Martin Ågren <martin.agren@xxxxxxxxx> writes:
> A further upshot of this patch is that `active_cache_changed`, which is
> defined as `the_index.cache_changed`, now only has a few users left.
I am undecided if this is a *good* thing. In a few codepaths where
we make a speculative update to the on-disk index file, I find that
the explicit mention of active_cache_changed clarifies what is going
on. Perhaps once my (and other old timers') eyes get used to this
new SKIP_IF_UNCHANGED symbol, it would start serving the same
purpose ;-)
> We could have made the new flag behave the other way round
> ("FORCE_WRITE"), but that could break existing users behind their backs.
Yes, that would break topics in flight that add new calls to
write_locked_index(); they want to write out the index even when
active_cache_changed says there is no update.
Looking at a typical pre/post image of this change like this place:
> hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
> refresh_cache(REFRESH_QUIET);
> - if (active_cache_changed &&
> - write_locked_index(&the_index, &lock, COMMIT_LOCK))
> + if (write_locked_index(&the_index, &lock,
> + COMMIT_LOCK | SKIP_IF_UNCHANGED))
> return error(_("Unable to write index."));
> - rollback_lock_file(&lock);
this certainly simplifies what the caller needs to do.
> @@ -638,6 +639,9 @@ extern int read_index_unmerged(struct index_state *);
> * With `COMMIT_LOCK`, the lock is always committed or rolled back.
> * Without it, the lock is closed, but neither committed nor rolled
> * back.
> + *
> + * If `SKIP_IF_UNCHANGED` is given and the index is unchanged, nothing
> + * is written (and the lock is rolled back if `COMMIT_LOCK` is given).
> */
> extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);
Good.
