summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2022-08-03 20:36:09 (GMT)
committerJunio C Hamano <gitster@pobox.com>2022-08-03 20:36:09 (GMT)
commit966ff64a3042f879bca83e2376128cf5f39737a7 (patch)
tree9e03fa16df286180840c091d9b1de725122f09b1
parent4e0d160bbc88c3486ff7ccae179e4730aab5dd28 (diff)
parentc23fc075c6b9601e0fe7f4c8e5399a6f0cbff13e (diff)
downloadgit-966ff64a3042f879bca83e2376128cf5f39737a7.zip
git-966ff64a3042f879bca83e2376128cf5f39737a7.tar.gz
git-966ff64a3042f879bca83e2376128cf5f39737a7.tar.bz2
Merge branch 'en/merge-restore-to-pristine'
When "git merge" finds that it cannot perform a merge, it should restore the working tree to the state before the command was initiated, but in some corner cases it didn't. * en/merge-restore-to-pristine: merge: do not exit restore_state() prematurely merge: ensure we can actually restore pre-merge state merge: make restore_state() restore staged state too merge: fix save_state() to work when there are stat-dirty files merge: do not abort early if one strategy fails to handle the merge merge: abort if index does not match HEAD for trivial merges merge-resolve: abort if index does not match HEAD merge-ort-wrappers: make printed message match the one from recursive
-rw-r--r--builtin/merge.c57
-rwxr-xr-xgit-merge-resolve.sh10
-rw-r--r--merge-ort-wrappers.c4
-rwxr-xr-xt/t6402-merge-rename.sh2
-rwxr-xr-xt/t6424-merge-unrelated-index-changes.sh65
-rwxr-xr-xt/t6439-merge-co-error-msgs.sh1
-rwxr-xr-xt/t7607-merge-state.sh32
7 files changed, 154 insertions, 17 deletions
diff --git a/builtin/merge.c b/builtin/merge.c
index 23170f2..f7c92c0 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -313,8 +313,16 @@ static int save_state(struct object_id *stash)
int len;
struct child_process cp = CHILD_PROCESS_INIT;
struct strbuf buffer = STRBUF_INIT;
+ struct lock_file lock_file = LOCK_INIT;
+ int fd;
int rc = -1;
+ fd = repo_hold_locked_index(the_repository, &lock_file, 0);
+ refresh_cache(REFRESH_QUIET);
+ if (0 <= fd)
+ repo_update_index_if_able(the_repository, &lock_file);
+ rollback_lock_file(&lock_file);
+
strvec_pushl(&cp.args, "stash", "create", NULL);
cp.out = -1;
cp.git_cmd = 1;
@@ -375,22 +383,26 @@ static void reset_hard(const struct object_id *oid, int verbose)
static void restore_state(const struct object_id *head,
const struct object_id *stash)
{
- const char *args[] = { "stash", "apply", NULL, NULL };
-
- if (is_null_oid(stash))
- return;
+ struct strvec args = STRVEC_INIT;
reset_hard(head, 1);
- args[2] = oid_to_hex(stash);
+ if (is_null_oid(stash))
+ goto refresh_cache;
+
+ strvec_pushl(&args, "stash", "apply", "--index", "--quiet", NULL);
+ strvec_push(&args, oid_to_hex(stash));
/*
* It is OK to ignore error here, for example when there was
* nothing to restore.
*/
- run_command_v_opt(args, RUN_GIT_CMD);
+ run_command_v_opt(args.v, RUN_GIT_CMD);
+ strvec_clear(&args);
- refresh_cache(REFRESH_QUIET);
+refresh_cache:
+ if (discard_cache() < 0 || read_cache() < 0)
+ die(_("could not read index"));
}
/* This is called when no merge was necessary. */
@@ -754,8 +766,10 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
else
clean = merge_recursive(&o, head, remoteheads->item,
reversed, &result);
- if (clean < 0)
- exit(128);
+ if (clean < 0) {
+ rollback_lock_file(&lock);
+ return 2;
+ }
if (write_locked_index(&the_index, &lock,
COMMIT_LOCK | SKIP_IF_UNCHANGED))
die(_("unable to write %s"), get_index_file());
@@ -1599,6 +1613,21 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
*/
refresh_cache(REFRESH_QUIET);
if (allow_trivial && fast_forward != FF_ONLY) {
+ /*
+ * Must first ensure that index matches HEAD before
+ * attempting a trivial merge.
+ */
+ struct tree *head_tree = get_commit_tree(head_commit);
+ struct strbuf sb = STRBUF_INIT;
+
+ if (repo_index_has_changes(the_repository, head_tree,
+ &sb)) {
+ error(_("Your local changes to the following files would be overwritten by merge:\n %s"),
+ sb.buf);
+ strbuf_release(&sb);
+ return 2;
+ }
+
/* See if it is really trivial. */
git_committer_info(IDENT_STRICT);
printf(_("Trying really trivial in-index merge...\n"));
@@ -1655,12 +1684,12 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
* tree in the index -- this means that the index must be in
* sync with the head commit. The strategies are responsible
* to ensure this.
+ *
+ * Stash away the local changes so that we can try more than one
+ * and/or recover from merge strategies bailing while leaving the
+ * index and working tree polluted.
*/
- if (use_strategies_nr == 1 ||
- /*
- * Stash away the local changes so that we can try more than one.
- */
- save_state(&stash))
+ if (save_state(&stash))
oidclr(&stash);
for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {
diff --git a/git-merge-resolve.sh b/git-merge-resolve.sh
index 343fe7b..77e9312 100755
--- a/git-merge-resolve.sh
+++ b/git-merge-resolve.sh
@@ -5,6 +5,16 @@
#
# Resolve two trees, using enhanced multi-base read-tree.
+. git-sh-setup
+
+# Abort if index does not match HEAD
+if ! git diff-index --quiet --cached HEAD --
+then
+ gettextln "Error: Your local changes to the following files would be overwritten by merge"
+ git diff-index --cached --name-only HEAD -- | sed -e 's/^/ /'
+ exit 2
+fi
+
# The first parameters up to -- are merge bases; the rest are heads.
bases= head= remotes= sep_seen=
for arg
diff --git a/merge-ort-wrappers.c b/merge-ort-wrappers.c
index ad04106..748924a 100644
--- a/merge-ort-wrappers.c
+++ b/merge-ort-wrappers.c
@@ -10,8 +10,8 @@ static int unclean(struct merge_options *opt, struct tree *head)
struct strbuf sb = STRBUF_INIT;
if (head && repo_index_has_changes(opt->repo, head, &sb)) {
- fprintf(stderr, _("Your local changes to the following files would be overwritten by merge:\n %s"),
- sb.buf);
+ error(_("Your local changes to the following files would be overwritten by merge:\n %s"),
+ sb.buf);
strbuf_release(&sb);
return -1;
}
diff --git a/t/t6402-merge-rename.sh b/t/t6402-merge-rename.sh
index 3a32b1a..772238e 100755
--- a/t/t6402-merge-rename.sh
+++ b/t/t6402-merge-rename.sh
@@ -210,7 +210,7 @@ test_expect_success 'updated working tree file should prevent the merge' '
echo >>M one line addition &&
cat M >M.saved &&
git update-index M &&
- test_expect_code 128 git pull --no-rebase . yellow &&
+ test_expect_code 2 git pull --no-rebase . yellow &&
test_cmp M M.saved &&
rm -f M.saved
'
diff --git a/t/t6424-merge-unrelated-index-changes.sh b/t/t6424-merge-unrelated-index-changes.sh
index b6e424a..a61f20c 100755
--- a/t/t6424-merge-unrelated-index-changes.sh
+++ b/t/t6424-merge-unrelated-index-changes.sh
@@ -114,6 +114,39 @@ test_expect_success 'resolve, non-trivial' '
test_path_is_missing .git/MERGE_HEAD
'
+test_expect_success 'resolve, trivial, related file removed' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git rm a &&
+ test_path_is_missing a &&
+
+ test_must_fail git merge -s resolve C^0 &&
+
+ test_path_is_missing a &&
+ test_path_is_missing .git/MERGE_HEAD
+'
+
+test_expect_success 'resolve, non-trivial, related file removed' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ git rm a &&
+ test_path_is_missing a &&
+
+ # We also ask for recursive in order to turn off the "allow_trivial"
+ # setting in builtin/merge.c, and ensure that resolve really does
+ # correctly fail the merge (I guess this also tests that recursive
+ # correctly fails the merge, but the main thing we are attempting
+ # to test here is resolve and are just using the side effect of
+ # adding recursive to ensure that resolve is actually tested rather
+ # than the trivial merge codepath)
+ test_must_fail git merge -s resolve -s recursive D^0 &&
+
+ test_path_is_missing a &&
+ test_path_is_missing .git/MERGE_HEAD
+'
+
test_expect_success 'recursive' '
git reset --hard &&
git checkout B^0 &&
@@ -242,4 +275,36 @@ test_expect_success 'subtree' '
test_path_is_missing .git/MERGE_HEAD
'
+test_expect_success 'avoid failure due to stat-dirty files' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ # Make "a" be stat-dirty
+ test-tool chmtime =+1 a &&
+
+ # stat-dirty file should not prevent stash creation in builtin/merge.c
+ git merge -s resolve -s recursive D^0
+'
+
+test_expect_success 'with multiple strategies, recursive or ort failure do not early abort' '
+ git reset --hard &&
+ git checkout B^0 &&
+
+ test_seq 0 10 >a &&
+ git add a &&
+ git rev-parse :a >expect &&
+
+ sane_unset GIT_TEST_MERGE_ALGORITHM &&
+ test_must_fail git merge -s recursive -s ort -s octopus C^0 >output 2>&1 &&
+
+ grep "Trying merge strategy recursive..." output &&
+ grep "Trying merge strategy ort..." output &&
+ grep "Trying merge strategy octopus..." output &&
+ grep "No merge strategy handled the merge." output &&
+
+ # Changes to "a" should remain staged
+ git rev-parse :a >actual &&
+ test_cmp expect actual
+'
+
test_done
diff --git a/t/t6439-merge-co-error-msgs.sh b/t/t6439-merge-co-error-msgs.sh
index 5bfb027..52cf0c8 100755
--- a/t/t6439-merge-co-error-msgs.sh
+++ b/t/t6439-merge-co-error-msgs.sh
@@ -47,6 +47,7 @@ test_expect_success 'untracked files overwritten by merge (fast and non-fast for
export GIT_MERGE_VERBOSITY &&
test_must_fail git merge branch 2>out2
) &&
+ echo "Merge with strategy ${GIT_TEST_MERGE_ALGORITHM:-ort} failed." >>expect &&
test_cmp out2 expect &&
git reset --hard HEAD^
'
diff --git a/t/t7607-merge-state.sh b/t/t7607-merge-state.sh
new file mode 100755
index 0000000..89a62ac
--- /dev/null
+++ b/t/t7607-merge-state.sh
@@ -0,0 +1,32 @@
+#!/bin/sh
+
+test_description="Test that merge state is as expected after failed merge"
+
+GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
+export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
+. ./test-lib.sh
+
+test_expect_success 'Ensure we restore original state if no merge strategy handles it' '
+ test_commit --no-tag "Initial" base base &&
+
+ for b in branch1 branch2 branch3
+ do
+ git checkout -b $b main &&
+ test_commit --no-tag "Change on $b" base $b || return 1
+ done &&
+
+ git checkout branch1 &&
+ # This is a merge that octopus cannot handle. Note, that it does not
+ # just hit conflicts, it completely fails and says that it cannot
+ # handle this type of merge.
+ test_expect_code 2 git merge branch2 branch3 >output 2>&1 &&
+ grep "fatal: merge program failed" output &&
+ grep "Should not be doing an octopus" output &&
+
+ # Make sure we did not leave stray changes around when no appropriate
+ # merge strategy was found
+ git diff --exit-code --name-status &&
+ test_path_is_missing .git/MERGE_HEAD
+'
+
+test_done