Skip to content

Commit c18ac30

Browse files
gitstergitster
committed
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
2 parents 2b9afea + 55f39cf commit c18ac30

File tree

9 files changed

+124
-179
lines changed

9 files changed

+124
-179
lines changed

‎Documentation/git-merge.txt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,9 +130,9 @@ merge' may need to update.
130130

131131
To avoid recording unrelated changes in the merge commit,
132132
'git pull' and 'git merge' will also abort if there are any changes
133-
registered in the index relative to the `HEAD` commit. (One
134-
exception is when the changed index entries are in the state that
135-
would result from the merge already.)
133+
registered in the index relative to the `HEAD` commit. (Special
134+
narrow exceptions to this rule may exist depending on which merge
135+
strategy is in use, but generally, the index must match HEAD.)
136136

137137
If all named commits are already ancestors of `HEAD`, 'git merge'
138138
will exit early with the message "Already up to date."

‎builtin/am.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1766,7 +1766,7 @@ static void am_run(struct am_state *state, int resume)
17661766

17671767
refresh_and_write_cache();
17681768

1769-
if (index_has_changes(&sb)) {
1769+
if (index_has_changes(&the_index, NULL, &sb)) {
17701770
write_state_bool(state, "dirtyindex", 1);
17711771
die(_("Dirty index: cannot apply patches (dirty: %s)"), sb.buf);
17721772
}
@@ -1823,7 +1823,8 @@ static void am_run(struct am_state *state, int resume)
18231823
* Applying the patch to an earlier tree and merging
18241824
* the result may have produced the same tree as ours.
18251825
*/
1826-
if (!apply_status && !index_has_changes(NULL)) {
1826+
if (!apply_status &&
1827+
!index_has_changes(&the_index, NULL, NULL)) {
18271828
say(state, stdout, _("No changes -- Patch already applied."));
18281829
goto next;
18291830
}
@@ -1877,7 +1878,7 @@ static void am_resolve(struct am_state *state)
18771878

18781879
say(state, stdout, _("Applying: %.*s"), linelen(state->msg), state->msg);
18791880

1880-
if (!index_has_changes(NULL)) {
1881+
if (!index_has_changes(&the_index, NULL, NULL)) {
18811882
printf_ln(_("No changes - did you forget to use 'git add'?\n"
18821883
"If there is nothing left to stage, chances are that something else\n"
18831884
"already introduced the same changes; you might want to skip this patch."));

‎cache.h‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,7 @@ struct cache_entry {
220220
/* Forward structure decls */
221221
struct pathspec;
222222
struct child_process;
223+
struct tree;
223224

224225
/*
225226
* 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
696697
extern int unmerged_index(const struct index_state *);
697698

698699
/**
699-
* Returns 1 if the index differs from HEAD, 0 otherwise. When on an unborn
700-
* branch, returns 1 if there are entries in the index, 0 otherwise. If an
701-
* strbuf is provided, the space-separated list of files that differ will be
702-
* appended to it.
703-
*/
704-
extern int index_has_changes(struct strbuf *sb);
700+
* Returns 1 if istate differs from tree, 0 otherwise. If tree is NULL,
701+
* compares istate to HEAD. If tree is NULL and on an unborn branch,
702+
* returns 1 if there are entries in istate, 0 otherwise. If an strbuf is
703+
* provided, the space-separated list of files that differ will be appended
704+
* to it.
705+
*/
706+
extern int index_has_changes(const struct index_state *istate,
707+
struct tree *tree,
708+
struct strbuf *sb);
705709

706710
extern int verify_path(const char *path, unsigned mode);
707711
extern int strcmp_offset(const char *s1, const char *s2, size_t *first_change);

‎merge-recursive.c‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3281,20 +3281,20 @@ int merge_trees(struct merge_options *o,
32813281
struct tree **result)
32823282
{
32833283
int code, clean;
3284+
struct strbuf sb = STRBUF_INIT;
3285+
3286+
if (!o->call_depth && index_has_changes(&the_index, head, &sb)) {
3287+
err(o, _("Your local changes to the following files would be overwritten by merge:\n %s"),
3288+
sb.buf);
3289+
return -1;
3290+
}
32843291

32853292
if (o->subtree_shift) {
32863293
merge = shift_tree_object(head, merge, o->subtree_shift);
32873294
common = shift_tree_object(head, common, o->subtree_shift);
32883295
}
32893296

32903297
if (oid_eq(&common->object.oid, &merge->object.oid)) {
3291-
struct strbuf sb = STRBUF_INIT;
3292-
3293-
if (!o->call_depth && index_has_changes(&sb)) {
3294-
err(o, _("Dirty index: cannot merge (dirty: %s)"),
3295-
sb.buf);
3296-
return 0;
3297-
}
32983298
output(o, 0, _("Already up to date!"));
32993299
*result = head;
33003300
return 1;

‎merge.c‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -14,37 +14,6 @@ static const char *merge_argument(struct commit *commit)
1414
return oid_to_hex(commit ? &commit->object.oid : the_hash_algo->empty_tree);
1515
}
1616

17-
int index_has_changes(struct strbuf *sb)
18-
{
19-
struct object_id head;
20-
int i;
21-
22-
if (!get_oid_tree("HEAD", &head)) {
23-
struct diff_options opt;
24-
25-
diff_setup(&opt);
26-
opt.flags.exit_with_status = 1;
27-
if (!sb)
28-
opt.flags.quick = 1;
29-
do_diff_cache(&head, &opt);
30-
diffcore_std(&opt);
31-
for (i = 0; sb && i < diff_queued_diff.nr; i++) {
32-
if (i)
33-
strbuf_addch(sb, ' ');
34-
strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
35-
}
36-
diff_flush(&opt);
37-
return opt.flags.has_changes != 0;
38-
} else {
39-
for (i = 0; sb && i < active_nr; i++) {
40-
if (i)
41-
strbuf_addch(sb, ' ');
42-
strbuf_addstr(sb, active_cache[i]->name);
43-
}
44-
return !!active_nr;
45-
}
46-
}
47-
4817
int try_merge_command(const char *strategy, size_t xopts_nr,
4918
const char **xopts, struct commit_list *common,
5019
const char *head_arg, struct commit_list *remotes)

‎read-cache.c‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#define NO_THE_INDEX_COMPATIBILITY_MACROS
77
#include "cache.h"
88
#include "config.h"
9+
#include "diff.h"
10+
#include "diffcore.h"
911
#include "tempfile.h"
1012
#include "lockfile.h"
1113
#include "cache-tree.h"
@@ -2120,6 +2122,44 @@ int unmerged_index(const struct index_state *istate)
21202122
return 0;
21212123
}
21222124

2125+
int index_has_changes(const struct index_state *istate,
2126+
struct tree *tree,
2127+
struct strbuf *sb)
2128+
{
2129+
struct object_id cmp;
2130+
int i;
2131+
2132+
if (istate != &the_index) {
2133+
BUG("index_has_changes cannot yet accept istate != &the_index; do_diff_cache needs updating first.");
2134+
}
2135+
if (tree)
2136+
cmp = tree->object.oid;
2137+
if (tree || !get_oid_tree("HEAD", &cmp)) {
2138+
struct diff_options opt;
2139+
2140+
diff_setup(&opt);
2141+
opt.flags.exit_with_status = 1;
2142+
if (!sb)
2143+
opt.flags.quick = 1;
2144+
do_diff_cache(&cmp, &opt);
2145+
diffcore_std(&opt);
2146+
for (i = 0; sb && i < diff_queued_diff.nr; i++) {
2147+
if (i)
2148+
strbuf_addch(sb, ' ');
2149+
strbuf_addstr(sb, diff_queued_diff.queue[i]->two->path);
2150+
}
2151+
diff_flush(&opt);
2152+
return opt.flags.has_changes != 0;
2153+
} else {
2154+
for (i = 0; sb && i < istate->cache_nr; i++) {
2155+
if (i)
2156+
strbuf_addch(sb, ' ');
2157+
strbuf_addstr(sb, istate->cache[i]->name);
2158+
}
2159+
return !!istate->cache_nr;
2160+
}
2161+
}
2162+
21232163
#define WRITE_BUFFER_SIZE 8192
21242164
static unsigned char write_buffer[WRITE_BUFFER_SIZE];
21252165
static unsigned long write_buffer_len;

‎t/t6044-merge-unrelated-index-changes.sh‎

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ test_expect_success 'ff update, important file modified' '
8282
touch subdir/e &&
8383
git add subdir/e &&
8484
85-
test_must_fail git merge E^0
85+
test_must_fail git merge E^0 &&
86+
test_path_is_missing .git/MERGE_HEAD
8687
'
8788

8889
test_expect_success 'resolve, trivial' '
@@ -91,7 +92,8 @@ test_expect_success 'resolve, trivial' '
9192
9293
touch random_file && git add random_file &&
9394
94-
test_must_fail git merge -s resolve C^0
95+
test_must_fail git merge -s resolve C^0 &&
96+
test_path_is_missing .git/MERGE_HEAD
9597
'
9698

9799
test_expect_success 'resolve, non-trivial' '
@@ -100,7 +102,8 @@ test_expect_success 'resolve, non-trivial' '
100102
101103
touch random_file && git add random_file &&
102104
103-
test_must_fail git merge -s resolve D^0
105+
test_must_fail git merge -s resolve D^0 &&
106+
test_path_is_missing .git/MERGE_HEAD
104107
'
105108

106109
test_expect_success 'recursive' '
@@ -109,7 +112,8 @@ test_expect_success 'recursive' '
109112
110113
touch random_file && git add random_file &&
111114
112-
test_must_fail git merge -s recursive C^0
115+
test_must_fail git merge -s recursive C^0 &&
116+
test_path_is_missing .git/MERGE_HEAD
113117
'
114118

115119
test_expect_success 'recursive, when merge branch matches merge base' '
@@ -118,7 +122,45 @@ test_expect_success 'recursive, when merge branch matches merge base' '
118122
119123
touch random_file && git add random_file &&
120124
121-
test_must_fail git merge -s recursive F^0
125+
test_must_fail git merge -s recursive F^0 &&
126+
test_path_is_missing .git/MERGE_HEAD
127+
'
128+
129+
test_expect_success 'merge-recursive, when index==head but head!=HEAD' '
130+
git reset --hard &&
131+
git checkout C^0 &&
132+
133+
# Make index match B
134+
git diff C B -- | git apply --cached &&
135+
# Merge B & F, with B as "head"
136+
git merge-recursive A -- B F > out &&
137+
test_i18ngrep "Already up to date" out
138+
'
139+
140+
test_expect_success 'recursive, when file has staged changes not matching HEAD nor what a merge would give' '
141+
git reset --hard &&
142+
git checkout B^0 &&
143+
144+
mkdir subdir &&
145+
test_seq 1 10 >subdir/a &&
146+
git add subdir/a &&
147+
148+
# We have staged changes; merge should error out
149+
test_must_fail git merge -s recursive E^0 2>err &&
150+
test_i18ngrep "changes to the following files would be overwritten" err
151+
'
152+
153+
test_expect_success 'recursive, when file has staged changes matching what a merge would give' '
154+
git reset --hard &&
155+
git checkout B^0 &&
156+
157+
mkdir subdir &&
158+
test_seq 1 11 >subdir/a &&
159+
git add subdir/a &&
160+
161+
# We have staged changes; merge should error out
162+
test_must_fail git merge -s recursive E^0 2>err &&
163+
test_i18ngrep "changes to the following files would be overwritten" err
122164
'
123165

124166
test_expect_success 'octopus, unrelated file touched' '
@@ -127,7 +169,8 @@ test_expect_success 'octopus, unrelated file touched' '
127169
128170
touch random_file && git add random_file &&
129171
130-
test_must_fail git merge C^0 D^0
172+
test_must_fail git merge C^0 D^0 &&
173+
test_path_is_missing .git/MERGE_HEAD
131174
'
132175

133176
test_expect_success 'octopus, related file removed' '
@@ -136,7 +179,8 @@ test_expect_success 'octopus, related file removed' '
136179
137180
git rm b &&
138181
139-
test_must_fail git merge C^0 D^0
182+
test_must_fail git merge C^0 D^0 &&
183+
test_path_is_missing .git/MERGE_HEAD
140184
'
141185

142186
test_expect_success 'octopus, related file modified' '
@@ -145,7 +189,8 @@ test_expect_success 'octopus, related file modified' '
145189
146190
echo 12 >>a && git add a &&
147191
148-
test_must_fail git merge C^0 D^0
192+
test_must_fail git merge C^0 D^0 &&
193+
test_path_is_missing .git/MERGE_HEAD
149194
'
150195

151196
test_expect_success 'ours' '
@@ -154,7 +199,8 @@ test_expect_success 'ours' '
154199
155200
touch random_file && git add random_file &&
156201
157-
test_must_fail git merge -s ours C^0
202+
test_must_fail git merge -s ours C^0 &&
203+
test_path_is_missing .git/MERGE_HEAD
158204
'
159205

160206
test_expect_success 'subtree' '
@@ -163,7 +209,8 @@ test_expect_success 'subtree' '
163209
164210
touch random_file && git add random_file &&
165211
166-
test_must_fail git merge -s subtree E^0
212+
test_must_fail git merge -s subtree E^0 &&
213+
test_path_is_missing .git/MERGE_HEAD
167214
'
168215

169216
test_done

‎t/t7504-commit-msg-hook.sh‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ test_expect_success 'merge bypasses failing hook with --no-verify' '
157157
test_when_finished "git branch -D newbranch" &&
158158
test_when_finished "git checkout -f master" &&
159159
git checkout --orphan newbranch &&
160+
git rm -f file &&
160161
: >file2 &&
161162
git add file2 &&
162163
git commit --no-verify file2 -m in-side-branch &&
@@ -168,7 +169,7 @@ test_expect_success 'merge bypasses failing hook with --no-verify' '
168169
chmod -x "$HOOK"
169170
test_expect_success POSIXPERM 'with non-executable hook' '
170171
171-
echo "content" >> file &&
172+
echo "content" >file &&
172173
git add file &&
173174
git commit -m "content"
174175
@@ -249,6 +250,7 @@ test_expect_success 'hook called in git-merge picks up commit message' '
249250
test_when_finished "git branch -D newbranch" &&
250251
test_when_finished "git checkout -f master" &&
251252
git checkout --orphan newbranch &&
253+
git rm -f file &&
252254
: >file2 &&
253255
git add file2 &&
254256
git commit --no-verify file2 -m in-side-branch &&

0 commit comments

Comments
 (0)