Merge branch 'en/dirty-merge-fixes'

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
This commit is contained in:
Junio C Hamano 2018-08-02 15:30:44 -07:00
Родитель 2b9afea372 55f39cf755
Коммит c18ac30e9e
9 изменённых файлов: 123 добавлений и 178 удалений

Просмотреть файл

@ -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."

Просмотреть файл

@ -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."));

14
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.
* 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(struct strbuf *sb);
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);

Просмотреть файл

@ -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;

31
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)

Просмотреть файл

@ -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;

Просмотреть файл

@ -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

Просмотреть файл

@ -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 &&

Просмотреть файл

@ -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