On Fri, Mar 29, 2019 at 05:38:59PM +0700, Nguyễn Thái Ngọc Duy wrote:
> After a successful switch, if a merge, cherry-pick or revert is ongoing,
> it is canceled. This behavior has been with us from the very early
> beginning, soon after git-merge was created but never actually
> documented [1]. It may be a good idea to be transparent and tell the
> user if some operation is canceled.
After this entered 'next' last week, today it greeted me with 167(!)
of these warnings... before I even had my breakfast.
Now, my script does a lot of repeated cherry-picks and expects that
rerere is able to deal with most of the conflicts, i.e. it does
approximately this:
if ! git cherry-pick $oid >/dev/null 2>&1
then
if was_the_conflict_resolved
then
echo "using previous conflict resolution"
git commit --no-edit --cleanup=strip --quiet
else
die "uh-oh"
fi
fi
That 'git commit' in there always prints:
warning: cancelling a cherry picking in progress
I don't understand why committing after a cherry-pick is considered
"cancelling"... in my view it's finishing it and there should be no
warning whatsoever.
> I consider this a better way of telling the user than just adding a
> sentence or two in git-checkout.txt, which will be mostly ignored
> anyway.
>
> PS. Originally I wanted to print more details like
>
> warning: cancelling an in-progress merge from <SHA-1>
>
> which may allow some level of undo if the user wants to. But that seems
> a lot more work. Perhaps it can be improved later if people still want
> that.
>
> [1] ... and I will try not to argue whether it is a sensible behavior.
> There is some more discussion here if people are interested:
> CACsJy8Axa5WsLSjiscjnxVK6jQHkfs-gH959=YtUvQkWriAk5w@xxxxxxxxxxxxxx
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
> branch.c | 11 +++++++----
> branch.h | 2 +-
> builtin/am.c | 2 +-
> builtin/checkout.c | 2 +-
> builtin/rebase.c | 4 ++--
> builtin/reset.c | 2 +-
> builtin/revert.c | 2 +-
> 7 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/branch.c b/branch.c
> index 28b81a7e02..8dd5bb9f1c 100644
> --- a/branch.c
> +++ b/branch.c
> @@ -337,11 +337,14 @@ void create_branch(struct repository *r,
> free(real_ref);
> }
>
> -void remove_branch_state(struct repository *r)
> +void remove_branch_state(struct repository *r, int verbose)
> {
> - unlink(git_path_cherry_pick_head(r));
> - unlink(git_path_revert_head(r));
> - unlink(git_path_merge_head(r));
> + if (!unlink(git_path_cherry_pick_head(r)) && verbose)
> + warning(_("cancelling a cherry picking in progress"));
> + if (!unlink(git_path_revert_head(r)) && verbose)
> + warning(_("cancelling a revert in progress"));
> + if (!unlink(git_path_merge_head(r)) && verbose)
> + warning(_("cancelling a merge in progress"));
> unlink(git_path_merge_rr(r));
> unlink(git_path_merge_msg(r));
> unlink(git_path_merge_mode(r));
> diff --git a/branch.h b/branch.h
> index 29c1afa4d0..aed045901e 100644
> --- a/branch.h
> +++ b/branch.h
> @@ -64,7 +64,7 @@ extern int validate_new_branchname(const char *name, struct strbuf *ref, int for
> * Remove information about the state of working on the current
> * branch. (E.g., MERGE_HEAD)
> */
> -void remove_branch_state(struct repository *r);
> +void remove_branch_state(struct repository *r, int verbose);
>
> /*
> * Configure local branch "local" as downstream to branch "remote"
> diff --git a/builtin/am.c b/builtin/am.c
> index 4fb107a9d1..99b66508fd 100644
> --- a/builtin/am.c
> +++ b/builtin/am.c
> @@ -1957,7 +1957,7 @@ static int clean_index(const struct object_id *head, const struct object_id *rem
> if (merge_tree(remote_tree))
> return -1;
>
> - remove_branch_state(the_repository);
> + remove_branch_state(the_repository, 0);
>
> return 0;
> }
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 0e6037b296..f66bd2f56d 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -899,7 +899,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
> delete_reflog(old_branch_info->path);
> }
> }
> - remove_branch_state(the_repository);
> + remove_branch_state(the_repository, !opts->quiet);
> strbuf_release(&msg);
> if (!opts->quiet &&
> (new_branch_info->path || (!opts->force_detach && !strcmp(new_branch_info->name, "HEAD"))))
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 52114cbf0d..646d0f9fb1 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -1272,7 +1272,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> if (reset_head(NULL, "reset", NULL, RESET_HEAD_HARD,
> NULL, NULL) < 0)
> die(_("could not discard worktree changes"));
> - remove_branch_state(the_repository);
> + remove_branch_state(the_repository, 0);
> if (read_basic_state(&options))
> exit(1);
> goto run_rebase;
> @@ -1292,7 +1292,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
> NULL, NULL) < 0)
> die(_("could not move back to %s"),
> oid_to_hex(&options.orig_head));
> - remove_branch_state(the_repository);
> + remove_branch_state(the_repository, 0);
> ret = finish_rebase(&options);
> goto cleanup;
> }
> diff --git a/builtin/reset.c b/builtin/reset.c
> index 7882829a95..6d9397c844 100644
> --- a/builtin/reset.c
> +++ b/builtin/reset.c
> @@ -420,7 +420,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
> print_new_head_line(lookup_commit_reference(the_repository, &oid));
> }
> if (!pathspec.nr)
> - remove_branch_state(the_repository);
> + remove_branch_state(the_repository, 0);
>
> return update_ref_status;
> }
> diff --git a/builtin/revert.c b/builtin/revert.c
> index a47b53ceaf..ebf2789225 100644
> --- a/builtin/revert.c
> +++ b/builtin/revert.c
> @@ -196,7 +196,7 @@ static int run_sequencer(int argc, const char **argv, struct replay_opts *opts)
> if (cmd == 'q') {
> int ret = sequencer_remove_state(opts);
> if (!ret)
> - remove_branch_state(the_repository);
> + remove_branch_state(the_repository, 0);
> return ret;
> }
> if (cmd == 'c')
> --
> 2.21.0.479.g47ac719cd3
>
