[PATCH v15 0/5] checkout: 'autostash' for branch switching

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

 * When the initial merge fails, the caller now knows for sure it was a
   conflict (not some unrelated error) before kicking off the autostash.

 * If the second attempt also fails, the user now actually sees why.
   Previously errors like "would be overwritten by checkout" were being
   swallowed.

 * The stash is reapplied right after the second attempt, so it won't get
   left behind if something else goes wrong later in the checkout.

 * A couple of clean-up steps only run when we actually stashed, so regular
   checkouts aren't paying for work they don't need.

 * Added a small header "The following paths have local changes:" before the
   list of changed files, so it doesn't look like part of the conflict
   advice above it.

 * Reworded the conflict advice into a single paragraph. Feels less shouty
   and doesn't say "local changes" three times.

 * Trimmed and consolidated tests in t7201. A bunch of the old ones couldn't
   actually fail, or were really testing other things; the conflict-path
   tests are now a single test that checks the stash entry directly.

Harald Nordgren (5):
  stash: add --label-ours, --label-theirs, --label-base for apply
  sequencer: allow create_autostash to run silently
  sequencer: teach autostash apply to take optional conflict marker
    labels
  checkout: rollback lock on early returns in merge_working_tree
  checkout -m: autostash when switching branches

 Documentation/git-checkout.adoc |  58 +++++------
 Documentation/git-stash.adoc    |  11 ++-
 Documentation/git-switch.adoc   |  33 +++----
 builtin/checkout.c              | 165 +++++++++++++++-----------------
 builtin/commit.c                |   3 +-
 builtin/merge.c                 |  15 ++-
 builtin/stash.c                 |  28 ++++--
 sequencer.c                     |  69 +++++++++----
 sequencer.h                     |   7 +-
 t/t3420-rebase-autostash.sh     |  16 ++--
 t/t3903-stash.sh                |  24 +++++
 t/t7201-co.sh                   |  61 +++++++++++-
 t/t7600-merge.sh                |   3 +-
 xdiff-interface.c               |  12 +++
 xdiff-interface.h               |   1 +
 xdiff/xmerge.c                  |   6 +-
 16 files changed, 335 insertions(+), 177 deletions(-)


base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2234%2FHaraldNordgren%2Fcheckout_autostash-v15
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2234/HaraldNordgren/checkout_autostash-v15
Pull-Request: https://github.com/git/git/pull/2234

Range-diff vs v14:

 1:  e18c25599a = 1:  aba8e6a9dc stash: add --label-ours, --label-theirs, --label-base for apply
 2:  ce29b10264 = 2:  89e0bfa803 sequencer: allow create_autostash to run silently
 3:  73051d1762 = 3:  a428ce7328 sequencer: teach autostash apply to take optional conflict marker labels
 4:  191058d8e3 = 4:  f358424085 checkout: rollback lock on early returns in merge_working_tree
 5:  86f33df1eb ! 5:  96b14db827 checkout -m: autostash when switching branches
     @@ builtin/checkout.c
       #include "setup.h"
       #include "submodule.h"
       #include "symlinks.h"
     +@@ builtin/checkout.c: struct checkout_opts {
     + 	.auto_advance = 1, \
     + }
     + 
     ++#define MERGE_WORKING_TREE_UNPACK_FAILED (-2)
     ++
     + struct branch_info {
     + 	char *name; /* The short name used */
     + 	char *path; /* The full name of a real branch */
     +@@ builtin/checkout.c: static void setup_branch_path(struct branch_info *branch)
     + 
     + static void init_topts(struct unpack_trees_options *topts, int merge,
     + 		       int show_progress, int overwrite_ignore,
     +-		       struct commit *old_commit)
     ++		       struct commit *old_commit, bool show_unpack_errors)
     + {
     + 	memset(topts, 0, sizeof(*topts));
     + 	topts->head_idx = -1;
     +@@ builtin/checkout.c: static void init_topts(struct unpack_trees_options *topts, int merge,
     + 	topts->initial_checkout = is_index_unborn(the_repository->index);
     + 	topts->update = 1;
     + 	topts->merge = 1;
     +-	topts->quiet = merge && old_commit;
     ++	topts->quiet = merge && old_commit && !show_unpack_errors;
     + 	topts->verbose_update = show_progress;
     + 	topts->fn = twoway_merge;
     + 	topts->preserve_ignored = !overwrite_ignore;
     +@@ builtin/checkout.c: static void init_topts(struct unpack_trees_options *topts, int merge,
     + static int merge_working_tree(const struct checkout_opts *opts,
     + 			      struct branch_info *old_branch_info,
     + 			      struct branch_info *new_branch_info,
     +-			      int *writeout_error)
     ++			      int *writeout_error,
     ++			      bool show_unpack_errors)
     + {
     + 	int ret;
     + 	struct lock_file lock_file = LOCK_INIT;
     +@@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *opts,
     + 
     + 		/* 2-way merge to the new branch */
     + 		init_topts(&topts, opts->merge, opts->show_progress,
     +-			   opts->overwrite_ignore, old_branch_info->commit);
     ++			   opts->overwrite_ignore, old_branch_info->commit,
     ++			   show_unpack_errors);
     + 		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,
       		ret = unpack_trees(2, trees, &topts);
       		clear_unpack_trees_porcelain(&topts);
     @@ builtin/checkout.c: static int merge_working_tree(const struct checkout_opts *op
      -				return ret;
      -			}
      +			rollback_lock_file(&lock_file);
     -+			return ret;
     ++			return MERGE_WORKING_TREE_UNPACK_FAILED;
       		}
       	}
       
     @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       			do_merge = 0;
       	}
       
     -+	if (old_branch_info.name)
     ++	if (old_branch_info.name) {
      +		stash_label_base = old_branch_info.name;
     -+	else if (old_branch_info.commit) {
     ++	} else if (old_branch_info.commit) {
      +		strbuf_add_unique_abbrev(&old_commit_shortname,
      +					 &old_branch_info.commit->object.oid,
      +					 DEFAULT_ABBREV);
     @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
      +	}
      +
       	if (do_merge) {
     - 		ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
     -+		if (ret == -1 && opts->merge) {
     +-		ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
     ++		ret = merge_working_tree(opts, &old_branch_info, new_branch_info,
     ++					 &writeout_error, false);
     ++		if (ret == MERGE_WORKING_TREE_UNPACK_FAILED && opts->merge) {
      +			strbuf_addf(&autostash_msg,
      +				    "autostash while switching to '%s'",
      +				    new_branch_info->name);
     @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
      +					     "CHECKOUT_AUTOSTASH_HEAD",
      +					     autostash_msg.buf, true);
      +			created_autostash = 1;
     -+			ret = merge_working_tree(opts, &old_branch_info, new_branch_info, &writeout_error);
     ++			ret = merge_working_tree(opts, &old_branch_info, new_branch_info,
     ++						 &writeout_error, true);
     ++		}
     ++		if (created_autostash) {
     ++			if (opts->conflict_style >= 0) {
     ++				struct strbuf cfg = STRBUF_INIT;
     ++				strbuf_addf(&cfg, "merge.conflictStyle=%s",
     ++					    conflict_style_name(opts->conflict_style));
     ++				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);
      +		}
       		if (ret) {
     -+			if (created_autostash)
     -+				apply_autostash_ref(the_repository,
     -+						    "CHECKOUT_AUTOSTASH_HEAD",
     -+						    new_branch_info->name,
     -+						    "local",
     -+						    stash_label_base,
     -+						    autostash_msg.buf);
       			branch_info_release(&old_branch_info);
      -			return ret;
      +			strbuf_release(&old_commit_shortname);
     @@ builtin/checkout.c: static int switch_branches(const struct checkout_opts *opts,
       
       	update_refs_for_switch(opts, &old_branch_info, new_branch_info);
       
     -+	if (opts->conflict_style >= 0) {
     -+		struct strbuf cfg = STRBUF_INIT;
     -+		strbuf_addf(&cfg, "merge.conflictStyle=%s",
     -+			    conflict_style_name(opts->conflict_style));
     -+		git_config_push_parameter(cfg.buf);
     -+		strbuf_release(&cfg);
     ++	if (created_autostash) {
     ++		discard_index(the_repository->index);
     ++		if (repo_read_index(the_repository) < 0)
     ++			die(_("index file corrupt"));
     ++
     ++		if (!opts->quiet && new_branch_info->commit) {
     ++			printf(_("The following paths have local changes:\n"));
     ++			show_local_changes(&new_branch_info->commit->object,
     ++					   &opts->diff_options);
     ++		}
      +	}
     -+	apply_autostash_ref(the_repository, "CHECKOUT_AUTOSTASH_HEAD",
     -+			    new_branch_info->name, "local",
     -+			    stash_label_base,
     -+			    autostash_msg.buf);
     -+
     -+	discard_index(the_repository->index);
     -+	if (repo_read_index(the_repository) < 0)
     -+		die(_("index file corrupt"));
     -+
     -+	if (created_autostash && !opts->quiet && new_branch_info->commit)
     -+		show_local_changes(&new_branch_info->commit->object,
     -+				   &opts->diff_options);
      +
       	ret = post_checkout_hook(old_branch_info.commit, new_branch_info->commit, 1);
       	branch_info_release(&old_branch_info);
     @@ sequencer.c: static int apply_save_autostash_oid(const char *stash_oid, int atte
       			ret = error(_("cannot store %s"), stash_oid);
      +		else if (attempt_apply)
      +			fprintf(stderr,
     -+				_("Your local changes are stashed, however, applying it to carry\n"
     -+				  "forward your local changes resulted in conflicts:\n"
     -+				  "\n"
     -+				  " - You can try resolving them now.  If you resolved them\n"
     -+				  "   successfully, discard the stash entry with \"git stash drop\".\n"
     -+				  "\n"
     -+				  " - Alternatively you can \"git reset --hard\" if you do not want\n"
     -+				  "   to deal with them right now, and later \"git stash pop\" to\n"
     -+				  "   recover your local changes.\n"));
     ++				_("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
       			fprintf(stderr,
      -				_("%s\n"
     @@ t/t3420-rebase-autostash.sh: create_expected_failure_apply () {
      -	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.
     -+	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.
     ++	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".
       	EOF
       }
       
     @@ t/t3420-rebase-autostash.sh: create_expected_failure_apply () {
      -	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.
     -+	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.
     ++	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".
       	Successfully rebased and updated refs/heads/rebased-feature-branch.
       	EOF
       }
      
       ## t/t7201-co.sh ##
     +@@ t/t7201-co.sh: test_expect_success 'checkout -m with dirty tree' '
     + 
     + 	test "$(git symbolic-ref HEAD)" = "refs/heads/side" &&
     + 
     +-	printf "M\t%s\n" one >expect.messages &&
     ++	printf "The following paths have local changes:\nM\t%s\n" one >expect.messages &&
     + 	test_cmp expect.messages messages &&
     + 
     + 	fill "M	one" "A	three" "D	two" >expect.main &&
      @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>' '
       	test_cmp expect two
       '
       
     -+test_expect_success 'checkout --merge --conflict=zdiff3 <branch>' '
     -+	git checkout -f main &&
     -+	git reset --hard &&
     -+	git clean -f &&
     -+
     -+	fill a b X d e >two &&
     -+	git checkout --merge --conflict=zdiff3 simple &&
     -+
     -+	cat <<-EOF >expect &&
     -+	a
     -+	<<<<<<< simple
     -+	c
     -+	||||||| main
     -+	b
     -+	c
     -+	d
     -+	=======
     -+	b
     -+	X
     -+	d
     -+	>>>>>>> local
     -+	e
     -+	EOF
     -+	test_cmp expect two
     -+'
     -+
     -+test_expect_success 'checkout -m respects merge.conflictStyle config' '
     -+	git checkout -f main &&
     -+	git reset --hard &&
     -+	git clean -f &&
     -+
     -+	test_config merge.conflictStyle diff3 &&
     -+	fill b d >two &&
     -+	git checkout -m simple &&
     -+
     -+	cat <<-EOF >expect &&
     -+	<<<<<<< simple
     -+	a
     -+	c
     -+	e
     -+	||||||| main
     -+	a
     -+	b
     -+	c
     -+	d
     -+	e
     -+	=======
     -+	b
     -+	d
     -+	>>>>>>> local
     -+	EOF
     -+	test_cmp expect two
     -+'
     -+
     -+test_expect_success 'checkout -m skips stash when no conflict' '
     -+	git checkout -f main &&
     -+	git clean -f &&
     -+
     -+	fill 0 x y z >same &&
     -+	git stash list >stash-before &&
     -+	git checkout -m side >actual 2>&1 &&
     -+	test_grep ! "Created autostash" actual &&
     -+	git stash list >stash-after &&
     -+	test_cmp stash-before stash-after &&
     -+	fill 0 x y z >expect &&
     -+	test_cmp expect same
     -+'
     -+
     -+test_expect_success 'checkout -m skips stash with non-conflicting dirty index' '
     -+	git checkout -f main &&
     -+	git clean -f &&
     -+
     -+	fill 0 x y z >same &&
     -+	git add same &&
     -+	git checkout -m side >actual 2>&1 &&
     -+	test_grep ! "Created autostash" actual &&
     -+	fill 0 x y z >expect &&
     -+	test_cmp expect same
     -+'
     -+
     -+test_expect_success 'checkout -m stashes and applies on conflicting changes' '
     -+	git checkout -f main &&
     -+	git clean -f &&
     -+
     -+	fill 1 2 3 4 5 6 7 >one &&
     -+	git checkout -m side >actual 2>&1 &&
     -+	test_grep ! "Created autostash" actual &&
     -+	test_grep "Applied autostash" actual &&
     -+	fill 1 2 3 4 5 6 7 >expect &&
     -+	test_cmp expect one
     -+'
     -+
      +test_expect_success 'checkout -m with mixed staged and unstaged changes' '
      +	git checkout -f main &&
      +	git clean -f &&
     @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>'
      +	git add same &&
      +	fill 1 2 3 4 5 6 7 >one &&
      +	git checkout -m side >actual 2>&1 &&
     -+	test_grep ! "Created autostash" actual &&
      +	test_grep "Applied autostash" actual &&
      +	fill 0 x y z >expect &&
      +	test_cmp expect same &&
     @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>'
      +	test_cmp expect one
      +'
      +
     -+test_expect_success 'checkout -m stashes on truly conflicting changes' '
     ++test_expect_success 'checkout -m creates a recoverable stash on conflict' '
      +	git checkout -f main &&
      +	git clean -f &&
      +
     @@ t/t7201-co.sh: test_expect_success 'checkout --merge --conflict=diff3 <branch>'
      +	test_must_fail git checkout side 2>stderr &&
      +	test_grep "Your local changes" stderr &&
      +	git checkout -m side >actual 2>&1 &&
     -+	test_grep ! "Created autostash" actual &&
      +	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 &&
     ++	git show --format=%B --diff-merges=1 refs/stash >actual &&
     ++	sed /^index/d actual >actual.trimmed &&
     ++	cat >expect <<-EOF &&
     ++	On main: autostash while switching to ${SQ}side${SQ}
     ++
     ++	diff --git a/one b/one
     ++	--- a/one
     ++	+++ b/one
     ++	@@ -3,6 +3,3 @@
     ++	 3
     ++	 4
     ++	 5
     ++	-6
     ++	-7
     ++	-8
     ++	EOF
     ++	test_cmp expect actual.trimmed &&
      +	git stash drop &&
      +	git reset --hard
      +'
      +
     -+test_expect_success 'checkout -m produces usable stash on conflict' '
     -+	git checkout -f main &&
     -+	git clean -f &&
     -+
     ++test_expect_success 'checkout -m which would overwrite untracked file' '
     ++	git checkout -f --detach main &&
     ++	test_commit another-file &&
     ++	git checkout HEAD^ &&
     ++	>another-file.t &&
      +	fill 1 2 3 4 5 >one &&
     -+	git checkout -m side >actual 2>&1 &&
     -+	test_grep "recover your local changes" actual &&
     -+	git checkout -f main &&
     -+	git stash pop &&
     -+	fill 1 2 3 4 5 >expect &&
     -+	test_cmp expect one
     -+'
     -+
     -+test_expect_success 'checkout -m autostash message includes target branch' '
     -+	git checkout -f main &&
     -+	git clean -f &&
     -+
     -+	fill 1 2 3 4 5 >one &&
     -+	git checkout -m side >actual 2>&1 &&
     -+	git stash list >stash-list &&
     -+	test_grep "autostash while switching to .side." stash-list &&
     -+	git stash drop &&
     -+	git checkout -f main &&
     -+	git reset --hard
     -+'
     -+
     -+test_expect_success 'checkout -m stashes on staged conflicting changes' '
     -+	git checkout -f main &&
     -+	git clean -f &&
     -+
     -+	fill 1 2 3 4 5 >one &&
     -+	git add one &&
     -+	git checkout -m side >actual 2>&1 &&
     -+	test_grep ! "Created autostash" actual &&
     -+	test_grep "resulted in conflicts" actual &&
     -+	test_grep "git stash drop" actual &&
     -+	git stash drop &&
     -+	git reset --hard
     -+'
     -+
     -+test_expect_success 'checkout -m applies stash cleanly with non-overlapping changes in same file' '
     -+	git checkout -f main &&
     -+	git reset --hard &&
     -+	git clean -f &&
     -+
     -+	git checkout -b nonoverlap_base &&
     -+	fill a b c d >file &&
     -+	git add file &&
     -+	git commit -m "add file" &&
     -+
     -+	git checkout -b nonoverlap_child &&
     -+	fill a b c INSERTED d >file &&
     -+	git commit -a -m "insert line near end of file" &&
     -+
     -+	fill DIRTY a b c INSERTED d >file &&
     -+
     -+	git stash list >stash-before &&
     -+	git checkout -m nonoverlap_base 2>stderr &&
     -+	test_grep "Applied autostash" stderr &&
     -+	test_grep ! "resulted in conflicts" stderr &&
     -+
     -+	git stash list >stash-after &&
     -+	test_cmp stash-before stash-after &&
     -+
     -+	fill DIRTY a b c d >expect &&
     -+	test_cmp expect file &&
     -+
     -+	git checkout -f main &&
     -+	git branch -D nonoverlap_base &&
     -+	git branch -D nonoverlap_child
     -+'
     -+
     -+test_expect_success 'checkout -m -b skips stash with dirty tree' '
     -+	git checkout -f main &&
     -+	git clean -f &&
     -+
     -+	fill 0 x y z >same &&
     -+	git checkout -m -b newbranch >actual 2>&1 &&
     -+	test_grep ! "Created autostash" actual &&
     -+	fill 0 x y z >expect &&
     -+	test_cmp expect same &&
     -+	git checkout main &&
     -+	git branch -D newbranch
     ++	test_must_fail git checkout -m @{-1} 2>err &&
     ++	test_grep "would be overwritten by checkout" err &&
     ++	test_grep "another-file.t" err
      +'
      +
       test_expect_success 'switch to another branch while carrying a deletion' '
     @@ t/t7600-merge.sh: test_expect_success 'merge with conflicted --autostash changes
       	test_when_finished "test_might_fail git stash drop" &&
       	git merge --autostash c3 2>err &&
      -	test_grep "Applying autostash resulted in conflicts." err &&
     -+	test_grep "your local changes resulted in conflicts" err &&
     ++	test_grep "applying them" err &&
     ++	test_grep "resulted in conflicts" err &&
       	git show HEAD:file >merge-result &&
       	test_cmp result.1-9 merge-result &&
       	git stash show -p >actual &&

-- 
gitgitgadget