git status: Fix false positive "new commits" output for dirty submodules
Testing if the output "new commits" should appear in the long format of "git status" is done by comparing the hashes of the diffpair. This always resulted in printing "new commits" for submodules that contained untracked or modified content, even if they did not contain new commits. The reason was that match_stat_with_submodule() did set the "changed" flag for dirty submodules, resulting in two->sha1 being set to the null_sha1 at the call sites, which indicates that new commits are present. This is changed so that when no new commits are present, the same object names are in the sha1 field for both sides of the filepair, and the working tree side will have the "dirty_submodule" flag set when appropriate. For a submodule to be seen as modified even when it just has a dirty work tree, some conditions had to be extended to also check for the "dirty_submodule" flag. Unfortunately the test case that should have found this bug had been changed incorrectly too. It is fixed and extended to test for other combinations too. Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jens Lehmann committed
Mar 12, 2010 at 22:23 UTC
85adbf2f751a91429de6b431c45737ba9d7e9e00
3 files changed
+88
-9
diff-lib.c
+2
-4
@@ -72,8 +72,6 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
72
&& !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)
73
&& (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES))) {
74
*dirty_submodule = is_submodule_modified(ce->name);
75
- if (*dirty_submodule)
76
- changed = 1;
75
}
76
return changed;
77
}
@@ -202,7 +200,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
200
}
201
changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
202
ce_option, &dirty_submodule);
205
- if (!changed) {
203
+ if (!changed && !dirty_submodule) {
204
ce_mark_uptodate(ce);
205
if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
206
continue;
@@ -333,7 +331,7 @@ static int show_modified(struct rev_info *revs,
331
}
332
333
oldmode = old->ce_mode;
336
- if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
334
+ if (mode == oldmode && !hashcmp(sha1, old->sha1) && !dirty_submodule &&
335
!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
336
return 0;
337
diff.c
+5
-2
@@ -2032,7 +2032,7 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
2032
char *data = xmalloc(100), *dirty = "";
2033
2034
/* Are we looking at the work tree? */
2035
- if (!s->sha1_valid && s->dirty_submodule)
2035
+ if (s->dirty_submodule)
2036
dirty = "-dirty";
2037
2038
len = snprintf(data, 100,
@@ -3081,7 +3081,8 @@ int diff_unmodified_pair(struct diff_filepair *p)
3081
* dealing with a change.
3082
*/
3083
if (one->sha1_valid && two->sha1_valid &&
3084
- !hashcmp(one->sha1, two->sha1))
3084
+ !hashcmp(one->sha1, two->sha1) &&
3085
+ !one->dirty_submodule && !two->dirty_submodule)
3086
return 1; /* no change */
3087
if (!one->sha1_valid && !two->sha1_valid)
3088
return 1; /* both look at the same file on the filesystem. */
@@ -3216,6 +3217,8 @@ static void diff_resolve_rename_copy(void)
3217
}
3218
else if (hashcmp(p->one->sha1, p->two->sha1) ||
3219
p->one->mode != p->two->mode ||
3220
+ p->one->dirty_submodule ||
3221
+ p->two->dirty_submodule ||
3222
is_null_sha1(p->one->sha1))
3223
p->status = DIFF_STATUS_MODIFIED;
3224
else {
t/t7506-status-submodule.sh
+81
-3
@@ -34,7 +34,7 @@ test_expect_success 'status with modified file in submodule' '
34
(cd sub && git reset --hard) &&
35
echo "changed" >sub/foo &&
36
git status >output &&
37
- grep "modified: sub (new commits, modified content)" output
37
+ grep "modified: sub (modified content)" output
38
'
39
40
test_expect_success 'status with modified file in submodule (porcelain)' '
@@ -49,7 +49,7 @@ test_expect_success 'status with modified file in submodule (porcelain)' '
49
test_expect_success 'status with added file in submodule' '
50
(cd sub && git reset --hard && echo >foo && git add foo) &&
51
git status >output &&
52
- grep "modified: sub (new commits, modified content)" output
52
+ grep "modified: sub (modified content)" output
53
'
54
55
test_expect_success 'status with added file in submodule (porcelain)' '
@@ -64,7 +64,7 @@ test_expect_success 'status with untracked file in submodule' '
64
(cd sub && git reset --hard) &&
65
echo "content" >sub/new-file &&
66
git status >output &&
67
- grep "modified: sub (new commits, untracked content)" output
67
+ grep "modified: sub (untracked content)" output
68
'
69
70
test_expect_success 'status with untracked file in submodule (porcelain)' '
@@ -74,6 +74,84 @@ test_expect_success 'status with untracked file in submodule (porcelain)' '
74
EOF
75
'
76
77
+test_expect_success 'status with added and untracked file in submodule' '
78
+ (cd sub && git reset --hard && echo >foo && git add foo) &&
79
+ echo "content" >sub/new-file &&
80
+ git status >output &&
81
+ grep "modified: sub (modified content, untracked content)" output
82
+'
83
+
84
+test_expect_success 'status with added and untracked file in submodule (porcelain)' '
85
+ (cd sub && git reset --hard && echo >foo && git add foo) &&
86
+ echo "content" >sub/new-file &&
87
+ git status --porcelain >output &&
88
+ diff output - <<-\EOF
89
+ M sub
90
+ EOF
91
+'
92
+
93
+test_expect_success 'status with modified file in modified submodule' '
94
+ (cd sub && git reset --hard) &&
95
+ rm sub/new-file &&
96
+ (cd sub && echo "next change" >foo && git commit -m "next change" foo) &&
97
+ echo "changed" >sub/foo &&
98
+ git status >output &&
99
+ grep "modified: sub (new commits, modified content)" output
100
+'
101
+
102
+test_expect_success 'status with modified file in modified submodule (porcelain)' '
103
+ (cd sub && git reset --hard) &&
104
+ echo "changed" >sub/foo &&
105
+ git status --porcelain >output &&
106
+ diff output - <<-\EOF
107
+ M sub
108
+ EOF
109
+'
110
+
111
+test_expect_success 'status with added file in modified submodule' '
112
+ (cd sub && git reset --hard && echo >foo && git add foo) &&
113
+ git status >output &&
114
+ grep "modified: sub (new commits, modified content)" output
115
+'
116
+
117
+test_expect_success 'status with added file in modified submodule (porcelain)' '
118
+ (cd sub && git reset --hard && echo >foo && git add foo) &&
119
+ git status --porcelain >output &&
120
+ diff output - <<-\EOF
121
+ M sub
122
+ EOF
123
+'
124
+
125
+test_expect_success 'status with untracked file in modified submodule' '
126
+ (cd sub && git reset --hard) &&
127
+ echo "content" >sub/new-file &&
128
+ git status >output &&
129
+ grep "modified: sub (new commits, untracked content)" output
130
+'
131
+
132
+test_expect_success 'status with untracked file in modified submodule (porcelain)' '
133
+ git status --porcelain >output &&
134
+ diff output - <<-\EOF
135
+ M sub
136
+ EOF
137
+'
138
+
139
+test_expect_success 'status with added and untracked file in modified submodule' '
140
+ (cd sub && git reset --hard && echo >foo && git add foo) &&
141
+ echo "content" >sub/new-file &&
142
+ git status >output &&
143
+ grep "modified: sub (new commits, modified content, untracked content)" output
144
+'
145
+
146
+test_expect_success 'status with added and untracked file in modified submodule (porcelain)' '
147
+ (cd sub && git reset --hard && echo >foo && git add foo) &&
148
+ echo "content" >sub/new-file &&
149
+ git status --porcelain >output &&
150
+ diff output - <<-\EOF
151
+ M sub
152
+ EOF
153
+'
154
+
155
test_expect_success 'rm submodule contents' '
156
rm -rf sub/* sub/.git
157
'