[PATCH 0/2] checkout -m: refine autostash fallback

[PATCH 0/2] checkout -m: refine autostash fallback

[Harald Nordgren via GitGitGadget]

Avoiding checkout -m autostash retries when no tracked local changes exist
and visually separating autostash conflict advice from the subsequent
branch-switch message.

Addresses #leftoverbits from here:
https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/

Harald Nordgren (2):
  sequencer: teach autostash apply to report conflicts
  checkout -m: refine autostash fallback

 builtin/checkout.c | 18 ++++++++++++++----
 builtin/commit.c   |  2 +-
 builtin/merge.c    |  6 +++---
 sequencer.c        | 29 +++++++++++++++++++----------
 sequencer.h        |  3 ++-
 t/t7201-co.sh      | 17 ++++++++++++++++-
 6 files changed, 55 insertions(+), 20 deletions(-)


base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v1
Pull-Request: https://github.com/git/git/pull/2364
-- 
gitgitgadget

[PATCH 1/2] sequencer: teach autostash apply to report conflicts

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

Add a conflicted parameter to apply_save_autostash_oid() and
apply_save_autostash_ref() so callers can learn whether applying the
stash resulted in conflicts.  Thread the parameter through
apply_autostash_ref() and update existing callers to pass NULL.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/checkout.c |  3 ++-
 builtin/commit.c   |  2 +-
 builtin/merge.c    |  6 +++---
 sequencer.c        | 29 +++++++++++++++++++----------
 sequencer.h        |  3 ++-
 5 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index aee84ca897..72aafa4049 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1242,7 +1242,8 @@ static int switch_branches(const struct checkout_opts *opts,
 					    new_branch_info->name,
 					    "local",
 					    stash_label_base,
-					    autostash_msg.buf);
+					    autostash_msg.buf,
+					    NULL);
 		}
 		if (ret) {
 			branch_info_release(&old_branch_info);
diff --git a/builtin/commit.c b/builtin/commit.c
index 28f6174503..d678a81865 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1980,7 +1980,7 @@ int cmd_commit(int argc,
 	}
 
 	apply_autostash_ref(the_repository, "MERGE_AUTOSTASH",
-			    NULL, NULL, NULL, NULL);
+			    NULL, NULL, NULL, NULL, NULL);
 
 cleanup:
 	free_commit_extra_headers(extra);
diff --git a/builtin/merge.c b/builtin/merge.c
index 5b46a596f0..cecb8fb716 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -538,7 +538,7 @@ static void finish(struct commit *head_commit,
 
 	if (new_head)
 		apply_autostash_ref(the_repository, "MERGE_AUTOSTASH",
-				    NULL, NULL, NULL, NULL);
+				    NULL, NULL, NULL, NULL, NULL);
 	strbuf_release(&reflog_message);
 }
 
@@ -1680,7 +1680,7 @@ int cmd_merge(int argc,
 					  &commit->object.oid,
 					  overwrite_ignore)) {
 			apply_autostash_ref(the_repository, "MERGE_AUTOSTASH",
-					    NULL, NULL, NULL, NULL);
+					    NULL, NULL, NULL, NULL, NULL);
 			ret = 1;
 			goto done;
 		}
@@ -1844,7 +1844,7 @@ int cmd_merge(int argc,
 			fprintf(stderr, _("Merge with strategy %s failed.\n"),
 				use_strategies[0]->name);
 		apply_autostash_ref(the_repository, "MERGE_AUTOSTASH",
-				    NULL, NULL, NULL, NULL);
+				    NULL, NULL, NULL, NULL, NULL);
 		ret = 2;
 		goto done;
 	} else if (best_strategy == wt_strategy)
diff --git a/sequencer.c b/sequencer.c
index 1355a99a09..91a70e39a1 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4748,7 +4748,8 @@ void create_autostash_ref(struct repository *r, const char *refname,
 static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 				    const char *label_ours, const char *label_theirs,
 				    const char *label_base,
-				    const char *stash_msg)
+				    const char *stash_msg,
+				    bool *conflicted)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
 	int ret = 0;
@@ -4783,14 +4784,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 		strvec_push(&store.args, stash_oid);
 		if (run_command(&store))
 			ret = error(_("cannot store %s"), stash_oid);
-		else if (attempt_apply)
+		else if (attempt_apply) {
+			if (conflicted)
+				*conflicted = true;
 			fprintf(stderr,
 				_("Your local changes are stashed, however applying them\n"
 				  "resulted in conflicts.  You can either resolve the conflicts\n"
 				  "and then discard the stash with \"git stash drop\", or, if you\n"
 				  "do not want to resolve them now, run \"git reset --hard\" and\n"
 				  "apply the local changes later by running \"git stash pop\".\n"));
-		else
+		} else
 			fprintf(stderr,
 				_("Autostash exists; creating a new stash entry.\n"
 				  "Your changes are safe in the stash.\n"
@@ -4814,7 +4817,7 @@ static int apply_save_autostash(const char *path, int attempt_apply)
 	strbuf_trim(&stash_oid);
 
 	ret = apply_save_autostash_oid(stash_oid.buf, attempt_apply,
-				      NULL, NULL, NULL, NULL);
+				      NULL, NULL, NULL, NULL, NULL);
 
 	unlink(path);
 	strbuf_release(&stash_oid);
@@ -4833,19 +4836,24 @@ int apply_autostash(const char *path)
 
 int apply_autostash_oid(const char *stash_oid)
 {
-	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL);
+	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL,
+				       NULL);
 }
 
 static int apply_save_autostash_ref(struct repository *r, const char *refname,
 				    int attempt_apply,
 				    const char *label_ours, const char *label_theirs,
 				    const char *label_base,
-				    const char *stash_msg)
+				    const char *stash_msg,
+				    bool *conflicted)
 {
 	struct object_id stash_oid;
 	char stash_oid_hex[GIT_MAX_HEXSZ + 1];
 	int flag, ret;
 
+	if (conflicted)
+		*conflicted = false;
+
 	if (!refs_ref_exists(get_main_ref_store(r), refname))
 		return 0;
 
@@ -4858,7 +4866,7 @@ static int apply_save_autostash_ref(struct repository *r, const char *refname,
 	oid_to_hex_r(stash_oid_hex, &stash_oid);
 	ret = apply_save_autostash_oid(stash_oid_hex, attempt_apply,
 				       label_ours, label_theirs, label_base,
-				       stash_msg);
+				       stash_msg, conflicted);
 
 	refs_delete_ref(get_main_ref_store(r), "", refname,
 			&stash_oid, REF_NO_DEREF);
@@ -4869,16 +4877,17 @@ static int apply_save_autostash_ref(struct repository *r, const char *refname,
 int save_autostash_ref(struct repository *r, const char *refname)
 {
 	return apply_save_autostash_ref(r, refname, 0,
-					NULL, NULL, NULL, NULL);
+					NULL, NULL, NULL, NULL, NULL);
 }
 
 int apply_autostash_ref(struct repository *r, const char *refname,
 			const char *label_ours, const char *label_theirs,
-			const char *label_base, const char *stash_msg)
+			const char *label_base, const char *stash_msg,
+			bool *conflicted)
 {
 	return apply_save_autostash_ref(r, refname, 1,
 					label_ours, label_theirs, label_base,
-					stash_msg);
+					stash_msg, conflicted);
 }
 
 static int checkout_onto(struct repository *r, struct replay_opts *opts,
diff --git a/sequencer.h b/sequencer.h
index 64a9c7fb1b..b39528b6d0 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -237,7 +237,8 @@ int apply_autostash(const char *path);
 int apply_autostash_oid(const char *stash_oid);
 int apply_autostash_ref(struct repository *r, const char *refname,
 			const char *label_ours, const char *label_theirs,
-			const char *label_base, const char *stash_msg);
+			const char *label_base, const char *stash_msg,
+			bool *conflicted);
 
 #define SUMMARY_INITIAL_COMMIT   (1 << 0)
 #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
-- 
gitgitgadget

Re: [PATCH 1/2] sequencer: teach autostash apply to report conflicts

[Phillip Wood]

Hi Harald

On 25/07/2026 16:34, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> Add a conflicted parameter to apply_save_autostash_oid() and
> apply_save_autostash_ref() so callers can learn whether applying the
> stash resulted in conflicts.  Thread the parameter through
> apply_autostash_ref() and update existing callers to pass NULL.

It would be nicer to use the return value to indicate 
success/error/conflicts rather than adding yet another parameter. Apart 
from that this looks good.

Thanks

Phillip

> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>   builtin/checkout.c |  3 ++-
>   builtin/commit.c   |  2 +-
>   builtin/merge.c    |  6 +++---
>   sequencer.c        | 29 +++++++++++++++++++----------
>   sequencer.h        |  3 ++-
>   5 files changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index aee84ca897..72aafa4049 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -1242,7 +1242,8 @@ static int switch_branches(const struct checkout_opts *opts,
>   					    new_branch_info->name,
>   					    "local",
>   					    stash_label_base,
> -					    autostash_msg.buf);
> +					    autostash_msg.buf,
> +					    NULL);
>   		}
>   		if (ret) {
>   			branch_info_release(&old_branch_info);
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 28f6174503..d678a81865 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -1980,7 +1980,7 @@ int cmd_commit(int argc,
>   	}
>   
>   	apply_autostash_ref(the_repository, "MERGE_AUTOSTASH",
> -			    NULL, NULL, NULL, NULL);
> +			    NULL, NULL, NULL, NULL, NULL);
>   
>   cleanup:
>   	free_commit_extra_headers(extra);
> diff --git a/builtin/merge.c b/builtin/merge.c
> index 5b46a596f0..cecb8fb716 100644
> --- a/builtin/merge.c
> +++ b/builtin/merge.c
> @@ -538,7 +538,7 @@ static void finish(struct commit *head_commit,
>   
>   	if (new_head)
>   		apply_autostash_ref(the_repository, "MERGE_AUTOSTASH",
> -				    NULL, NULL, NULL, NULL);
> +				    NULL, NULL, NULL, NULL, NULL);
>   	strbuf_release(&reflog_message);
>   }
>   
> @@ -1680,7 +1680,7 @@ int cmd_merge(int argc,
>   					  &commit->object.oid,
>   					  overwrite_ignore)) {
>   			apply_autostash_ref(the_repository, "MERGE_AUTOSTASH",
> -					    NULL, NULL, NULL, NULL);
> +					    NULL, NULL, NULL, NULL, NULL);
>   			ret = 1;
>   			goto done;
>   		}
> @@ -1844,7 +1844,7 @@ int cmd_merge(int argc,
>   			fprintf(stderr, _("Merge with strategy %s failed.\n"),
>   				use_strategies[0]->name);
>   		apply_autostash_ref(the_repository, "MERGE_AUTOSTASH",
> -				    NULL, NULL, NULL, NULL);
> +				    NULL, NULL, NULL, NULL, NULL);
>   		ret = 2;
>   		goto done;
>   	} else if (best_strategy == wt_strategy)
> diff --git a/sequencer.c b/sequencer.c
> index 1355a99a09..91a70e39a1 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4748,7 +4748,8 @@ void create_autostash_ref(struct repository *r, const char *refname,
>   static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
>   				    const char *label_ours, const char *label_theirs,
>   				    const char *label_base,
> -				    const char *stash_msg)
> +				    const char *stash_msg,
> +				    bool *conflicted)
>   {
>   	struct child_process child = CHILD_PROCESS_INIT;
>   	int ret = 0;
> @@ -4783,14 +4784,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
>   		strvec_push(&store.args, stash_oid);
>   		if (run_command(&store))
>   			ret = error(_("cannot store %s"), stash_oid);
> -		else if (attempt_apply)
> +		else if (attempt_apply) {
> +			if (conflicted)
> +				*conflicted = true;
>   			fprintf(stderr,
>   				_("Your local changes are stashed, however applying them\n"
>   				  "resulted in conflicts.  You can either resolve the conflicts\n"
>   				  "and then discard the stash with \"git stash drop\", or, if you\n"
>   				  "do not want to resolve them now, run \"git reset --hard\" and\n"
>   				  "apply the local changes later by running \"git stash pop\".\n"));
> -		else
> +		} else
>   			fprintf(stderr,
>   				_("Autostash exists; creating a new stash entry.\n"
>   				  "Your changes are safe in the stash.\n"
> @@ -4814,7 +4817,7 @@ static int apply_save_autostash(const char *path, int attempt_apply)
>   	strbuf_trim(&stash_oid);
>   
>   	ret = apply_save_autostash_oid(stash_oid.buf, attempt_apply,
> -				      NULL, NULL, NULL, NULL);
> +				      NULL, NULL, NULL, NULL, NULL);
>   
>   	unlink(path);
>   	strbuf_release(&stash_oid);
> @@ -4833,19 +4836,24 @@ int apply_autostash(const char *path)
>   
>   int apply_autostash_oid(const char *stash_oid)
>   {
> -	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL);
> +	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL,
> +				       NULL);
>   }
>   
>   static int apply_save_autostash_ref(struct repository *r, const char *refname,
>   				    int attempt_apply,
>   				    const char *label_ours, const char *label_theirs,
>   				    const char *label_base,
> -				    const char *stash_msg)
> +				    const char *stash_msg,
> +				    bool *conflicted)
>   {
>   	struct object_id stash_oid;
>   	char stash_oid_hex[GIT_MAX_HEXSZ + 1];
>   	int flag, ret;
>   
> +	if (conflicted)
> +		*conflicted = false;
> +
>   	if (!refs_ref_exists(get_main_ref_store(r), refname))
>   		return 0;
>   
> @@ -4858,7 +4866,7 @@ static int apply_save_autostash_ref(struct repository *r, const char *refname,
>   	oid_to_hex_r(stash_oid_hex, &stash_oid);
>   	ret = apply_save_autostash_oid(stash_oid_hex, attempt_apply,
>   				       label_ours, label_theirs, label_base,
> -				       stash_msg);
> +				       stash_msg, conflicted);
>   
>   	refs_delete_ref(get_main_ref_store(r), "", refname,
>   			&stash_oid, REF_NO_DEREF);
> @@ -4869,16 +4877,17 @@ static int apply_save_autostash_ref(struct repository *r, const char *refname,
>   int save_autostash_ref(struct repository *r, const char *refname)
>   {
>   	return apply_save_autostash_ref(r, refname, 0,
> -					NULL, NULL, NULL, NULL);
> +					NULL, NULL, NULL, NULL, NULL);
>   }
>   
>   int apply_autostash_ref(struct repository *r, const char *refname,
>   			const char *label_ours, const char *label_theirs,
> -			const char *label_base, const char *stash_msg)
> +			const char *label_base, const char *stash_msg,
> +			bool *conflicted)
>   {
>   	return apply_save_autostash_ref(r, refname, 1,
>   					label_ours, label_theirs, label_base,
> -					stash_msg);
> +					stash_msg, conflicted);
>   }
>   
>   static int checkout_onto(struct repository *r, struct replay_opts *opts,
> diff --git a/sequencer.h b/sequencer.h
> index 64a9c7fb1b..b39528b6d0 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -237,7 +237,8 @@ int apply_autostash(const char *path);
>   int apply_autostash_oid(const char *stash_oid);
>   int apply_autostash_ref(struct repository *r, const char *refname,
>   			const char *label_ours, const char *label_theirs,
> -			const char *label_base, const char *stash_msg);
> +			const char *label_base, const char *stash_msg,
> +			bool *conflicted);
>   
>   #define SUMMARY_INITIAL_COMMIT   (1 << 0)
>   #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)

Re: [PATCH 1/2] sequencer: teach autostash apply to report conflicts

[Harald Nordgren]

> > Add a conflicted parameter to apply_save_autostash_oid() and
> > apply_save_autostash_ref() so callers can learn whether applying the
> > stash resulted in conflicts.  Thread the parameter through
> > apply_autostash_ref() and update existing callers to pass NULL.
>
> It would be nicer to use the return value to indicate
> success/error/conflicts rather than adding yet another parameter. Apart
> from that this looks good.

Is the int flexible enough to separate autostash conflict from other
errors? Or should we introduce an enum here?


Harald

Re: [PATCH 1/2] sequencer: teach autostash apply to report conflicts

[phillip.wood123]

On 31/08/2026 11:18, Harald Nordgren wrote:
>>> Add a conflicted parameter to apply_save_autostash_oid() and
>>> apply_save_autostash_ref() so callers can learn whether applying the
>>> stash resulted in conflicts.  Thread the parameter through
>>> apply_autostash_ref() and update existing callers to pass NULL.
>>
>> It would be nicer to use the return value to indicate
>> success/error/conflicts rather than adding yet another parameter. Apart
>> from that this looks good.
> 
> Is the int flexible enough to separate autostash conflict from other
> errors? Or should we introduce an enum here?

I think a tri-state enum would be clearest - the callers are only 
interested it "stash applied without conflicts", "stash applied with 
conflicts" and "something went wrong - could not apply stash"

Thanks

Phillip

[PATCH 2/2] checkout -m: refine autostash fallback

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

When unpack_trees() fails under "git checkout -m", only create an
autostash and retry if there are tracked local changes.  Without such
changes, the fallback cannot help and merely repeats the same failure.

Use the conflict result from apply_autostash_ref() to print a blank line
before the branch-switch message, visually separating it from the
conflict advice.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/checkout.c | 17 +++++++++++++----
 t/t7201-co.sh      | 17 ++++++++++++++++-
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 72aafa4049..2e8b2a2348 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -838,10 +838,11 @@ static void init_topts(struct unpack_trees_options *topts,
 static int merge_working_tree(const struct checkout_opts *opts,
 			      struct branch_info *old_branch_info,
 			      struct branch_info *new_branch_info,
-			      bool quiet,
+			      bool allow_autostash,
 			      int *writeout_error)
 {
 	int ret;
+	bool can_autostash = false;
 	struct lock_file lock_file = LOCK_INIT;
 	struct tree *new_tree;
 
@@ -888,9 +889,13 @@ static int merge_working_tree(const struct checkout_opts *opts,
 			return 1;
 		}
 
+		if (allow_autostash)
+			can_autostash = has_unstaged_changes(the_repository, 1) ||
+				has_uncommitted_changes(the_repository, 1);
+
 		/* 2-way merge to the new branch */
 		init_topts(&topts, opts->show_progress,
-			   opts->overwrite_ignore, quiet);
+			   opts->overwrite_ignore, can_autostash);
 		init_checkout_metadata(&topts.meta, new_branch_info->refname,
 				       new_branch_info->commit ?
 				       &new_branch_info->commit->object.oid :
@@ -917,7 +922,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
 		clear_unpack_trees_porcelain(&topts);
 		if (ret == -1) {
 			rollback_lock_file(&lock_file);
-			return MERGE_WORKING_TREE_UNPACK_FAILED;
+			return can_autostash ?
+				MERGE_WORKING_TREE_UNPACK_FAILED : 1;
 		}
 	}
 
@@ -1166,6 +1172,7 @@ static int switch_branches(const struct checkout_opts *opts,
 	int flag, writeout_error = 0;
 	int do_merge = 1;
 	int created_autostash = 0;
+	bool autostash_conflicted = false;
 	struct strbuf old_commit_shortname = STRBUF_INIT;
 	struct strbuf autostash_msg = STRBUF_INIT;
 	const char *stash_label_base = NULL;
@@ -1243,7 +1250,7 @@ static int switch_branches(const struct checkout_opts *opts,
 					    "local",
 					    stash_label_base,
 					    autostash_msg.buf,
-					    NULL);
+					    &autostash_conflicted);
 		}
 		if (ret) {
 			branch_info_release(&old_branch_info);
@@ -1256,6 +1263,8 @@ static int switch_branches(const struct checkout_opts *opts,
 	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
 		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
 
+	if (autostash_conflicted && !opts->quiet)
+		fputc('\n', stderr);
 	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
 
 	if (created_autostash) {
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 0ddd1ad7aa..f9696dab36 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -240,6 +240,14 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
 	test_grep "git stash drop" actual &&
 	test_grep "git stash pop" actual &&
 	test_grep "The following paths have local changes" actual &&
+	sed -n "/apply the local changes later/,/Switched to branch/p" \
+		actual >separator.actual &&
+	cat >separator.expect <<-EOF &&
+	apply the local changes later by running "git stash pop".
+
+	Switched to branch ${SQ}side${SQ}
+	EOF
+	test_cmp separator.expect separator.actual &&
 	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
 	sed /^index/d actual >actual.trimmed &&
 	cat >expect <<-EOF &&
@@ -262,11 +270,18 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
 	git reset --hard
 '
 
-test_expect_success 'checkout -m which would overwrite untracked file' '
+test_expect_success 'checkout -m only retries untracked-file failure with local changes' '
 	git checkout -f --detach main &&
 	test_commit another-file &&
 	git checkout HEAD^ &&
 	>another-file.t &&
+	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/trace" \
+		git checkout -m @{-1} 2>err &&
+	test_grep "untracked working tree files" err &&
+	grep "\"region_enter\".*\"category\":\"index\",\"label\":\"refresh\"" \
+		trace >refresh.events &&
+	test_line_count = 1 refresh.events &&
+
 	fill 1 2 3 4 5 >one &&
 	test_must_fail git checkout -m @{-1} 2>err &&
 	q_to_tab >expect <<-\EOF &&
-- 
gitgitgadget

Re: [PATCH 2/2] checkout -m: refine autostash fallback

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> When unpack_trees() fails under "git checkout -m", only create an
> autostash and retry if there are tracked local changes.  Without such
> changes, the fallback cannot help and merely repeats the same failure.
>
> Use the conflict result from apply_autostash_ref() to print a blank line
> before the branch-switch message, visually separating it from the
> conflict advice.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>  builtin/checkout.c | 17 +++++++++++++----
>  t/t7201-co.sh      | 17 ++++++++++++++++-
>  2 files changed, 29 insertions(+), 5 deletions(-)

Thanks.  Will queue.

Re: [PATCH 2/2] checkout -m: refine autostash fallback

[Phillip Wood]

Hi Harald

On 25/07/2026 16:34, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> When unpack_trees() fails under "git checkout -m", only create an
> autostash and retry if there are tracked local changes.  Without such
> changes, the fallback cannot help and merely repeats the same failure.

Unfortunately to do that we have to look for local changes before the 
first call to unpack_trees() so we're trading an occasional 
inconvenience of an unnecessary stash and unstash for the cost of 
looking for local changes on every invocation of "git checkout -m". I 
don't think that is a good trade off, especially as there is no 
guarantee that stashing the local changes will make unpack_trees() 
succeed. To do this effectively would require refactoring unpack_trees() 
to write its error messages to a buffer and return an error flag that 
indicates all the errors that were encountered. We could then check if 
the only thing that prevented upack_trees() from succeeding was local 
changes to files and stash them, or if there are other errors print the 
error message. I suspect such a change is far from straight forward.

> Use the conflict result from apply_autostash_ref() to print a blank line
> before the branch-switch message, visually separating it from the
> conflict advice.


This change is very welcome and could happily be squashed into the first 
patch as it motivates the changes in it.

> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 0ddd1ad7aa..f9696dab36 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -240,6 +240,14 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
>   	test_grep "git stash drop" actual &&
>   	test_grep "git stash pop" actual &&
>   	test_grep "The following paths have local changes" actual &&
> +	sed -n "/apply the local changes later/,/Switched to branch/p" \
> +		actual >separator.actual &&
> +	cat >separator.expect <<-EOF &&
> +	apply the local changes later by running "git stash pop".
> +
> +	Switched to branch ${SQ}side${SQ}
> +	EOF
> +	test_cmp separator.expect separator.actual &&

I wonder whether we should just bite the bullet and check what gets 
printed to the screen with test_cmp, rather than grepping for all these 
separate parts of the message. Is there something in the message that 
makes that difficult?

Thanks

Phillip

>   	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
>   	sed /^index/d actual >actual.trimmed &&
>   	cat >expect <<-EOF &&
> @@ -262,11 +270,18 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
>   	git reset --hard
>   '
>   
> -test_expect_success 'checkout -m which would overwrite untracked file' '
> +test_expect_success 'checkout -m only retries untracked-file failure with local changes' '
>   	git checkout -f --detach main &&
>   	test_commit another-file &&
>   	git checkout HEAD^ &&
>   	>another-file.t &&
> +	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/trace" \
> +		git checkout -m @{-1} 2>err &&
> +	test_grep "untracked working tree files" err &&
> +	grep "\"region_enter\".*\"category\":\"index\",\"label\":\"refresh\"" \
> +		trace >refresh.events &&
> +	test_line_count = 1 refresh.events &&
> +
>   	fill 1 2 3 4 5 >one &&
>   	test_must_fail git checkout -m @{-1} 2>err &&
>   	q_to_tab >expect <<-\EOF &&

Re: [PATCH 0/2] checkout -m: refine autostash fallback

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Avoiding checkout -m autostash retries when no tracked local changes exist
> and visually separating autostash conflict advice from the subsequent
> branch-switch message.
>
> Addresses #leftoverbits from here:
> https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/
>
> Harald Nordgren (2):
>   sequencer: teach autostash apply to report conflicts
>   checkout -m: refine autostash fallback
>
>  builtin/checkout.c | 18 ++++++++++++++----
>  builtin/commit.c   |  2 +-
>  builtin/merge.c    |  6 +++---
>  sequencer.c        | 29 +++++++++++++++++++----------
>  sequencer.h        |  3 ++-
>  t/t7201-co.sh      | 17 ++++++++++++++++-
>  6 files changed, 55 insertions(+), 20 deletions(-)
>
>
> base-commit: 9a0c4701dcd5725c4184599322b52933ff5005ca
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v1
> Pull-Request: https://github.com/git/git/pull/2364

This topic unfortunately has seen no interests from others on the
list.  Asking Phillip for help, as the leftoverbits comment cited in
the cover letter is from him.

Thanks.

Re: [PATCH 0/2] checkout -m: refine autostash fallback

[Phillip Wood]

On 26/08/2026 20:14, Junio C Hamano wrote:
>
> This topic unfortunately has seen no interests from others on the
> list.  Asking Phillip for help, as the leftoverbits comment cited in
> the cover letter is from him.

That'll teach me to make suggestions for further work when I'm reviewing 
patches! Thanks for following up on those comments Harald. I've left 
some comments, in short I think the changes to the error message are a 
good idea, but I'm not sure sure about the other change in patch 2 though.

Thanks

Phillip

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

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

When "git checkout -m" stashes the user's local changes and then fails
to re-apply them because of conflicts, the conflict advice is printed
directly on top of the branch-switch message ("Switched to branch ..."),
making the two messages hard to tell apart.  Print a blank line in
between when the stash apply conflicted, so that the advice and the
branch-switch message are visually distinct.

To learn whether the stash apply resulted in conflicts, make the
autostash apply functions return 1 when the stash could not be applied
due to conflicts (and was stored back), while success and error remain
0 and -1 respectively.  Checkout can then use the return value to decide
whether to print the blank line.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    checkout -m: refine autostash fallback
    
    Avoiding checkout -m autostash retries when no tracked local changes
    exist and visually separating autostash conflict advice from the
    subsequent branch-switch message. Addresses #leftoverbits from here:
    https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/
    
    Changes in v2:
    
     * Simplify logic and combine to one commit.
     * Test full output with test_cmp.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v2
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v1:

 1:  b501b5fcd0 < -:  ---------- sequencer: teach autostash apply to report conflicts
 2:  37becf38c2 ! 1:  b44c53fcf2 checkout -m: refine autostash fallback
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    checkout -m: refine autostash fallback
     +    checkout: print blank line after autostash conflict advice
      
     -    When unpack_trees() fails under "git checkout -m", only create an
     -    autostash and retry if there are tracked local changes.  Without such
     -    changes, the fallback cannot help and merely repeats the same failure.
     +    When "git checkout -m" stashes the user's local changes and then fails
     +    to re-apply them because of conflicts, the conflict advice is printed
     +    directly on top of the branch-switch message ("Switched to branch ..."),
     +    making the two messages hard to tell apart.  Print a blank line in
     +    between when the stash apply conflicted, so that the advice and the
     +    branch-switch message are visually distinct.
      
     -    Use the conflict result from apply_autostash_ref() to print a blank line
     -    before the branch-switch message, visually separating it from the
     -    conflict advice.
     +    To learn whether the stash apply resulted in conflicts, make the
     +    autostash apply functions return 1 when the stash could not be applied
     +    due to conflicts (and was stored back), while success and error remain
     +    0 and -1 respectively.  Checkout can then use the return value to decide
     +    whether to print the blank line.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## builtin/checkout.c ##
     -@@ builtin/checkout.c: static void init_topts(struct unpack_trees_options *topts,
     - static int merge_working_tree(const struct checkout_opts *opts,
     - 			      struct branch_info *old_branch_info,
     - 			      struct branch_info *new_branch_info,
     --			      bool quiet,
     -+			      bool allow_autostash,
     - 			      int *writeout_error)
     - {
     - 	int ret;
     -+	bool can_autostash = false;
     - 	struct lock_file lock_file = LOCK_INIT;
     - 	struct tree *new_tree;
     - 
     -@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
     - 			return 1;
     - 		}
     - 
     -+		if (allow_autostash)
     -+			can_autostash = has_unstaged_changes(the_repository, 1) ||
     -+				has_uncommitted_changes(the_repository, 1);
     -+
     - 		/* 2-way merge to the new branch */
     - 		init_topts(&topts, opts->show_progress,
     --			   opts->overwrite_ignore, quiet);
     -+			   opts->overwrite_ignore, can_autostash);
     - 		init_checkout_metadata(&topts.meta, new_branch_info->refname,
     - 				       new_branch_info->commit ?
     - 				       &new_branch_info->commit->object.oid :
     -@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
     - 		clear_unpack_trees_porcelain(&topts);
     - 		if (ret == -1) {
     - 			rollback_lock_file(&lock_file);
     --			return MERGE_WORKING_TREE_UNPACK_FAILED;
     -+			return can_autostash ?
     -+				MERGE_WORKING_TREE_UNPACK_FAILED : 1;
     - 		}
     - 	}
     - 
      @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       	int flag, writeout_error = 0;
       	int do_merge = 1;
       	int created_autostash = 0;
     -+	bool autostash_conflicted = false;
     ++	int autostash_res = 0;
       	struct strbuf old_commit_shortname = STRBUF_INIT;
       	struct strbuf autostash_msg = STRBUF_INIT;
       	const char *stash_label_base = NULL;
      @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
     - 					    "local",
     - 					    stash_label_base,
     - 					    autostash_msg.buf,
     --					    NULL);
     -+					    &autostash_conflicted);
     + 				git_config_push_parameter(cfg.buf);
     + 				strbuf_release(&cfg);
     + 			}
     +-			apply_autostash_ref(the_repository,
     +-					    "CHECKOUT_AUTOSTASH_HEAD",
     +-					    new_branch_info->name,
     +-					    "local",
     +-					    stash_label_base,
     +-					    autostash_msg.buf);
     ++			autostash_res = apply_autostash_ref(the_repository,
     ++				    "CHECKOUT_AUTOSTASH_HEAD",
     ++				    new_branch_info->name,
     ++				    "local",
     ++				    stash_label_base,
     ++				    autostash_msg.buf);
       		}
       		if (ret) {
       			branch_info_release(&old_branch_info);
     @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
       		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
       
     -+	if (autostash_conflicted && !opts->quiet)
     ++	if (autostash_res == 1 && !opts->quiet)
      +		fputc('\n', stderr);
       	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
       
       	if (created_autostash) {
      
     + ## sequencer.c ##
     +@@ sequencer.c: 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;
     + 	}
     + 
     + 	if (attempt_apply && !ret)
     +
       ## t/t7201-co.sh ##
      @@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '
     - 	test_grep "git stash drop" actual &&
     - 	test_grep "git stash pop" actual &&
     - 	test_grep "The following paths have local changes" actual &&
     -+	sed -n "/apply the local changes later/,/Switched to branch/p" \
     -+		actual >separator.actual &&
     -+	cat >separator.expect <<-EOF &&
     + 	test_must_fail git checkout side 2>stderr &&
     + 	test_grep "Your local changes" stderr &&
     + 	git checkout -m side >actual 2>&1 &&
     +-	test_grep "resulted in conflicts" actual &&
     +-	test_grep "git stash drop" actual &&
     +-	test_grep "git stash pop" actual &&
     +-	test_grep "The following paths have local changes" actual &&
     ++	cat >expect <<-EOF &&
     ++	Your local changes are stashed, however applying them
     ++	resulted in conflicts.  You can either resolve the conflicts
     ++	and then discard the stash with "git stash drop", or, if you
     ++	do not want to resolve them now, run "git reset --hard" and
      +	apply the local changes later by running "git stash pop".
      +
      +	Switched to branch ${SQ}side${SQ}
     ++	The following paths have local changes:
     ++	M	one
      +	EOF
     -+	test_cmp separator.expect separator.actual &&
     ++	test_cmp expect actual &&
       	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
       	sed /^index/d actual >actual.trimmed &&
       	cat >expect <<-EOF &&
     -@@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '
     - 	git reset --hard
     - '
     - 
     --test_expect_success 'checkout -m which would overwrite untracked file' '
     -+test_expect_success 'checkout -m only retries untracked-file failure with local changes' '
     - 	git checkout -f --detach main &&
     - 	test_commit another-file &&
     - 	git checkout HEAD^ &&
     - 	>another-file.t &&
     -+	test_must_fail env GIT_TRACE2_EVENT="$(pwd)/trace" \
     -+		git checkout -m @{-1} 2>err &&
     -+	test_grep "untracked working tree files" err &&
     -+	grep "\"region_enter\".*\"category\":\"index\",\"label\":\"refresh\"" \
     -+		trace >refresh.events &&
     -+	test_line_count = 1 refresh.events &&
     -+
     - 	fill 1 2 3 4 5 >one &&
     - 	test_must_fail git checkout -m @{-1} 2>err &&
     - 	q_to_tab >expect <<-\EOF &&


 builtin/checkout.c | 15 +++++++++------
 sequencer.c        |  3 ++-
 t/t7201-co.sh      | 16 ++++++++++++----
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 55e3a89a85..e4b7383f10 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1166,6 +1166,7 @@ static int switch_branches(const struct checkout_opts *opts,
 	int flag, writeout_error = 0;
 	int do_merge = 1;
 	int created_autostash = 0;
+	int autostash_res = 0;
 	struct strbuf old_commit_shortname = STRBUF_INIT;
 	struct strbuf autostash_msg = STRBUF_INIT;
 	const char *stash_label_base = NULL;
@@ -1237,12 +1238,12 @@ static int switch_branches(const struct checkout_opts *opts,
 				git_config_push_parameter(cfg.buf);
 				strbuf_release(&cfg);
 			}
-			apply_autostash_ref(the_repository,
-					    "CHECKOUT_AUTOSTASH_HEAD",
-					    new_branch_info->name,
-					    "local",
-					    stash_label_base,
-					    autostash_msg.buf);
+			autostash_res = apply_autostash_ref(the_repository,
+				    "CHECKOUT_AUTOSTASH_HEAD",
+				    new_branch_info->name,
+				    "local",
+				    stash_label_base,
+				    autostash_msg.buf);
 		}
 		if (ret) {
 			branch_info_release(&old_branch_info);
@@ -1255,6 +1256,8 @@ static int switch_branches(const struct checkout_opts *opts,
 	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
 		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
 
+	if (autostash_res == 1 && !opts->quiet)
+		fputc('\n', stderr);
 	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
 
 	if (created_autostash) {
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;
 	}
 
 	if (attempt_apply && !ret)
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 0ddd1ad7aa..9ea9462914 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -236,10 +236,18 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
 	test_must_fail git checkout side 2>stderr &&
 	test_grep "Your local changes" stderr &&
 	git checkout -m side >actual 2>&1 &&
-	test_grep "resulted in conflicts" actual &&
-	test_grep "git stash drop" actual &&
-	test_grep "git stash pop" actual &&
-	test_grep "The following paths have local changes" actual &&
+	cat >expect <<-EOF &&
+	Your local changes are stashed, however applying them
+	resulted in conflicts.  You can either resolve the conflicts
+	and then discard the stash with "git stash drop", or, if you
+	do not want to resolve them now, run "git reset --hard" and
+	apply the local changes later by running "git stash pop".
+
+	Switched to branch ${SQ}side${SQ}
+	The following paths have local changes:
+	M	one
+	EOF
+	test_cmp expect actual &&
 	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
 	sed /^index/d actual >actual.trimmed &&
 	cat >expect <<-EOF &&

base-commit: c73e85354c275c9d409b26445089bc16940fc527
-- 
gitgitgadget

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

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> To learn whether the stash apply resulted in conflicts, make the
> autostash apply functions return 1 when the stash could not be applied
> due to conflicts (and was stored back), while success and error remain
> 0 and -1 respectively.  Checkout can then use the return value to decide
> whether to print the blank line.

> @@ -1237,12 +1238,12 @@ static int switch_branches(const struct checkout_opts *opts,
>  				git_config_push_parameter(cfg.buf);
>  				strbuf_release(&cfg);
>  			}
> -			apply_autostash_ref(the_repository,
> -					    "CHECKOUT_AUTOSTASH_HEAD",
> -					    new_branch_info->name,
> -					    "local",
> -					    stash_label_base,
> -					    autostash_msg.buf);
> +			autostash_res = apply_autostash_ref(the_repository,
> +				    "CHECKOUT_AUTOSTASH_HEAD",
> +				    new_branch_info->name,
> +				    "local",
> +				    stash_label_base,
> +				    autostash_msg.buf);
>  		}
>  		if (ret) {
>  			branch_info_release(&old_branch_info);
> @@ -1255,6 +1256,8 @@ static int switch_branches(const struct checkout_opts *opts,
>  	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
>  		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
>  
> +	if (autostash_res == 1 && !opts->quiet)
> +		fputc('\n', stderr);
>  	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
>  
>  	if (created_autostash) {
> 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.

The reasons for a non-zero return value from run_command() range
from failing to find the command to run, to the command crashing, to
the command voluntarily exiting with a fixed exit status to signal
how exactly it exited to the spawning process.

Skimming bultin/stash.c::do_apply_stash(), I find this bit:

	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);

	/*
	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
	 * merge was clean, and nonzero if the merge was unclean or encountered
	 * an error.
	 */
	ret = clean >= 0 ? !clean : clean;

If 'clean' is 0 (i.e., unclean merge), we assign 1 (== !clean) to
ret.  If 'clean' is 1 or more (presumably a clean merge?  Check what
merge_ort_nonrecursive() does for details---I didn't), return 0.  If
merge-ort gave an error (i.e., negative), use it as-is.  And this
'ret' is eventually returned to the caller apply_stash() , which
returns that value as-is.

The value is given back to builtin/stash.c::cmd_stash() and the more
detailed exit value unfortunately discarded ehre:

	if (fn)
		return !!fn(argc, argv, prefix, repo);

but if one wants to implement 

    > To learn whether the stash apply resulted in conflicts, make the
    > autostash apply functions return 1 when the stash could not be applied
    > due to conflicts (and was stored back),...

correctly, I think this patch needs a bit more work.  

 (1) Find the values returned by foo_stash() implementations of all
     'git stash foo' subcommands.  Do they follow the 0 == success,
     negative == failure, positive == something else convention?

 (2) Update the code above to something like

	if (fn) {
		int ret = fn(argc, argv, prefix, repo);

		if (ret < 0)
			return 1; /* as before */
		if (!ret)
			return 0; /* as before */
		return ret + 1;
	}

      or with whatever necessary tweak to allow the positive "I have
      something to tell the spawning process" case from the bog
      standard and boring 'return error("message");' cases.

 (3) document what each of 'git stash foo' signals with its exit
     status.

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.

Thanks.

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

[Phillip Wood]

On 31/08/2026 18:19, Junio C Hamano wrote:
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> 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

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

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> 	ret = run_command(&child);
> 	if (ret > 1)
> 		ret = -1;
>
> would catch run_command() failing and stash dying or being killed by a 
> signal.

;-)

[PATCH v3] checkout: separate autostash conflict advice from branch-switch message

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

"git checkout -m" stashes the user's local changes when it cannot
perform the checkout, and then applies the stash.  When applying the
stash results in conflicts, the advice on how to deal with them is
printed directly on top of the branch-switch message ("Switched to
branch ..."), making the two hard to tell apart.  Print a blank line
in between so that the advice and the branch-switch message are
visually distinct.

To make this possible, "git stash apply", "pop" and "branch" now exit
with status 2 when applying the stash entry resulted in conflicts, in
which case the stash entry is left in place; other failures exit with
status 1, as before.  The exit statuses are documented in the "git
stash" documentation.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    checkout -m: refine autostash fallback
    
    Avoiding checkout -m autostash retries when no tracked local changes
    exist and visually separating autostash conflict advice from the
    subsequent branch-switch message. Addresses #leftoverbits from here:
    https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/
    
    Changes in v3:
    
     * Use enum for git stash return values, to separate conflict from
       generic error.
    
    Changes in v2:
    
     * Simplify logic and combine to one commit.
     * Test full output with test_cmp.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v3
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v2:

 1:  b44c53fcf2 ! 1:  8e1979dd6c checkout: print blank line after autostash conflict advice
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    checkout: print blank line after autostash conflict advice
     +    checkout: separate autostash conflict advice from branch-switch message
      
     -    When "git checkout -m" stashes the user's local changes and then fails
     -    to re-apply them because of conflicts, the conflict advice is printed
     -    directly on top of the branch-switch message ("Switched to branch ..."),
     -    making the two messages hard to tell apart.  Print a blank line in
     -    between when the stash apply conflicted, so that the advice and the
     -    branch-switch message are visually distinct.
     +    "git checkout -m" stashes the user's local changes when it cannot
     +    perform the checkout, and then applies the stash.  When applying the
     +    stash results in conflicts, the advice on how to deal with them is
     +    printed directly on top of the branch-switch message ("Switched to
     +    branch ..."), making the two hard to tell apart.  Print a blank line
     +    in between so that the advice and the branch-switch message are
     +    visually distinct.
      
     -    To learn whether the stash apply resulted in conflicts, make the
     -    autostash apply functions return 1 when the stash could not be applied
     -    due to conflicts (and was stored back), while success and error remain
     -    0 and -1 respectively.  Checkout can then use the return value to decide
     -    whether to print the blank line.
     +    To make this possible, "git stash apply", "pop" and "branch" now exit
     +    with status 2 when applying the stash entry resulted in conflicts, in
     +    which case the stash entry is left in place; other failures exit with
     +    status 1, as before.  The exit statuses are documented in the "git
     +    stash" documentation.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     + ## Documentation/git-stash.adoc ##
     +@@ Documentation/git-stash.adoc: include::includes/cmd-config-section-all.adoc[]
     + :git-stash: 1
     + include::config/stash.adoc[]
     + 
     ++EXIT STATUS
     ++-----------
     ++
     ++The `git stash` subcommands exit with status 0 on success and non-zero
     ++on failure.  The subcommands that apply a stash entry, i.e. `apply`,
     ++`pop` and `branch`, exit with status 2 when applying the stash entry
     ++resulted in conflicts, in which case the stash entry is left in place.
     ++Other failures exit with status 1 (usage errors exit with status 129).
     ++
     + 
     + SEE ALSO
     + --------
     +
       ## builtin/checkout.c ##
      @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       	int flag, writeout_error = 0;
       	int do_merge = 1;
       	int created_autostash = 0;
     -+	int autostash_res = 0;
     ++	enum stash_apply_result autostash_res = STASH_APPLY_CLEAN;
       	struct strbuf old_commit_shortname = STRBUF_INIT;
       	struct strbuf autostash_msg = STRBUF_INIT;
       	const char *stash_label_base = NULL;
     @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
       		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
       
     -+	if (autostash_res == 1 && !opts->quiet)
     ++	if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet)
      +		fputc('\n', stderr);
       	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
       
       	if (created_autostash) {
      
     + ## builtin/stash.c ##
     +@@
     + #include "object-name.h"
     + #include "parse-options.h"
     + #include "refs.h"
     ++#include "stash.h"
     + #include "lockfile.h"
     + #include "cache-tree.h"
     + #include "unpack-trees.h"
     +@@ builtin/stash.c: static void unstage_changes_unless_new(struct object_id *orig_tree)
     + 		die(_("could not write index"));
     + }
     + 
     +-static int do_apply_stash(const char *prefix, struct stash_info *info,
     +-			  int index, int quiet,
     ++static enum stash_apply_result do_apply_stash(const char *prefix,
     ++					      struct stash_info *info,
     ++					      int index, int quiet,
     + 			  const char *label_ours, const char *label_theirs,
     + 			  const char *label_base)
     + {
     +@@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
     + 	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);
     + 
     + 	/*
     +-	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
     +-	 * merge was clean, and nonzero if the merge was unclean or encountered
     +-	 * an error.
     ++	 * Translate the value of 'clean' so 'ret' is STASH_APPLY_CLEAN
     ++	 * when the merge was clean, STASH_APPLY_CONFLICT when it was
     ++	 * unclean, and a negative value if it encountered an error.
     + 	 */
     +-	ret = clean >= 0 ? !clean : clean;
     ++	ret = clean >= 0 ? (clean ? STASH_APPLY_CLEAN : STASH_APPLY_CONFLICT)
     ++			 : clean;
     + 
     + 	if (ret < 0)
     + 		rollback_lock_file(&lock);
     +@@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
     + 
     + 	if (has_index) {
     + 		if (reset_tree(&index_tree, 0, 0))
     +-			ret = -1;
     ++			ret = STASH_APPLY_ERROR;
     + 	} else {
     + 		unstage_changes_unless_new(&c_tree);
     + 	}
     +@@ builtin/stash.c: int cmd_stash(int argc,
     + 	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
     + 		    (uintmax_t)pid);
     + 
     +-	if (fn)
     +-		return !!fn(argc, argv, prefix, repo);
     +-	else if (!argc)
     ++	if (fn) {
     ++		ret = fn(argc, argv, prefix, repo);
     ++
     ++		if (ret < 0)
     ++			return 1;
     ++		return ret;
     ++	} else if (!argc)
     + 		return !!push_stash_unassumed(0, NULL, prefix, repo);
     + 
     + 	/* Assume 'stash push' */
     +
       ## sequencer.c ##
     +@@
     + #include "commit.h"
     + #include "sequencer.h"
     + #include "run-command.h"
     ++#include "stash.h"
     + #include "hook.h"
     + #include "utf8.h"
     + #include "cache-tree.h"
     +@@ sequencer.c: void create_autostash_ref(struct repository *r, const char *refname,
     + 	create_autostash_internal(r, NULL, refname, message, silent);
     + }
     + 
     +-static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     +-				    const char *label_ours, const char *label_theirs,
     +-				    const char *label_base,
     +-				    const char *stash_msg)
     ++static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
     ++							int attempt_apply,
     ++							const char *label_ours,
     ++							const char *label_theirs,
     ++							const char *label_base,
     ++							const char *stash_msg)
     + {
     + 	struct child_process child = CHILD_PROCESS_INIT;
     +-	int ret = 0;
     ++	enum stash_apply_result ret = STASH_APPLY_CLEAN;
     + 
     + 	if (attempt_apply) {
     + 		child.git_cmd = 1;
      @@ sequencer.c: 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;
     + 		ret = run_command(&child);
     ++		if (ret && ret != STASH_APPLY_CONFLICT)
     ++			ret = STASH_APPLY_ERROR;
       	}
       
     - 	if (attempt_apply && !ret)
     +-	if (attempt_apply && !ret)
     ++	if (attempt_apply && ret == STASH_APPLY_CLEAN)
     + 		fprintf(stderr, _("Applied autostash.\n"));
     + 	else {
     + 		struct child_process store = CHILD_PROCESS_INIT;
     +@@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     + 		strvec_push(&store.args, stash_oid);
     + 		if (run_command(&store))
     + 			ret = error(_("cannot store %s"), stash_oid);
     +-		else if (attempt_apply)
     ++		else if (attempt_apply && ret == STASH_APPLY_CONFLICT)
     + 			fprintf(stderr,
     + 				_("Your local changes are stashed, however applying them\n"
     + 				  "resulted in conflicts.  You can either resolve the conflicts\n"
     + 				  "and then discard the stash with \"git stash drop\", or, if you\n"
     + 				  "do not want to resolve them now, run \"git reset --hard\" and\n"
     + 				  "apply the local changes later by running \"git stash pop\".\n"));
     ++		else if (attempt_apply)
     ++			ret = error(_("could not apply autostash"));
     + 		else
     + 			fprintf(stderr,
     + 				_("Autostash exists; creating a new stash entry.\n"
     +@@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     + 	return ret;
     + }
     + 
     +-static int apply_save_autostash(const char *path, int attempt_apply)
     ++static enum stash_apply_result apply_save_autostash(const char *path,
     ++						    int attempt_apply)
     + {
     + 	struct strbuf stash_oid = STRBUF_INIT;
     +-	int ret = 0;
     ++	enum stash_apply_result ret = STASH_APPLY_CLEAN;
     + 
     + 	if (!read_oneliner(&stash_oid, path,
     + 			   READ_ONELINER_SKIP_IF_EMPTY)) {
     + 		strbuf_release(&stash_oid);
     +-		return 0;
     ++		return STASH_APPLY_CLEAN;
     + 	}
     + 	strbuf_trim(&stash_oid);
     + 
     +@@ sequencer.c: static int apply_save_autostash(const char *path, int attempt_apply)
     + 	return ret;
     + }
     + 
     +-int save_autostash(const char *path)
     ++enum stash_apply_result save_autostash(const char *path)
     + {
     + 	return apply_save_autostash(path, 0);
     + }
     + 
     +-int apply_autostash(const char *path)
     ++enum stash_apply_result apply_autostash(const char *path)
     + {
     + 	return apply_save_autostash(path, 1);
     + }
     + 
     +-int apply_autostash_oid(const char *stash_oid)
     ++enum stash_apply_result apply_autostash_oid(const char *stash_oid)
     + {
     + 	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL);
     + }
     + 
     +-static int apply_save_autostash_ref(struct repository *r, const char *refname,
     +-				    int attempt_apply,
     +-				    const char *label_ours, const char *label_theirs,
     +-				    const char *label_base,
     +-				    const char *stash_msg)
     ++static enum stash_apply_result apply_save_autostash_ref(struct repository *r,
     ++							const char *refname,
     ++							int attempt_apply,
     ++							const char *label_ours,
     ++							const char *label_theirs,
     ++							const char *label_base,
     ++							const char *stash_msg)
     + {
     + 	struct object_id stash_oid;
     + 	char stash_oid_hex[GIT_MAX_HEXSZ + 1];
     +-	int flag, ret;
     ++	int flag;
     ++	enum stash_apply_result ret;
     + 
     + 	if (!refs_ref_exists(get_main_ref_store(r), refname))
     +-		return 0;
     ++		return STASH_APPLY_CLEAN;
     + 
     + 	if (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname,
     + 				     RESOLVE_REF_READING, &stash_oid, &flag))
     +-		return -1;
     ++		return STASH_APPLY_ERROR;
     + 	if (flag & REF_ISSYMREF)
     + 		return error(_("autostash reference is a symref"));
     + 
     +@@ sequencer.c: static int apply_save_autostash_ref(struct repository *r, const char *refname,
     + 	return ret;
     + }
     + 
     +-int save_autostash_ref(struct repository *r, const char *refname)
     ++enum stash_apply_result save_autostash_ref(struct repository *r,
     ++					   const char *refname)
     + {
     + 	return apply_save_autostash_ref(r, refname, 0,
     + 					NULL, NULL, NULL, NULL);
     + }
     + 
     +-int apply_autostash_ref(struct repository *r, const char *refname,
     +-			const char *label_ours, const char *label_theirs,
     +-			const char *label_base, const char *stash_msg)
     ++enum stash_apply_result apply_autostash_ref(struct repository *r,
     ++					    const char *refname,
     ++					    const char *label_ours,
     ++					    const char *label_theirs,
     ++					    const char *label_base,
     ++					    const char *stash_msg)
     + {
     + 	return apply_save_autostash_ref(r, refname, 1,
     + 					label_ours, label_theirs, label_base,
     +
     + ## sequencer.h ##
     +@@
     + 
     + #include "strbuf.h"
     + #include "strvec.h"
     ++#include "stash.h"
     + #include "wt-status.h"
     + 
     + struct commit;
     +@@ sequencer.h: void commit_post_rewrite(struct repository *r,
     + void create_autostash(struct repository *r, const char *path);
     + void create_autostash_ref(struct repository *r, const char *refname,
     + 			  const char *message, bool silent);
     +-int save_autostash(const char *path);
     +-int save_autostash_ref(struct repository *r, const char *refname);
     +-int apply_autostash(const char *path);
     +-int apply_autostash_oid(const char *stash_oid);
     +-int apply_autostash_ref(struct repository *r, const char *refname,
     +-			const char *label_ours, const char *label_theirs,
     +-			const char *label_base, const char *stash_msg);
     ++enum stash_apply_result save_autostash(const char *path);
     ++enum stash_apply_result save_autostash_ref(struct repository *r,
     ++					   const char *refname);
     ++enum stash_apply_result apply_autostash(const char *path);
     ++enum stash_apply_result apply_autostash_oid(const char *stash_oid);
     ++enum stash_apply_result apply_autostash_ref(struct repository *r,
     ++					    const char *refname,
     ++					    const char *label_ours,
     ++					    const char *label_theirs,
     ++					    const char *label_base,
     ++					    const char *stash_msg);
     + 
     + #define SUMMARY_INITIAL_COMMIT   (1 << 0)
     + #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
     +
     + ## stash.h (new) ##
     +@@
     ++#ifndef STASH_H
     ++#define STASH_H
     ++
     ++enum stash_apply_result {
     ++	/* The stash was applied cleanly, or there was nothing to apply. */
     ++	STASH_APPLY_CLEAN = 0,
     ++
     ++	/*
     ++	 * The stash could not be applied because it resulted in
     ++	 * conflicts.  The stash entry is left in place.  The "git stash
     ++	 * apply", "pop" and "branch" subcommands exit with this status
     ++	 * in this case.
     ++	 */
     ++	STASH_APPLY_CONFLICT = 2,
     ++
     ++	/* Something went wrong. */
     ++	STASH_APPLY_ERROR = -1,
     ++};
     ++
     ++#endif /* STASH_H */
     +
     + ## t/t3903-stash.sh ##
     +@@ t/t3903-stash.sh: test_expect_success 'apply with custom conflict labels' '
     + 	echo stashed >conflict-file &&
     + 	git stash push -m "stashed" &&
     + 	test_commit label-upstream conflict-file upstream-content &&
     +-	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
     ++	test_expect_code 2 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
     + 	test_grep "^<<<<<<< UP" conflict-file &&
     + 	test_grep "^||||||| Stash base" conflict-file &&
     + 	test_grep "^>>>>>>> STASH" conflict-file
     +@@ t/t3903-stash.sh: test_expect_success 'apply with empty conflict labels' '
     + 	echo stashed >conflict-file &&
     + 	git stash push -m "stashed" &&
     + 	test_commit empty-label-upstream conflict-file upstream-content &&
     +-	test_must_fail git stash apply --label-ours= --label-theirs= &&
     ++	test_expect_code 2 git stash apply --label-ours= --label-theirs= &&
     + 	test_grep "^<<<<<<<$" conflict-file &&
     + 	test_grep "^>>>>>>>$" conflict-file
     + '
     + 
     ++test_expect_success 'apply exits 2 on conflicts and keeps the stash entry' '
     ++	git reset --hard initial &&
     ++	test_commit exit-code-base conflict-file base-content &&
     ++	echo stashed >conflict-file &&
     ++	git stash push -m stashed &&
     ++	test_commit exit-code-upstream conflict-file upstream-content &&
     ++	test_expect_code 2 git stash apply &&
     ++	git stash list >list &&
     ++	test_grep stashed list
     ++'
     ++
     ++test_expect_success 'pop exits 2 on conflicts and keeps the stash entry' '
     ++	git reset --hard initial &&
     ++	test_commit pop-exit-code-base pop-file base-content &&
     ++	echo stashed >pop-file &&
     ++	git stash push -m pop-stashed &&
     ++	test_commit pop-exit-code-upstream pop-file upstream-content &&
     ++	test_expect_code 2 git stash pop &&
     ++	git stash list >list &&
     ++	test_grep pop-stashed list
     ++'
     ++
     + test_expect_success 'stash show --include-untracked includes untracked files' '
     + 	git reset --hard &&
     + 
      
       ## t/t7201-co.sh ##
      @@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '


 Documentation/git-stash.adoc |  9 +++++
 builtin/checkout.c           | 15 +++++----
 builtin/stash.c              | 27 +++++++++------
 sequencer.c                  | 65 ++++++++++++++++++++++--------------
 sequencer.h                  | 19 +++++++----
 stash.h                      | 20 +++++++++++
 t/t3903-stash.sh             | 26 +++++++++++++--
 t/t7201-co.sh                | 16 ++++++---
 8 files changed, 143 insertions(+), 54 deletions(-)
 create mode 100644 stash.h

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 50bb89f483..3e41ffcf43 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -426,6 +426,15 @@ include::includes/cmd-config-section-all.adoc[]
 :git-stash: 1
 include::config/stash.adoc[]
 
+EXIT STATUS
+-----------
+
+The `git stash` subcommands exit with status 0 on success and non-zero
+on failure.  The subcommands that apply a stash entry, i.e. `apply`,
+`pop` and `branch`, exit with status 2 when applying the stash entry
+resulted in conflicts, in which case the stash entry is left in place.
+Other failures exit with status 1 (usage errors exit with status 129).
+
 
 SEE ALSO
 --------
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 55e3a89a85..c960f98287 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1166,6 +1166,7 @@ static int switch_branches(const struct checkout_opts *opts,
 	int flag, writeout_error = 0;
 	int do_merge = 1;
 	int created_autostash = 0;
+	enum stash_apply_result autostash_res = STASH_APPLY_CLEAN;
 	struct strbuf old_commit_shortname = STRBUF_INIT;
 	struct strbuf autostash_msg = STRBUF_INIT;
 	const char *stash_label_base = NULL;
@@ -1237,12 +1238,12 @@ static int switch_branches(const struct checkout_opts *opts,
 				git_config_push_parameter(cfg.buf);
 				strbuf_release(&cfg);
 			}
-			apply_autostash_ref(the_repository,
-					    "CHECKOUT_AUTOSTASH_HEAD",
-					    new_branch_info->name,
-					    "local",
-					    stash_label_base,
-					    autostash_msg.buf);
+			autostash_res = apply_autostash_ref(the_repository,
+				    "CHECKOUT_AUTOSTASH_HEAD",
+				    new_branch_info->name,
+				    "local",
+				    stash_label_base,
+				    autostash_msg.buf);
 		}
 		if (ret) {
 			branch_info_release(&old_branch_info);
@@ -1255,6 +1256,8 @@ static int switch_branches(const struct checkout_opts *opts,
 	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
 		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
 
+	if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet)
+		fputc('\n', stderr);
 	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
 
 	if (created_autostash) {
diff --git a/builtin/stash.c b/builtin/stash.c
index 72c52571f8..86c7ac4ffa 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -10,6 +10,7 @@
 #include "object-name.h"
 #include "parse-options.h"
 #include "refs.h"
+#include "stash.h"
 #include "lockfile.h"
 #include "cache-tree.h"
 #include "unpack-trees.h"
@@ -640,8 +641,9 @@ static void unstage_changes_unless_new(struct object_id *orig_tree)
 		die(_("could not write index"));
 }
 
-static int do_apply_stash(const char *prefix, struct stash_info *info,
-			  int index, int quiet,
+static enum stash_apply_result do_apply_stash(const char *prefix,
+					      struct stash_info *info,
+					      int index, int quiet,
 			  const char *label_ours, const char *label_theirs,
 			  const char *label_base)
 {
@@ -716,11 +718,12 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);
 
 	/*
-	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
-	 * merge was clean, and nonzero if the merge was unclean or encountered
-	 * an error.
+	 * Translate the value of 'clean' so 'ret' is STASH_APPLY_CLEAN
+	 * when the merge was clean, STASH_APPLY_CONFLICT when it was
+	 * unclean, and a negative value if it encountered an error.
 	 */
-	ret = clean >= 0 ? !clean : clean;
+	ret = clean >= 0 ? (clean ? STASH_APPLY_CLEAN : STASH_APPLY_CONFLICT)
+			 : clean;
 
 	if (ret < 0)
 		rollback_lock_file(&lock);
@@ -739,7 +742,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 
 	if (has_index) {
 		if (reset_tree(&index_tree, 0, 0))
-			ret = -1;
+			ret = STASH_APPLY_ERROR;
 	} else {
 		unstage_changes_unless_new(&c_tree);
 	}
@@ -2492,9 +2495,13 @@ int cmd_stash(int argc,
 	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
 		    (uintmax_t)pid);
 
-	if (fn)
-		return !!fn(argc, argv, prefix, repo);
-	else if (!argc)
+	if (fn) {
+		ret = fn(argc, argv, prefix, repo);
+
+		if (ret < 0)
+			return 1;
+		return ret;
+	} else if (!argc)
 		return !!push_stash_unassumed(0, NULL, prefix, repo);
 
 	/* Assume 'stash push' */
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..328fd4262e 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -19,6 +19,7 @@
 #include "commit.h"
 #include "sequencer.h"
 #include "run-command.h"
+#include "stash.h"
 #include "hook.h"
 #include "utf8.h"
 #include "cache-tree.h"
@@ -4794,13 +4795,15 @@ void create_autostash_ref(struct repository *r, const char *refname,
 	create_autostash_internal(r, NULL, refname, message, silent);
 }
 
-static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
-				    const char *label_ours, const char *label_theirs,
-				    const char *label_base,
-				    const char *stash_msg)
+static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
+							int attempt_apply,
+							const char *label_ours,
+							const char *label_theirs,
+							const char *label_base,
+							const char *stash_msg)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
-	int ret = 0;
+	enum stash_apply_result ret = STASH_APPLY_CLEAN;
 
 	if (attempt_apply) {
 		child.git_cmd = 1;
@@ -4816,9 +4819,11 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 			strvec_pushf(&child.args, "--label-base=%s", label_base);
 		strvec_push(&child.args, stash_oid);
 		ret = run_command(&child);
+		if (ret && ret != STASH_APPLY_CONFLICT)
+			ret = STASH_APPLY_ERROR;
 	}
 
-	if (attempt_apply && !ret)
+	if (attempt_apply && ret == STASH_APPLY_CLEAN)
 		fprintf(stderr, _("Applied autostash.\n"));
 	else {
 		struct child_process store = CHILD_PROCESS_INIT;
@@ -4832,13 +4837,15 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 		strvec_push(&store.args, stash_oid);
 		if (run_command(&store))
 			ret = error(_("cannot store %s"), stash_oid);
-		else if (attempt_apply)
+		else if (attempt_apply && ret == STASH_APPLY_CONFLICT)
 			fprintf(stderr,
 				_("Your local changes are stashed, however applying them\n"
 				  "resulted in conflicts.  You can either resolve the conflicts\n"
 				  "and then discard the stash with \"git stash drop\", or, if you\n"
 				  "do not want to resolve them now, run \"git reset --hard\" and\n"
 				  "apply the local changes later by running \"git stash pop\".\n"));
+		else if (attempt_apply)
+			ret = error(_("could not apply autostash"));
 		else
 			fprintf(stderr,
 				_("Autostash exists; creating a new stash entry.\n"
@@ -4850,15 +4857,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 	return ret;
 }
 
-static int apply_save_autostash(const char *path, int attempt_apply)
+static enum stash_apply_result apply_save_autostash(const char *path,
+						    int attempt_apply)
 {
 	struct strbuf stash_oid = STRBUF_INIT;
-	int ret = 0;
+	enum stash_apply_result ret = STASH_APPLY_CLEAN;
 
 	if (!read_oneliner(&stash_oid, path,
 			   READ_ONELINER_SKIP_IF_EMPTY)) {
 		strbuf_release(&stash_oid);
-		return 0;
+		return STASH_APPLY_CLEAN;
 	}
 	strbuf_trim(&stash_oid);
 
@@ -4870,37 +4878,40 @@ static int apply_save_autostash(const char *path, int attempt_apply)
 	return ret;
 }
 
-int save_autostash(const char *path)
+enum stash_apply_result save_autostash(const char *path)
 {
 	return apply_save_autostash(path, 0);
 }
 
-int apply_autostash(const char *path)
+enum stash_apply_result apply_autostash(const char *path)
 {
 	return apply_save_autostash(path, 1);
 }
 
-int apply_autostash_oid(const char *stash_oid)
+enum stash_apply_result apply_autostash_oid(const char *stash_oid)
 {
 	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL);
 }
 
-static int apply_save_autostash_ref(struct repository *r, const char *refname,
-				    int attempt_apply,
-				    const char *label_ours, const char *label_theirs,
-				    const char *label_base,
-				    const char *stash_msg)
+static enum stash_apply_result apply_save_autostash_ref(struct repository *r,
+							const char *refname,
+							int attempt_apply,
+							const char *label_ours,
+							const char *label_theirs,
+							const char *label_base,
+							const char *stash_msg)
 {
 	struct object_id stash_oid;
 	char stash_oid_hex[GIT_MAX_HEXSZ + 1];
-	int flag, ret;
+	int flag;
+	enum stash_apply_result ret;
 
 	if (!refs_ref_exists(get_main_ref_store(r), refname))
-		return 0;
+		return STASH_APPLY_CLEAN;
 
 	if (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname,
 				     RESOLVE_REF_READING, &stash_oid, &flag))
-		return -1;
+		return STASH_APPLY_ERROR;
 	if (flag & REF_ISSYMREF)
 		return error(_("autostash reference is a symref"));
 
@@ -4915,15 +4926,19 @@ static int apply_save_autostash_ref(struct repository *r, const char *refname,
 	return ret;
 }
 
-int save_autostash_ref(struct repository *r, const char *refname)
+enum stash_apply_result save_autostash_ref(struct repository *r,
+					   const char *refname)
 {
 	return apply_save_autostash_ref(r, refname, 0,
 					NULL, NULL, NULL, NULL);
 }
 
-int apply_autostash_ref(struct repository *r, const char *refname,
-			const char *label_ours, const char *label_theirs,
-			const char *label_base, const char *stash_msg)
+enum stash_apply_result apply_autostash_ref(struct repository *r,
+					    const char *refname,
+					    const char *label_ours,
+					    const char *label_theirs,
+					    const char *label_base,
+					    const char *stash_msg)
 {
 	return apply_save_autostash_ref(r, refname, 1,
 					label_ours, label_theirs, label_base,
diff --git a/sequencer.h b/sequencer.h
index 64a9c7fb1b..804501b64c 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -3,6 +3,7 @@
 
 #include "strbuf.h"
 #include "strvec.h"
+#include "stash.h"
 #include "wt-status.h"
 
 struct commit;
@@ -231,13 +232,17 @@ void commit_post_rewrite(struct repository *r,
 void create_autostash(struct repository *r, const char *path);
 void create_autostash_ref(struct repository *r, const char *refname,
 			  const char *message, bool silent);
-int save_autostash(const char *path);
-int save_autostash_ref(struct repository *r, const char *refname);
-int apply_autostash(const char *path);
-int apply_autostash_oid(const char *stash_oid);
-int apply_autostash_ref(struct repository *r, const char *refname,
-			const char *label_ours, const char *label_theirs,
-			const char *label_base, const char *stash_msg);
+enum stash_apply_result save_autostash(const char *path);
+enum stash_apply_result save_autostash_ref(struct repository *r,
+					   const char *refname);
+enum stash_apply_result apply_autostash(const char *path);
+enum stash_apply_result apply_autostash_oid(const char *stash_oid);
+enum stash_apply_result apply_autostash_ref(struct repository *r,
+					    const char *refname,
+					    const char *label_ours,
+					    const char *label_theirs,
+					    const char *label_base,
+					    const char *stash_msg);
 
 #define SUMMARY_INITIAL_COMMIT   (1 << 0)
 #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
diff --git a/stash.h b/stash.h
new file mode 100644
index 0000000000..bf2f2e1a45
--- /dev/null
+++ b/stash.h
@@ -0,0 +1,20 @@
+#ifndef STASH_H
+#define STASH_H
+
+enum stash_apply_result {
+	/* The stash was applied cleanly, or there was nothing to apply. */
+	STASH_APPLY_CLEAN = 0,
+
+	/*
+	 * The stash could not be applied because it resulted in
+	 * conflicts.  The stash entry is left in place.  The "git stash
+	 * apply", "pop" and "branch" subcommands exit with this status
+	 * in this case.
+	 */
+	STASH_APPLY_CONFLICT = 2,
+
+	/* Something went wrong. */
+	STASH_APPLY_ERROR = -1,
+};
+
+#endif /* STASH_H */
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index da27a6599a..93e8e98216 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1797,7 +1797,7 @@ test_expect_success 'apply with custom conflict labels' '
 	echo stashed >conflict-file &&
 	git stash push -m "stashed" &&
 	test_commit label-upstream conflict-file upstream-content &&
-	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
+	test_expect_code 2 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
 	test_grep "^<<<<<<< UP" conflict-file &&
 	test_grep "^||||||| Stash base" conflict-file &&
 	test_grep "^>>>>>>> STASH" conflict-file
@@ -1809,11 +1809,33 @@ test_expect_success 'apply with empty conflict labels' '
 	echo stashed >conflict-file &&
 	git stash push -m "stashed" &&
 	test_commit empty-label-upstream conflict-file upstream-content &&
-	test_must_fail git stash apply --label-ours= --label-theirs= &&
+	test_expect_code 2 git stash apply --label-ours= --label-theirs= &&
 	test_grep "^<<<<<<<$" conflict-file &&
 	test_grep "^>>>>>>>$" conflict-file
 '
 
+test_expect_success 'apply exits 2 on conflicts and keeps the stash entry' '
+	git reset --hard initial &&
+	test_commit exit-code-base conflict-file base-content &&
+	echo stashed >conflict-file &&
+	git stash push -m stashed &&
+	test_commit exit-code-upstream conflict-file upstream-content &&
+	test_expect_code 2 git stash apply &&
+	git stash list >list &&
+	test_grep stashed list
+'
+
+test_expect_success 'pop exits 2 on conflicts and keeps the stash entry' '
+	git reset --hard initial &&
+	test_commit pop-exit-code-base pop-file base-content &&
+	echo stashed >pop-file &&
+	git stash push -m pop-stashed &&
+	test_commit pop-exit-code-upstream pop-file upstream-content &&
+	test_expect_code 2 git stash pop &&
+	git stash list >list &&
+	test_grep pop-stashed list
+'
+
 test_expect_success 'stash show --include-untracked includes untracked files' '
 	git reset --hard &&
 
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 0ddd1ad7aa..9ea9462914 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -236,10 +236,18 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
 	test_must_fail git checkout side 2>stderr &&
 	test_grep "Your local changes" stderr &&
 	git checkout -m side >actual 2>&1 &&
-	test_grep "resulted in conflicts" actual &&
-	test_grep "git stash drop" actual &&
-	test_grep "git stash pop" actual &&
-	test_grep "The following paths have local changes" actual &&
+	cat >expect <<-EOF &&
+	Your local changes are stashed, however applying them
+	resulted in conflicts.  You can either resolve the conflicts
+	and then discard the stash with "git stash drop", or, if you
+	do not want to resolve them now, run "git reset --hard" and
+	apply the local changes later by running "git stash pop".
+
+	Switched to branch ${SQ}side${SQ}
+	The following paths have local changes:
+	M	one
+	EOF
+	test_cmp expect actual &&
 	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
 	sed /^index/d actual >actual.trimmed &&
 	cat >expect <<-EOF &&

base-commit: c73e85354c275c9d409b26445089bc16940fc527
-- 
gitgitgadget

Re: [PATCH v3] checkout: separate autostash conflict advice from branch-switch message

[Phillip Wood]

Hi Harald

On 01/09/2026 10:49, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> "git checkout -m" stashes the user's local changes when it cannot
> perform the checkout, and then applies the stash.  When applying the
> stash results in conflicts, the advice on how to deal with them is
> printed directly on top of the branch-switch message ("Switched to
> branch ..."), making the two hard to tell apart.  Print a blank line
> in between so that the advice and the branch-switch message are
> visually distinct.
> 
> To make this possible, "git stash apply", "pop" and "branch" now exit
> with status 2 when applying the stash entry resulted in conflicts, in
> which case the stash entry is left in place; other failures exit with
> status 1, as before.  The exit statuses are documented in the "git
> stash" documentation.

Other commands such as merge-tree and merge strategies use 1 to indicate 
conflicts and another non-zero exit code for errors. That matches the 
way grep and diff use the exit code to distinguish differences from 
errors. It is confusing if we start using a different convention here. 
I've left a few comments below, but the exit code is my main concern. It 
would be nice to separate out the stash changes into a separate commit 
as well.

> diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
> index 50bb89f483..3e41ffcf43 100644
> --- a/Documentation/git-stash.adoc
> +++ b/Documentation/git-stash.adoc
> @@ -426,6 +426,15 @@ include::includes/cmd-config-section-all.adoc[]
>   :git-stash: 1
>   include::config/stash.adoc[]
>   
> +EXIT STATUS
> +-----------
> +
> +The `git stash` subcommands exit with status 0 on success and non-zero
> +on failure.  The subcommands that apply a stash entry, i.e. `apply`,
> +`pop` and `branch`, exit with status 2 when applying the stash entry
> +resulted in conflicts, in which case the stash entry is left in place.
> +Other failures exit with status 1 (usage errors exit with status 129).

Thanks for documenting this, I think we'd be better to avoid giving 
specific exit codes for errors and say "a non-zero exit code other than 
1" unless we have a good way of enforcing that.

> diff --git a/builtin/stash.c b/builtin/stash.c
> index 72c52571f8..86c7ac4ffa 100644
> --- a/builtin/stash.c
> +++ b/builtin/stash.c
> @@ -10,6 +10,7 @@
>   #include "object-name.h"
>   #include "parse-options.h"
>   #include "refs.h"
> +#include "stash.h"
>   #include "lockfile.h"
>   #include "cache-tree.h"
>   #include "unpack-trees.h"
> @@ -640,8 +641,9 @@ static void unstage_changes_unless_new(struct object_id *orig_tree)
>   		die(_("could not write index"));
>   }
>   
> -static int do_apply_stash(const char *prefix, struct stash_info *info,
> -			  int index, int quiet,
> +static enum stash_apply_result do_apply_stash(const char *prefix,
> +					      struct stash_info *info,
> +					      int index, int quiet,
>   			  const char *label_ours, const char *label_theirs,
>   			  const char *label_base)

The indentation is strange here

>   {
> @@ -716,11 +718,12 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
>   	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);
>   
>   	/*
> -	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
> -	 * merge was clean, and nonzero if the merge was unclean or encountered
> -	 * an error.
> +	 * Translate the value of 'clean' so 'ret' is STASH_APPLY_CLEAN
> +	 * when the merge was clean, STASH_APPLY_CONFLICT when it was
> +	 * unclean, and a negative value if it encountered an error.
>   	 */
> -	ret = clean >= 0 ? !clean : clean;
> +	ret = clean >= 0 ? (clean ? STASH_APPLY_CLEAN : STASH_APPLY_CONFLICT)
> +			 : clean;

Nested ternary operators are not particularly readable, if we stick with 
an exit code of 1 for conflicts the original code does not need to be 
modified.

>   
>   	if (ret < 0)
>   		rollback_lock_file(&lock);
> @@ -739,7 +742,7 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
>   
>   	if (has_index) {
>   		if (reset_tree(&index_tree, 0, 0))
> -			ret = -1;
> +			ret = STASH_APPLY_ERROR;

This seems a bit pointless when we're still returning -1 implicitly 
everywhere else where we have "return error(...).


>   	} else {
>   		unstage_changes_unless_new(&c_tree);
>   	}
> @@ -2492,9 +2495,13 @@ int cmd_stash(int argc,
>   	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
>   		    (uintmax_t)pid);
>   
> -	if (fn)
> -		return !!fn(argc, argv, prefix, repo);
> -	else if (!argc)
> +	if (fn) {
> +		ret = fn(argc, argv, prefix, repo);
> +
> +		if (ret < 0)
> +			return 1;
> +		return ret;

Looking at the callers of do_apply_stash(), apply_stash() returns the 
result of do_apply_stash(), pop_stash() and branch_stash() return the 
result of do_drop_stash() if do_apply_stash() returns 0. do_drop_stash() 
always returns 0 so we're safe, but that analysis should be in the 
commit message.


> @@ -4832,13 +4837,15 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
>   		strvec_push(&store.args, stash_oid);
>   		if (run_command(&store))
>   			ret = error(_("cannot store %s"), stash_oid);
> -		else if (attempt_apply)
> +		else if (attempt_apply && ret == STASH_APPLY_CONFLICT)
>   			fprintf(stderr,
>   				_("Your local changes are stashed, however applying them\n"
>   				  "resulted in conflicts.  You can either resolve the conflicts\n"
>   				  "and then discard the stash with \"git stash drop\", or, if you\n"
>   				  "do not want to resolve them now, run \"git reset --hard\" and\n"
>   				  "apply the local changes later by running \"git stash pop\".\n"));

We only print this if we know there were conflicts - good.

> +		else if (attempt_apply)
> +			ret = error(_("could not apply autostash"));

We know we've saved the stash so we should tell the user that we have, 
rather than leaving when wondering what's happened to their stashed changes.

The rest of the changes in this file look good.

> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
> index da27a6599a..93e8e98216 100755
> --- a/t/t3903-stash.sh
> +++ b/t/t3903-stash.sh
> @@ -1797,7 +1797,7 @@ test_expect_success 'apply with custom conflict labels' '
>   	echo stashed >conflict-file &&
>   	git stash push -m "stashed" &&
>   	test_commit label-upstream conflict-file upstream-content &&
> -	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
> +	test_expect_code 2 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
>   	test_grep "^<<<<<<< UP" conflict-file &&
>   	test_grep "^||||||| Stash base" conflict-file &&
>   	test_grep "^>>>>>>> STASH" conflict-file
> @@ -1809,11 +1809,33 @@ test_expect_success 'apply with empty conflict labels' '
>   	echo stashed >conflict-file &&
>   	git stash push -m "stashed" &&
>   	test_commit empty-label-upstream conflict-file upstream-content &&
> -	test_must_fail git stash apply --label-ours= --label-theirs= &&
> +	test_expect_code 2 git stash apply --label-ours= --label-theirs= &&
>   	test_grep "^<<<<<<<$" conflict-file &&
>   	test_grep "^>>>>>>>$" conflict-file
>   '
>   
> +test_expect_success 'apply exits 2 on conflicts and keeps the stash entry' '

Aren't we testing that above?

> +	git reset --hard initial &&
> +	test_commit exit-code-base conflict-file base-content &&

We've just reset to a known starting point that has paths file and 
other-file, so why do we need to create a new commit in order to stash 
something?

> +	echo stashed >conflict-file &&
> +	git stash push -m stashed &&
> +	test_commit exit-code-upstream conflict-file upstream-content &&
> +	test_expect_code 2 git stash apply &&
> +	git stash list >list &&
> +	test_grep stashed list
> +'
> +
> +test_expect_success 'pop exits 2 on conflicts and keeps the stash entry' '

This is good, we should be checking "stash branch" as well.

> diff --git a/t/t7201-co.sh b/t/t7201-co.sh
> index 0ddd1ad7aa..9ea9462914 100755
> --- a/t/t7201-co.sh
> +++ b/t/t7201-co.sh
> @@ -236,10 +236,18 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
>   	test_must_fail git checkout side 2>stderr &&
>   	test_grep "Your local changes" stderr &&
>   	git checkout -m side >actual 2>&1 &&
> -	test_grep "resulted in conflicts" actual &&
> -	test_grep "git stash drop" actual &&
> -	test_grep "git stash pop" actual &&
> -	test_grep "The following paths have local changes" actual &&
> +	cat >expect <<-EOF &&
> +	Your local changes are stashed, however applying them
> +	resulted in conflicts.  You can either resolve the conflicts
> +	and then discard the stash with "git stash drop", or, if you
> +	do not want to resolve them now, run "git reset --hard" and
> +	apply the local changes later by running "git stash pop".
> +
> +	Switched to branch ${SQ}side${SQ}
> +	The following paths have local changes:
> +	M	one
> +	EOF
> +	test_cmp expect actual &&

Nice, it is much easier to see what we're checking now

Thanks

Phillip

Re: [PATCH v3] checkout: separate autostash conflict advice from branch-switch message

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Hi Harald
>
> On 01/09/2026 10:49, Harald Nordgren via GitGitGadget wrote:
>> From: Harald Nordgren <haraldnordgren@gmail.com>
>> 
>> "git checkout -m" stashes the user's local changes when it cannot
>> perform the checkout, and then applies the stash.  When applying the
>> stash results in conflicts, the advice on how to deal with them is
>> printed directly on top of the branch-switch message ("Switched to
>> branch ..."), making the two hard to tell apart.  Print a blank line
>> in between so that the advice and the branch-switch message are
>> visually distinct.
>> 
>> To make this possible, "git stash apply", "pop" and "branch" now exit
>> with status 2 when applying the stash entry resulted in conflicts, in
>> which case the stash entry is left in place; other failures exit with
>> status 1, as before.  The exit statuses are documented in the "git
>> stash" documentation.
>
> Other commands such as merge-tree and merge strategies use 1 to indicate 
> conflicts and another non-zero exit code for errors. That matches the 
> way grep and diff use the exit code to distinguish differences from 
> errors. It is confusing if we start using a different convention here. 
> I've left a few comments below, but the exit code is my main concern. It 
> would be nice to separate out the stash changes into a separate commit 
> as well.

This is half my fault.  I should have made it clear that my sample
code was merely to illustrate the need and ability to allow the
callers to tell general errors and specific failure modes.  I agree
with you that "1 if X, other non-zero exit if error" would fly much
better.

Thanks.

[PATCH v4 0/2] checkout -m: refine autostash fallback

[Harald Nordgren via GitGitGadget]

Avoiding checkout -m autostash retries when no tracked local changes exist
and visually separating autostash conflict advice from the subsequent
branch-switch message. Addresses #leftoverbits from here:
https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/

Changes in v4:

 * Conflicts now exit with status 1 like merge-tree, other failures exit 128
   so exit 1 unambiguously means conflicts. Stash changes split into their
   own commit.
 * The autostash apply helpers use the return value (enum
   stash_apply_result) instead of an out-parameter, and only claim conflicts
   when git stash apply actually reported them.

Changes in v3:

 * Use enum for git stash return values, to separate conflict from generic
   error.

Changes in v2:

 * Simplify logic and combine to one commit.
 * Test full output with test_cmp.

Harald Nordgren (2):
  stash: reserve exit status 1 for conflicts
  checkout: separate autostash conflict advice from branch-switch
    message

 Documentation/git-stash.adoc |  9 +++++
 builtin/checkout.c           | 15 ++++----
 builtin/stash.c              | 32 ++++++++++++-----
 sequencer.c                  | 66 ++++++++++++++++++++++--------------
 sequencer.h                  | 19 +++++++----
 stash.h                      | 21 ++++++++++++
 t/t3903-stash.sh             | 25 ++++++++++++--
 t/t7201-co.sh                | 16 ++++++---
 8 files changed, 149 insertions(+), 54 deletions(-)
 create mode 100644 stash.h


base-commit: 1630431f326e15fcde608827b5ff38422528eb59
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v4
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v3:

 1:  8e1979dd6c ! 1:  ff43221802 checkout: separate autostash conflict advice from branch-switch message
     @@ Metadata
      Author: Harald Nordgren <haraldnordgren@gmail.com>
      
       ## Commit message ##
     -    checkout: separate autostash conflict advice from branch-switch message
     +    stash: reserve exit status 1 for conflicts
      
     -    "git checkout -m" stashes the user's local changes when it cannot
     -    perform the checkout, and then applies the stash.  When applying the
     -    stash results in conflicts, the advice on how to deal with them is
     -    printed directly on top of the branch-switch message ("Switched to
     -    branch ..."), making the two hard to tell apart.  Print a blank line
     -    in between so that the advice and the branch-switch message are
     -    visually distinct.
     +    "git stash apply", "pop" and "branch" exit with status 1 both when
     +    applying the stash entry resulted in conflicts and when they fail for
     +    other reasons, so callers cannot tell the two apart.
      
     -    To make this possible, "git stash apply", "pop" and "branch" now exit
     -    with status 2 when applying the stash entry resulted in conflicts, in
     -    which case the stash entry is left in place; other failures exit with
     -    status 1, as before.  The exit statuses are documented in the "git
     -    stash" documentation.
     +    Follow the convention of "git merge-tree" and the merge strategies,
     +    which exit with status 1 to indicate conflicts and with a different
     +    non-zero status for errors: those subcommands now exit with status 1
     +    only when applying the stash entry resulted in conflicts, in which
     +    case the stash entry is left in place, and exit with status 128, the
     +    status die() uses, when they fail for other reasons.  Document the
     +    exit statuses.
     +
     +    cmd_stash() used to collapse the return values of the subcommand
     +    implementations to a boolean.  It now maps negative values, which
     +    signal a failure, to 128 and passes everything else through as-is.
     +    The only implementations that return a positive value are "apply",
     +    "pop" and "branch", which return the value of do_apply_stash():
     +    "apply" returns it directly, and "pop" and "branch" drop the stash
     +    entry, via do_drop_stash(), which always returns 0, only when the
     +    application succeeded.  The positive value is always 1, as
     +    do_apply_stash() only returns a positive value when the three-way
     +    merge was unclean.
     +
     +    Make the convention explicit by introducing enum stash_apply_result
     +    with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
     +    STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
     +    too.  They spawn "git stash apply" and can now tell conflicts apart
     +    from other failures, e.g. a crash or death by signal of the child,
     +    which map to exit statuses above 1.  Since we know the stash entry
     +    was saved, tell users so in the error message instead of leaving them
     +    wondering what happened to their stashed changes.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ Documentation/git-stash.adoc: include::includes/cmd-config-section-all.adoc[]
      +EXIT STATUS
      +-----------
      +
     -+The `git stash` subcommands exit with status 0 on success and non-zero
     -+on failure.  The subcommands that apply a stash entry, i.e. `apply`,
     -+`pop` and `branch`, exit with status 2 when applying the stash entry
     -+resulted in conflicts, in which case the stash entry is left in place.
     -+Other failures exit with status 1 (usage errors exit with status 129).
     ++The `git stash` subcommands exit with status 0 on success.  The
     ++subcommands that apply a stash entry, i.e. `apply`, `pop` and `branch`,
     ++exit with status 1 when applying the stash entry resulted in conflicts,
     ++in which case the stash entry is left in place, and with a non-zero
     ++status other than 1 when they fail for other reasons.
      +
       
       SEE ALSO
       --------
      
     - ## builtin/checkout.c ##
     -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
     - 	int flag, writeout_error = 0;
     - 	int do_merge = 1;
     - 	int created_autostash = 0;
     -+	enum stash_apply_result autostash_res = STASH_APPLY_CLEAN;
     - 	struct strbuf old_commit_shortname = STRBUF_INIT;
     - 	struct strbuf autostash_msg = STRBUF_INIT;
     - 	const char *stash_label_base = NULL;
     -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
     - 				git_config_push_parameter(cfg.buf);
     - 				strbuf_release(&cfg);
     - 			}
     --			apply_autostash_ref(the_repository,
     --					    "CHECKOUT_AUTOSTASH_HEAD",
     --					    new_branch_info->name,
     --					    "local",
     --					    stash_label_base,
     --					    autostash_msg.buf);
     -+			autostash_res = apply_autostash_ref(the_repository,
     -+				    "CHECKOUT_AUTOSTASH_HEAD",
     -+				    new_branch_info->name,
     -+				    "local",
     -+				    stash_label_base,
     -+				    autostash_msg.buf);
     - 		}
     - 		if (ret) {
     - 			branch_info_release(&old_branch_info);
     -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
     - 	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
     - 		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
     - 
     -+	if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet)
     -+		fputc('\n', stderr);
     - 	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
     - 
     - 	if (created_autostash) {
     -
       ## builtin/stash.c ##
      @@
       #include "object-name.h"
     @@ builtin/stash.c: static void unstage_changes_unless_new(struct object_id *orig_t
       
      -static int do_apply_stash(const char *prefix, struct stash_info *info,
      -			  int index, int quiet,
     +-			  const char *label_ours, const char *label_theirs,
     +-			  const char *label_base)
      +static enum stash_apply_result do_apply_stash(const char *prefix,
      +					      struct stash_info *info,
      +					      int index, int quiet,
     - 			  const char *label_ours, const char *label_theirs,
     - 			  const char *label_base)
     ++					      const char *label_ours,
     ++					      const char *label_theirs,
     ++					      const char *label_base)
       {
     + 	int clean, ret;
     + 	int has_index = index;
      @@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
     - 	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);
       
       	/*
     --	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
     + 	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
      -	 * merge was clean, and nonzero if the merge was unclean or encountered
      -	 * an error.
     -+	 * Translate the value of 'clean' so 'ret' is STASH_APPLY_CLEAN
     -+	 * when the merge was clean, STASH_APPLY_CONFLICT when it was
     -+	 * unclean, and a negative value if it encountered an error.
     ++	 * merge was clean, and 1 if the merge was unclean or a negative value
     ++	 * if it encountered an error.
       	 */
     --	ret = clean >= 0 ? !clean : clean;
     -+	ret = clean >= 0 ? (clean ? STASH_APPLY_CLEAN : STASH_APPLY_CONFLICT)
     -+			 : clean;
     + 	ret = clean >= 0 ? !clean : clean;
       
     - 	if (ret < 0)
     - 		rollback_lock_file(&lock);
     -@@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
     - 
     - 	if (has_index) {
     - 		if (reset_tree(&index_tree, 0, 0))
     --			ret = -1;
     -+			ret = STASH_APPLY_ERROR;
     - 	} else {
     - 		unstage_changes_unless_new(&c_tree);
     - 	}
      @@ builtin/stash.c: int cmd_stash(int argc,
       	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
       		    (uintmax_t)pid);
     @@ builtin/stash.c: int cmd_stash(int argc,
      +	if (fn) {
      +		ret = fn(argc, argv, prefix, repo);
      +
     ++		/*
     ++		 * The subcommand implementations return 0 on success, a
     ++		 * negative value on failure, and STASH_APPLY_CONFLICT
     ++		 * when applying a stash entry resulted in conflicts.
     ++		 * Map failures to 128, the status die() uses, so that
     ++		 * exit status 1 unambiguously indicates conflicts.
     ++		 */
      +		if (ret < 0)
     -+			return 1;
     ++			return 128;
      +		return ret;
      +	} else if (!argc)
       		return !!push_stash_unassumed(0, NULL, prefix, repo);
     @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int atte
       			strvec_pushf(&child.args, "--label-base=%s", label_base);
       		strvec_push(&child.args, stash_oid);
       		ret = run_command(&child);
     -+		if (ret && ret != STASH_APPLY_CONFLICT)
     ++		if (ret > 1)
      +			ret = STASH_APPLY_ERROR;
       	}
       
     @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int atte
       				  "do not want to resolve them now, run \"git reset --hard\" and\n"
       				  "apply the local changes later by running \"git stash pop\".\n"));
      +		else if (attempt_apply)
     -+			ret = error(_("could not apply autostash"));
     ++			ret = error(_("could not apply autostash; "
     ++				      "your changes are safe in the stash"));
       		else
       			fprintf(stderr,
       				_("Autostash exists; creating a new stash entry.\n"
     @@ stash.h (new)
      +	 * The stash could not be applied because it resulted in
      +	 * conflicts.  The stash entry is left in place.  The "git stash
      +	 * apply", "pop" and "branch" subcommands exit with this status
     -+	 * in this case.
     ++	 * in this case, mirroring the convention of "git merge-tree" and
     ++	 * the merge strategies.
      +	 */
     -+	STASH_APPLY_CONFLICT = 2,
     ++	STASH_APPLY_CONFLICT = 1,
      +
      +	/* Something went wrong. */
      +	STASH_APPLY_ERROR = -1,
     @@ stash.h (new)
      +#endif /* STASH_H */
      
       ## t/t3903-stash.sh ##
     -@@ t/t3903-stash.sh: test_expect_success 'apply with custom conflict labels' '
     +@@ t/t3903-stash.sh: test_expect_success 'stash.index=false overridden by --index' '
     + 	test_cmp expect file
     + '
     + 
     +-test_expect_success 'apply with custom conflict labels' '
     ++test_expect_success 'apply exits 1 on conflicts' '
     + 	git reset --hard initial &&
     + 	test_commit label-base conflict-file base-content &&
       	echo stashed >conflict-file &&
       	git stash push -m "stashed" &&
       	test_commit label-upstream conflict-file upstream-content &&
      -	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
     -+	test_expect_code 2 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
     ++	test_expect_code 1 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
       	test_grep "^<<<<<<< UP" conflict-file &&
       	test_grep "^||||||| Stash base" conflict-file &&
       	test_grep "^>>>>>>> STASH" conflict-file
     @@ t/t3903-stash.sh: test_expect_success 'apply with empty conflict labels' '
       	git stash push -m "stashed" &&
       	test_commit empty-label-upstream conflict-file upstream-content &&
      -	test_must_fail git stash apply --label-ours= --label-theirs= &&
     -+	test_expect_code 2 git stash apply --label-ours= --label-theirs= &&
     ++	test_expect_code 1 git stash apply --label-ours= --label-theirs= &&
       	test_grep "^<<<<<<<$" conflict-file &&
       	test_grep "^>>>>>>>$" conflict-file
       '
       
     -+test_expect_success 'apply exits 2 on conflicts and keeps the stash entry' '
     ++test_expect_success 'pop exits 1 on conflicts and keeps the stash entry' '
      +	git reset --hard initial &&
     -+	test_commit exit-code-base conflict-file base-content &&
     -+	echo stashed >conflict-file &&
     -+	git stash push -m stashed &&
     -+	test_commit exit-code-upstream conflict-file upstream-content &&
     -+	test_expect_code 2 git stash apply &&
     ++	echo stashed >file &&
     ++	git stash push -m pop-stashed &&
     ++	test_commit pop-upstream file upstream-content &&
     ++	test_expect_code 1 git stash pop &&
      +	git stash list >list &&
     -+	test_grep stashed list
     ++	test_grep pop-stashed list
      +'
      +
     -+test_expect_success 'pop exits 2 on conflicts and keeps the stash entry' '
     ++test_expect_success 'stash branch exits with a non-1 status on errors' '
      +	git reset --hard initial &&
     -+	test_commit pop-exit-code-base pop-file base-content &&
     -+	echo stashed >pop-file &&
     -+	git stash push -m pop-stashed &&
     -+	test_commit pop-exit-code-upstream pop-file upstream-content &&
     -+	test_expect_code 2 git stash pop &&
     ++	echo stashed >file &&
     ++	git stash push -m branch-stashed &&
     ++	test_expect_code 128 git stash branch conflicting-branch refs/heads/does-not-exist &&
      +	git stash list >list &&
     -+	test_grep pop-stashed list
     ++	test_grep branch-stashed list
      +'
      +
       test_expect_success 'stash show --include-untracked includes untracked files' '
       	git reset --hard &&
       
     -
     - ## t/t7201-co.sh ##
     -@@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '
     - 	test_must_fail git checkout side 2>stderr &&
     - 	test_grep "Your local changes" stderr &&
     - 	git checkout -m side >actual 2>&1 &&
     --	test_grep "resulted in conflicts" actual &&
     --	test_grep "git stash drop" actual &&
     --	test_grep "git stash pop" actual &&
     --	test_grep "The following paths have local changes" actual &&
     -+	cat >expect <<-EOF &&
     -+	Your local changes are stashed, however applying them
     -+	resulted in conflicts.  You can either resolve the conflicts
     -+	and then discard the stash with "git stash drop", or, if you
     -+	do not want to resolve them now, run "git reset --hard" and
     -+	apply the local changes later by running "git stash pop".
     -+
     -+	Switched to branch ${SQ}side${SQ}
     -+	The following paths have local changes:
     -+	M	one
     -+	EOF
     -+	test_cmp expect actual &&
     - 	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
     - 	sed /^index/d actual >actual.trimmed &&
     - 	cat >expect <<-EOF &&
 -:  ---------- > 2:  935fa0a9ae checkout: separate autostash conflict advice from branch-switch message

-- 
gitgitgadget

[PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

"git stash apply", "pop" and "branch" exit with status 1 both when
applying the stash entry resulted in conflicts and when they fail for
other reasons, so callers cannot tell the two apart.

Follow the convention of "git merge-tree" and the merge strategies,
which exit with status 1 to indicate conflicts and with a different
non-zero status for errors: those subcommands now exit with status 1
only when applying the stash entry resulted in conflicts, in which
case the stash entry is left in place, and exit with status 128, the
status die() uses, when they fail for other reasons.  Document the
exit statuses.

cmd_stash() used to collapse the return values of the subcommand
implementations to a boolean.  It now maps negative values, which
signal a failure, to 128 and passes everything else through as-is.
The only implementations that return a positive value are "apply",
"pop" and "branch", which return the value of do_apply_stash():
"apply" returns it directly, and "pop" and "branch" drop the stash
entry, via do_drop_stash(), which always returns 0, only when the
application succeeded.  The positive value is always 1, as
do_apply_stash() only returns a positive value when the three-way
merge was unclean.

Make the convention explicit by introducing enum stash_apply_result
with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
too.  They spawn "git stash apply" and can now tell conflicts apart
from other failures, e.g. a crash or death by signal of the child,
which map to exit statuses above 1.  Since we know the stash entry
was saved, tell users so in the error message instead of leaving them
wondering what happened to their stashed changes.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-stash.adoc |  9 +++++
 builtin/stash.c              | 32 ++++++++++++-----
 sequencer.c                  | 66 ++++++++++++++++++++++--------------
 sequencer.h                  | 19 +++++++----
 stash.h                      | 21 ++++++++++++
 t/t3903-stash.sh             | 25 ++++++++++++--
 6 files changed, 128 insertions(+), 44 deletions(-)
 create mode 100644 stash.h

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 50bb89f483..fc6a9a008c 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -426,6 +426,15 @@ include::includes/cmd-config-section-all.adoc[]
 :git-stash: 1
 include::config/stash.adoc[]
 
+EXIT STATUS
+-----------
+
+The `git stash` subcommands exit with status 0 on success.  The
+subcommands that apply a stash entry, i.e. `apply`, `pop` and `branch`,
+exit with status 1 when applying the stash entry resulted in conflicts,
+in which case the stash entry is left in place, and with a non-zero
+status other than 1 when they fail for other reasons.
+
 
 SEE ALSO
 --------
diff --git a/builtin/stash.c b/builtin/stash.c
index 72c52571f8..d858b7603f 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -10,6 +10,7 @@
 #include "object-name.h"
 #include "parse-options.h"
 #include "refs.h"
+#include "stash.h"
 #include "lockfile.h"
 #include "cache-tree.h"
 #include "unpack-trees.h"
@@ -640,10 +641,12 @@ static void unstage_changes_unless_new(struct object_id *orig_tree)
 		die(_("could not write index"));
 }
 
-static int do_apply_stash(const char *prefix, struct stash_info *info,
-			  int index, int quiet,
-			  const char *label_ours, const char *label_theirs,
-			  const char *label_base)
+static enum stash_apply_result do_apply_stash(const char *prefix,
+					      struct stash_info *info,
+					      int index, int quiet,
+					      const char *label_ours,
+					      const char *label_theirs,
+					      const char *label_base)
 {
 	int clean, ret;
 	int has_index = index;
@@ -717,8 +720,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 
 	/*
 	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
-	 * merge was clean, and nonzero if the merge was unclean or encountered
-	 * an error.
+	 * merge was clean, and 1 if the merge was unclean or a negative value
+	 * if it encountered an error.
 	 */
 	ret = clean >= 0 ? !clean : clean;
 
@@ -2492,9 +2495,20 @@ int cmd_stash(int argc,
 	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
 		    (uintmax_t)pid);
 
-	if (fn)
-		return !!fn(argc, argv, prefix, repo);
-	else if (!argc)
+	if (fn) {
+		ret = fn(argc, argv, prefix, repo);
+
+		/*
+		 * The subcommand implementations return 0 on success, a
+		 * negative value on failure, and STASH_APPLY_CONFLICT
+		 * when applying a stash entry resulted in conflicts.
+		 * Map failures to 128, the status die() uses, so that
+		 * exit status 1 unambiguously indicates conflicts.
+		 */
+		if (ret < 0)
+			return 128;
+		return ret;
+	} else if (!argc)
 		return !!push_stash_unassumed(0, NULL, prefix, repo);
 
 	/* Assume 'stash push' */
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..b5dd855084 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -19,6 +19,7 @@
 #include "commit.h"
 #include "sequencer.h"
 #include "run-command.h"
+#include "stash.h"
 #include "hook.h"
 #include "utf8.h"
 #include "cache-tree.h"
@@ -4794,13 +4795,15 @@ void create_autostash_ref(struct repository *r, const char *refname,
 	create_autostash_internal(r, NULL, refname, message, silent);
 }
 
-static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
-				    const char *label_ours, const char *label_theirs,
-				    const char *label_base,
-				    const char *stash_msg)
+static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
+							int attempt_apply,
+							const char *label_ours,
+							const char *label_theirs,
+							const char *label_base,
+							const char *stash_msg)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
-	int ret = 0;
+	enum stash_apply_result ret = STASH_APPLY_CLEAN;
 
 	if (attempt_apply) {
 		child.git_cmd = 1;
@@ -4816,9 +4819,11 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 			strvec_pushf(&child.args, "--label-base=%s", label_base);
 		strvec_push(&child.args, stash_oid);
 		ret = run_command(&child);
+		if (ret > 1)
+			ret = STASH_APPLY_ERROR;
 	}
 
-	if (attempt_apply && !ret)
+	if (attempt_apply && ret == STASH_APPLY_CLEAN)
 		fprintf(stderr, _("Applied autostash.\n"));
 	else {
 		struct child_process store = CHILD_PROCESS_INIT;
@@ -4832,13 +4837,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 		strvec_push(&store.args, stash_oid);
 		if (run_command(&store))
 			ret = error(_("cannot store %s"), stash_oid);
-		else if (attempt_apply)
+		else if (attempt_apply && ret == STASH_APPLY_CONFLICT)
 			fprintf(stderr,
 				_("Your local changes are stashed, however applying them\n"
 				  "resulted in conflicts.  You can either resolve the conflicts\n"
 				  "and then discard the stash with \"git stash drop\", or, if you\n"
 				  "do not want to resolve them now, run \"git reset --hard\" and\n"
 				  "apply the local changes later by running \"git stash pop\".\n"));
+		else if (attempt_apply)
+			ret = error(_("could not apply autostash; "
+				      "your changes are safe in the stash"));
 		else
 			fprintf(stderr,
 				_("Autostash exists; creating a new stash entry.\n"
@@ -4850,15 +4858,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 	return ret;
 }
 
-static int apply_save_autostash(const char *path, int attempt_apply)
+static enum stash_apply_result apply_save_autostash(const char *path,
+						    int attempt_apply)
 {
 	struct strbuf stash_oid = STRBUF_INIT;
-	int ret = 0;
+	enum stash_apply_result ret = STASH_APPLY_CLEAN;
 
 	if (!read_oneliner(&stash_oid, path,
 			   READ_ONELINER_SKIP_IF_EMPTY)) {
 		strbuf_release(&stash_oid);
-		return 0;
+		return STASH_APPLY_CLEAN;
 	}
 	strbuf_trim(&stash_oid);
 
@@ -4870,37 +4879,40 @@ static int apply_save_autostash(const char *path, int attempt_apply)
 	return ret;
 }
 
-int save_autostash(const char *path)
+enum stash_apply_result save_autostash(const char *path)
 {
 	return apply_save_autostash(path, 0);
 }
 
-int apply_autostash(const char *path)
+enum stash_apply_result apply_autostash(const char *path)
 {
 	return apply_save_autostash(path, 1);
 }
 
-int apply_autostash_oid(const char *stash_oid)
+enum stash_apply_result apply_autostash_oid(const char *stash_oid)
 {
 	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL);
 }
 
-static int apply_save_autostash_ref(struct repository *r, const char *refname,
-				    int attempt_apply,
-				    const char *label_ours, const char *label_theirs,
-				    const char *label_base,
-				    const char *stash_msg)
+static enum stash_apply_result apply_save_autostash_ref(struct repository *r,
+							const char *refname,
+							int attempt_apply,
+							const char *label_ours,
+							const char *label_theirs,
+							const char *label_base,
+							const char *stash_msg)
 {
 	struct object_id stash_oid;
 	char stash_oid_hex[GIT_MAX_HEXSZ + 1];
-	int flag, ret;
+	int flag;
+	enum stash_apply_result ret;
 
 	if (!refs_ref_exists(get_main_ref_store(r), refname))
-		return 0;
+		return STASH_APPLY_CLEAN;
 
 	if (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname,
 				     RESOLVE_REF_READING, &stash_oid, &flag))
-		return -1;
+		return STASH_APPLY_ERROR;
 	if (flag & REF_ISSYMREF)
 		return error(_("autostash reference is a symref"));
 
@@ -4915,15 +4927,19 @@ static int apply_save_autostash_ref(struct repository *r, const char *refname,
 	return ret;
 }
 
-int save_autostash_ref(struct repository *r, const char *refname)
+enum stash_apply_result save_autostash_ref(struct repository *r,
+					   const char *refname)
 {
 	return apply_save_autostash_ref(r, refname, 0,
 					NULL, NULL, NULL, NULL);
 }
 
-int apply_autostash_ref(struct repository *r, const char *refname,
-			const char *label_ours, const char *label_theirs,
-			const char *label_base, const char *stash_msg)
+enum stash_apply_result apply_autostash_ref(struct repository *r,
+					    const char *refname,
+					    const char *label_ours,
+					    const char *label_theirs,
+					    const char *label_base,
+					    const char *stash_msg)
 {
 	return apply_save_autostash_ref(r, refname, 1,
 					label_ours, label_theirs, label_base,
diff --git a/sequencer.h b/sequencer.h
index 64a9c7fb1b..804501b64c 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -3,6 +3,7 @@
 
 #include "strbuf.h"
 #include "strvec.h"
+#include "stash.h"
 #include "wt-status.h"
 
 struct commit;
@@ -231,13 +232,17 @@ void commit_post_rewrite(struct repository *r,
 void create_autostash(struct repository *r, const char *path);
 void create_autostash_ref(struct repository *r, const char *refname,
 			  const char *message, bool silent);
-int save_autostash(const char *path);
-int save_autostash_ref(struct repository *r, const char *refname);
-int apply_autostash(const char *path);
-int apply_autostash_oid(const char *stash_oid);
-int apply_autostash_ref(struct repository *r, const char *refname,
-			const char *label_ours, const char *label_theirs,
-			const char *label_base, const char *stash_msg);
+enum stash_apply_result save_autostash(const char *path);
+enum stash_apply_result save_autostash_ref(struct repository *r,
+					   const char *refname);
+enum stash_apply_result apply_autostash(const char *path);
+enum stash_apply_result apply_autostash_oid(const char *stash_oid);
+enum stash_apply_result apply_autostash_ref(struct repository *r,
+					    const char *refname,
+					    const char *label_ours,
+					    const char *label_theirs,
+					    const char *label_base,
+					    const char *stash_msg);
 
 #define SUMMARY_INITIAL_COMMIT   (1 << 0)
 #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
diff --git a/stash.h b/stash.h
new file mode 100644
index 0000000000..14ba4f946d
--- /dev/null
+++ b/stash.h
@@ -0,0 +1,21 @@
+#ifndef STASH_H
+#define STASH_H
+
+enum stash_apply_result {
+	/* The stash was applied cleanly, or there was nothing to apply. */
+	STASH_APPLY_CLEAN = 0,
+
+	/*
+	 * The stash could not be applied because it resulted in
+	 * conflicts.  The stash entry is left in place.  The "git stash
+	 * apply", "pop" and "branch" subcommands exit with this status
+	 * in this case, mirroring the convention of "git merge-tree" and
+	 * the merge strategies.
+	 */
+	STASH_APPLY_CONFLICT = 1,
+
+	/* Something went wrong. */
+	STASH_APPLY_ERROR = -1,
+};
+
+#endif /* STASH_H */
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index da27a6599a..6529508b06 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1791,13 +1791,13 @@ test_expect_success 'stash.index=false overridden by --index' '
 	test_cmp expect file
 '
 
-test_expect_success 'apply with custom conflict labels' '
+test_expect_success 'apply exits 1 on conflicts' '
 	git reset --hard initial &&
 	test_commit label-base conflict-file base-content &&
 	echo stashed >conflict-file &&
 	git stash push -m "stashed" &&
 	test_commit label-upstream conflict-file upstream-content &&
-	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
+	test_expect_code 1 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
 	test_grep "^<<<<<<< UP" conflict-file &&
 	test_grep "^||||||| Stash base" conflict-file &&
 	test_grep "^>>>>>>> STASH" conflict-file
@@ -1809,11 +1809,30 @@ test_expect_success 'apply with empty conflict labels' '
 	echo stashed >conflict-file &&
 	git stash push -m "stashed" &&
 	test_commit empty-label-upstream conflict-file upstream-content &&
-	test_must_fail git stash apply --label-ours= --label-theirs= &&
+	test_expect_code 1 git stash apply --label-ours= --label-theirs= &&
 	test_grep "^<<<<<<<$" conflict-file &&
 	test_grep "^>>>>>>>$" conflict-file
 '
 
+test_expect_success 'pop exits 1 on conflicts and keeps the stash entry' '
+	git reset --hard initial &&
+	echo stashed >file &&
+	git stash push -m pop-stashed &&
+	test_commit pop-upstream file upstream-content &&
+	test_expect_code 1 git stash pop &&
+	git stash list >list &&
+	test_grep pop-stashed list
+'
+
+test_expect_success 'stash branch exits with a non-1 status on errors' '
+	git reset --hard initial &&
+	echo stashed >file &&
+	git stash push -m branch-stashed &&
+	test_expect_code 128 git stash branch conflicting-branch refs/heads/does-not-exist &&
+	git stash list >list &&
+	test_grep branch-stashed list
+'
+
 test_expect_success 'stash show --include-untracked includes untracked files' '
 	git reset --hard &&
 
-- 
gitgitgadget

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> "git stash apply", "pop" and "branch" exit with status 1 both when
> applying the stash entry resulted in conflicts and when they fail for
> other reasons, so callers cannot tell the two apart.
>
> Follow the convention of "git merge-tree" and the merge strategies,
> which exit with status 1 to indicate conflicts and with a different
> non-zero status for errors: those subcommands now exit with status 1
> only when applying the stash entry resulted in conflicts, in which
> case the stash entry is left in place, and exit with status 128, the
> status die() uses, when they fail for other reasons.  Document the
> exit statuses.
>
> cmd_stash() used to collapse the return values of the subcommand
> implementations to a boolean.  It now maps negative values, which
> signal a failure, to 128 and passes everything else through as-is.
> The only implementations that return a positive value are "apply",
> "pop" and "branch", which return the value of do_apply_stash():
> "apply" returns it directly, and "pop" and "branch" drop the stash
> entry, via do_drop_stash(), which always returns 0, only when the
> application succeeded.  The positive value is always 1, as
> do_apply_stash() only returns a positive value when the three-way
> merge was unclean.
>
> Make the convention explicit by introducing enum stash_apply_result
> with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
> STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
> too.  They spawn "git stash apply" and can now tell conflicts apart
> from other failures, e.g. a crash or death by signal of the child,
> which map to exit statuses above 1.  Since we know the stash entry
> was saved, tell users so in the error message instead of leaving them
> wondering what happened to their stashed changes.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---

The above is on the overly verbose side.  The first two paragraphs
give enough discussion and the remainder mostly repeats with small
details sprinkled in, which can probably be shortened to 1/4 of the
amount of text, but it is OK.

> diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
> index 50bb89f483..fc6a9a008c 100644
> --- a/Documentation/git-stash.adoc
> +++ b/Documentation/git-stash.adoc
> @@ -426,6 +426,15 @@ include::includes/cmd-config-section-all.adoc[]
>  :git-stash: 1
>  include::config/stash.adoc[]
>  
> +EXIT STATUS
> +-----------
> +
> +The `git stash` subcommands exit with status 0 on success.  The
> +subcommands that apply a stash entry, i.e. `apply`, `pop` and `branch`,
> +exit with status 1 when applying the stash entry resulted in conflicts,
> +in which case the stash entry is left in place, and with a non-zero
> +status other than 1 when they fail for other reasons.
> +

Great.

> +static enum stash_apply_result do_apply_stash(const char *prefix,
> +					      struct stash_info *info,
> +					      int index, int quiet,
> +					      const char *label_ours,
> +					      const char *label_theirs,
> +					      const char *label_base)
>  {
>  	int clean, ret;
>  	int has_index = index;
> @@ -717,8 +720,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
>  
>  	/*
>  	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
> -	 * merge was clean, and nonzero if the merge was unclean or encountered
> -	 * an error.
> +	 * merge was clean, and 1 if the merge was unclean or a negative value
> +	 * if it encountered an error.
>  	 */
>  	ret = clean >= 0 ? !clean : clean;

OK.

> +	if (fn) {
> +		ret = fn(argc, argv, prefix, repo);
> +
> +		/*
> +		 * The subcommand implementations return 0 on success, a
> +		 * negative value on failure, and STASH_APPLY_CONFLICT
> +		 * when applying a stash entry resulted in conflicts.
> +		 * Map failures to 128, the status die() uses, so that
> +		 * exit status 1 unambiguously indicates conflicts.
> +		 */
> +		if (ret < 0)
> +			return 128;
> +		return ret;
> +	} else if (!argc)
>  		return !!push_stash_unassumed(0, NULL, prefix, repo);

Style.  Once one of "if", "else if" and "else" cascade gains
{braches}, others should do so as well.

> +static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
> +							int attempt_apply,
> +							const char *label_ours,
> +							const char *label_theirs,
> +							const char *label_base,
> +							const char *stash_msg)
>  {
>  	struct child_process child = CHILD_PROCESS_INIT;
> -	int ret = 0;
> +	enum stash_apply_result ret = STASH_APPLY_CLEAN;
>  
>  	if (attempt_apply) {
>  		child.git_cmd = 1;
> @@ -4816,9 +4819,11 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
>  			strvec_pushf(&child.args, "--label-base=%s", label_base);
>  		strvec_push(&child.args, stash_oid);
>  		ret = run_command(&child);
> +		if (ret > 1)
> +			ret = STASH_APPLY_ERROR;

This kind of code that assigns any random "int" that is returned by
run_command() to "enum ret" that has much narrower valid value range
and then makes corrections annoys me a bit.

One way to do this cleanly might be to make a small helper function
do_stash_apply(), and use it like so:

	if (attempt_apply)
		ret = do_stash_apply(stash_oid, label_ours, label_theirs,
				     label_base);

The implementation of do_stash_apply() would be like what you have
in "if (attempt_apply) {...}" block, perhaps like:

	static enum stash_apply_result do_stash_apply(const char *stash_oid,
						      const char *label_ours,
						      const char *label_theirs,
						      const char *label_base)
	{
		struct child_process child = CHILD_PROCESS_INIT;

		child.git_cmd = 1;
		...
                strvec_push(&child.args, stash_oid);
                switch (run_command(&child)) {
		case 0: return STASH_APPLY_CLEAN;
		case 1: return STASH_APPLY_CONFLICT;
		default: return STASH_APPLY_ERROR;
		}
	}

> -	if (attempt_apply && !ret)
> +	if (attempt_apply && ret == STASH_APPLY_CLEAN)
>  		fprintf(stderr, _("Applied autostash.\n"));
>  	else {
>  		struct child_process store = CHILD_PROCESS_INIT;

Good, and the rest of this function is good.

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> From: Harald Nordgren <haraldnordgren@gmail.com>
>>
>> "git stash apply", "pop" and "branch" exit with status 1 both when
>> applying the stash entry resulted in conflicts and when they fail for
>> other reasons, so callers cannot tell the two apart.
>>
>> Follow the convention of "git merge-tree" and the merge strategies,
>> which exit with status 1 to indicate conflicts and with a different
>> non-zero status for errors: those subcommands now exit with status 1
>> only when applying the stash entry resulted in conflicts, in which
>> case the stash entry is left in place, and exit with status 128, the
>> status die() uses, when they fail for other reasons.  Document the
>> exit statuses.
>>
>> cmd_stash() used to collapse the return values of the subcommand
>> implementations to a boolean.  It now maps negative values, which
>> signal a failure, to 128 and passes everything else through as-is.
>> The only implementations that return a positive value are "apply",
>> "pop" and "branch", which return the value of do_apply_stash():
>> "apply" returns it directly, and "pop" and "branch" drop the stash
>> entry, via do_drop_stash(), which always returns 0, only when the
>> application succeeded.  The positive value is always 1, as
>> do_apply_stash() only returns a positive value when the three-way
>> merge was unclean.
>>
>> Make the convention explicit by introducing enum stash_apply_result
>> with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
>> STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
>> too.  They spawn "git stash apply" and can now tell conflicts apart
>> from other failures, e.g. a crash or death by signal of the child,
>> which map to exit statuses above 1.  Since we know the stash entry
>> was saved, tell users so in the error message instead of leaving them
>> wondering what happened to their stashed changes.
>>
>> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>> ---
>
> The above is on the overly verbose side.  The first two paragraphs
> give enough discussion and the remainder mostly repeats with small
> details sprinkled in, which can probably be shortened to 1/4 of the
> amount of text, but it is OK.

Just for fun, I complained to an AI agent I had nearby with the
above four lines of critique, which spit back the following as a
replacement for the last two paragraphs.

    Update cmd_stash() to map negative return values to 128 while
    passing positive values through.  Formalize this with enum
    stash_apply_result and update the autostash helpers accordingly
    to distinguish conflicts from errors and inform users that their
    stash was preserved.

As I said it is OK already, you do not have to adopt this shortened
version, but I personally think that this level of detail is enough.

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Phillip Wood]

On 02/09/2026 20:51, Junio C Hamano wrote:
> "Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Harald Nordgren <haraldnordgren@gmail.com>
>>
>> "git stash apply", "pop" and "branch" exit with status 1 both when
>> applying the stash entry resulted in conflicts and when they fail for
>> other reasons, so callers cannot tell the two apart.
>>
>> Follow the convention of "git merge-tree" and the merge strategies,
>> which exit with status 1 to indicate conflicts and with a different
>> non-zero status for errors: those subcommands now exit with status 1
>> only when applying the stash entry resulted in conflicts, in which
>> case the stash entry is left in place, and exit with status 128, the
>> status die() uses, when they fail for other reasons.  Document the
>> exit statuses.
>>
>> cmd_stash() used to collapse the return values of the subcommand
>> implementations to a boolean.  It now maps negative values, which
>> signal a failure, to 128 and passes everything else through as-is.
>> The only implementations that return a positive value are "apply",
>> "pop" and "branch", which return the value of do_apply_stash():
>> "apply" returns it directly, and "pop" and "branch" drop the stash
>> entry, via do_drop_stash(), which always returns 0, only when the
>> application succeeded.  The positive value is always 1, as
>> do_apply_stash() only returns a positive value when the three-way
>> merge was unclean.
>>
>> Make the convention explicit by introducing enum stash_apply_result
>> with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
>> STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
>> too.  They spawn "git stash apply" and can now tell conflicts apart
>> from other failures, e.g. a crash or death by signal of the child,
>> which map to exit statuses above 1.  Since we know the stash entry
>> was saved, tell users so in the error message instead of leaving them
>> wondering what happened to their stashed changes.
>>
>> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>> ---
> 
> The above is on the overly verbose side.  The first two paragraphs
> give enough discussion and the remainder mostly repeats with small
> details sprinkled in, which can probably be shortened to 1/4 of the
> amount of text, but it is OK.

I think the analysis in the middle of the third paragraph is useful to 
make it clear that the return paths have been audited correctly. I agree 
the rest could be condensed or cut.

Thanks

Phillip

> 
>> diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
>> index 50bb89f483..fc6a9a008c 100644
>> --- a/Documentation/git-stash.adoc
>> +++ b/Documentation/git-stash.adoc
>> @@ -426,6 +426,15 @@ include::includes/cmd-config-section-all.adoc[]
>>   :git-stash: 1
>>   include::config/stash.adoc[]
>>   
>> +EXIT STATUS
>> +-----------
>> +
>> +The `git stash` subcommands exit with status 0 on success.  The
>> +subcommands that apply a stash entry, i.e. `apply`, `pop` and `branch`,
>> +exit with status 1 when applying the stash entry resulted in conflicts,
>> +in which case the stash entry is left in place, and with a non-zero
>> +status other than 1 when they fail for other reasons.
>> +
> 
> Great.
> 
>> +static enum stash_apply_result do_apply_stash(const char *prefix,
>> +					      struct stash_info *info,
>> +					      int index, int quiet,
>> +					      const char *label_ours,
>> +					      const char *label_theirs,
>> +					      const char *label_base)
>>   {
>>   	int clean, ret;
>>   	int has_index = index;
>> @@ -717,8 +720,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
>>   
>>   	/*
>>   	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
>> -	 * merge was clean, and nonzero if the merge was unclean or encountered
>> -	 * an error.
>> +	 * merge was clean, and 1 if the merge was unclean or a negative value
>> +	 * if it encountered an error.
>>   	 */
>>   	ret = clean >= 0 ? !clean : clean;
> 
> OK.
> 
>> +	if (fn) {
>> +		ret = fn(argc, argv, prefix, repo);
>> +
>> +		/*
>> +		 * The subcommand implementations return 0 on success, a
>> +		 * negative value on failure, and STASH_APPLY_CONFLICT
>> +		 * when applying a stash entry resulted in conflicts.
>> +		 * Map failures to 128, the status die() uses, so that
>> +		 * exit status 1 unambiguously indicates conflicts.
>> +		 */
>> +		if (ret < 0)
>> +			return 128;
>> +		return ret;
>> +	} else if (!argc)
>>   		return !!push_stash_unassumed(0, NULL, prefix, repo);
> 
> Style.  Once one of "if", "else if" and "else" cascade gains
> {braches}, others should do so as well.
> 
>> +static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
>> +							int attempt_apply,
>> +							const char *label_ours,
>> +							const char *label_theirs,
>> +							const char *label_base,
>> +							const char *stash_msg)
>>   {
>>   	struct child_process child = CHILD_PROCESS_INIT;
>> -	int ret = 0;
>> +	enum stash_apply_result ret = STASH_APPLY_CLEAN;
>>   
>>   	if (attempt_apply) {
>>   		child.git_cmd = 1;
>> @@ -4816,9 +4819,11 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
>>   			strvec_pushf(&child.args, "--label-base=%s", label_base);
>>   		strvec_push(&child.args, stash_oid);
>>   		ret = run_command(&child);
>> +		if (ret > 1)
>> +			ret = STASH_APPLY_ERROR;
> 
> This kind of code that assigns any random "int" that is returned by
> run_command() to "enum ret" that has much narrower valid value range
> and then makes corrections annoys me a bit.
> 
> One way to do this cleanly might be to make a small helper function
> do_stash_apply(), and use it like so:
> 
> 	if (attempt_apply)
> 		ret = do_stash_apply(stash_oid, label_ours, label_theirs,
> 				     label_base);
> 
> The implementation of do_stash_apply() would be like what you have
> in "if (attempt_apply) {...}" block, perhaps like:
> 
> 	static enum stash_apply_result do_stash_apply(const char *stash_oid,
> 						      const char *label_ours,
> 						      const char *label_theirs,
> 						      const char *label_base)
> 	{
> 		struct child_process child = CHILD_PROCESS_INIT;
> 
> 		child.git_cmd = 1;
> 		...
>                  strvec_push(&child.args, stash_oid);
>                  switch (run_command(&child)) {
> 		case 0: return STASH_APPLY_CLEAN;
> 		case 1: return STASH_APPLY_CONFLICT;
> 		default: return STASH_APPLY_ERROR;
> 		}
> 	}
> 
>> -	if (attempt_apply && !ret)
>> +	if (attempt_apply && ret == STASH_APPLY_CLEAN)
>>   		fprintf(stderr, _("Applied autostash.\n"));
>>   	else {
>>   		struct child_process store = CHILD_PROCESS_INIT;
> 
> Good, and the rest of this function is good.

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Harald Nordgren]

> >> +    if (fn) {
> >> +            ret = fn(argc, argv, prefix, repo);
> >> +
> >> +            /*
> >> +             * The subcommand implementations return 0 on success, a
> >> +             * negative value on failure, and STASH_APPLY_CONFLICT
> >> +             * when applying a stash entry resulted in conflicts.
> >> +             * Map failures to 128, the status die() uses, so that
> >> +             * exit status 1 unambiguously indicates conflicts.
> >> +             */
> >> +            if (ret < 0)
> >> +                    return 128;
> >> +            return ret;
> >> +    } else if (!argc)
> >>              return !!push_stash_unassumed(0, NULL, prefix, repo);
> >
> > Style.  Once one of "if", "else if" and "else" cascade gains
> > {braches}, others should do so as well.

I would love to have a linter rule for this, since I keep forgetting
and it's a waste of time for others to remind me.

Maybe we should copy this over from Linux?
https://github.com/torvalds/linux/blob/v7.2/scripts/checkpatch.pl#L6270


Harald

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Junio C Hamano]

Harald Nordgren <haraldnordgren@gmail.com> writes:

>> >> +    if (fn) {
>> >> +            ret = fn(argc, argv, prefix, repo);
>> >> +
>> >> +            /*
>> >> +             * The subcommand implementations return 0 on success, a
>> >> +             * negative value on failure, and STASH_APPLY_CONFLICT
>> >> +             * when applying a stash entry resulted in conflicts.
>> >> +             * Map failures to 128, the status die() uses, so that
>> >> +             * exit status 1 unambiguously indicates conflicts.
>> >> +             */
>> >> +            if (ret < 0)
>> >> +                    return 128;
>> >> +            return ret;
>> >> +    } else if (!argc)
>> >>              return !!push_stash_unassumed(0, NULL, prefix, repo);
>> >
>> > Style.  Once one of "if", "else if" and "else" cascade gains
>> > {braches}, others should do so as well.
>
> I would love to have a linter rule for this, since I keep forgetting
> and it's a waste of time for others to remind me.

Does our .clang-format file already cover this?

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Harald Nordgren]

> >> > Style.  Once one of "if", "else if" and "else" cascade gains
> >> > {braches}, others should do so as well.
> >
> > I would love to have a linter rule for this, since I keep forgetting
> > and it's a waste of time for others to remind me.
>
> Does our .clang-format file already cover this?

I don't think so. When GitHub CI runs, it doesn't complain about
mismatched braces.


Harald

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Junio C Hamano]

Harald Nordgren <haraldnordgren@gmail.com> writes:

>> >> > Style.  Once one of "if", "else if" and "else" cascade gains
>> >> > {braches}, others should do so as well.
>> >
>> > I would love to have a linter rule for this, since I keep forgetting
>> > and it's a waste of time for others to remind me.
>>
>> Does our .clang-format file already cover this?
>
> I don't think so. When GitHub CI runs, it doesn't complain about
> mismatched braces.

A local "make style" while you develop runs

	git clang-format --style file --diff --extensions c,h

which tells the command to check only the parts of the system that
you touched.

I do not think it is used in CI.  

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Harald Nordgren]

> A local "make style" while you develop runs
>
>         git clang-format --style file --diff --extensions c,h
>
> which tells the command to check only the parts of the system that
> you touched.
>
> I do not think it is used in CI.

It seems to run as part of CI and doesn't catch it then:
'.github/workflows/check-style.yml'


Harald

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Phillip Wood]

[Cc'd Karthik for a view on the CI style job]

On 04/09/2026 09:16, Harald Nordgren wrote:
>> A local "make style" while you develop runs
>>
>>          git clang-format --style file --diff --extensions c,h
>>
>> which tells the command to check only the parts of the system that
>> you touched.
>>
>> I do not think it is used in CI.
> 
> It seems to run as part of CI and doesn't catch it then:
> '.github/workflows/check-style.yml'

My recollection is that we made that job never fail because clang-format 
does not always do a good job of following our style guide and the 
output is not guaranteed to be stable across different versions. That 
makes its value debatable as I don't think many people (anyone?) bother 
checking the output to see what suggestions it made. It would be a lot 
simpler for contributors if we just devolved style decisions to 
clang-format so no one had to think about or comment on the style. 
Whether that tradeoff is worth it depends on how far the output of 
clang-format deviates from our preferred style.

Thanks

Phillip

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Harald Nordgren]

On Fri, Sep 4, 2026 at 5:09 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> [Cc'd Karthik for a view on the CI style job]
>
> On 04/09/2026 09:16, Harald Nordgren wrote:
> >> A local "make style" while you develop runs
> >>
> >>          git clang-format --style file --diff --extensions c,h
> >>
> >> which tells the command to check only the parts of the system that
> >> you touched.
> >>
> >> I do not think it is used in CI.
> >
> > It seems to run as part of CI and doesn't catch it then:
> > '.github/workflows/check-style.yml'
>
> My recollection is that we made that job never fail because clang-format
> does not always do a good job of following our style guide and the
> output is not guaranteed to be stable across different versions.

Thanks for the history! Seems that when linter output is not
consistent between versions, then version pinning is necessary.

> That
> makes its value debatable as I don't think many people (anyone?) bother
> checking the output to see what suggestions it made. It would be a lot
> simpler for contributors if we just devolved style decisions to
> clang-format so no one had to think about or comment on the style.

My professional opinion is that a consistent style, auto-enforceable +
auto-fixable, is extremely valuable, because it removes an entire
class of code review discussions. I almost never care which style is
chosen, as long as it's the same everywhere.

I tried today to apply the full clang-format output and it was A LOT.
Doesn't mean it shouldn't be done, but it would need to be done at a
carefully chosen time because there will be lots of conflicts on all
in-flight topics.

> Whether that tradeoff is worth it depends on how far the output of
> clang-format deviates from our preferred style.

In the best of worlds, the preferred style is exactly what the linter
dictates, then these two are equivalent.


Harald

Re: [PATCH v4 1/2] stash: reserve exit status 1 for conflicts

[Junio C Hamano]

Harald Nordgren <haraldnordgren@gmail.com> writes:

>> A local "make style" while you develop runs
>>
>>         git clang-format --style file --diff --extensions c,h
>>
>> which tells the command to check only the parts of the system that
>> you touched.
>>
>> I do not think it is used in CI.
>
> It seems to run as part of CI and doesn't catch it then:
> '.github/workflows/check-style.yml'

I never make any pull request to this project, so I didn't even
notice its existence.

My question was more about what is in .clang-format file we already
have and if it would have caught this.  I was actually hoping that
"RemoveBracesLLVM: true" we have at the end, which is explained like
so

    # Remove optional braces of control statements (if, else, for, and while)
    # according to the LLVM coding style. This avoids braces on simple
    # single-statement bodies of statements but keeps braces if one side of
    # if/else if/.../else cascade has multi-statement body.

would help us, but it only can do a small subset of what we want,
i.e., excess braces around both or either if/else body are removed
(see [*] below), and it cannot add braces around all the bodies of
if/else if.../else cascade only when one of them require them, it
seems.


[*] effect of RemoveBracesLLVM.

git clang-format --style file --diff --extensions c,h
diff --git a/git.c b/git.c
index dfd62d4010..c8df287d70 100644
--- a/git.c
+++ b/git.c
@@ -61,11 +61,10 @@ static void exclude_helpers_from_list(struct string_list *list)
 	size_t i = 0;
 
 	while (i < list->nr) {
-		if (strstr(list->items[i].string, "--")) {
+		if (strstr(list->items[i].string, "--"))
 			unsorted_string_list_delete_item(list, i, 0);
-		} else {
+		else
 			i++;
-		}
 	}
 }
 
make: *** [Makefile:3503: style] Error 1

[PATCH v4 2/2] checkout: separate autostash conflict advice from branch-switch message

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

"git checkout -m" stashes the user's local changes when it cannot
perform the checkout, and then applies the stash.  When applying the
stash results in conflicts, the advice on how to deal with them is
printed directly on top of the branch-switch message ("Switched to
branch ..."), making the two hard to tell apart.  Print a blank line
in between so that the advice and the branch-switch message are
visually distinct.

apply_autostash_ref() reports whether applying the stash resulted in
conflicts via its enum stash_apply_result return value, so only print
the blank line in the conflicted case.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/checkout.c | 15 +++++++++------
 t/t7201-co.sh      | 16 ++++++++++++----
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 55e3a89a85..c960f98287 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1166,6 +1166,7 @@ static int switch_branches(const struct checkout_opts *opts,
 	int flag, writeout_error = 0;
 	int do_merge = 1;
 	int created_autostash = 0;
+	enum stash_apply_result autostash_res = STASH_APPLY_CLEAN;
 	struct strbuf old_commit_shortname = STRBUF_INIT;
 	struct strbuf autostash_msg = STRBUF_INIT;
 	const char *stash_label_base = NULL;
@@ -1237,12 +1238,12 @@ static int switch_branches(const struct checkout_opts *opts,
 				git_config_push_parameter(cfg.buf);
 				strbuf_release(&cfg);
 			}
-			apply_autostash_ref(the_repository,
-					    "CHECKOUT_AUTOSTASH_HEAD",
-					    new_branch_info->name,
-					    "local",
-					    stash_label_base,
-					    autostash_msg.buf);
+			autostash_res = apply_autostash_ref(the_repository,
+				    "CHECKOUT_AUTOSTASH_HEAD",
+				    new_branch_info->name,
+				    "local",
+				    stash_label_base,
+				    autostash_msg.buf);
 		}
 		if (ret) {
 			branch_info_release(&old_branch_info);
@@ -1255,6 +1256,8 @@ static int switch_branches(const struct checkout_opts *opts,
 	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
 		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
 
+	if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet)
+		fputc('\n', stderr);
 	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
 
 	if (created_autostash) {
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 0ddd1ad7aa..9ea9462914 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -236,10 +236,18 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
 	test_must_fail git checkout side 2>stderr &&
 	test_grep "Your local changes" stderr &&
 	git checkout -m side >actual 2>&1 &&
-	test_grep "resulted in conflicts" actual &&
-	test_grep "git stash drop" actual &&
-	test_grep "git stash pop" actual &&
-	test_grep "The following paths have local changes" actual &&
+	cat >expect <<-EOF &&
+	Your local changes are stashed, however applying them
+	resulted in conflicts.  You can either resolve the conflicts
+	and then discard the stash with "git stash drop", or, if you
+	do not want to resolve them now, run "git reset --hard" and
+	apply the local changes later by running "git stash pop".
+
+	Switched to branch ${SQ}side${SQ}
+	The following paths have local changes:
+	M	one
+	EOF
+	test_cmp expect actual &&
 	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
 	sed /^index/d actual >actual.trimmed &&
 	cat >expect <<-EOF &&
-- 
gitgitgadget

Re: [PATCH v4 2/2] checkout: separate autostash conflict advice from branch-switch message

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> "git checkout -m" stashes the user's local changes when it cannot
> perform the checkout, and then applies the stash.  When applying the
> stash results in conflicts, the advice on how to deal with them is
> printed directly on top of the branch-switch message ("Switched to
> branch ..."), making the two hard to tell apart.  Print a blank line
> in between so that the advice and the branch-switch message are
> visually distinct.
>
> apply_autostash_ref() reports whether applying the stash resulted in
> conflicts via its enum stash_apply_result return value, so only print
> the blank line in the conflicted case.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>  builtin/checkout.c | 15 +++++++++------

This iteration looks good to me.

Re: [PATCH v4 0/2] checkout -m: refine autostash fallback

[Phillip Wood]

Hi Harald

On 02/09/2026 19:29, Harald Nordgren via GitGitGadget wrote:
> Avoiding checkout -m autostash retries when no tracked local changes exist
> and visually separating autostash conflict advice from the subsequent
> branch-switch message. Addresses #leftoverbits from here:
> https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/
> 
> Changes in v4:
> 
>   * Conflicts now exit with status 1 like merge-tree, other failures exit 128
>     so exit 1 unambiguously means conflicts. Stash changes split into their
>     own commit.

Thanks for changing the exit codes, I don't have anything to add to 
Junio's comments.

Thanks

Phillip

>   * The autostash apply helpers use the return value (enum
>     stash_apply_result) instead of an out-parameter, and only claim conflicts
>     when git stash apply actually reported them.
> 
> Changes in v3:
> 
>   * Use enum for git stash return values, to separate conflict from generic
>     error.
> 
> Changes in v2:
> 
>   * Simplify logic and combine to one commit.
>   * Test full output with test_cmp.
> 
> Harald Nordgren (2):
>    stash: reserve exit status 1 for conflicts
>    checkout: separate autostash conflict advice from branch-switch
>      message
> 
>   Documentation/git-stash.adoc |  9 +++++
>   builtin/checkout.c           | 15 ++++----
>   builtin/stash.c              | 32 ++++++++++++-----
>   sequencer.c                  | 66 ++++++++++++++++++++++--------------
>   sequencer.h                  | 19 +++++++----
>   stash.h                      | 21 ++++++++++++
>   t/t3903-stash.sh             | 25 ++++++++++++--
>   t/t7201-co.sh                | 16 ++++++---
>   8 files changed, 149 insertions(+), 54 deletions(-)
>   create mode 100644 stash.h
> 
> 
> base-commit: 1630431f326e15fcde608827b5ff38422528eb59
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v4
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v4
> Pull-Request: https://github.com/git/git/pull/2364
> 
> Range-diff vs v3:
> 
>   1:  8e1979dd6c ! 1:  ff43221802 checkout: separate autostash conflict advice from branch-switch message
>       @@ Metadata
>        Author: Harald Nordgren <haraldnordgren@gmail.com>
>        
>         ## Commit message ##
>       -    checkout: separate autostash conflict advice from branch-switch message
>       +    stash: reserve exit status 1 for conflicts
>        
>       -    "git checkout -m" stashes the user's local changes when it cannot
>       -    perform the checkout, and then applies the stash.  When applying the
>       -    stash results in conflicts, the advice on how to deal with them is
>       -    printed directly on top of the branch-switch message ("Switched to
>       -    branch ..."), making the two hard to tell apart.  Print a blank line
>       -    in between so that the advice and the branch-switch message are
>       -    visually distinct.
>       +    "git stash apply", "pop" and "branch" exit with status 1 both when
>       +    applying the stash entry resulted in conflicts and when they fail for
>       +    other reasons, so callers cannot tell the two apart.
>        
>       -    To make this possible, "git stash apply", "pop" and "branch" now exit
>       -    with status 2 when applying the stash entry resulted in conflicts, in
>       -    which case the stash entry is left in place; other failures exit with
>       -    status 1, as before.  The exit statuses are documented in the "git
>       -    stash" documentation.
>       +    Follow the convention of "git merge-tree" and the merge strategies,
>       +    which exit with status 1 to indicate conflicts and with a different
>       +    non-zero status for errors: those subcommands now exit with status 1
>       +    only when applying the stash entry resulted in conflicts, in which
>       +    case the stash entry is left in place, and exit with status 128, the
>       +    status die() uses, when they fail for other reasons.  Document the
>       +    exit statuses.
>       +
>       +    cmd_stash() used to collapse the return values of the subcommand
>       +    implementations to a boolean.  It now maps negative values, which
>       +    signal a failure, to 128 and passes everything else through as-is.
>       +    The only implementations that return a positive value are "apply",
>       +    "pop" and "branch", which return the value of do_apply_stash():
>       +    "apply" returns it directly, and "pop" and "branch" drop the stash
>       +    entry, via do_drop_stash(), which always returns 0, only when the
>       +    application succeeded.  The positive value is always 1, as
>       +    do_apply_stash() only returns a positive value when the three-way
>       +    merge was unclean.
>       +
>       +    Make the convention explicit by introducing enum stash_apply_result
>       +    with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
>       +    STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
>       +    too.  They spawn "git stash apply" and can now tell conflicts apart
>       +    from other failures, e.g. a crash or death by signal of the child,
>       +    which map to exit statuses above 1.  Since we know the stash entry
>       +    was saved, tell users so in the error message instead of leaving them
>       +    wondering what happened to their stashed changes.
>        
>            Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
>        
>       @@ Documentation/git-stash.adoc: include::includes/cmd-config-section-all.adoc[]
>        +EXIT STATUS
>        +-----------
>        +
>       -+The `git stash` subcommands exit with status 0 on success and non-zero
>       -+on failure.  The subcommands that apply a stash entry, i.e. `apply`,
>       -+`pop` and `branch`, exit with status 2 when applying the stash entry
>       -+resulted in conflicts, in which case the stash entry is left in place.
>       -+Other failures exit with status 1 (usage errors exit with status 129).
>       ++The `git stash` subcommands exit with status 0 on success.  The
>       ++subcommands that apply a stash entry, i.e. `apply`, `pop` and `branch`,
>       ++exit with status 1 when applying the stash entry resulted in conflicts,
>       ++in which case the stash entry is left in place, and with a non-zero
>       ++status other than 1 when they fail for other reasons.
>        +
>         
>         SEE ALSO
>         --------
>        
>       - ## builtin/checkout.c ##
>       -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
>       - 	int flag, writeout_error = 0;
>       - 	int do_merge = 1;
>       - 	int created_autostash = 0;
>       -+	enum stash_apply_result autostash_res = STASH_APPLY_CLEAN;
>       - 	struct strbuf old_commit_shortname = STRBUF_INIT;
>       - 	struct strbuf autostash_msg = STRBUF_INIT;
>       - 	const char *stash_label_base = NULL;
>       -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
>       - 				git_config_push_parameter(cfg.buf);
>       - 				strbuf_release(&cfg);
>       - 			}
>       --			apply_autostash_ref(the_repository,
>       --					    "CHECKOUT_AUTOSTASH_HEAD",
>       --					    new_branch_info->name,
>       --					    "local",
>       --					    stash_label_base,
>       --					    autostash_msg.buf);
>       -+			autostash_res = apply_autostash_ref(the_repository,
>       -+				    "CHECKOUT_AUTOSTASH_HEAD",
>       -+				    new_branch_info->name,
>       -+				    "local",
>       -+				    stash_label_base,
>       -+				    autostash_msg.buf);
>       - 		}
>       - 		if (ret) {
>       - 			branch_info_release(&old_branch_info);
>       -@@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
>       - 	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
>       - 		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
>       -
>       -+	if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet)
>       -+		fputc('\n', stderr);
>       - 	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
>       -
>       - 	if (created_autostash) {
>       -
>         ## builtin/stash.c ##
>        @@
>         #include "object-name.h"
>       @@ builtin/stash.c: static void unstage_changes_unless_new(struct object_id *orig_t
>         
>        -static int do_apply_stash(const char *prefix, struct stash_info *info,
>        -			  int index, int quiet,
>       +-			  const char *label_ours, const char *label_theirs,
>       +-			  const char *label_base)
>        +static enum stash_apply_result do_apply_stash(const char *prefix,
>        +					      struct stash_info *info,
>        +					      int index, int quiet,
>       - 			  const char *label_ours, const char *label_theirs,
>       - 			  const char *label_base)
>       ++					      const char *label_ours,
>       ++					      const char *label_theirs,
>       ++					      const char *label_base)
>         {
>       + 	int clean, ret;
>       + 	int has_index = index;
>        @@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
>       - 	clean = merge_ort_nonrecursive(&o, head, merge, merge_base);
>         
>         	/*
>       --	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
>       + 	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
>        -	 * merge was clean, and nonzero if the merge was unclean or encountered
>        -	 * an error.
>       -+	 * Translate the value of 'clean' so 'ret' is STASH_APPLY_CLEAN
>       -+	 * when the merge was clean, STASH_APPLY_CONFLICT when it was
>       -+	 * unclean, and a negative value if it encountered an error.
>       ++	 * merge was clean, and 1 if the merge was unclean or a negative value
>       ++	 * if it encountered an error.
>         	 */
>       --	ret = clean >= 0 ? !clean : clean;
>       -+	ret = clean >= 0 ? (clean ? STASH_APPLY_CLEAN : STASH_APPLY_CONFLICT)
>       -+			 : clean;
>       + 	ret = clean >= 0 ? !clean : clean;
>         
>       - 	if (ret < 0)
>       - 		rollback_lock_file(&lock);
>       -@@ builtin/stash.c: static int do_apply_stash(const char *prefix, struct stash_info *info,
>       -
>       - 	if (has_index) {
>       - 		if (reset_tree(&index_tree, 0, 0))
>       --			ret = -1;
>       -+			ret = STASH_APPLY_ERROR;
>       - 	} else {
>       - 		unstage_changes_unless_new(&c_tree);
>       - 	}
>        @@ builtin/stash.c: int cmd_stash(int argc,
>         	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
>         		    (uintmax_t)pid);
>       @@ builtin/stash.c: int cmd_stash(int argc,
>        +	if (fn) {
>        +		ret = fn(argc, argv, prefix, repo);
>        +
>       ++		/*
>       ++		 * The subcommand implementations return 0 on success, a
>       ++		 * negative value on failure, and STASH_APPLY_CONFLICT
>       ++		 * when applying a stash entry resulted in conflicts.
>       ++		 * Map failures to 128, the status die() uses, so that
>       ++		 * exit status 1 unambiguously indicates conflicts.
>       ++		 */
>        +		if (ret < 0)
>       -+			return 1;
>       ++			return 128;
>        +		return ret;
>        +	} else if (!argc)
>         		return !!push_stash_unassumed(0, NULL, prefix, repo);
>       @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int atte
>         			strvec_pushf(&child.args, "--label-base=%s", label_base);
>         		strvec_push(&child.args, stash_oid);
>         		ret = run_command(&child);
>       -+		if (ret && ret != STASH_APPLY_CONFLICT)
>       ++		if (ret > 1)
>        +			ret = STASH_APPLY_ERROR;
>         	}
>         
>       @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int atte
>         				  "do not want to resolve them now, run \"git reset --hard\" and\n"
>         				  "apply the local changes later by running \"git stash pop\".\n"));
>        +		else if (attempt_apply)
>       -+			ret = error(_("could not apply autostash"));
>       ++			ret = error(_("could not apply autostash; "
>       ++				      "your changes are safe in the stash"));
>         		else
>         			fprintf(stderr,
>         				_("Autostash exists; creating a new stash entry.\n"
>       @@ stash.h (new)
>        +	 * The stash could not be applied because it resulted in
>        +	 * conflicts.  The stash entry is left in place.  The "git stash
>        +	 * apply", "pop" and "branch" subcommands exit with this status
>       -+	 * in this case.
>       ++	 * in this case, mirroring the convention of "git merge-tree" and
>       ++	 * the merge strategies.
>        +	 */
>       -+	STASH_APPLY_CONFLICT = 2,
>       ++	STASH_APPLY_CONFLICT = 1,
>        +
>        +	/* Something went wrong. */
>        +	STASH_APPLY_ERROR = -1,
>       @@ stash.h (new)
>        +#endif /* STASH_H */
>        
>         ## t/t3903-stash.sh ##
>       -@@ t/t3903-stash.sh: test_expect_success 'apply with custom conflict labels' '
>       +@@ t/t3903-stash.sh: test_expect_success 'stash.index=false overridden by --index' '
>       + 	test_cmp expect file
>       + '
>       +
>       +-test_expect_success 'apply with custom conflict labels' '
>       ++test_expect_success 'apply exits 1 on conflicts' '
>       + 	git reset --hard initial &&
>       + 	test_commit label-base conflict-file base-content &&
>         	echo stashed >conflict-file &&
>         	git stash push -m "stashed" &&
>         	test_commit label-upstream conflict-file upstream-content &&
>        -	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
>       -+	test_expect_code 2 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
>       ++	test_expect_code 1 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
>         	test_grep "^<<<<<<< UP" conflict-file &&
>         	test_grep "^||||||| Stash base" conflict-file &&
>         	test_grep "^>>>>>>> STASH" conflict-file
>       @@ t/t3903-stash.sh: test_expect_success 'apply with empty conflict labels' '
>         	git stash push -m "stashed" &&
>         	test_commit empty-label-upstream conflict-file upstream-content &&
>        -	test_must_fail git stash apply --label-ours= --label-theirs= &&
>       -+	test_expect_code 2 git stash apply --label-ours= --label-theirs= &&
>       ++	test_expect_code 1 git stash apply --label-ours= --label-theirs= &&
>         	test_grep "^<<<<<<<$" conflict-file &&
>         	test_grep "^>>>>>>>$" conflict-file
>         '
>         
>       -+test_expect_success 'apply exits 2 on conflicts and keeps the stash entry' '
>       ++test_expect_success 'pop exits 1 on conflicts and keeps the stash entry' '
>        +	git reset --hard initial &&
>       -+	test_commit exit-code-base conflict-file base-content &&
>       -+	echo stashed >conflict-file &&
>       -+	git stash push -m stashed &&
>       -+	test_commit exit-code-upstream conflict-file upstream-content &&
>       -+	test_expect_code 2 git stash apply &&
>       ++	echo stashed >file &&
>       ++	git stash push -m pop-stashed &&
>       ++	test_commit pop-upstream file upstream-content &&
>       ++	test_expect_code 1 git stash pop &&
>        +	git stash list >list &&
>       -+	test_grep stashed list
>       ++	test_grep pop-stashed list
>        +'
>        +
>       -+test_expect_success 'pop exits 2 on conflicts and keeps the stash entry' '
>       ++test_expect_success 'stash branch exits with a non-1 status on errors' '
>        +	git reset --hard initial &&
>       -+	test_commit pop-exit-code-base pop-file base-content &&
>       -+	echo stashed >pop-file &&
>       -+	git stash push -m pop-stashed &&
>       -+	test_commit pop-exit-code-upstream pop-file upstream-content &&
>       -+	test_expect_code 2 git stash pop &&
>       ++	echo stashed >file &&
>       ++	git stash push -m branch-stashed &&
>       ++	test_expect_code 128 git stash branch conflicting-branch refs/heads/does-not-exist &&
>        +	git stash list >list &&
>       -+	test_grep pop-stashed list
>       ++	test_grep branch-stashed list
>        +'
>        +
>         test_expect_success 'stash show --include-untracked includes untracked files' '
>         	git reset --hard &&
>         
>       -
>       - ## t/t7201-co.sh ##
>       -@@ t/t7201-co.sh: test_expect_success 'checkout -m creates a recoverable stash on conflict' '
>       - 	test_must_fail git checkout side 2>stderr &&
>       - 	test_grep "Your local changes" stderr &&
>       - 	git checkout -m side >actual 2>&1 &&
>       --	test_grep "resulted in conflicts" actual &&
>       --	test_grep "git stash drop" actual &&
>       --	test_grep "git stash pop" actual &&
>       --	test_grep "The following paths have local changes" actual &&
>       -+	cat >expect <<-EOF &&
>       -+	Your local changes are stashed, however applying them
>       -+	resulted in conflicts.  You can either resolve the conflicts
>       -+	and then discard the stash with "git stash drop", or, if you
>       -+	do not want to resolve them now, run "git reset --hard" and
>       -+	apply the local changes later by running "git stash pop".
>       -+
>       -+	Switched to branch ${SQ}side${SQ}
>       -+	The following paths have local changes:
>       -+	M	one
>       -+	EOF
>       -+	test_cmp expect actual &&
>       - 	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
>       - 	sed /^index/d actual >actual.trimmed &&
>       - 	cat >expect <<-EOF &&
>   -:  ---------- > 2:  935fa0a9ae checkout: separate autostash conflict advice from branch-switch message
> 

[PATCH v5 0/2] checkout -m: refine autostash fallback

[Harald Nordgren via GitGitGadget]

Avoiding checkout -m autostash retries when no tracked local changes exist
and visually separating autostash conflict advice from the subsequent
branch-switch message. Addresses #leftoverbits from here:
https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/

Changes in v5:

 * Improve commit messages.
 * Create helper do_stash_apply.

Changes in v4:

 * Conflicts now exit with status 1 like merge-tree, other failures exit 128
   so exit 1 unambiguously means conflicts. Stash changes split into their
   own commit.
 * The autostash apply helpers use the return value (enum
   stash_apply_result) instead of an out-parameter, and only claim conflicts
   when git stash apply actually reported them.

Changes in v3:

 * Use enum for git stash return values, to separate conflict from generic
   error.

Changes in v2:

 * Simplify logic and combine to one commit.
 * Test full output with test_cmp.

Harald Nordgren (2):
  stash: reserve exit status 1 for conflicts
  checkout: separate autostash conflict advice from branch-switch
    message

 Documentation/git-stash.adoc |   9 +++
 builtin/checkout.c           |  15 +++--
 builtin/stash.c              |  33 +++++++---
 sequencer.c                  | 113 ++++++++++++++++++++++-------------
 sequencer.h                  |  19 +++---
 stash.h                      |  21 +++++++
 t/t3903-stash.sh             |  25 +++++++-
 t/t7201-co.sh                |  16 +++--
 8 files changed, 181 insertions(+), 70 deletions(-)
 create mode 100644 stash.h


base-commit: 3cb9185f65410273787f74333cc027d2ea5daada
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2364%2FHaraldNordgren%2Fhn%2Fgit-checkout-m-leftoverbits-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2364/HaraldNordgren/hn/git-checkout-m-leftoverbits-v5
Pull-Request: https://github.com/git/git/pull/2364

Range-diff vs v4:

 1:  ff43221802 ! 1:  fe22b1bfa6 stash: reserve exit status 1 for conflicts
     @@ Commit message
          status die() uses, when they fail for other reasons.  Document the
          exit statuses.
      
     -    cmd_stash() used to collapse the return values of the subcommand
     -    implementations to a boolean.  It now maps negative values, which
     -    signal a failure, to 128 and passes everything else through as-is.
     -    The only implementations that return a positive value are "apply",
     -    "pop" and "branch", which return the value of do_apply_stash():
     -    "apply" returns it directly, and "pop" and "branch" drop the stash
     -    entry, via do_drop_stash(), which always returns 0, only when the
     -    application succeeded.  The positive value is always 1, as
     -    do_apply_stash() only returns a positive value when the three-way
     -    merge was unclean.
     -
     -    Make the convention explicit by introducing enum stash_apply_result
     -    with the values STASH_APPLY_CLEAN, STASH_APPLY_CONFLICT and
     -    STASH_APPLY_ERROR, and use it for the in-process autostash helpers,
     -    too.  They spawn "git stash apply" and can now tell conflicts apart
     -    from other failures, e.g. a crash or death by signal of the child,
     -    which map to exit statuses above 1.  Since we know the stash entry
     -    was saved, tell users so in the error message instead of leaving them
     -    wondering what happened to their stashed changes.
     +    The only subcommand implementations that can return a positive value
     +    are "apply", "pop" and "branch", which return the value of
     +    do_apply_stash(): "apply" returns it directly, and "pop" and "branch"
     +    drop the stash entry, via do_drop_stash(), which always returns 0,
     +    only when the application succeeded.  do_apply_stash() only returns a
     +    positive value when the three-way merge was unclean.  cmd_stash() now
     +    maps negative values to 128 and passes positive values through as the
     +    exit status, so exit status 1 unambiguously indicates conflicts.
     +    enum stash_apply_result makes the convention explicit, and the
     +    autostash helpers use it to tell users that their stashed changes
     +    were saved when applying them fails.
      
          Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
      
     @@ builtin/stash.c: int cmd_stash(int argc,
      +		if (ret < 0)
      +			return 128;
      +		return ret;
     -+	} else if (!argc)
     ++	} else if (!argc) {
       		return !!push_stash_unassumed(0, NULL, prefix, repo);
     ++	}
       
       	/* Assume 'stash push' */
     + 	strvec_push(&args, "push");
      
       ## sequencer.c ##
      @@
     @@ sequencer.c: void create_autostash_ref(struct repository *r, const char *refname
      -				    const char *label_ours, const char *label_theirs,
      -				    const char *label_base,
      -				    const char *stash_msg)
     ++static enum stash_apply_result do_stash_apply(const char *stash_oid,
     ++					      const char *label_ours,
     ++					      const char *label_theirs,
     ++					      const char *label_base)
     + {
     + 	struct child_process child = CHILD_PROCESS_INIT;
     +-	int ret = 0;
     + 
     +-	if (attempt_apply) {
     +-		child.git_cmd = 1;
     +-		child.no_stdout = 1;
     +-		child.no_stderr = 1;
     +-		strvec_push(&child.args, "stash");
     +-		strvec_push(&child.args, "apply");
     +-		if (label_ours)
     +-			strvec_pushf(&child.args, "--label-ours=%s", label_ours);
     +-		if (label_theirs)
     +-			strvec_pushf(&child.args, "--label-theirs=%s", label_theirs);
     +-		if (label_base)
     +-			strvec_pushf(&child.args, "--label-base=%s", label_base);
     +-		strvec_push(&child.args, stash_oid);
     +-		ret = run_command(&child);
     +-	}
     +-
     +-	if (attempt_apply && !ret)
     ++	child.git_cmd = 1;
     ++	child.no_stdout = 1;
     ++	child.no_stderr = 1;
     ++	strvec_push(&child.args, "stash");
     ++	strvec_push(&child.args, "apply");
     ++	if (label_ours)
     ++		strvec_pushf(&child.args, "--label-ours=%s", label_ours);
     ++	if (label_theirs)
     ++		strvec_pushf(&child.args, "--label-theirs=%s", label_theirs);
     ++	if (label_base)
     ++		strvec_pushf(&child.args, "--label-base=%s", label_base);
     ++	strvec_push(&child.args, stash_oid);
     ++
     ++	switch (run_command(&child)) {
     ++	case 0:
     ++		return STASH_APPLY_CLEAN;
     ++	case STASH_APPLY_CONFLICT:
     ++		return STASH_APPLY_CONFLICT;
     ++	default:
     ++		return STASH_APPLY_ERROR;
     ++	}
     ++}
     ++
      +static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
      +							int attempt_apply,
      +							const char *label_ours,
      +							const char *label_theirs,
      +							const char *label_base,
      +							const char *stash_msg)
     - {
     - 	struct child_process child = CHILD_PROCESS_INIT;
     --	int ret = 0;
     ++{
      +	enum stash_apply_result ret = STASH_APPLY_CLEAN;
     - 
     - 	if (attempt_apply) {
     - 		child.git_cmd = 1;
     -@@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
     - 			strvec_pushf(&child.args, "--label-base=%s", label_base);
     - 		strvec_push(&child.args, stash_oid);
     - 		ret = run_command(&child);
     -+		if (ret > 1)
     -+			ret = STASH_APPLY_ERROR;
     - 	}
     - 
     --	if (attempt_apply && !ret)
     ++
     ++	if (attempt_apply)
     ++		ret = do_stash_apply(stash_oid, label_ours, label_theirs,
     ++				     label_base);
     ++
      +	if (attempt_apply && ret == STASH_APPLY_CLEAN)
       		fprintf(stderr, _("Applied autostash.\n"));
       	else {
 2:  935fa0a9ae = 2:  d18ff3ea9a checkout: separate autostash conflict advice from branch-switch message

-- 
gitgitgadget

[PATCH v5 1/2] stash: reserve exit status 1 for conflicts

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

"git stash apply", "pop" and "branch" exit with status 1 both when
applying the stash entry resulted in conflicts and when they fail for
other reasons, so callers cannot tell the two apart.

Follow the convention of "git merge-tree" and the merge strategies,
which exit with status 1 to indicate conflicts and with a different
non-zero status for errors: those subcommands now exit with status 1
only when applying the stash entry resulted in conflicts, in which
case the stash entry is left in place, and exit with status 128, the
status die() uses, when they fail for other reasons.  Document the
exit statuses.

The only subcommand implementations that can return a positive value
are "apply", "pop" and "branch", which return the value of
do_apply_stash(): "apply" returns it directly, and "pop" and "branch"
drop the stash entry, via do_drop_stash(), which always returns 0,
only when the application succeeded.  do_apply_stash() only returns a
positive value when the three-way merge was unclean.  cmd_stash() now
maps negative values to 128 and passes positive values through as the
exit status, so exit status 1 unambiguously indicates conflicts.
enum stash_apply_result makes the convention explicit, and the
autostash helpers use it to tell users that their stashed changes
were saved when applying them fails.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 Documentation/git-stash.adoc |   9 +++
 builtin/stash.c              |  33 +++++++---
 sequencer.c                  | 113 ++++++++++++++++++++++-------------
 sequencer.h                  |  19 +++---
 stash.h                      |  21 +++++++
 t/t3903-stash.sh             |  25 +++++++-
 6 files changed, 160 insertions(+), 60 deletions(-)
 create mode 100644 stash.h

diff --git a/Documentation/git-stash.adoc b/Documentation/git-stash.adoc
index 50bb89f483..fc6a9a008c 100644
--- a/Documentation/git-stash.adoc
+++ b/Documentation/git-stash.adoc
@@ -426,6 +426,15 @@ include::includes/cmd-config-section-all.adoc[]
 :git-stash: 1
 include::config/stash.adoc[]
 
+EXIT STATUS
+-----------
+
+The `git stash` subcommands exit with status 0 on success.  The
+subcommands that apply a stash entry, i.e. `apply`, `pop` and `branch`,
+exit with status 1 when applying the stash entry resulted in conflicts,
+in which case the stash entry is left in place, and with a non-zero
+status other than 1 when they fail for other reasons.
+
 
 SEE ALSO
 --------
diff --git a/builtin/stash.c b/builtin/stash.c
index 72c52571f8..7a9843413b 100644
--- a/builtin/stash.c
+++ b/builtin/stash.c
@@ -10,6 +10,7 @@
 #include "object-name.h"
 #include "parse-options.h"
 #include "refs.h"
+#include "stash.h"
 #include "lockfile.h"
 #include "cache-tree.h"
 #include "unpack-trees.h"
@@ -640,10 +641,12 @@ static void unstage_changes_unless_new(struct object_id *orig_tree)
 		die(_("could not write index"));
 }
 
-static int do_apply_stash(const char *prefix, struct stash_info *info,
-			  int index, int quiet,
-			  const char *label_ours, const char *label_theirs,
-			  const char *label_base)
+static enum stash_apply_result do_apply_stash(const char *prefix,
+					      struct stash_info *info,
+					      int index, int quiet,
+					      const char *label_ours,
+					      const char *label_theirs,
+					      const char *label_base)
 {
 	int clean, ret;
 	int has_index = index;
@@ -717,8 +720,8 @@ static int do_apply_stash(const char *prefix, struct stash_info *info,
 
 	/*
 	 * If 'clean' >= 0, reverse the value for 'ret' so 'ret' is 0 when the
-	 * merge was clean, and nonzero if the merge was unclean or encountered
-	 * an error.
+	 * merge was clean, and 1 if the merge was unclean or a negative value
+	 * if it encountered an error.
 	 */
 	ret = clean >= 0 ? !clean : clean;
 
@@ -2492,10 +2495,22 @@ int cmd_stash(int argc,
 	strbuf_addf(&stash_index_path, "%s.stash.%" PRIuMAX, index_file,
 		    (uintmax_t)pid);
 
-	if (fn)
-		return !!fn(argc, argv, prefix, repo);
-	else if (!argc)
+	if (fn) {
+		ret = fn(argc, argv, prefix, repo);
+
+		/*
+		 * The subcommand implementations return 0 on success, a
+		 * negative value on failure, and STASH_APPLY_CONFLICT
+		 * when applying a stash entry resulted in conflicts.
+		 * Map failures to 128, the status die() uses, so that
+		 * exit status 1 unambiguously indicates conflicts.
+		 */
+		if (ret < 0)
+			return 128;
+		return ret;
+	} else if (!argc) {
 		return !!push_stash_unassumed(0, NULL, prefix, repo);
+	}
 
 	/* Assume 'stash push' */
 	strvec_push(&args, "push");
diff --git a/sequencer.c b/sequencer.c
index 65afd100d9..e4a6ddfd01 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -19,6 +19,7 @@
 #include "commit.h"
 #include "sequencer.h"
 #include "run-command.h"
+#include "stash.h"
 #include "hook.h"
 #include "utf8.h"
 #include "cache-tree.h"
@@ -4794,31 +4795,50 @@ void create_autostash_ref(struct repository *r, const char *refname,
 	create_autostash_internal(r, NULL, refname, message, silent);
 }
 
-static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
-				    const char *label_ours, const char *label_theirs,
-				    const char *label_base,
-				    const char *stash_msg)
+static enum stash_apply_result do_stash_apply(const char *stash_oid,
+					      const char *label_ours,
+					      const char *label_theirs,
+					      const char *label_base)
 {
 	struct child_process child = CHILD_PROCESS_INIT;
-	int ret = 0;
 
-	if (attempt_apply) {
-		child.git_cmd = 1;
-		child.no_stdout = 1;
-		child.no_stderr = 1;
-		strvec_push(&child.args, "stash");
-		strvec_push(&child.args, "apply");
-		if (label_ours)
-			strvec_pushf(&child.args, "--label-ours=%s", label_ours);
-		if (label_theirs)
-			strvec_pushf(&child.args, "--label-theirs=%s", label_theirs);
-		if (label_base)
-			strvec_pushf(&child.args, "--label-base=%s", label_base);
-		strvec_push(&child.args, stash_oid);
-		ret = run_command(&child);
-	}
-
-	if (attempt_apply && !ret)
+	child.git_cmd = 1;
+	child.no_stdout = 1;
+	child.no_stderr = 1;
+	strvec_push(&child.args, "stash");
+	strvec_push(&child.args, "apply");
+	if (label_ours)
+		strvec_pushf(&child.args, "--label-ours=%s", label_ours);
+	if (label_theirs)
+		strvec_pushf(&child.args, "--label-theirs=%s", label_theirs);
+	if (label_base)
+		strvec_pushf(&child.args, "--label-base=%s", label_base);
+	strvec_push(&child.args, stash_oid);
+
+	switch (run_command(&child)) {
+	case 0:
+		return STASH_APPLY_CLEAN;
+	case STASH_APPLY_CONFLICT:
+		return STASH_APPLY_CONFLICT;
+	default:
+		return STASH_APPLY_ERROR;
+	}
+}
+
+static enum stash_apply_result apply_save_autostash_oid(const char *stash_oid,
+							int attempt_apply,
+							const char *label_ours,
+							const char *label_theirs,
+							const char *label_base,
+							const char *stash_msg)
+{
+	enum stash_apply_result ret = STASH_APPLY_CLEAN;
+
+	if (attempt_apply)
+		ret = do_stash_apply(stash_oid, label_ours, label_theirs,
+				     label_base);
+
+	if (attempt_apply && ret == STASH_APPLY_CLEAN)
 		fprintf(stderr, _("Applied autostash.\n"));
 	else {
 		struct child_process store = CHILD_PROCESS_INIT;
@@ -4832,13 +4852,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 		strvec_push(&store.args, stash_oid);
 		if (run_command(&store))
 			ret = error(_("cannot store %s"), stash_oid);
-		else if (attempt_apply)
+		else if (attempt_apply && ret == STASH_APPLY_CONFLICT)
 			fprintf(stderr,
 				_("Your local changes are stashed, however applying them\n"
 				  "resulted in conflicts.  You can either resolve the conflicts\n"
 				  "and then discard the stash with \"git stash drop\", or, if you\n"
 				  "do not want to resolve them now, run \"git reset --hard\" and\n"
 				  "apply the local changes later by running \"git stash pop\".\n"));
+		else if (attempt_apply)
+			ret = error(_("could not apply autostash; "
+				      "your changes are safe in the stash"));
 		else
 			fprintf(stderr,
 				_("Autostash exists; creating a new stash entry.\n"
@@ -4850,15 +4873,16 @@ static int apply_save_autostash_oid(const char *stash_oid, int attempt_apply,
 	return ret;
 }
 
-static int apply_save_autostash(const char *path, int attempt_apply)
+static enum stash_apply_result apply_save_autostash(const char *path,
+						    int attempt_apply)
 {
 	struct strbuf stash_oid = STRBUF_INIT;
-	int ret = 0;
+	enum stash_apply_result ret = STASH_APPLY_CLEAN;
 
 	if (!read_oneliner(&stash_oid, path,
 			   READ_ONELINER_SKIP_IF_EMPTY)) {
 		strbuf_release(&stash_oid);
-		return 0;
+		return STASH_APPLY_CLEAN;
 	}
 	strbuf_trim(&stash_oid);
 
@@ -4870,37 +4894,40 @@ static int apply_save_autostash(const char *path, int attempt_apply)
 	return ret;
 }
 
-int save_autostash(const char *path)
+enum stash_apply_result save_autostash(const char *path)
 {
 	return apply_save_autostash(path, 0);
 }
 
-int apply_autostash(const char *path)
+enum stash_apply_result apply_autostash(const char *path)
 {
 	return apply_save_autostash(path, 1);
 }
 
-int apply_autostash_oid(const char *stash_oid)
+enum stash_apply_result apply_autostash_oid(const char *stash_oid)
 {
 	return apply_save_autostash_oid(stash_oid, 1, NULL, NULL, NULL, NULL);
 }
 
-static int apply_save_autostash_ref(struct repository *r, const char *refname,
-				    int attempt_apply,
-				    const char *label_ours, const char *label_theirs,
-				    const char *label_base,
-				    const char *stash_msg)
+static enum stash_apply_result apply_save_autostash_ref(struct repository *r,
+							const char *refname,
+							int attempt_apply,
+							const char *label_ours,
+							const char *label_theirs,
+							const char *label_base,
+							const char *stash_msg)
 {
 	struct object_id stash_oid;
 	char stash_oid_hex[GIT_MAX_HEXSZ + 1];
-	int flag, ret;
+	int flag;
+	enum stash_apply_result ret;
 
 	if (!refs_ref_exists(get_main_ref_store(r), refname))
-		return 0;
+		return STASH_APPLY_CLEAN;
 
 	if (!refs_resolve_ref_unsafe(get_main_ref_store(r), refname,
 				     RESOLVE_REF_READING, &stash_oid, &flag))
-		return -1;
+		return STASH_APPLY_ERROR;
 	if (flag & REF_ISSYMREF)
 		return error(_("autostash reference is a symref"));
 
@@ -4915,15 +4942,19 @@ static int apply_save_autostash_ref(struct repository *r, const char *refname,
 	return ret;
 }
 
-int save_autostash_ref(struct repository *r, const char *refname)
+enum stash_apply_result save_autostash_ref(struct repository *r,
+					   const char *refname)
 {
 	return apply_save_autostash_ref(r, refname, 0,
 					NULL, NULL, NULL, NULL);
 }
 
-int apply_autostash_ref(struct repository *r, const char *refname,
-			const char *label_ours, const char *label_theirs,
-			const char *label_base, const char *stash_msg)
+enum stash_apply_result apply_autostash_ref(struct repository *r,
+					    const char *refname,
+					    const char *label_ours,
+					    const char *label_theirs,
+					    const char *label_base,
+					    const char *stash_msg)
 {
 	return apply_save_autostash_ref(r, refname, 1,
 					label_ours, label_theirs, label_base,
diff --git a/sequencer.h b/sequencer.h
index 64a9c7fb1b..804501b64c 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -3,6 +3,7 @@
 
 #include "strbuf.h"
 #include "strvec.h"
+#include "stash.h"
 #include "wt-status.h"
 
 struct commit;
@@ -231,13 +232,17 @@ void commit_post_rewrite(struct repository *r,
 void create_autostash(struct repository *r, const char *path);
 void create_autostash_ref(struct repository *r, const char *refname,
 			  const char *message, bool silent);
-int save_autostash(const char *path);
-int save_autostash_ref(struct repository *r, const char *refname);
-int apply_autostash(const char *path);
-int apply_autostash_oid(const char *stash_oid);
-int apply_autostash_ref(struct repository *r, const char *refname,
-			const char *label_ours, const char *label_theirs,
-			const char *label_base, const char *stash_msg);
+enum stash_apply_result save_autostash(const char *path);
+enum stash_apply_result save_autostash_ref(struct repository *r,
+					   const char *refname);
+enum stash_apply_result apply_autostash(const char *path);
+enum stash_apply_result apply_autostash_oid(const char *stash_oid);
+enum stash_apply_result apply_autostash_ref(struct repository *r,
+					    const char *refname,
+					    const char *label_ours,
+					    const char *label_theirs,
+					    const char *label_base,
+					    const char *stash_msg);
 
 #define SUMMARY_INITIAL_COMMIT   (1 << 0)
 #define SUMMARY_SHOW_AUTHOR_DATE (1 << 1)
diff --git a/stash.h b/stash.h
new file mode 100644
index 0000000000..14ba4f946d
--- /dev/null
+++ b/stash.h
@@ -0,0 +1,21 @@
+#ifndef STASH_H
+#define STASH_H
+
+enum stash_apply_result {
+	/* The stash was applied cleanly, or there was nothing to apply. */
+	STASH_APPLY_CLEAN = 0,
+
+	/*
+	 * The stash could not be applied because it resulted in
+	 * conflicts.  The stash entry is left in place.  The "git stash
+	 * apply", "pop" and "branch" subcommands exit with this status
+	 * in this case, mirroring the convention of "git merge-tree" and
+	 * the merge strategies.
+	 */
+	STASH_APPLY_CONFLICT = 1,
+
+	/* Something went wrong. */
+	STASH_APPLY_ERROR = -1,
+};
+
+#endif /* STASH_H */
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index da27a6599a..6529508b06 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -1791,13 +1791,13 @@ test_expect_success 'stash.index=false overridden by --index' '
 	test_cmp expect file
 '
 
-test_expect_success 'apply with custom conflict labels' '
+test_expect_success 'apply exits 1 on conflicts' '
 	git reset --hard initial &&
 	test_commit label-base conflict-file base-content &&
 	echo stashed >conflict-file &&
 	git stash push -m "stashed" &&
 	test_commit label-upstream conflict-file upstream-content &&
-	test_must_fail git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
+	test_expect_code 1 git -c merge.conflictStyle=diff3 stash apply --label-ours=UP --label-theirs=STASH &&
 	test_grep "^<<<<<<< UP" conflict-file &&
 	test_grep "^||||||| Stash base" conflict-file &&
 	test_grep "^>>>>>>> STASH" conflict-file
@@ -1809,11 +1809,30 @@ test_expect_success 'apply with empty conflict labels' '
 	echo stashed >conflict-file &&
 	git stash push -m "stashed" &&
 	test_commit empty-label-upstream conflict-file upstream-content &&
-	test_must_fail git stash apply --label-ours= --label-theirs= &&
+	test_expect_code 1 git stash apply --label-ours= --label-theirs= &&
 	test_grep "^<<<<<<<$" conflict-file &&
 	test_grep "^>>>>>>>$" conflict-file
 '
 
+test_expect_success 'pop exits 1 on conflicts and keeps the stash entry' '
+	git reset --hard initial &&
+	echo stashed >file &&
+	git stash push -m pop-stashed &&
+	test_commit pop-upstream file upstream-content &&
+	test_expect_code 1 git stash pop &&
+	git stash list >list &&
+	test_grep pop-stashed list
+'
+
+test_expect_success 'stash branch exits with a non-1 status on errors' '
+	git reset --hard initial &&
+	echo stashed >file &&
+	git stash push -m branch-stashed &&
+	test_expect_code 128 git stash branch conflicting-branch refs/heads/does-not-exist &&
+	git stash list >list &&
+	test_grep branch-stashed list
+'
+
 test_expect_success 'stash show --include-untracked includes untracked files' '
 	git reset --hard &&
 
-- 
gitgitgadget

[PATCH v5 2/2] checkout: separate autostash conflict advice from branch-switch message

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

"git checkout -m" stashes the user's local changes when it cannot
perform the checkout, and then applies the stash.  When applying the
stash results in conflicts, the advice on how to deal with them is
printed directly on top of the branch-switch message ("Switched to
branch ..."), making the two hard to tell apart.  Print a blank line
in between so that the advice and the branch-switch message are
visually distinct.

apply_autostash_ref() reports whether applying the stash resulted in
conflicts via its enum stash_apply_result return value, so only print
the blank line in the conflicted case.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 builtin/checkout.c | 15 +++++++++------
 t/t7201-co.sh      | 16 ++++++++++++----
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 55e3a89a85..c960f98287 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1166,6 +1166,7 @@ static int switch_branches(const struct checkout_opts *opts,
 	int flag, writeout_error = 0;
 	int do_merge = 1;
 	int created_autostash = 0;
+	enum stash_apply_result autostash_res = STASH_APPLY_CLEAN;
 	struct strbuf old_commit_shortname = STRBUF_INIT;
 	struct strbuf autostash_msg = STRBUF_INIT;
 	const char *stash_label_base = NULL;
@@ -1237,12 +1238,12 @@ static int switch_branches(const struct checkout_opts *opts,
 				git_config_push_parameter(cfg.buf);
 				strbuf_release(&cfg);
 			}
-			apply_autostash_ref(the_repository,
-					    "CHECKOUT_AUTOSTASH_HEAD",
-					    new_branch_info->name,
-					    "local",
-					    stash_label_base,
-					    autostash_msg.buf);
+			autostash_res = apply_autostash_ref(the_repository,
+				    "CHECKOUT_AUTOSTASH_HEAD",
+				    new_branch_info->name,
+				    "local",
+				    stash_label_base,
+				    autostash_msg.buf);
 		}
 		if (ret) {
 			branch_info_release(&old_branch_info);
@@ -1255,6 +1256,8 @@ static int switch_branches(const struct checkout_opts *opts,
 	if (!opts->quiet && !old_branch_info.path && old_branch_info.commit && new_branch_info->commit != old_branch_info.commit)
 		orphaned_commit_warning(old_branch_info.commit, new_branch_info->commit);
 
+	if (autostash_res == STASH_APPLY_CONFLICT && !opts->quiet)
+		fputc('\n', stderr);
 	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
 
 	if (created_autostash) {
diff --git a/t/t7201-co.sh b/t/t7201-co.sh
index 0ddd1ad7aa..9ea9462914 100755
--- a/t/t7201-co.sh
+++ b/t/t7201-co.sh
@@ -236,10 +236,18 @@ test_expect_success 'checkout -m creates a recoverable stash on conflict' '
 	test_must_fail git checkout side 2>stderr &&
 	test_grep "Your local changes" stderr &&
 	git checkout -m side >actual 2>&1 &&
-	test_grep "resulted in conflicts" actual &&
-	test_grep "git stash drop" actual &&
-	test_grep "git stash pop" actual &&
-	test_grep "The following paths have local changes" actual &&
+	cat >expect <<-EOF &&
+	Your local changes are stashed, however applying them
+	resulted in conflicts.  You can either resolve the conflicts
+	and then discard the stash with "git stash drop", or, if you
+	do not want to resolve them now, run "git reset --hard" and
+	apply the local changes later by running "git stash pop".
+
+	Switched to branch ${SQ}side${SQ}
+	The following paths have local changes:
+	M	one
+	EOF
+	test_cmp expect actual &&
 	git log -p -1 --format="%gs%n%B" -g --diff-merges=1 refs/stash >actual &&
 	sed /^index/d actual >actual.trimmed &&
 	cat >expect <<-EOF &&
-- 
gitgitgadget

Re: [PATCH v5 0/2] checkout -m: refine autostash fallback

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> Avoiding checkout -m autostash retries when no tracked local changes exist
> and visually separating autostash conflict advice from the subsequent
> branch-switch message. Addresses #leftoverbits from here:
> https://lore.kernel.org/git/cfd09dbf-8d77-4464-8030-3a0ffb4aeae7@gmail.com/
>
> Changes in v5:
>
>  * Improve commit messages.
>  * Create helper do_stash_apply.

Both patches look good to me.  Thanks.  Will queue.