Re: [PATCH v3] checkout: add --autostash option for branch switching
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 13, 2026, 17:16 UTC
- Message-ID
- <xmqqv7ez6455.fsf@gitster.g>
- In-Reply-To
- <pull.2234.v3.git.git.1773393818235.gitgitgadget@gmail.com>
"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 21 quoted lines
> From: Harald Nordgren <haraldnordgren@gmail.com> > > When switching branches, local modifications in the working tree can > prevent the checkout from succeeding. While "git rebase" and "git > merge" already support --autostash to handle this case automatically, > "git checkout" and "git switch" require users to manually stash and > unstash their changes. > > Teach "git checkout" and "git switch" to accept --autostash and > --no-autostash options that automatically create a temporary stash > entry before the branch switch begins and apply it after the switch > completes. If the stash application results in conflicts, the stash > entry is saved to the stash list so the user can resolve them later. > > Also add a checkout.autoStash configuration option that enables this > behavior by default, which can be overridden with --no-autostash on > the command line. > > Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com> > --- > checkout: 'autostash' for branch switching
Almost all of the above is now stale, as we no longer call this the "autostash" feature. It turned into a project to vastly improve the "checkout --merge" option, so the proposed log message needs to be revamped to match.
Using the autostash mechanism to simplify checkout.c is a great direction, I think, provided we can maintain the high-quality conflict information (branch labels) that users expect from '-m'.
For a new iteration that invalidates almost everything from the previous round, it is easier if range-diff were omitted from the reviewers' point of view (I am not a GGG user, so I do not know if there is such an option to do so). It is not a huge deal as we can simply ignore the large block of lines as irrelevant ;-)
Show 24 quoted lines
> Documentation/git-checkout.adoc | 50 +++++----- > Documentation/git-switch.adoc | 26 ++--- > builtin/checkout.c | 160 +++++++++++++++--------------- > sequencer.c | 2 +- > t/t7201-co.sh | 170 +++++++++++++++++++++++++++++++- > xdiff-interface.c | 12 +++ > xdiff-interface.h | 1 + > 7 files changed, 293 insertions(+), 128 deletions(-) > > diff --git a/Documentation/git-checkout.adoc b/Documentation/git-checkout.adoc > index 43ccf47cf6..1e8adf6ef3 100644 > --- a/Documentation/git-checkout.adoc > +++ b/Documentation/git-checkout.adoc > @@ -251,20 +251,17 @@ working tree, by copying them from elsewhere, extracting a tarball, etc. > are different between the current branch and the branch to > which you are switching, the command refuses to switch > branches in order to preserve your modifications in context. > + With this option, the conflicting local changes are > + automatically stashed before the switch and reapplied > + afterwards. If the local changes do not overlap with the > + differences between branches, the switch proceeds without > + stashing. If reapplying the stash results in conflicts, the > + entry is saved to the stash list so you can use `git stash > + pop` to recover and `git stash drop` when done.
Great.
> -+ > -When switching branches with `--merge`, staged changes may be lost.
Hmph, I have to admit that I never do "checkout" to go to a different branch with a dirty index that does not match the current HEAD, with or without "--merge"; in fact, not just "I never do" myself, I never thought about what happens when somebody does so, so I do not offhand know what the current code without this patch does, or what the updated code would. In "stash && switch && unstash" flow, the last "unstash" is what decides if the original changes in the index is reapplied to the index or not, IIRC, so we still may lose the distinction between what has been and what has not been added to the index after all, depending on the way how the new code does the "unstash" step.
Show 20 quoted lines
> @@ -578,39 +575,36 @@ $ git checkout mytopic > error: You have local changes to 'frotz'; not switching branches. > ------------ > > -You can give the `-m` flag to the command, which would try a > -three-way merge: > +You can give the `-m` flag to the command, which would save the local > +changes in a stash entry and reset the working tree to allow switching: > > ------------ > $ git checkout -m mytopic > -Auto-merging frotz > +Created autostash: 7a9afa3 > +Applied autostash. > ------------ > > -After this three-way merge, the local modifications are _not_ > +After the switch, the local modifications are reapplied and are _not_ > registered in your index file, so `git diff` would show you what > changes you made since the tip of the new branch.
OK.
Show 7 quoted lines
> === 3. Merge conflict > > -When a merge conflict happens during switching branches with > -the `-m` option, you would see something like this: > +When the locally modified files overlap with files that need to be > +updated by the branch switch, the changes are stashed and reapplied > +after the switch. If the stash application results in conflicts,
In the original text of this third example (and as I understand, these examples are written to be read more-or-less independently), the `-m` option was explicitly mentioned to make it clear that this "conflict" situation is relevant only when the option is used. In the updated text, it is not. We should mention `-m` somewhere in the updated text as well: When the `--merge` (`-m`) option is in effect, _this_ happens.
Show 12 quoted lines
> +they are not resolved and the stash is saved to the stash list: > > ------------ > $ git checkout -m mytopic > -Auto-merging frotz > -ERROR: Merge conflict in frotz > -fatal: merge program failed > +Created autostash: 7a9afa3 > +Applying autostash resulted in conflicts. > +Your changes are safe in the stash. > +You can run "git stash pop" or "git stash drop" at any time. > ------------
Nice. It may probably be on a bit too verbose side, though. The fact we are creating a stash entry or the object name of it is not interesting to the user unless there is a conflict, so I would have thought we would not say "Created autostash" at all, and the last three lines would be replaced with something like the following and shown _only_ when the application of the stashed changes did not succeed cleanly:
Your local changes are stashed, however, applying it to carry
forward your local changes resulted in conflicts: - You can try resolving them now. If you resolved them
successfully, discard the stash entry with "git stash drop". - Alternatively you can "git reset --hard" if you do not want
to deal with them right now, and later "git stash pop" to
recover your local changes.Show 9 quoted lines
> -At this point, `git diff` shows the changes cleanly merged as in > -the previous example, as well as the changes in the conflicted > -files. Edit and resolve the conflict and mark it resolved with > -`git add` as usual: > - > ------------- > -$ edit frotz > -$ git add frotz > -------------
After resolving and adding, they need to do "git stash drop" it, but other than that, this part of the example is still valid, isn't it?
The improvement this patch brings in is that the user now has another choice, i.e.,
Instead of editing and resolving the conflicts right now, you
can clear the slate with `git reset --hard` and resurrect the
local changes, safely stored in the stash at your leisure, with
`git stash pop`.> +At this point, `git stash pop` can be used to recover and resolve > +the conflicts, and `git stash drop` to discard the stash when done.
This is not giving a wrong instruction per-se, but my suggestion above can be used to extend it if we wanted to. It may make it easier to understand to the readers, or it may make it way too verbose. I dunno.
Show 35 quoted lines
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index 1d1667fa4c..d3b3b815a7 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -17,7 +17,6 @@
> #include "merge-ll.h"
> #include "lockfile.h"
> #include "mem-pool.h"
> -#include "merge-ort-wrappers.h"
> #include "object-file.h"
> #include "object-name.h"
> #include "odb.h"
> @@ -30,6 +29,7 @@
> #include "repo-settings.h"
> #include "resolve-undo.h"
> #include "revision.h"
> +#include "sequencer.h"
> #include "setup.h"
> #include "submodule.h"
> #include "symlinks.h"
> @@ -845,83 +845,8 @@ static int merge_working_tree(const struct checkout_opts *opts,
>
> ret = unpack_trees(2, trees, &topts);
> clear_unpack_trees_porcelain(&topts);
> - if (ret == -1) {
> - /*
> - * Unpack couldn't do a trivial merge; either
> - * give up or do a real merge, depending on
> - * whether the merge flag was used.
> - */
> -...
> - }
> + if (ret == -1)
> + return 1;
> }So pleased to see so much custom code getting removed ;-).
Show 13 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index aafd0bc959..72f0afe609 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -4677,7 +4677,7 @@ static void create_autostash_internal(struct repository *r,
> &oid, null_oid(the_hash_algo), 0, UPDATE_REFS_DIE_ON_ERR);
> }
>
> - printf(_("Created autostash: %s\n"), buf.buf);
> + fprintf(stderr, _("Created autostash: %s\n"), buf.buf);
> if (reset_head(r, &ropts) < 0)
> die(_("could not reset --hard"));
> discard_index(r->index);Is this change something all the callers that passes control through this part desire? Don't some of them want the output go to the standard output, not standard error?
If we are going in the direction I suggested above (i.e., be silent about our use of stashes before it becomes relevant to the user), this part may have to become a conditional
if (!silent)
printf(_("Created autostash: %s\n"), buf.buf);or something, where all the current callers before the "checkout --merge" adds a new caller would pass silent==0 to.
Whatever changes this function needs, it looks more like a separate preparatory patch to me.
Show 21 quoted lines
> diff --git a/t/t7201-co.sh b/t/t7201-co.sh > index 9bcf7c0b40..9ace3962ba 100755 > --- a/t/t7201-co.sh > +++ b/t/t7201-co.sh > @@ -171,14 +171,14 @@ test_expect_success 'format of merge conflict from checkout -m' ' > test_cmp expect current && > > cat <<-EOF >expect && > - <<<<<<< simple > + <<<<<<< Updated upstream > a > c > e > ======= > b > d > - >>>>>>> local > + >>>>>>> Stashed changes > EOF > test_cmp expect two > '
Historically, 'git checkout -m' provided the branch names (e.g., "<<<<<<< simple"), which is very helpful context. It would be great if we could find a way to pass the branch names through as labels to the stash application. It may require a preparatory change to "git stash apply" to allow customizing the labels---the underlying machinery at the lowest level does have ability to use labels the callers want it to use, so it may be just the matter of plumbing it through the callchain. I didn't check.
Show 35 quoted lines
> diff --git a/xdiff-interface.c b/xdiff-interface.c
> index f043330f2a..5ee2b96d0a 100644
> --- a/xdiff-interface.c
> +++ b/xdiff-interface.c
> @@ -325,6 +325,18 @@ int parse_conflict_style_name(const char *value)
> return -1;
> }
>
> +const char *conflict_style_name(int style)
> +{
> + switch (style) {
> + case XDL_MERGE_DIFF3:
> + return "diff3";
> + case XDL_MERGE_ZEALOUS_DIFF3:
> + return "zdiff3";
> + default:
> + return "merge";
> + }
> +}
> +
> int git_xmerge_style = -1;
>
> int git_xmerge_config(const char *var, const char *value,
> diff --git a/xdiff-interface.h b/xdiff-interface.h
> index fbc4ceec40..ce54e1c0e0 100644
> --- a/xdiff-interface.h
> +++ b/xdiff-interface.h
> @@ -55,6 +55,7 @@ void xdiff_set_find_func(xdemitconf_t *xecfg, const char *line, int cflags);
> void xdiff_clear_find_func(xdemitconf_t *xecfg);
> struct config_context;
> int parse_conflict_style_name(const char *value);
> +const char *conflict_style_name(int style);
> int git_xmerge_config(const char *var, const char *value,
> const struct config_context *ctx, void *cb);
> extern int git_xmerge_style;This may be a useful addition, but it would probably need to become a separate preparatory patch, possibly with its own test.