git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Han-Wen Nienhuys <hanwenn@gmail.com>,
	Han-Wen Nienhuys <hanwen@google.com>
Subject: Re: [PATCH 2/3] Treat CHERRY_PICK_HEAD as a pseudo ref
Date: Tue, 18 Aug 2020 14:05:16 -0700	[thread overview]
Message-ID: <xmqqsgcjwtpv.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <06a8e8cbd1370ba3d4ea8a0a9f1e69d27b1d62c4.1597753075.git.gitgitgadget@gmail.com> (Han-Wen Nienhuys via GitGitGadget's message of "Tue, 18 Aug 2020 12:17:54 +0000")

"Han-Wen Nienhuys via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Han-Wen Nienhuys <hanwen@google.com>
>
> Check for existence and delete CHERRY_PICK_HEAD through ref functions.
> This will help cherry-pick work with alternate ref storage backends.

These two sentences are true, but this patch does another thing that
is not advertised here.  It stops recommending removal of
CHERRY_PICK_HEAD from the filesystem with "rm" (or using "git
update-ref -d" for that matter) as a way to avoid recording the
current commit as a cherry-pick.

The intent of the warning message touched by this patch, I think, is
that there is CHERRY_PICK_HEAD, but that might be a leftover one
that does not have to do anything to do with the commit the user is
making, and continuing blindly may record the commit incorrectly
(perhaps 'cherry-picked-from' trailer is added incorrectly?
existing notes data would be copied to the newly created commit?).

To recover from the situation, the user would want to abort the
commit while keeping the changes made to the working tree and to the
index to avoid the result recorded as a cherry-pick of an irrelevant
commit, get rid of CHERRY_PICK_HEAD and then attempt the "git
commit" again.

Does "git cherry-pick --abort" only remove CHERRY_PICK_HEAD without
doing any other damage to the working tree files and to the index?

> Signed-off-by: Han-Wen Nienhuys <hanwen@google.com>
> ---
>  builtin/commit.c | 34 +++++++++++++++++++---------------
>  builtin/merge.c  |  2 +-
>  path.c           |  1 -
>  path.h           |  7 ++++---
>  sequencer.c      | 42 ++++++++++++++++++++++++++----------------
>  wt-status.c      |  4 ++--
>  6 files changed, 52 insertions(+), 38 deletions(-)
>
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 69ac78d5e5..619b71bcb4 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -847,21 +847,25 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>  			if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
>  				!merge_contains_scissors)
>  				wt_status_add_cut_line(s->fp);
> -			status_printf_ln(s, GIT_COLOR_NORMAL,
> -			    whence == FROM_MERGE
> -				? _("\n"
> -					"It looks like you may be committing a merge.\n"
> -					"If this is not correct, please remove the file\n"
> -					"	%s\n"
> -					"and try again.\n")
> -				: _("\n"
> -					"It looks like you may be committing a cherry-pick.\n"
> -					"If this is not correct, please remove the file\n"
> -					"	%s\n"
> -					"and try again.\n"),
> -				whence == FROM_MERGE ?
> -					git_path_merge_head(the_repository) :
> -					git_path_cherry_pick_head(the_repository));
> +			if (whence == FROM_MERGE)
> +				status_printf_ln(
> +					s, GIT_COLOR_NORMAL,
> +
> +					_("\n"
> +					  "It looks like you may be committing a merge.\n"
> +					  "If this is not correct, please remove the file\n"
> +					  "	%s\n"
> +					  "and try again.\n"),
> +					git_path_merge_head(the_repository));
> +			else
> +				status_printf_ln(
> +					s, GIT_COLOR_NORMAL,
> +
> +					_("\n"
> +					  "It looks like you may be committing a cherry-pick.\n"
> +					  "If this is not correct, please run\n"
> +					  "	git cherry-pick --abort\n"
> +					  "and try again.\n"));
>  		}

I do not know about this change (see above).

> diff --git a/builtin/merge.c b/builtin/merge.c
> index 74829a838e..b9a89ba858 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -1348,7 +1348,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
>  		else
>  			die(_("You have not concluded your merge (MERGE_HEAD exists)."));
>  	}
> -	if (file_exists(git_path_cherry_pick_head(the_repository))) {
> +	if (ref_exists("CHERRY_PICK_HEAD")) {

This, and all the rest, look quite sensible.

> diff --git a/wt-status.c b/wt-status.c
> index d75399085d..c6abf2f3ca 100644
> --- a/wt-status.c
> +++ b/wt-status.c
> @@ -1672,8 +1672,8 @@ void wt_status_get_state(struct repository *r,
>  		state->merge_in_progress = 1;
>  	} else if (wt_status_check_rebase(NULL, state)) {
>  		;		/* all set */
> -	} else if (!stat(git_path_cherry_pick_head(r), &st) &&
> -			!get_oid("CHERRY_PICK_HEAD", &oid)) {
> +	} else if (refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") &&
> +		   !get_oid("CHERRY_PICK_HEAD", &oid)) {
>  		state->cherry_pick_in_progress = 1;
>  		oidcpy(&state->cherry_pick_head_oid, &oid);
>  	}

  reply	other threads:[~2020-08-18 21:05 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-18 12:17 [PATCH 0/3] Use refs API for handling sundry pseudorefs Han-Wen Nienhuys via GitGitGadget
2020-08-18 12:17 ` [PATCH 1/3] Make refs_ref_exists public Han-Wen Nienhuys via GitGitGadget
2020-08-18 20:39   ` Junio C Hamano
2020-08-18 12:17 ` [PATCH 2/3] Treat CHERRY_PICK_HEAD as a pseudo ref Han-Wen Nienhuys via GitGitGadget
2020-08-18 21:05   ` Junio C Hamano [this message]
2020-08-19 15:04     ` Han-Wen Nienhuys
2020-08-19 16:14       ` Junio C Hamano
2020-08-18 12:17 ` [PATCH 3/3] Treat REVERT_HEAD " Han-Wen Nienhuys via GitGitGadget
2020-08-18 21:06   ` Junio C Hamano
2020-08-18 20:33 ` [PATCH 0/3] Use refs API for handling sundry pseudorefs Junio C Hamano
2020-08-19 15:15 ` [PATCH v2 0/4] " Han-Wen Nienhuys via GitGitGadget
2020-08-19 15:15   ` [PATCH v2 1/4] refs: make refs_ref_exists public Han-Wen Nienhuys via GitGitGadget
2020-08-19 15:15   ` [PATCH v2 2/4] sequencer: treat CHERRY_PICK_HEAD as a pseudo ref Han-Wen Nienhuys via GitGitGadget
2020-08-19 15:15   ` [PATCH v2 3/4] builtin/commit: suggest update-ref for pseudoref removal Han-Wen Nienhuys via GitGitGadget
2020-08-19 21:24     ` Junio C Hamano
2020-08-21 14:57       ` Han-Wen Nienhuys
2020-08-19 15:15   ` [PATCH v2 4/4] sequencer: treat REVERT_HEAD as a pseudo ref Han-Wen Nienhuys via GitGitGadget
2020-08-21 16:59   ` [PATCH v3 0/4] Use refs API for handling sundry pseudorefs Han-Wen Nienhuys via GitGitGadget
2020-08-21 16:59     ` [PATCH v3 1/4] refs: make refs_ref_exists public Han-Wen Nienhuys via GitGitGadget
2020-08-21 16:59     ` [PATCH v3 2/4] sequencer: treat CHERRY_PICK_HEAD as a pseudo ref Han-Wen Nienhuys via GitGitGadget
2020-08-21 16:59     ` [PATCH v3 3/4] builtin/commit: suggest update-ref for pseudoref removal Han-Wen Nienhuys via GitGitGadget
2020-08-21 16:59     ` [PATCH v3 4/4] sequencer: treat REVERT_HEAD as a pseudo ref Han-Wen Nienhuys via GitGitGadget
2020-08-21 18:34     ` [PATCH v3 0/4] Use refs API for handling sundry pseudorefs Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqsgcjwtpv.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=hanwen@google.com \
    --cc=hanwenn@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).