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>

diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt
index ba873657cf2f9d139d2ddb7c19e6865cc159a64d..67f1a910f3c330a2e7c7389e43e7c120a54ec882 100644 (file)
--- a/Documentation/git-status.txt
+++ b/Documentation/git-status.txt
@@ -181,6 +181,15 @@ in which case `XY` are `!!`.
     !           !    ignored
     -------------------------------------------------
 
     !           !    ignored
     -------------------------------------------------
 

+Submodules have more state and instead report
+               M    the submodule has a different HEAD than
+                    recorded in the index
+               m    the submodule has modified content
+               ?    the submodule has untracked files
+since modified content or untracked files in a submodule cannot be added
+via `git add` in the superproject to prepare a commit.
+
+

 If -b is used the short-format status is preceded by a line
 
     ## branchname tracking info
 If -b is used the short-format status is preceded by a line
 
     ## branchname tracking info


@@ -210,6 +219,8 @@ field from the first filename).  Third, filenames containing special
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 
 characters are not specially formatted; no quoting or
 backslash-escaping is performed.
 

+Any submodule changes are reported as modified `M` instead of `m` or single `?`.
+

 Porcelain Format Version 2
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 Porcelain Format Version 2
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 5aa6db584cd0dbddf6060eeb46f4c1c9836bc935..a6e5c5bd5607dcf5ebd398179e30deba319ab127 100755 (executable)
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -268,6 +268,14 @@ cat >expect.modified <<EOF
  M submod
 EOF
 
  M submod
 EOF
 

+cat >expect.modified_inside <<EOF
+ m submod
+EOF
+
+cat >expect.modified_untracked <<EOF
+ ? submod
+EOF
+

 cat >expect.cached <<EOF
 D  submod
 EOF
 cat >expect.cached <<EOF
 D  submod
 EOF


@@ -421,7 +429,7 @@ test_expect_success 'rm of a populated submodule with modifications fails unless
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&

-       test_cmp expect.modified actual &&
+       test_cmp expect.modified_inside actual &&

        git rm -f submod &&
        test ! -d submod &&
        git status -s -uno --ignore-submodules=none >actual &&
        git rm -f submod &&
        test ! -d submod &&
        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
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&

-       test_cmp expect.modified actual &&
+       test_cmp expect.modified_untracked actual &&

        git rm -f submod &&
        test ! -d submod &&
        git status -s -uno --ignore-submodules=none >actual &&
        git rm -f submod &&
        test ! -d submod &&
        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
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&

-       test_cmp expect.modified actual &&
+       test_cmp expect.modified_inside actual &&

        git rm -f submod &&
        test ! -d submod &&
        git status -s -uno --ignore-submodules=none >actual &&
        git rm -f submod &&
        test ! -d submod &&
        git status -s -uno --ignore-submodules=none >actual &&


@@ -636,7 +644,7 @@ test_expect_success 'rm of a populated nested submodule with nested modification
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&

-       test_cmp expect.modified actual &&
+       test_cmp expect.modified_inside actual &&

        git rm -f submod &&
        test ! -d submod &&
        git status -s -uno --ignore-submodules=none >actual &&
        git rm -f submod &&
        test ! -d submod &&
        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
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&
        test -d submod &&
        test -f submod/.git &&
        git status -s -uno --ignore-submodules=none >actual &&

-       test_cmp expect.modified actual &&
+       test_cmp expect.modified_inside actual &&

        git rm -f submod &&
        test ! -d submod &&
        git status -s -uno --ignore-submodules=none >actual &&
        git rm -f submod &&
        test ! -d submod &&
        git status -s -uno --ignore-submodules=none >actual &&

diff --git a/t/t7506-status-submodule.sh b/t/t7506-status-submodule.sh
index 51f8d0d03426dd671487c6b7915861f5a4570e35..1fa2ff29093b7728dcaec367515fbb48ff265de0 100755 (executable)
--- a/t/t7506-status-submodule.sh
+++ b/t/t7506-status-submodule.sh
@@ -17,6 +17,12 @@ test_create_repo_with_commit () {
        )
 }
 
        )
 }
 

+sanitize_output () {
+       sed -e "s/$_x40/HASH/" -e "s/$_x40/HASH/" output >output2 &&
+       mv output2 output
+}
+
+

 test_expect_success 'setup' '
        test_create_repo_with_commit sub &&
        echo output > .gitignore &&
 test_expect_success 'setup' '
        test_create_repo_with_commit sub &&
        echo output > .gitignore &&


@@ -50,6 +56,15 @@ test_expect_success 'status with modified file in submodule (porcelain)' '
        EOF
 '
 
        EOF
 '
 

+test_expect_success 'status with modified file in submodule (short)' '
+       (cd sub && git reset --hard) &&
+       echo "changed" >sub/foo &&
+       git status --short >output &&
+       diff output - <<-\EOF
+        m sub
+       EOF
+'
+

 test_expect_success 'status with added file in submodule' '
        (cd sub && git reset --hard && echo >foo && git add foo) &&
        git status >output &&
 test_expect_success 'status with added file in submodule' '
        (cd sub && git reset --hard && echo >foo && git add foo) &&
        git status >output &&


@@ -64,6 +79,14 @@ test_expect_success 'status with added file in submodule (porcelain)' '
        EOF
 '
 
        EOF
 '
 

+test_expect_success 'status with added file in submodule (short)' '
+       (cd sub && git reset --hard && echo >foo && git add foo) &&
+       git status --short >output &&
+       diff output - <<-\EOF
+        m sub
+       EOF
+'
+

 test_expect_success 'status with untracked file in submodule' '
        (cd sub && git reset --hard) &&
        echo "content" >sub/new-file &&
 test_expect_success 'status with untracked file in submodule' '
        (cd sub && git reset --hard) &&
        echo "content" >sub/new-file &&


@@ -83,6 +106,13 @@ test_expect_success 'status with untracked file in submodule (porcelain)' '
        EOF
 '
 
        EOF
 '
 

+test_expect_success 'status with untracked file in submodule (short)' '
+       git status --short >output &&
+       diff output - <<-\EOF
+        ? sub
+       EOF
+'
+

 test_expect_success 'status with added and untracked file in submodule' '
        (cd sub && git reset --hard && echo >foo && git add foo) &&
        echo "content" >sub/new-file &&
 test_expect_success 'status with added and untracked file in submodule' '
        (cd sub && git reset --hard && echo >foo && git add foo) &&
        echo "content" >sub/new-file &&


@@ -287,4 +317,91 @@ test_expect_success 'diff --submodule with merge conflict in .gitmodules' '
        test_cmp diff_submodule_actual diff_submodule_expect
 '
 
        test_cmp diff_submodule_actual diff_submodule_expect
 '
 

+# We'll setup different cases for further testing:
+# sub1 will contain a nested submodule,
+# sub2 will have an untracked file
+# sub3 will have an untracked repository
+test_expect_success 'setup superproject with untracked file in nested submodule' '
+       (
+               cd super &&
+               git clean -dfx &&
+               rm .gitmodules &&
+               git submodule add -f ./sub1 &&
+               git submodule add -f ./sub2 &&
+               git submodule add -f ./sub1 sub3 &&
+               git commit -a -m "messy merge in superproject" &&
+               (
+                       cd sub1 &&
+                       git submodule add ../sub2 &&
+                       git commit -a -m "add sub2 to sub1"
+               ) &&
+               git add sub1 &&
+               git commit -a -m "update sub1 to contain nested sub"
+       ) &&
+       echo content >super/sub1/sub2/file &&
+       echo content >super/sub2/file &&
+       git -C super/sub3 clone ../../sub2 untracked_repository
+'
+
+test_expect_success 'status with untracked file in nested submodule (porcelain)' '
+       git -C super status --porcelain >output &&
+       diff output - <<-\EOF
+        M sub1
+        M sub2
+        M sub3
+       EOF
+'
+
+test_expect_success 'status with untracked file in nested submodule (porcelain=2)' '
+       git -C super status --porcelain=2 >output &&
+       sanitize_output output &&
+       diff output - <<-\EOF
+       1 .M S.M. 160000 160000 160000 HASH HASH sub1
+       1 .M S..U 160000 160000 160000 HASH HASH sub2
+       1 .M S..U 160000 160000 160000 HASH HASH sub3
+       EOF
+'
+
+test_expect_success 'status with untracked file in nested submodule (short)' '
+       git -C super status --short >output &&
+       diff output - <<-\EOF
+        m sub1
+        ? sub2
+        ? sub3
+       EOF
+'
+
+test_expect_success 'setup superproject with modified file in nested submodule' '
+       git -C super/sub1/sub2 add file &&
+       git -C super/sub2 add file
+'
+
+test_expect_success 'status with added file in nested submodule (porcelain)' '
+       git -C super status --porcelain >output &&
+       diff output - <<-\EOF
+        M sub1
+        M sub2
+        M sub3
+       EOF
+'
+
+test_expect_success 'status with added file in nested submodule (porcelain=2)' '
+       git -C super status --porcelain=2 >output &&
+       sanitize_output output &&
+       diff output - <<-\EOF
+       1 .M S.M. 160000 160000 160000 HASH HASH sub1
+       1 .M S.M. 160000 160000 160000 HASH HASH sub2
+       1 .M S..U 160000 160000 160000 HASH HASH sub3
+       EOF
+'
+
+test_expect_success 'status with added file in nested submodule (short)' '
+       git -C super status --short >output &&
+       diff output - <<-\EOF
+        m sub1
+        m sub2
+        ? sub3
+       EOF
+'
+

 test_done
 test_done

diff --git a/wt-status.c b/wt-status.c
index 308cf3779eb05b0953edd5aaeeae26f95104240f..03754849626d1bf117e3821fb7cdffcd594f694f 100644 (file)
--- a/wt-status.c
+++ b/wt-status.c
@@ -407,6 +407,16 @@ static void wt_longstatus_print_change_data(struct wt_status *s,
        strbuf_release(&twobuf);
 }
 
        strbuf_release(&twobuf);
 }
 

+static char short_submodule_status(struct wt_status_change_data *d) {
+       if (d->new_submodule_commits)
+               return 'M';
+       if (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
+               return 'm';
+       if (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
+               return '?';
+       return d->worktree_status;
+}
+

 static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
                                         struct diff_options *options,
                                         void *data)
 static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
                                         struct diff_options *options,
                                         void *data)


@@ -431,10 +441,13 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,
                }
                if (!d->worktree_status)
                        d->worktree_status = p->status;
                }
                if (!d->worktree_status)
                        d->worktree_status = p->status;

-               d->dirty_submodule = p->two->dirty_submodule;
-               if (S_ISGITLINK(p->two->mode))
+               if (S_ISGITLINK(p->two->mode)) {
+                       d->dirty_submodule = p->two->dirty_submodule;

                        d->new_submodule_commits = !!oidcmp(&p->one->oid,
                                                            &p->two->oid);
                        d->new_submodule_commits = !!oidcmp(&p->one->oid,
                                                            &p->two->oid);

+                       if (s->status_format == STATUS_FORMAT_SHORT)
+                               d->worktree_status = short_submodule_status(d);
+               }

 
                switch (p->status) {
                case DIFF_STATUS_ADDED:
 
                switch (p->status) {
                case DIFF_STATUS_ADDED: