short status: improve reporting for submodule changes

If I add an untracked file to a submodule or modify a tracked file, currently "git status --short" treats the change in the same way as changes to the current HEAD of the submodule: $ git clone --quiet --recurse-submodules https://gerrit.googlesource.com/gerrit $ echo hello >gerrit/plugins/replication/stray-file $ sed -i -e 's/.*//' gerrit/plugins/replication/.mailmap $ git -C gerrit status --short M plugins/replication This is by analogy with ordinary files, where "M" represents a change that has not been added yet to the index. But this change cannot be added to the index without entering the submodule, "git add"-ing it, and running "git commit", so the analogy is counterproductive. Introduce new status letters " ?" and " m" for this. These are similar to the existing "??" and " M" but mean that the submodule (not the parent project) has new untracked files and modified files, respectively. The user can use "git add" and "git commit" from within the submodule to add them. Changes to the submodule's HEAD commit can be recorded in the index with a plain "git add -u" and are shown with " M", like today. To avoid excessive clutter, show at most one of " ?", " m", and " M" for the submodule. They represent increasing levels of change --- the last one that applies is shown (e.g., " m" if there are both modified files and untracked files in the submodule, or " M" if the submodule's HEAD has been modified and it has untracked files). While making these changes, we need to make sure to not break porcelain level 1, which shares code with "status --short". We only change "git status --short". Non-short "git status" and "git status --porcelain=2" already handle these cases by showing more detail: $ git -C gerrit status --porcelain=2 1 .M S.MU 160000 160000 160000 305c864db28eb0c77c8499bc04c87de3f849cf3c 305c864db28eb0c77c8499bc04c87de3f849cf3c plugins/replication $ git -C gerrit status [...] modified: plugins/replication (modified content, untracked content) Scripts caring about these distinctions should use --porcelain=2. Helped-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Stefan Beller <sbeller@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Stefan Beller committed Mar 29, 2017 at 15:26 UTC dd6962dd731e6ec679f772d2bb14657f79062580
4 files changed +156 -7
Documentation/git-status.txt
+11
@@ -181,6 +181,15 @@ in which case `XY` are `!!`.
181 ! ! ignored
182 -------------------------------------------------
183
184 +Submodules have more state and instead report
185 + M the submodule has a different HEAD than
186 + recorded in the index
187 + m the submodule has modified content
188 + ? the submodule has untracked files
189 +since modified content or untracked files in a submodule cannot be added
190 +via `git add` in the superproject to prepare a commit.
191 +
192 +
193 If -b is used the short-format status is preceded by a line
194
195 ## branchname tracking info
@@ -210,6 +219,8 @@ field from the first filename). Third, filenames containing special
219 characters are not specially formatted; no quoting or
220 backslash-escaping is performed.
221
222 +Any submodule changes are reported as modified `M` instead of `m` or single `?`.
223 +
224 Porcelain Format Version 2
225 ~~~~~~~~~~~~~~~~~~~~~~~~~~
226
t/t3600-rm.sh
+13 -5
@@ -268,6 +268,14 @@ cat >expect.modified <<EOF
268 M submod
269 EOF
270
271 +cat >expect.modified_inside <<EOF
272 + m submod
273 +EOF
274 +
275 +cat >expect.modified_untracked <<EOF
276 + ? submod
277 +EOF
278 +
279 cat >expect.cached <<EOF
280 D submod
281 EOF
@@ -421,7 +429,7 @@ test_expect_success 'rm of a populated submodule with modifications fails unless
429 test -d submod &&
430 test -f submod/.git &&
431 git status -s -uno --ignore-submodules=none >actual &&
424 - test_cmp expect.modified actual &&
432 + test_cmp expect.modified_inside actual &&
433 git rm -f submod &&
434 test ! -d submod &&
435 git status -s -uno --ignore-submodules=none >actual &&
@@ -436,7 +444,7 @@ test_expect_success 'rm of a populated submodule with untracked files fails unle
444 test -d submod &&
445 test -f submod/.git &&
446 git status -s -uno --ignore-submodules=none >actual &&
439 - test_cmp expect.modified actual &&
447 + test_cmp expect.modified_untracked actual &&
448 git rm -f submod &&
449 test ! -d submod &&
450 git status -s -uno --ignore-submodules=none >actual &&
@@ -621,7 +629,7 @@ test_expect_success 'rm of a populated nested submodule with different nested HE
629 test -d submod &&
630 test -f submod/.git &&
631 git status -s -uno --ignore-submodules=none >actual &&
624 - test_cmp expect.modified actual &&
632 + test_cmp expect.modified_inside actual &&
633 git rm -f submod &&
634 test ! -d submod &&
635 git status -s -uno --ignore-submodules=none >actual &&
@@ -636,7 +644,7 @@ test_expect_success 'rm of a populated nested submodule with nested modification
644 test -d submod &&
645 test -f submod/.git &&
646 git status -s -uno --ignore-submodules=none >actual &&
639 - test_cmp expect.modified actual &&
647 + test_cmp expect.modified_inside actual &&
648 git rm -f submod &&
649 test ! -d submod &&
650 git status -s -uno --ignore-submodules=none >actual &&
@@ -651,7 +659,7 @@ test_expect_success 'rm of a populated nested submodule with nested untracked fi
659 test -d submod &&
660 test -f submod/.git &&
661 git status -s -uno --ignore-submodules=none >actual &&
654 - test_cmp expect.modified actual &&
662 + test_cmp expect.modified_inside actual &&
663 git rm -f submod &&
664 test ! -d submod &&
665 git status -s -uno --ignore-submodules=none >actual &&
t/t7506-status-submodule.sh
+117
@@ -17,6 +17,12 @@ test_create_repo_with_commit () {
17 )
18 }
19
20 +sanitize_output () {
21 + sed -e "s/$_x40/HASH/" -e "s/$_x40/HASH/" output >output2 &&
22 + mv output2 output
23 +}
24 +
25 +
26 test_expect_success 'setup' '
27 test_create_repo_with_commit sub &&
28 echo output > .gitignore &&
@@ -50,6 +56,15 @@ test_expect_success 'status with modified file in submodule (porcelain)' '
56 EOF
57 '
58
59 +test_expect_success 'status with modified file in submodule (short)' '
60 + (cd sub && git reset --hard) &&
61 + echo "changed" >sub/foo &&
62 + git status --short >output &&
63 + diff output - <<-\EOF
64 + m sub
65 + EOF
66 +'
67 +
68 test_expect_success 'status with added file in submodule' '
69 (cd sub && git reset --hard && echo >foo && git add foo) &&
70 git status >output &&
@@ -64,6 +79,14 @@ test_expect_success 'status with added file in submodule (porcelain)' '
79 EOF
80 '
81
82 +test_expect_success 'status with added file in submodule (short)' '
83 + (cd sub && git reset --hard && echo >foo && git add foo) &&
84 + git status --short >output &&
85 + diff output - <<-\EOF
86 + m sub
87 + EOF
88 +'
89 +
90 test_expect_success 'status with untracked file in submodule' '
91 (cd sub && git reset --hard) &&
92 echo "content" >sub/new-file &&
@@ -83,6 +106,13 @@ test_expect_success 'status with untracked file in submodule (porcelain)' '
106 EOF
107 '
108
109 +test_expect_success 'status with untracked file in submodule (short)' '
110 + git status --short >output &&
111 + diff output - <<-\EOF
112 + ? sub
113 + EOF
114 +'
115 +
116 test_expect_success 'status with added and untracked file in submodule' '
117 (cd sub && git reset --hard && echo >foo && git add foo) &&
118 echo "content" >sub/new-file &&
@@ -287,4 +317,91 @@ test_expect_success 'diff --submodule with merge conflict in .gitmodules' '
317 test_cmp diff_submodule_actual diff_submodule_expect
318 '
319
320 +# We'll setup different cases for further testing:
321 +# sub1 will contain a nested submodule,
322 +# sub2 will have an untracked file
323 +# sub3 will have an untracked repository
324 +test_expect_success 'setup superproject with untracked file in nested submodule' '
325 + (
326 + cd super &&
327 + git clean -dfx &&
328 + rm .gitmodules &&
329 + git submodule add -f ./sub1 &&
330 + git submodule add -f ./sub2 &&
331 + git submodule add -f ./sub1 sub3 &&
332 + git commit -a -m "messy merge in superproject" &&
333 + (
334 + cd sub1 &&
335 + git submodule add ../sub2 &&
336 + git commit -a -m "add sub2 to sub1"
337 + ) &&
338 + git add sub1 &&
339 + git commit -a -m "update sub1 to contain nested sub"
340 + ) &&
341 + echo content >super/sub1/sub2/file &&
342 + echo content >super/sub2/file &&
343 + git -C super/sub3 clone ../../sub2 untracked_repository
344 +'
345 +
346 +test_expect_success 'status with untracked file in nested submodule (porcelain)' '
347 + git -C super status --porcelain >output &&
348 + diff output - <<-\EOF
349 + M sub1
350 + M sub2
351 + M sub3
352 + EOF
353 +'
354 +
355 +test_expect_success 'status with untracked file in nested submodule (porcelain=2)' '
356 + git -C super status --porcelain=2 >output &&
357 + sanitize_output output &&
358 + diff output - <<-\EOF
359 + 1 .M S.M. 160000 160000 160000 HASH HASH sub1
360 + 1 .M S..U 160000 160000 160000 HASH HASH sub2
361 + 1 .M S..U 160000 160000 160000 HASH HASH sub3
362 + EOF
363 +'
364 +
365 +test_expect_success 'status with untracked file in nested submodule (short)' '
366 + git -C super status --short >output &&
367 + diff output - <<-\EOF
368 + m sub1
369 + ? sub2
370 + ? sub3
371 + EOF
372 +'
373 +
374 +test_expect_success 'setup superproject with modified file in nested submodule' '
375 + git -C super/sub1/sub2 add file &&
376 + git -C super/sub2 add file
377 +'
378 +
379 +test_expect_success 'status with added file in nested submodule (porcelain)' '
380 + git -C super status --porcelain >output &&
381 + diff output - <<-\EOF
382 + M sub1
383 + M sub2
384 + M sub3
385 + EOF
386 +'
387 +
388 +test_expect_success 'status with added file in nested submodule (porcelain=2)' '
389 + git -C super status --porcelain=2 >output &&
390 + sanitize_output output &&
391 + diff output - <<-\EOF
392 + 1 .M S.M. 160000 160000 160000 HASH HASH sub1
393 + 1 .M S.M. 160000 160000 160000 HASH HASH sub2
394 + 1 .M S..U 160000 160000 160000 HASH HASH sub3
395 + EOF
396 +'
397 +
398 +test_expect_success 'status with added file in nested submodule (short)' '
399 + git -C super status --short >output &&
400 + diff output - <<-\EOF
401 + m sub1
402 + m sub2
403 + ? sub3
404 + EOF
405 +'
406 +
407 test_done
wt-status.c
+15 -2
@@ -407,6 +407,16 @@ static void wt_longstatus_print_change_data(struct wt_status *s,
407 strbuf_release(&twobuf);
408 }
409
410 +static char short_submodule_status(struct wt_status_change_data *d) {
411 + if (d->new_submodule_commits)
412 + return 'M';
413 + if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
414 + return 'm';
415 + if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
416 + return '?';
417 + return d->worktree_status;
418 +}
419 +
420 static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
421 struct diff_options *options,
422 void *data)
@@ -431,10 +441,13 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
441 }
442 if (!d->worktree_status)
443 d->worktree_status = p->status;
434 - d->dirty_submodule = p->two->dirty_submodule;
435 - if (S_ISGITLINK(p->two->mode))
444 + if (S_ISGITLINK(p->two->mode)) {
445 + d->dirty_submodule = p->two->dirty_submodule;
446 d->new_submodule_commits = !!oidcmp(&p->one->oid,
447 &p->two->oid);
448 + if (s->status_format == STATUS_FORMAT_SHORT)
449 + d->worktree_status = short_submodule_status(d);
450 + }
451
452 switch (p->status) {
453 case DIFF_STATUS_ADDED: