write_locked_index(): add flag to avoid writing unchanged index
We have several callers like if (active_cache_changed && write_locked_index(...)) handle_error(); rollback_lock_file(...); where the final rollback is needed because "!active_cache_changed" shortcuts the if-expression. There are also a few variants of this, including some if-else constructs that make it more clear when the explicit rollback is really needed. Teach `write_locked_index()` to take a new flag SKIP_IF_UNCHANGED and simplify the callers. Leave the most complicated of the callers (in builtin/update-index.c) unchanged. Rewriting it to use this new flag would end up duplicating logic. We could have made the new flag behave the other way round ("FORCE_WRITE"), but that could break existing users behind their backs. Let's take the more conservative approach. We can still migrate existing callers to use our new flag. Later we might even be able to flip the default, possibly without entirely ignoring the risk to in-flight or out-of-tree topics. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Martin Ågren committed
Mar 1, 2018 at 21:40 UTC
610008146ed1647bb1da6a098e314b8929ff213e
10 files changed
+37
-40
builtin/add.c
+3
-4
@@ -534,10 +534,9 @@ int cmd_add(int argc, const char **argv, const char *prefix)
534
unplug_bulk_checkin();
535
536
finish:
537
- if (active_cache_changed) {
538
- if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
539
- die(_("Unable to write new index file"));
540
- }
537
+ if (write_locked_index(&the_index, &lock_file,
538
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
539
+ die(_("Unable to write new index file"));
540
541
UNLEAK(pathspec);
542
UNLEAK(dir);
builtin/commit.c
+3
-7
@@ -389,13 +389,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
389
if (active_cache_changed
390
|| !cache_tree_fully_valid(active_cache_tree))
391
update_main_cache_tree(WRITE_TREE_SILENT);
392
- if (active_cache_changed) {
393
- if (write_locked_index(&the_index, &index_lock,
394
- COMMIT_LOCK))
395
- die(_("unable to write new_index file"));
396
- } else {
397
- rollback_lock_file(&index_lock);
398
- }
392
+ if (write_locked_index(&the_index, &index_lock,
393
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
394
+ die(_("unable to write new_index file"));
395
commit_style = COMMIT_AS_IS;
396
ret = get_index_file();
397
goto out;
builtin/merge.c
+6
-9
@@ -651,10 +651,9 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
651
652
hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
653
refresh_cache(REFRESH_QUIET);
654
- if (active_cache_changed &&
655
- write_locked_index(&the_index, &lock, COMMIT_LOCK))
654
+ if (write_locked_index(&the_index, &lock,
655
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
656
return error(_("Unable to write index."));
657
- rollback_lock_file(&lock);
657
658
if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) {
659
int clean, x;
@@ -691,10 +690,9 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
690
remoteheads->item, reversed, &result);
691
if (clean < 0)
692
exit(128);
694
- if (active_cache_changed &&
695
- write_locked_index(&the_index, &lock, COMMIT_LOCK))
693
+ if (write_locked_index(&the_index, &lock,
694
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
695
die (_("unable to write %s"), get_index_file());
697
- rollback_lock_file(&lock);
696
return clean ? 0 : 1;
697
} else {
698
return try_merge_command(strategy, xopts_nr, xopts,
@@ -810,10 +808,9 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
808
809
hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
810
refresh_cache(REFRESH_QUIET);
813
- if (active_cache_changed &&
814
- write_locked_index(&the_index, &lock, COMMIT_LOCK))
811
+ if (write_locked_index(&the_index, &lock,
812
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
813
return error(_("Unable to write index."));
816
- rollback_lock_file(&lock);
814
815
write_tree_trivial(&result_tree);
816
printf(_("Wonderful.\n"));
builtin/mv.c
+2
-2
@@ -293,8 +293,8 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
293
if (gitmodules_modified)
294
stage_updated_gitmodules(&the_index);
295
296
- if (active_cache_changed &&
297
- write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
296
+ if (write_locked_index(&the_index, &lock_file,
297
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
298
die(_("Unable to write new index file"));
299
300
return 0;
builtin/rm.c
+3
-4
@@ -385,10 +385,9 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
385
stage_updated_gitmodules(&the_index);
386
}
387
388
- if (active_cache_changed) {
389
- if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
390
- die(_("Unable to write new index file"));
391
- }
388
+ if (write_locked_index(&the_index, &lock_file,
389
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
390
+ die(_("Unable to write new index file"));
391
392
return 0;
393
}
cache.h
+4
@@ -599,6 +599,7 @@ extern int read_index_unmerged(struct index_state *);
599
600
/* For use with `write_locked_index()`. */
601
#define COMMIT_LOCK (1 << 0)
602
+#define SKIP_IF_UNCHANGED (1 << 1)
603
604
/*
605
* Write the index while holding an already-taken lock. Close the lock,
@@ -615,6 +616,9 @@ extern int read_index_unmerged(struct index_state *);
616
* With `COMMIT_LOCK`, the lock is always committed or rolled back.
617
* Without it, the lock is closed, but neither committed nor rolled
618
* back.
619
+ *
620
+ * If `SKIP_IF_UNCHANGED` is given and the index is unchanged, nothing
621
+ * is written (and the lock is rolled back if `COMMIT_LOCK` is given).
622
*/
623
extern int write_locked_index(struct index_state *, struct lock_file *lock, unsigned flags);
624
merge-recursive.c
+2
-3
@@ -2223,10 +2223,9 @@ int merge_recursive_generic(struct merge_options *o,
2223
return clean;
2224
}
2225
2226
- if (active_cache_changed &&
2227
- write_locked_index(&the_index, &lock, COMMIT_LOCK))
2226
+ if (write_locked_index(&the_index, &lock,
2227
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
2228
return err(o, _("Unable to write index."));
2229
- rollback_lock_file(&lock);
2229
2230
return clean ? 0 : 1;
2231
}
read-cache.c
+6
@@ -2538,6 +2538,12 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
2538
int new_shared_index, ret;
2539
struct split_index *si = istate->split_index;
2540
2541
+ if ((flags & SKIP_IF_UNCHANGED) && !istate->cache_changed) {
2542
+ if (flags & COMMIT_LOCK)
2543
+ rollback_lock_file(lock);
2544
+ return 0;
2545
+ }
2546
+
2547
if (istate->fsmonitor_last_update)
2548
fill_fsmonitor_bitmap(istate);
2549
rerere.c
+3
-5
@@ -719,11 +719,9 @@ static void update_paths(struct string_list *update)
719
item->string);
720
}
721
722
- if (active_cache_changed) {
723
- if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
724
- die("Unable to write new index file");
725
- } else
726
- rollback_lock_file(&index_lock);
722
+ if (write_locked_index(&the_index, &index_lock,
723
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
724
+ die("Unable to write new index file");
725
}
726
727
static void remove_variant(struct rerere_id *id)
sequencer.c
+5
-6
@@ -517,15 +517,14 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
517
return clean;
518
}
519
520
- if (active_cache_changed &&
521
- write_locked_index(&the_index, &index_lock, COMMIT_LOCK))
520
+ if (write_locked_index(&the_index, &index_lock,
521
+ COMMIT_LOCK | SKIP_IF_UNCHANGED))
522
/*
523
* TRANSLATORS: %s will be "revert", "cherry-pick" or
524
* "rebase -i".
525
*/
526
return error(_("%s: Unable to write new index file"),
527
_(action_name(opts)));
528
- rollback_lock_file(&index_lock);
528
529
if (!clean)
530
append_conflicts_hint(msgbuf);
@@ -1713,13 +1712,13 @@ static int read_and_refresh_cache(struct replay_opts *opts)
1712
_(action_name(opts)));
1713
}
1714
refresh_index(&the_index, REFRESH_QUIET|REFRESH_UNMERGED, NULL, NULL, NULL);
1716
- if (the_index.cache_changed && index_fd >= 0) {
1717
- if (write_locked_index(&the_index, &index_lock, COMMIT_LOCK)) {
1715
+ if (index_fd >= 0) {
1716
+ if (write_locked_index(&the_index, &index_lock,
1717
+ COMMIT_LOCK | SKIP_IF_UNCHANGED)) {
1718
return error(_("git %s: failed to refresh the index"),
1719
_(action_name(opts)));
1720
}
1721
}
1722
- rollback_lock_file(&index_lock);
1722
return 0;
1723
}
1724