Skip to content

Commit b6a7a06

Browse files
gitstergitster
committed
Merge branch 'jl/submodule-diff-dirtiness'
* jl/submodule-diff-dirtiness: git status: ignoring untracked files must apply to submodules too git status: Fix false positive "new commits" output for dirty submodules Refactor dirty submodule detection in diff-lib.c git status: Show detailed dirty status of submodules in long format git diff --submodule: Show detailed dirty status of submodules
2 parents 797d443 + 3bfc450 commit b6a7a06

File tree

10 files changed

+221
-43
lines changed

10 files changed

+221
-43
lines changed

‎diff-lib.c‎

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,27 @@ static int check_removed(const struct cache_entry *ce, struct stat *st)
5555
return 0;
5656
}
5757

58+
/*
59+
* Has a file changed or has a submodule new commits or a dirty work tree?
60+
*
61+
* Return 1 when changes are detected, 0 otherwise. If the DIRTY_SUBMODULES
62+
* option is set, the caller does not only want to know if a submodule is
63+
* modified at all but wants to know all the conditions that are met (new
64+
* commits, untracked content and/or modified content).
65+
*/
66+
static int match_stat_with_submodule(struct diff_options *diffopt,
67+
struct cache_entry *ce, struct stat *st,
68+
unsigned ce_option, unsigned *dirty_submodule)
69+
{
70+
int changed = ce_match_stat(ce, st, ce_option);
71+
if (S_ISGITLINK(ce->ce_mode)
72+
&& !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)
73+
&& (!changed || DIFF_OPT_TST(diffopt, DIRTY_SUBMODULES))) {
74+
*dirty_submodule = is_submodule_modified(ce->name, DIFF_OPT_TST(diffopt, IGNORE_UNTRACKED_IN_SUBMODULES));
75+
}
76+
return changed;
77+
}
78+
5879
int run_diff_files(struct rev_info *revs, unsigned int option)
5980
{
6081
int entries, i;
@@ -177,15 +198,9 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
177198
ce->sha1, ce->name, 0);
178199
continue;
179200
}
180-
changed = ce_match_stat(ce, &st, ce_option);
181-
if (S_ISGITLINK(ce->ce_mode)
182-
&& !DIFF_OPT_TST(&revs->diffopt, IGNORE_SUBMODULES)
183-
&& (!changed || (revs->diffopt.output_format & DIFF_FORMAT_PATCH))
184-
&& is_submodule_modified(ce->name)) {
185-
changed = 1;
186-
dirty_submodule = 1;
187-
}
188-
if (!changed) {
201+
changed = match_stat_with_submodule(&revs->diffopt, ce, &st,
202+
ce_option, &dirty_submodule);
203+
if (!changed && !dirty_submodule) {
189204
ce_mark_uptodate(ce);
190205
if (!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
191206
continue;
@@ -240,14 +255,8 @@ static int get_stat_data(struct cache_entry *ce,
240255
}
241256
return -1;
242257
}
243-
changed = ce_match_stat(ce, &st, 0);
244-
if (S_ISGITLINK(ce->ce_mode)
245-
&& !DIFF_OPT_TST(diffopt, IGNORE_SUBMODULES)
246-
&& (!changed || (diffopt->output_format & DIFF_FORMAT_PATCH))
247-
&& is_submodule_modified(ce->name)) {
248-
changed = 1;
249-
*dirty_submodule = 1;
250-
}
258+
changed = match_stat_with_submodule(diffopt, ce, &st,
259+
0, dirty_submodule);
251260
if (changed) {
252261
mode = ce_mode_from_stat(ce, st.st_mode);
253262
sha1 = null_sha1;
@@ -322,7 +331,7 @@ static int show_modified(struct rev_info *revs,
322331
}
323332

324333
oldmode = old->ce_mode;
325-
if (mode == oldmode && !hashcmp(sha1, old->sha1) &&
334+
if (mode == oldmode && !hashcmp(sha1, old->sha1) && !dirty_submodule &&
326335
!DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
327336
return 0;
328337

‎diff.c‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2032,7 +2032,7 @@ static int diff_populate_gitlink(struct diff_filespec *s, int size_only)
20322032
char *data = xmalloc(100), *dirty = "";
20332033

20342034
/* Are we looking at the work tree? */
2035-
if (!s->sha1_valid && s->dirty_submodule)
2035+
if (s->dirty_submodule)
20362036
dirty = "-dirty";
20372037

20382038
len = snprintf(data, 100,
@@ -2628,6 +2628,12 @@ int diff_setup_done(struct diff_options *options)
26282628
*/
26292629
if (options->pickaxe)
26302630
DIFF_OPT_SET(options, RECURSIVE);
2631+
/*
2632+
* When patches are generated, submodules diffed against the work tree
2633+
* must be checked for dirtiness too so it can be shown in the output
2634+
*/
2635+
if (options->output_format & DIFF_FORMAT_PATCH)
2636+
DIFF_OPT_SET(options, DIRTY_SUBMODULES);
26312637

26322638
if (options->detect_rename && options->rename_limit < 0)
26332639
options->rename_limit = diff_rename_limit_default;
@@ -3086,7 +3092,8 @@ int diff_unmodified_pair(struct diff_filepair *p)
30863092
* dealing with a change.
30873093
*/
30883094
if (one->sha1_valid && two->sha1_valid &&
3089-
!hashcmp(one->sha1, two->sha1))
3095+
!hashcmp(one->sha1, two->sha1) &&
3096+
!one->dirty_submodule && !two->dirty_submodule)
30903097
return 1; /* no change */
30913098
if (!one->sha1_valid && !two->sha1_valid)
30923099
return 1; /* both look at the same file on the filesystem. */
@@ -3221,6 +3228,8 @@ static void diff_resolve_rename_copy(void)
32213228
}
32223229
else if (hashcmp(p->one->sha1, p->two->sha1) ||
32233230
p->one->mode != p->two->mode ||
3231+
p->one->dirty_submodule ||
3232+
p->two->dirty_submodule ||
32243233
is_null_sha1(p->one->sha1))
32253234
p->status = DIFF_STATUS_MODIFIED;
32263235
else {

‎diff.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
6969
#define DIFF_OPT_ALLOW_TEXTCONV (1 << 21)
7070
#define DIFF_OPT_DIFF_FROM_CONTENTS (1 << 22)
7171
#define DIFF_OPT_SUBMODULE_LOG (1 << 23)
72+
#define DIFF_OPT_DIRTY_SUBMODULES (1 << 24)
73+
#define DIFF_OPT_IGNORE_UNTRACKED_IN_SUBMODULES (1 << 25)
7274

7375
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
7476
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)

‎diffcore.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ struct diff_filespec {
4242
#define DIFF_FILE_VALID(spec) (((spec)->mode) != 0)
4343
unsigned should_free : 1; /* data should be free()'ed */
4444
unsigned should_munmap : 1; /* data should be munmap()'ed */
45-
unsigned dirty_submodule : 1; /* For submodules: its work tree is dirty */
45+
unsigned dirty_submodule : 2; /* For submodules: its work tree is dirty */
46+
#define DIRTY_SUBMODULE_UNTRACKED 1
47+
#define DIRTY_SUBMODULE_MODIFIED 2
4648

4749
struct userdiff_driver *driver;
4850
/* data should be considered "binary"; -1 means "don't know yet" */

‎submodule.c‎

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include "commit.h"
66
#include "revision.h"
77
#include "run-command.h"
8+
#include "diffcore.h"
89

910
static int add_submodule_odb(const char *path)
1011
{
@@ -85,13 +86,21 @@ void show_submodule_summary(FILE *f, const char *path,
8586
message = "(revision walker failed)";
8687
}
8788

89+
if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
90+
fprintf(f, "Submodule %s contains untracked content\n", path);
91+
if (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
92+
fprintf(f, "Submodule %s contains modified content\n", path);
93+
94+
if (!hashcmp(one, two)) {
95+
strbuf_release(&sb);
96+
return;
97+
}
98+
8899
strbuf_addf(&sb, "Submodule %s %s..", path,
89100
find_unique_abbrev(one, DEFAULT_ABBREV));
90101
if (!fast_backward && !fast_forward)
91102
strbuf_addch(&sb, '.');
92103
strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
93-
if (dirty_submodule)
94-
strbuf_add(&sb, "-dirty", 6);
95104
if (message)
96105
strbuf_addf(&sb, " %s\n", message);
97106
else
@@ -121,17 +130,21 @@ void show_submodule_summary(FILE *f, const char *path,
121130
strbuf_release(&sb);
122131
}
123132

124-
int is_submodule_modified(const char *path)
133+
unsigned is_submodule_modified(const char *path, int ignore_untracked)
125134
{
126-
int len, i;
135+
int i;
136+
ssize_t len;
127137
struct child_process cp;
128138
const char *argv[] = {
129139
"status",
130140
"--porcelain",
131141
NULL,
142+
NULL,
132143
};
133144
const char *env[LOCAL_REPO_ENV_SIZE + 3];
134145
struct strbuf buf = STRBUF_INIT;
146+
unsigned dirty_submodule = 0;
147+
const char *line, *next_line;
135148

136149
for (i = 0; i < LOCAL_REPO_ENV_SIZE; i++)
137150
env[i] = local_repo_env[i];
@@ -151,6 +164,9 @@ int is_submodule_modified(const char *path)
151164
env[i++] = strbuf_detach(&buf, NULL);
152165
env[i] = NULL;
153166

167+
if (ignore_untracked)
168+
argv[2] = "-uno";
169+
154170
memset(&cp, 0, sizeof(cp));
155171
cp.argv = argv;
156172
cp.env = env;
@@ -161,6 +177,25 @@ int is_submodule_modified(const char *path)
161177
die("Could not run git status --porcelain");
162178

163179
len = strbuf_read(&buf, cp.out, 1024);
180+
line = buf.buf;
181+
while (len > 2) {
182+
if ((line[0] == '?') && (line[1] == '?')) {
183+
dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
184+
if (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
185+
break;
186+
} else {
187+
dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
188+
if (ignore_untracked ||
189+
(dirty_submodule & DIRTY_SUBMODULE_UNTRACKED))
190+
break;
191+
}
192+
next_line = strchr(line, '\n');
193+
if (!next_line)
194+
break;
195+
next_line++;
196+
len -= (next_line - line);
197+
line = next_line;
198+
}
164199
close(cp.out);
165200

166201
if (finish_command(&cp))
@@ -169,5 +204,5 @@ int is_submodule_modified(const char *path)
169204
for (i = LOCAL_REPO_ENV_SIZE; env[i]; i++)
170205
free((char *)env[i]);
171206
strbuf_release(&buf);
172-
return len != 0;
207+
return dirty_submodule;
173208
}

‎submodule.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ void show_submodule_summary(FILE *f, const char *path,
55
unsigned char one[20], unsigned char two[20],
66
unsigned dirty_submodule,
77
const char *del, const char *add, const char *reset);
8-
int is_submodule_modified(const char *path);
8+
unsigned is_submodule_modified(const char *path, int ignore_untracked);
99

1010
#endif

‎t/t4041-diff-submodule.sh‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -201,23 +201,24 @@ test_expect_success 'submodule contains untracked content' "
201201
echo new > sm1/new-file &&
202202
git diff-index -p --submodule=log HEAD >actual &&
203203
diff actual - <<-EOF
204-
Submodule sm1 $head6..$head6-dirty:
204+
Submodule sm1 contains untracked content
205205
EOF
206206
"
207207

208208
test_expect_success 'submodule contains untracked and modifed content' "
209209
echo new > sm1/foo6 &&
210210
git diff-index -p --submodule=log HEAD >actual &&
211211
diff actual - <<-EOF
212-
Submodule sm1 $head6..$head6-dirty:
212+
Submodule sm1 contains untracked content
213+
Submodule sm1 contains modified content
213214
EOF
214215
"
215216

216217
test_expect_success 'submodule contains modifed content' "
217218
rm -f sm1/new-file &&
218219
git diff-index -p --submodule=log HEAD >actual &&
219220
diff actual - <<-EOF
220-
Submodule sm1 $head6..$head6-dirty:
221+
Submodule sm1 contains modified content
221222
EOF
222223
"
223224

@@ -235,7 +236,8 @@ test_expect_success 'modified submodule contains untracked content' "
235236
echo new > sm1/new-file &&
236237
git diff-index -p --submodule=log HEAD >actual &&
237238
diff actual - <<-EOF
238-
Submodule sm1 $head6..$head8-dirty:
239+
Submodule sm1 contains untracked content
240+
Submodule sm1 $head6..$head8:
239241
> change
240242
EOF
241243
"
@@ -244,7 +246,9 @@ test_expect_success 'modified submodule contains untracked and modifed content'
244246
echo modification >> sm1/foo6 &&
245247
git diff-index -p --submodule=log HEAD >actual &&
246248
diff actual - <<-EOF
247-
Submodule sm1 $head6..$head8-dirty:
249+
Submodule sm1 contains untracked content
250+
Submodule sm1 contains modified content
251+
Submodule sm1 $head6..$head8:
248252
> change
249253
EOF
250254
"
@@ -253,7 +257,8 @@ test_expect_success 'modified submodule contains modifed content' "
253257
rm -f sm1/new-file &&
254258
git diff-index -p --submodule=log HEAD >actual &&
255259
diff actual - <<-EOF
256-
Submodule sm1 $head6..$head8-dirty:
260+
Submodule sm1 contains modified content
261+
Submodule sm1 $head6..$head8:
257262
> change
258263
EOF
259264
"

0 commit comments

Comments
 (0)