Thanks Martin for the review of the last round! Changes compared to the previous round: - always pass COMMIT_LOCK to write_locked_index - don't write the error message in repo_refresh_and_write_index, but let the caller handle that. This means that we no longer change any error messages. Potential cleanups in that area can come later. - Drop the lock variable to the scope it needs. Range diff below: 1: 7249a3cf4e ! 1: 1f25fe227c factor out refresh_and_write_cache function @@ builtin/am.c: static void am_run(struct am_state *state, int resume) unlink(am_path(state, "dirtyindex")); - refresh_and_write_cache(); -+ if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK) < 0) -+ die(_("failed to refresh cache")); ++ if (refresh_and_write_cache(REFRESH_QUIET, 0) < 0) ++ die(_("unable to write index file")); if (repo_index_has_changes(the_repository, NULL, &sb)) { write_state_bool(state, "dirtyindex", 1); @@ cache.h: void fill_stat_cache_info(struct index_state *istate, struct cache_entr +/* + * Refresh the index and write it to disk. + * ++ * 'refresh_flags' is passed directly to 'refresh_index()', while ++ * 'COMMIT_LOCK | write_flags' is passed to 'write_locked_index()', so ++ * the lockfile is always either committed or rolled back. ++ * + * Return 1 if refreshing the cache failed, -1 if writing the cache to + * disk failed, 0 on success. + */ @@ read-cache.c: static void show_file(const char * fmt, const char * name, int in_ + repo_hold_locked_index(repo, &lock_file, LOCK_DIE_ON_ERROR); + if (refresh_index(repo->index, refresh_flags, pathspec, seen, header_msg)) + return 1; -+ if (write_locked_index(repo->index, &lock_file, write_flags)) -+ return error(_("unable to write index file")); ++ if (write_locked_index(repo->index, &lock_file, COMMIT_LOCK | write_flags)) ++ return -1; + return 0; +} + 2: de5b8c1529 ! 2: 148a65d649 merge: use refresh_and_write_cache @@ Commit message ## builtin/merge.c ## @@ builtin/merge.c: static int try_merge_strategy(const char *strategy, struct commit_list *common, - struct lock_file lock = LOCK_INIT; + struct commit_list *remoteheads, + struct commit *head) + { +- struct lock_file lock = LOCK_INIT; const char *head_arg = "HEAD"; - hold_locked_index(&lock, LOCK_DIE_ON_ERROR); - refresh_cache(REFRESH_QUIET); - if (write_locked_index(&the_index, &lock, - COMMIT_LOCK | SKIP_IF_UNCHANGED)) -- return error(_("Unable to write index.")); -+ if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK | SKIP_IF_UNCHANGED) < 0) -+ return -1; ++ if (refresh_and_write_cache(REFRESH_QUIET, SKIP_IF_UNCHANGED) < 0) + return error(_("Unable to write index.")); if (!strcmp(strategy, "recursive") || !strcmp(strategy, "subtree")) { ++ struct lock_file lock = LOCK_INIT; int clean, x; + struct commit *result; + struct commit_list *reversed = NULL; @@ builtin/merge.c: static int merge_trivial(struct commit *head, struct commit_list *remoteheads) { struct object_id result_tree, result_commit; @@ builtin/merge.c: static int merge_trivial(struct commit *head, struct commit_lis - refresh_cache(REFRESH_QUIET); - if (write_locked_index(&the_index, &lock, - COMMIT_LOCK | SKIP_IF_UNCHANGED)) -- return error(_("Unable to write index.")); -+ if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK | SKIP_IF_UNCHANGED) < 0) -+ return -1; ++ if (refresh_and_write_cache(REFRESH_QUIET, SKIP_IF_UNCHANGED) < 0) + return error(_("Unable to write index.")); write_tree_trivial(&result_tree); - printf(_("Wonderful.\n")); 3: d9efda0f2a ! 3: e0f6815192 stash: make sure to write refreshed cache @@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info read_cache_preload(NULL); - if (refresh_cache(REFRESH_QUIET)) -+ if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK)) ++ if (refresh_and_write_cache(REFRESH_QUIET, 0)) return -1; if (write_cache_as_tree(&c_tree, 0, NULL)) @@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info if (quiet) { - if (refresh_cache(REFRESH_QUIET)) -+ if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK)) ++ if (refresh_and_write_cache(REFRESH_QUIET, 0)) warning("could not refresh index"); } else { struct child_process cp = CHILD_PROCESS_INIT; @@ builtin/stash.c: static int do_create_stash(const struct pathspec *ps, struct st read_cache_preload(NULL); - refresh_cache(REFRESH_QUIET); -+ if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK) < 0) { ++ if (refresh_and_write_cache(REFRESH_QUIET, 0) < 0) { + ret = -1; + goto done; + } @@ builtin/stash.c: static int do_push_stash(const struct pathspec *ps, const char } - if (refresh_cache(REFRESH_QUIET)) { -+ if (refresh_and_write_cache(REFRESH_QUIET, COMMIT_LOCK)) { ++ if (refresh_and_write_cache(REFRESH_QUIET, 0)) { ret = -1; goto done; } Thomas Gummerer (3): factor out refresh_and_write_cache function merge: use refresh_and_write_cache stash: make sure to write refreshed cache builtin/am.c | 16 ++-------------- builtin/merge.c | 17 +++++------------ builtin/stash.c | 11 +++++++---- cache.h | 13 +++++++++++++ read-cache.c | 17 +++++++++++++++++ t/t3903-stash.sh | 16 ++++++++++++++++ 6 files changed, 60 insertions(+), 30 deletions(-) -- 2.23.0.rc2.194.ge5444969c9