Thread (37 messages) flat view 37 messages, 4 authors, 20d ago

Re: [PATCH v2] checkout: print blank line after autostash conflict advice

From: Phillip Wood <hidden>
Date: 2026-09-01 09:31:54

On 31/08/2026 18:19, Junio C Hamano wrote:
"Harald Nordgren via GitGitGadget" [off-list ref] writes:
quoted
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..5ed9ae86c4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4815,7 +4815,8 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
  		if (label_base)
  			strvec_pushf(&child.args, "--label-base=%s", label_base);
  		strvec_push(&child.args, stash_oid);
-		ret = run_command(&child);
+		if (run_command(&child))
+			ret = 1;
  	}
This does not look like the right way to have the function return 1
if the objective is to do so only when the spawned "git stash apply
<oid>" process fails due to conflicts.

[...]
 > > For expediency, it may be OK to assume any and all failures from
"git stash apply <oid>" come from a conflicted stash application in
your first version.  If that is what your reviewer recommended, I
would agree.  But let's help users and future developers (who do not
necessarily have to be you) by leaving a note that this code is not
doing what it claims to do and needs more work in the code.
I think if the objective of this patch is to tell the caller whether the 
conflicts message was printed or not then it is correct because the 
existing code is too caviler about printing that message. We should at 
least tighten that even if we don't change "git stash" (which I agree we 
should fix at some point).

	ret = run_command(&child);
	if (ret > 1)
		ret = -1;

would catch run_command() failing and stash dying or being killed by a 
signal. Then we should change the code below so that it only claims 
there were conflicts when "ret == 1" and prints a new error message 
explaining that "git stash apply" failed when "ret == -1"

Thanks

Phillip
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help