Re: [PATCH 10/11] merge-ort: write $GIT_DIR/AUTO_MERGE whenever we hit a conflict
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 8, 2021, 13:11 UTC
- Message-ID
- <87v9a1iycx.fsf@evledraar.gmail.com>
- In-Reply-To
- <d8c6eb39aa7c39263b7f0333ec92a3cc65c367ec.1614905738.git.gitgitgadget@gmail.com>
On Fri, Mar 05 2021, Elijah Newren via GitGitGadget wrote:
Show 23 quoted lines
> From: Elijah Newren <newren@gmail.com> > > There are a variety of questions users might ask while resolving > conflicts: > * What changes have been made since the previous (first) parent? > * What changes are staged? > * What is still unstaged? (or what is still conflicted?) > * What changes did I make to resolve conflicts so far? > The first three of these have simple answers: > * git diff HEAD > * git diff --cached > * git diff > There was no way to answer the final question previously. Adding one > is trivial in merge-ort, since it works by creating a tree representing > what should be written to the working copy complete with conflict > markers. Simply write that tree to .git/AUTO_MERGE, allowing users to > answer the fourth question with > * git diff AUTO_MERGE > > I avoided using a name like "MERGE_AUTO", because that would be > merge-specific (much like MERGE_HEAD, REBASE_HEAD, REVERT_HEAD, > CHERRY_PICK_HEAD) and I wanted a name that didn't change depending on > which type of operation the merge was part of.
That's a really cool feature. I'm starting to like this "ort" thing :)
(After knowing almost nothing about it until a few days ago...)
Show 63 quoted lines
> Ensure that paths which clean out other temporary operation-specific
> files (e.g. CHERRY_PICK_HEAD, MERGE_MSG, rebase-merge/ state directory)
> also clean out this AUTO_MERGE file.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
> branch.c | 1 +
> builtin/rebase.c | 1 +
> merge-ort.c | 10 ++++++++++
> path.c | 1 +
> path.h | 2 ++
> sequencer.c | 5 +++++
> 6 files changed, 20 insertions(+)
>
> diff --git a/branch.c b/branch.c
> index 9c9dae1eae32..b71a2de29dbe 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -344,6 +344,7 @@ void remove_merge_branch_state(struct repository *r)
> unlink(git_path_merge_rr(r));
> unlink(git_path_merge_msg(r));
> unlink(git_path_merge_mode(r));
> + unlink(git_path_auto_merge(r));
> save_autostash(git_path_merge_autostash(r));
> }
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index de400f9a1973..7d9afe118fd4 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -739,6 +739,7 @@ static int finish_rebase(struct rebase_options *opts)
> int ret = 0;
>
> delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
> + unlink(git_path_auto_merge(the_repository));
> apply_autostash(state_dir_path("autostash", opts));
> close_object_store(the_repository->objects);
> /*
> diff --git a/merge-ort.c b/merge-ort.c
> index 37b69cbe0f9a..cf927cd160e1 100644
> --- a/merge-ort.c
> +++ b/merge-ort.c
> @@ -3362,6 +3362,9 @@ void merge_switch_to_result(struct merge_options *opt,
> {
> assert(opt->priv == NULL);
> if (result->clean >= 0 && update_worktree_and_index) {
> + const char *filename;
> + FILE *fp;
> +
> trace2_region_enter("merge", "checkout", opt->repo);
> if (checkout(opt, head, result->tree)) {
> /* failure to function */
> @@ -3380,6 +3383,13 @@ void merge_switch_to_result(struct merge_options *opt,
> }
> opt->priv = NULL;
> trace2_region_leave("merge", "record_conflicted", opt->repo);
> +
> + trace2_region_enter("merge", "write_auto_merge", opt->repo);
> + filename = git_path_auto_merge(opt->repo);
> + fp = xfopen(filename, "w");
> + fprintf(fp, "%s\n", oid_to_hex(&result->tree->object.oid));
> + fclose(fp);
> + trace2_region_leave("merge", "write_auto_merge", opt->repo);This isn't a new problem since you're just folling an existing pattern, but here you (rightly) do xopen()< and the:n
Show 43 quoted lines
> }
>
> if (display_update_msgs) {
> diff --git a/path.c b/path.c
> index 7b385e5eb282..9e883eb52446 100644
> --- a/path.c
> +++ b/path.c
> @@ -1534,5 +1534,6 @@ REPO_GIT_PATH_FUNC(merge_rr, "MERGE_RR")
> REPO_GIT_PATH_FUNC(merge_mode, "MERGE_MODE")
> REPO_GIT_PATH_FUNC(merge_head, "MERGE_HEAD")
> REPO_GIT_PATH_FUNC(merge_autostash, "MERGE_AUTOSTASH")
> +REPO_GIT_PATH_FUNC(auto_merge, "AUTO_MERGE")
> REPO_GIT_PATH_FUNC(fetch_head, "FETCH_HEAD")
> REPO_GIT_PATH_FUNC(shallow, "shallow")
> diff --git a/path.h b/path.h
> index e7e77da6aaa5..251c78d98000 100644
> --- a/path.h
> +++ b/path.h
> @@ -176,6 +176,7 @@ struct path_cache {
> const char *merge_mode;
> const char *merge_head;
> const char *merge_autostash;
> + const char *auto_merge;
> const char *fetch_head;
> const char *shallow;
> };
> @@ -191,6 +192,7 @@ const char *git_path_merge_rr(struct repository *r);
> const char *git_path_merge_mode(struct repository *r);
> const char *git_path_merge_head(struct repository *r);
> const char *git_path_merge_autostash(struct repository *r);
> +const char *git_path_auto_merge(struct repository *r);
> const char *git_path_fetch_head(struct repository *r);
> const char *git_path_shallow(struct repository *r);
>
> diff --git a/sequencer.c b/sequencer.c
> index d2332d3e1787..472cdd8c620d 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2096,6 +2096,7 @@ static int do_pick_commit(struct repository *r,
> refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
> NULL, 0);
> unlink(git_path_merge_msg(r));
> + unlink(git_path_auto_merge(r));Shouldn't this & the rest ideally be at least unlink_or_warn()?
Show 28 quoted lines
> fprintf(stderr,
> _("dropping %s %s -- patch contents already upstream\n"),
> oid_to_hex(&commit->object.oid), msg.subject);
> @@ -2451,6 +2452,8 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose)
> need_cleanup = 1;
> }
>
> + unlink(git_path_auto_merge(r));
> +
> if (!need_cleanup)
> return;
>
> @@ -4111,6 +4114,7 @@ static int pick_commits(struct repository *r,
> unlink(rebase_path_stopped_sha());
> unlink(rebase_path_amend());
> unlink(git_path_merge_head(r));
> + unlink(git_path_auto_merge(r));
> delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
>
> if (item->command == TODO_BREAK) {
> @@ -4505,6 +4509,7 @@ static int commit_staged_changes(struct repository *r,
> return error(_("could not commit staged changes."));
> unlink(rebase_path_amend());
> unlink(git_path_merge_head(r));
> + unlink(git_path_auto_merge(r));
> if (final_fixup) {
> unlink(rebase_path_fixup_msg());
> unlink(rebase_path_squash_msg());