Re: [PATCH 1/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

2 messages, 2 authors, 2021-07-30 · open the first message on its own page

Re: [PATCH 1/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

From: Junio C Hamano <hidden>
Date: 2021-07-29 20:56:51

"ZheNing Hu via GitGitGadget" [off-list ref] writes:
quoted hunk
-static void print_advice(struct repository *r, int show_hint,
-			 struct replay_opts *opts)
+static char *check_need_delete_cherry_pick_head(struct repository *r)
 {
 	char *msg = getenv("GIT_CHERRY_PICK_HELP");
 
 	if (msg) {
-		fprintf(stderr, "%s\n", msg);
 		/*
 		 * A conflict has occurred but the porcelain
 		 * (typically rebase --interactive) wants to take care
@@ -411,18 +409,22 @@ static void print_advice(struct repository *r, int show_hint,
 		 */
 		refs_delete_ref(get_main_ref_store(r), "", "CHERRY_PICK_HEAD",
 				NULL, 0);
-		return;
+		return msg;
 	}
+	return NULL;
+}
The patch is a bit hard to read, but I do not think this improves
the root cause of the issue at all, which is that "do we need to
delete the pseudoref?" decision is tied to and based on the presense
of the custom message in the environment variable.

The original issue was that we declared (erroneously) that the
presence of a custom message _means_ the caller is not a human but a
sequencing machinery that wants to deal with what is done via the
CHERRY_PICK_HEAD pseudoref itself, and your original issue was that
when a human user has the custom message envirnoment (either on
purpose or by mistake), the logic _misidentifies_ the call as if it
is made from a sequencing machinery and we lose the CHERRY_PICK_HEAD
pseudoref.  The root cause is mistaking the correlation (sequencing
machinery uses the environment and it also does not want pseudoref)
as the causation (presense of the environment means we should remove
pseudoref).

Re: [PATCH 1/2] [GSOC] cherry-pick: fix bug when used with GIT_CHERRY_PICK_HELP

From: ZheNing Hu <hidden>
Date: 2021-07-30 14:15:01

Junio C Hamano [off-list ref] 于2021年7月30日周五 上午4:56写道:
The patch is a bit hard to read, but I do not think this improves
the root cause of the issue at all, which is that "do we need to
delete the pseudoref?" decision is tied to and based on the presense
of the custom message in the environment variable.
My original idea was to distinguish between cherry-pick calls by machines
(git-rebase--preserve-merges.sh) and cherry-pick calls by people through
an additional option "rebase-preserve-merges-mode".
The original issue was that we declared (erroneously) that the
presence of a custom message _means_ the caller is not a human but a
sequencing machinery that wants to deal with what is done via the
CHERRY_PICK_HEAD pseudoref itself, and your original issue was that
when a human user has the custom message envirnoment (either on
purpose or by mistake), the logic _misidentifies_ the call as if it
is made from a sequencing machinery and we lose the CHERRY_PICK_HEAD
pseudoref.  The root cause is mistaking the correlation (sequencing
machinery uses the environment and it also does not want pseudoref)
as the causation (presense of the environment means we should remove
pseudoref).
Ok... Now I guess what you mean is to provide an option for git cherry-pick
to specifically delete "CHERRY_PICK_HEAD", and this options is "hidden",
it is not provided to users, it is only reserved for sequencing
machinery. In this
way, we separate print_advice() from deleting "CHERRY_PICK_HEAD".
Then in the future, GIT_CHERRY_PICK_HELP will be deleted.

Thanks,
--
ZheNing Hu
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help