checkout: prevent losing staged changes with --merge

When --merge is specified, we may need to do a real merge (instead of three-way tree unpacking), the steps are best seen in git-checkout.sh version before it's removed: # Match the index to the working tree, and do a three-way. git diff-files --name-only | git update-index --remove --stdin && work=`git write-tree` && git read-tree $v --reset -u $new || exit git merge-recursive $old -- $new $work # Do not register the cleanly merged paths in the index yet. # this is not a real merge before committing, but just carrying # the working tree changes along. unmerged=`git ls-files -u` git read-tree $v --reset $new case "$unmerged" in '') ;; *) ( z40=0000000000000000000000000000000000000000 echo "$unmerged" | sed -e 's/^[0-7]* [0-9a-f]* /'"0 $z40 /" echo "$unmerged" ) | git update-index --index-info ;; esac Notice the last 'read-tree --reset' step. We restore worktree back to 'new' tree after worktree's messed up by merge-recursive. If there are staged changes before this whole command sequence is executed, they are lost because they are unlikely part of the 'new' tree to be restored. There is no easy way to fix this. Elijah may have something up his sleeves [1], but until then, check if there are staged changes and refuse to run and lose them. The user would need to do "git reset" to continue in this case. A note about the test update. 'checkout -m' in that test will fail because a deletion is staged. This 'checkout -m' was previously needed to verify quietness behavior of unpack-trees. But a different check has been put in place in the last patch. We can safely drop 'checkout -m' now. [1] CABPp-BFoL_U=bzON4SEMaQSKU2TKwnOgNqjt5MUaOejTKGUJxw@mail.gmail.com Reported-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Mar 22, 2019 at 16:31 UTC 6eff409e8a760645ae5357d1e95e7e7ff3c04456
2 files changed +11 -10
builtin/checkout.c
+10 -1
@@ -725,7 +725,10 @@ static int merge_working_tree(const struct checkout_opts *opts,
725 */
726 struct tree *result;
727 struct tree *work;
728 + struct tree *old_tree;
729 struct merge_options o;
730 + struct strbuf sb = STRBUF_INIT;
731 +
732 if (!opts->merge)
733 return 1;
734
@@ -735,6 +738,12 @@ static int merge_working_tree(const struct checkout_opts *opts,
738 */
739 if (!old_branch_info->commit)
740 return 1;
741 + old_tree = get_commit_tree(old_branch_info->commit);
742 +
743 + if (repo_index_has_changes(the_repository, old_tree, &sb))
744 + die(_("cannot continue with staged changes in "
745 + "the following files:\n%s"), sb.buf);
746 + strbuf_release(&sb);
747
748 /* Do more real merge */
749
@@ -772,7 +781,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
781 ret = merge_trees(&o,
782 get_commit_tree(new_branch_info->commit),
783 work,
775 - get_commit_tree(old_branch_info->commit),
784 + old_tree,
785 &result);
786 if (ret < 0)
787 exit(128);
t/t7201-co.sh
+1 -9
@@ -224,15 +224,7 @@ test_expect_success 'switch to another branch while carrying a deletion' '
224 test_i18ngrep overwritten errs &&
225
226 test_must_fail git read-tree --quiet -m -u HEAD simple 2>errs &&
227 - test_must_be_empty errs &&
228 -
229 - git checkout --merge simple 2>errs &&
230 - test_i18ngrep ! overwritten errs &&
231 - git ls-files -u &&
232 - test_must_fail git cat-file -t :0:two &&
233 - test "$(git cat-file -t :1:two)" = blob &&
234 - test "$(git cat-file -t :2:two)" = blob &&
235 - test_must_fail git cat-file -t :3:two
227 + test_must_be_empty errs
228 '
229
230 test_expect_success 'checkout to detach HEAD (with advice declined)' '