From: Phillip Wood <phillip.wood123@gmail.com>
To: Harald Nordgren via GitGitGadget <gitgitgadget@gmail.com>,
git@vger.kernel.org
Cc: Harald Nordgren <haraldnordgren@gmail.com>
Subject: Re: [PATCH v4 0/2] checkout -m: refine autostash fallback
Date: Thu, 3 Sep 2026 15:00:27 +0100 [thread overview]
Message-ID: <66283358-9b87-4485-a036-ba91e6d7fcad@gmail.com> (raw)
In-Reply-To: <pull.2364.v4.git.git.1788373743.gitgitgadget@gmail.com>
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
>
next prev parent reply other threads:[~2026-09-03 14:00 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-25 15:34 [PATCH 0/2] checkout -m: refine autostash fallback Harald Nordgren via GitGitGadget
2026-07-25 15:34 ` [PATCH 1/2] sequencer: teach autostash apply to report conflicts Harald Nordgren via GitGitGadget
2026-08-27 13:01 ` Phillip Wood
2026-08-31 10:18 ` Harald Nordgren
2026-08-31 13:15 ` phillip.wood123
2026-07-25 15:34 ` [PATCH 2/2] checkout -m: refine autostash fallback Harald Nordgren via GitGitGadget
2026-07-28 22:49 ` Junio C Hamano
2026-08-27 13:05 ` Phillip Wood
2026-08-26 19:14 ` [PATCH 0/2] " Junio C Hamano
2026-08-27 13:12 ` Phillip Wood
2026-08-31 12:00 ` [PATCH v2] checkout: print blank line after autostash conflict advice Harald Nordgren via GitGitGadget
2026-08-31 17:19 ` Junio C Hamano
2026-09-01 9:31 ` Phillip Wood
2026-09-01 13:50 ` Junio C Hamano
2026-09-01 9:49 ` [PATCH v3] checkout: separate autostash conflict advice from branch-switch message Harald Nordgren via GitGitGadget
2026-09-01 13:42 ` Phillip Wood
2026-09-01 17:31 ` Junio C Hamano
2026-09-02 18:29 ` [PATCH v4 0/2] checkout -m: refine autostash fallback Harald Nordgren via GitGitGadget
2026-09-02 18:29 ` [PATCH v4 1/2] stash: reserve exit status 1 for conflicts Harald Nordgren via GitGitGadget
2026-09-02 19:51 ` Junio C Hamano
2026-09-02 20:08 ` Junio C Hamano
2026-09-03 13:57 ` Phillip Wood
2026-09-03 14:45 ` Harald Nordgren
2026-09-03 18:42 ` Junio C Hamano
2026-09-03 19:09 ` Harald Nordgren
2026-09-03 19:45 ` Junio C Hamano
2026-09-04 8:16 ` Harald Nordgren
2026-09-04 15:09 ` Phillip Wood
2026-09-04 16:42 ` Harald Nordgren
2026-09-04 15:21 ` Junio C Hamano
2026-09-02 18:29 ` [PATCH v4 2/2] checkout: separate autostash conflict advice from branch-switch message Harald Nordgren via GitGitGadget
2026-09-02 19:52 ` Junio C Hamano
2026-09-03 14:00 ` Phillip Wood [this message]
2026-09-03 14:39 ` [PATCH v5 0/2] checkout -m: refine autostash fallback Harald Nordgren via GitGitGadget
2026-09-03 14:39 ` [PATCH v5 1/2] stash: reserve exit status 1 for conflicts Harald Nordgren via GitGitGadget
2026-09-03 14:39 ` [PATCH v5 2/2] checkout: separate autostash conflict advice from branch-switch message Harald Nordgren via GitGitGadget
2026-09-03 18:53 ` [PATCH v5 0/2] checkout -m: refine autostash fallback Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=66283358-9b87-4485-a036-ba91e6d7fcad@gmail.com \
--to=phillip.wood123@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
--cc=haraldnordgren@gmail.com \
--cc=phillip.wood@dunelm.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox