]> git.ipfire.org Git - thirdparty/git.git/commitdiff
Merge branch 'en/dirty-merge-fixes'
authorJunio C Hamano <gitster@pobox.com>
Thu, 2 Aug 2018 22:30:44 +0000 (15:30 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 2 Aug 2018 22:30:45 +0000 (15:30 -0700)
The recursive merge strategy did not properly ensure there was no
change between HEAD and the index before performing its operation,
which has been corrected.

* en/dirty-merge-fixes:
  merge: fix misleading pre-merge check documentation
  merge-recursive: enforce rule that index matches head before merging
  t6044: add more testcases with staged changes before a merge is invoked
  merge-recursive: fix assumption that head tree being merged is HEAD
  merge-recursive: make sure when we say we abort that we actually abort
  t6044: add a testcase for index matching head, when head doesn't match HEAD
  t6044: verify that merges expected to abort actually abort
  index_has_changes(): avoid assuming operating on the_index
  read-cache.c: move index_has_changes() from merge.c

Documentation/git-merge.txt
builtin/am.c
cache.h
merge-recursive.c
merge.c
read-cache.c
t/t6044-merge-unrelated-index-changes.sh
t/t7504-commit-msg-hook.sh
t/t7611-merge-abort.sh

index 537dca7faaed98f9c0e0364d35d63de7182f5071..eb36837f86e21423e0a543eceb67313eed27d310 100644 (file)
@@ -130,9 +130,9 @@ merge' may need to update.
 
 To avoid recording unrelated changes in the merge commit,
 'git pull' and 'git merge' will also abort if there are any changes
-registered in the index relative to the `HEAD` commit.  (One
-exception is when the changed index entries are in the state that
-would result from the merge already.)
+registered in the index relative to the `HEAD` commit.  (Special
+narrow exceptions to this rule may exist depending on which merge
+strategy is in use, but generally, the index must match HEAD.)
 
 If all named commits are already ancestors of `HEAD`, 'git merge'
 will exit early with the message "Already up to date."
index b6eeb46c4b65aefbc83b25d11d2ea28646990137..2c19e69f585a46330718ddea1c13a095cefe9d19 100644 (file)
@@ -1766,7 +1766,7 @@ static void am_run(struct am_state *state, int resume)
 
        refresh_and_write_cache();
 
-       if (index_has_changes(&sb)) {
+       if (index_has_changes(&the_index, NULL, &sb)) {
                write_state_bool(state, "dirtyindex", 1);
                die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf);
        }
@@ -1823,7 +1823,8 @@ static void am_run(struct am_state *state, int resume)
                         * Applying the patch to an earlier tree and merging
                         * the result may have produced the same tree as ours.
                         */
-                       if (!apply_status && !index_has_changes(NULL)) {
+                       if (!apply_status &&
+                           !index_has_changes(&the_index, NULL, NULL)) {
                                say(state, stdout, _("No changes -- Patch already applied."));
                                goto next;
                        }
@@ -1877,7 +1878,7 @@ static void am_resolve(struct am_state *state)
 
        say(state, stdout, _("Applying: %.*s"), linelen(state->msg), state->msg);
 
-       if (!index_has_changes(NULL)) {
+       if (!index_has_changes(&the_index, NULL, NULL)) {
                printf_ln(_("No changes - did you forget to use 'git add'?\n"
                        "If there is nothing left to stage, chances are that something else\n"
                        "already introduced the same changes; you might want to skip this patch."));
diff --git a/cache.h b/cache.h
index 05388e597b22632fb557603e1b173ce5bab69f4e..87785fc83218cff355f02e2e92a7f819594e5b15 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -220,6 +220,7 @@ struct cache_entry {
 /* Forward structure decls */
 struct pathspec;
 struct child_process;
+struct tree;
 
 /*
  * Copy the sha1 and stat state of a cache entry from one to
@@ -696,12 +697,15 @@ extern void move_index_extensions(struct index_state *dst, struct index_state *s
 extern int unmerged_index(const struct index_state *);
 
 /**
- * Returns 1 if the index differs from HEAD, 0 otherwise. When on an unborn
- * branch, returns 1 if there are entries in the index, 0 otherwise. If an
- * strbuf is provided, the space-separated list of files that differ will be
- * appended to it.
- */
-extern int index_has_changes(struct strbuf *sb);
+ * Returns 1 if istate differs from tree, 0 otherwise.  If tree is NULL,
+ * compares istate to HEAD.  If tree is NULL and on an unborn branch,
+ * returns 1 if there are entries in istate, 0 otherwise.  If an strbuf is
+ * provided, the space-separated list of files that differ will be appended
+ * to it.
+ */
+extern int index_has_changes(const struct index_state *istate,
+                            struct tree *tree,
+                            struct strbuf *sb);
 
 extern int verify_path(const char *path, unsigned mode);
 extern int strcmp_offset(const char *s1, const char *s2, size_t *first_change);
index 07d792fd10c6bcee9ca12367fc2ad708fb26f9c4..1446e92beaf5a85376684cef0104dcd226a7e539 100644 (file)
@@ -3281,6 +3281,13 @@ int merge_trees(struct merge_options *o,
                struct tree **result)
 {
        int code, clean;
+       struct strbuf sb = STRBUF_INIT;
+
+       if (!o->call_depth && index_has_changes(&the_index, head, &sb)) {
+               err(o, _("Your local changes to the following files would be overwritten by merge:\n  %s"),
+                   sb.buf);
+               return -1;
+       }
 
        if (o->subtree_shift) {
                merge = shift_tree_object(head, merge, o->subtree_shift);
@@ -3288,13 +3295,6 @@ int merge_trees(struct merge_options *o,
        }
 
        if (oid_eq(&common->object.oid, &merge->object.oid)) {
-               struct strbuf sb = STRBUF_INIT;
-
-               if (!o->call_depth && index_has_changes(&sb)) {
-                       err(o, _("Dirty index: cannot merge (dirty: %s)"),
-                           sb.buf);
-                       return 0;
-               }
                output(o, 0, _("Already up to date!"));
                *result = head;
                return 1;
diff --git a/merge.c b/merge.c
index 0783858739f84028df6eef85d3673c944fabb912..e30e03fb84a7677520d208a5688bb7170c22bf8e 100644 (file)
--- a/merge.c
+++ b/merge.c
@@ -14,37 +14,6 @@ static const char *merge_argument(struct commit *commit)
        return oid_to_hex(commit ? &commit->object.oid : the_hash_algo->empty_tree);
 }
 
-int index_has_changes(struct strbuf *sb)
-{
-       struct object_id head;
-       int i;
-
-       if (!get_oid_tree("HEAD", &head)) {
-               struct diff_options opt;
-
-               diff_setup(&opt);
-               opt.flags.exit_with_status = 1;
-               if (!sb)
-                       opt.flags.quick = 1;
-               do_diff_cache(&head, &opt);
-               diffcore_std(&opt);
-               for (i = 0; sb && i < diff_queued_diff.nr; i++) {
-                       if (i)
-                               strbuf_addch(sb, ' ');
-                       strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
-               }
-               diff_flush(&opt);
-               return opt.flags.has_changes != 0;
-       } else {
-               for (i = 0; sb && i < active_nr; i++) {
-                       if (i)
-                               strbuf_addch(sb, ' ');
-                       strbuf_addstr(sb, active_cache[i]->name);
-               }
-               return !!active_nr;
-       }
-}
-
 int try_merge_command(const char *strategy, size_t xopts_nr,
                      const char **xopts, struct commit_list *common,
                      const char *head_arg, struct commit_list *remotes)
index 56eac508362f272e5d777c98b7f31c0f043c1d89..880849fc8ad8454a756ac0849dff288ad8f38eaa 100644 (file)
@@ -6,6 +6,8 @@
 #define NO_THE_INDEX_COMPATIBILITY_MACROS
 #include "cache.h"
 #include "config.h"
+#include "diff.h"
+#include "diffcore.h"
 #include "tempfile.h"
 #include "lockfile.h"
 #include "cache-tree.h"
@@ -2120,6 +2122,44 @@ int unmerged_index(const struct index_state *istate)
        return 0;
 }
 
+int index_has_changes(const struct index_state *istate,
+                     struct tree *tree,
+                     struct strbuf *sb)
+{
+       struct object_id cmp;
+       int i;
+
+       if (istate != &the_index) {
+               BUG("index_has_changes cannot yet accept istate != &the_index; do_diff_cache needs updating first.");
+       }
+       if (tree)
+               cmp = tree->object.oid;
+       if (tree || !get_oid_tree("HEAD", &cmp)) {
+               struct diff_options opt;
+
+               diff_setup(&opt);
+               opt.flags.exit_with_status = 1;
+               if (!sb)
+                       opt.flags.quick = 1;
+               do_diff_cache(&cmp, &opt);
+               diffcore_std(&opt);
+               for (i = 0; sb && i < diff_queued_diff.nr; i++) {
+                       if (i)
+                               strbuf_addch(sb, ' ');
+                       strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
+               }
+               diff_flush(&opt);
+               return opt.flags.has_changes != 0;
+       } else {
+               for (i = 0; sb && i < istate->cache_nr; i++) {
+                       if (i)
+                               strbuf_addch(sb, ' ');
+                       strbuf_addstr(sb, istate->cache[i]->name);
+               }
+               return !!istate->cache_nr;
+       }
+}
+
 #define WRITE_BUFFER_SIZE 8192
 static unsigned char write_buffer[WRITE_BUFFER_SIZE];
 static unsigned long write_buffer_len;
index 23b86fb9778980ab813d5e252065ebe34fe4736d..5e3779ebc9310bfb25d0c6579f234ecf4fabd12e 100755 (executable)
@@ -82,7 +82,8 @@ test_expect_success 'ff update, important file modified' '
        touch subdir/e &&
        git add subdir/e &&
 
-       test_must_fail git merge E^0
+       test_must_fail git merge E^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'resolve, trivial' '
@@ -91,7 +92,8 @@ test_expect_success 'resolve, trivial' '
 
        touch random_file && git add random_file &&
 
-       test_must_fail git merge -s resolve C^0
+       test_must_fail git merge -s resolve C^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'resolve, non-trivial' '
@@ -100,7 +102,8 @@ test_expect_success 'resolve, non-trivial' '
 
        touch random_file && git add random_file &&
 
-       test_must_fail git merge -s resolve D^0
+       test_must_fail git merge -s resolve D^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'recursive' '
@@ -109,7 +112,8 @@ test_expect_success 'recursive' '
 
        touch random_file && git add random_file &&
 
-       test_must_fail git merge -s recursive C^0
+       test_must_fail git merge -s recursive C^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'recursive, when merge branch matches merge base' '
@@ -118,7 +122,45 @@ test_expect_success 'recursive, when merge branch matches merge base' '
 
        touch random_file && git add random_file &&
 
-       test_must_fail git merge -s recursive F^0
+       test_must_fail git merge -s recursive F^0 &&
+       test_path_is_missing .git/MERGE_HEAD
+'
+
+test_expect_success 'merge-recursive, when index==head but head!=HEAD' '
+       git reset --hard &&
+       git checkout C^0 &&
+
+       # Make index match B
+       git diff C B -- | git apply --cached &&
+       # Merge B & F, with B as "head"
+       git merge-recursive A -- B F > out &&
+       test_i18ngrep "Already up to date" out
+'
+
+test_expect_success 'recursive, when file has staged changes not matching HEAD nor what a merge would give' '
+       git reset --hard &&
+       git checkout B^0 &&
+
+       mkdir subdir &&
+       test_seq 1 10 >subdir/a &&
+       git add subdir/a &&
+
+       # We have staged changes; merge should error out
+       test_must_fail git merge -s recursive E^0 2>err &&
+       test_i18ngrep "changes to the following files would be overwritten" err
+'
+
+test_expect_success 'recursive, when file has staged changes matching what a merge would give' '
+       git reset --hard &&
+       git checkout B^0 &&
+
+       mkdir subdir &&
+       test_seq 1 11 >subdir/a &&
+       git add subdir/a &&
+
+       # We have staged changes; merge should error out
+       test_must_fail git merge -s recursive E^0 2>err &&
+       test_i18ngrep "changes to the following files would be overwritten" err
 '
 
 test_expect_success 'octopus, unrelated file touched' '
@@ -127,7 +169,8 @@ test_expect_success 'octopus, unrelated file touched' '
 
        touch random_file && git add random_file &&
 
-       test_must_fail git merge C^0 D^0
+       test_must_fail git merge C^0 D^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'octopus, related file removed' '
@@ -136,7 +179,8 @@ test_expect_success 'octopus, related file removed' '
 
        git rm b &&
 
-       test_must_fail git merge C^0 D^0
+       test_must_fail git merge C^0 D^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'octopus, related file modified' '
@@ -145,7 +189,8 @@ test_expect_success 'octopus, related file modified' '
 
        echo 12 >>a && git add a &&
 
-       test_must_fail git merge C^0 D^0
+       test_must_fail git merge C^0 D^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'ours' '
@@ -154,7 +199,8 @@ test_expect_success 'ours' '
 
        touch random_file && git add random_file &&
 
-       test_must_fail git merge -s ours C^0
+       test_must_fail git merge -s ours C^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_expect_success 'subtree' '
@@ -163,7 +209,8 @@ test_expect_success 'subtree' '
 
        touch random_file && git add random_file &&
 
-       test_must_fail git merge -s subtree E^0
+       test_must_fail git merge -s subtree E^0 &&
+       test_path_is_missing .git/MERGE_HEAD
 '
 
 test_done
index 302a3a2082a9da593ace637a2c32d8f4cf992cb3..31b9c6a2c1d3c30aa7d6d8a2eb836e7b67831d0c 100755 (executable)
@@ -157,6 +157,7 @@ test_expect_success 'merge bypasses failing hook with --no-verify' '
        test_when_finished "git branch -D newbranch" &&
        test_when_finished "git checkout -f master" &&
        git checkout --orphan newbranch &&
+       git rm -f file &&
        : >file2 &&
        git add file2 &&
        git commit --no-verify file2 -m in-side-branch &&
@@ -168,7 +169,7 @@ test_expect_success 'merge bypasses failing hook with --no-verify' '
 chmod -x "$HOOK"
 test_expect_success POSIXPERM 'with non-executable hook' '
 
-       echo "content" >file &&
+       echo "content" >file &&
        git add file &&
        git commit -m "content"
 
@@ -249,6 +250,7 @@ test_expect_success 'hook called in git-merge picks up commit message' '
        test_when_finished "git branch -D newbranch" &&
        test_when_finished "git checkout -f master" &&
        git checkout --orphan newbranch &&
+       git rm -f file &&
        : >file2 &&
        git add file2 &&
        git commit --no-verify file2 -m in-side-branch &&
index 7b4798e8e4791c78e94da1354ef7279fa456983d..7c84a518aa398cef2f7f39e23e02a9160323d968 100755 (executable)
@@ -187,31 +187,6 @@ test_expect_success 'Fail clean merge with matching dirty worktree' '
        test_cmp expect actual
 '
 
-test_expect_success 'Abort clean merge with matching dirty index' '
-       git add bar &&
-       git diff --staged > expect &&
-       git merge --no-commit clean_branch &&
-       test -f .git/MERGE_HEAD &&
-       ### When aborting the merge, git will discard all staged changes,
-       ### including those that were staged pre-merge. In other words,
-       ### --abort will LOSE any staged changes (the staged changes that
-       ### are lost must match the merge result, or the merge would not
-       ### have been allowed to start). Change expectations accordingly:
-       rm expect &&
-       touch expect &&
-       # Abort merge
-       git merge --abort &&
-       test ! -f .git/MERGE_HEAD &&
-       test "$pre_merge_head" = "$(git rev-parse HEAD)" &&
-       git diff --staged > actual &&
-       test_cmp expect actual &&
-       test -z "$(git diff)"
-'
-
-test_expect_success 'Reset worktree changes' '
-       git reset --hard "$pre_merge_head"
-'
-
 test_expect_success 'Fail conflicting merge with matching dirty worktree' '
        echo barf > bar &&
        git diff > expect &&
@@ -223,97 +198,4 @@ test_expect_success 'Fail conflicting merge with matching dirty worktree' '
        test_cmp expect actual
 '
 
-test_expect_success 'Abort conflicting merge with matching dirty index' '
-       git add bar &&
-       git diff --staged > expect &&
-       test_must_fail git merge conflict_branch &&
-       test -f .git/MERGE_HEAD &&
-       ### When aborting the merge, git will discard all staged changes,
-       ### including those that were staged pre-merge. In other words,
-       ### --abort will LOSE any staged changes (the staged changes that
-       ### are lost must match the merge result, or the merge would not
-       ### have been allowed to start). Change expectations accordingly:
-       rm expect &&
-       touch expect &&
-       # Abort merge
-       git merge --abort &&
-       test ! -f .git/MERGE_HEAD &&
-       test "$pre_merge_head" = "$(git rev-parse HEAD)" &&
-       git diff --staged > actual &&
-       test_cmp expect actual &&
-       test -z "$(git diff)"
-'
-
-test_expect_success 'Reset worktree changes' '
-       git reset --hard "$pre_merge_head"
-'
-
-test_expect_success 'Abort merge with pre- and post-merge worktree changes' '
-       # Pre-merge worktree changes
-       echo xyzzy > foo &&
-       echo barf > bar &&
-       git add bar &&
-       git diff > expect &&
-       git diff --staged > expect-staged &&
-       # Perform merge
-       test_must_fail git merge conflict_branch &&
-       test -f .git/MERGE_HEAD &&
-       # Post-merge worktree changes
-       echo yzxxz > foo &&
-       echo blech > baz &&
-       ### When aborting the merge, git will discard staged changes (bar)
-       ### and unmerged changes (baz). Other changes that are neither
-       ### staged nor marked as unmerged (foo), will be preserved. For
-       ### these changed, git cannot tell pre-merge changes apart from
-       ### post-merge changes, so the post-merge changes will be
-       ### preserved. Change expectations accordingly:
-       git diff -- foo > expect &&
-       rm expect-staged &&
-       touch expect-staged &&
-       # Abort merge
-       git merge --abort &&
-       test ! -f .git/MERGE_HEAD &&
-       test "$pre_merge_head" = "$(git rev-parse HEAD)" &&
-       git diff > actual &&
-       test_cmp expect actual &&
-       git diff --staged > actual-staged &&
-       test_cmp expect-staged actual-staged
-'
-
-test_expect_success 'Reset worktree changes' '
-       git reset --hard "$pre_merge_head"
-'
-
-test_expect_success 'Abort merge with pre- and post-merge index changes' '
-       # Pre-merge worktree changes
-       echo xyzzy > foo &&
-       echo barf > bar &&
-       git add bar &&
-       git diff > expect &&
-       git diff --staged > expect-staged &&
-       # Perform merge
-       test_must_fail git merge conflict_branch &&
-       test -f .git/MERGE_HEAD &&
-       # Post-merge worktree changes
-       echo yzxxz > foo &&
-       echo blech > baz &&
-       git add foo bar &&
-       ### When aborting the merge, git will discard all staged changes
-       ### (foo, bar and baz), and no changes will be preserved. Whether
-       ### the changes were staged pre- or post-merge does not matter
-       ### (except for not preventing starting the merge).
-       ### Change expectations accordingly:
-       rm expect expect-staged &&
-       touch expect &&
-       touch expect-staged &&
-       # Abort merge
-       git merge --abort &&
-       test ! -f .git/MERGE_HEAD &&
-       test "$pre_merge_head" = "$(git rev-parse HEAD)" &&
-       git diff > actual &&
-       test_cmp expect actual &&
-       git diff --staged > actual-staged &&
-       test_cmp expect-staged actual-staged
-'
-
 test_done