Stefan Beller wrote:
> Suppose I have a superproject 'super', with two submodules 'super/sub'
> and 'super/sub1'. 'super/sub' itself contains a submodule
> 'super/sub/subsub'. Now suppose I run, from within 'super':
>
> echo hi >sub/subsub/stray-file
> echo hi >sub1/stray-file
>
> Currently we get would see the following output in git-status:
>
> git status --short
> m sub
> ? sub1
>
> With this patch applied, the untracked file in the nested submodule is
> turned into an untracked file on the 'super' level as well.
s/turned into/displayed as/
> git status --short
> ? sub
> ? sub1
>
> This doesn't change the output of 'git status --porcelain=1' for nested
> submodules, because its output is always ' M' for either untracked files
> or local modifications no matter the nesting level of the submodule.
>
> 'git status --porcelain=2' is affected by this change in a nested
> submodule, though. Without this patch it would report the direct submodule
> as modified and having no untracked files. With this patch it would report
> untracked files. Chalk this up as a bug fix.
I think that's reasonable, and the documentation does a good job of
describing it.
Does this have any effect on the default mode of 'git status' (without
--short or --porcelain)?
[...]
> --- a/Documentation/git-status.txt
> +++ b/Documentation/git-status.txt
> @@ -187,6 +187,8 @@ Submodules have more state and instead report
> m the submodule has modified content
> ? the submodule has untracked files
>
> +Note that 'm' and '?' are applied recursively, e.g. if a nested submodule
> +in a submodule contains an untracked file, this is reported as '?' as well.
Language nits:
* Can simplify by leaving out "Note that ".
* s/, e\.g\./. For example,/
Should this say a brief word about rationale? The obvious way to
describe it would involve "git add --recurse-submodules", which alas
doesn't exist yet. But we could say
this is reported as '?' as well,
since the change cannot be added using "git add -u" from within any of the
submodules.
[...]
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1078,8 +1078,27 @@ unsigned is_submodule_modified(const char *path, int
> ignore_untracked)
> /* regular untracked files */
> if (buf.buf[0] == '?')
> dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
> - else
> - dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
> +
> + if (buf.buf[0] == 'u' ||
> + buf.buf[0] == '1' ||
> + buf.buf[0] == '2') {
> + /*
> + * T XY SSSS:
> + * T = line type, XY = status, SSSS = submodule state
> + */
> + if (buf.len < 1 + 1 + 2 + 1 + 4)
optional: Can shorten:
/* T = line type, XY = status, SSSS = submodule state */
if (buf.len < strlen("T XY SSSS"))
...
That produces the same code at run time because compilers are able to
inline the strlen of a constant. What you already have also seems
sensible, though.
[...]
> + die("BUG: invalid status --porcelain=2 line %s",
> + buf.buf);
> +
> + /* regular unmerged and renamed files */
> + if (buf.buf[5] == 'S' && buf.buf[8] == 'U')
> + /* nested untracked file */
> + dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
> +
> + if (memcmp(buf.buf + 5, "S..U", 4))
> + /* other change */
> + dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
sanity check: What does this do for a "2" line indicating a sub-submodule
that has been renamed that contains an untracked file? Do we need to
rely on some other indication to show this as a change?
Enumerating some more cases, since I keep finding myself getting lost:
- if the HEAD commit of "sub" changes, we show this as " M sub".
What should we show if the HEAD commit of "sub/subsub" changes?
I think this should be " m".
- if "sub" is renamed, we show this as "R sub -> newname".
What should we show if "sub/subsub" is renamed? It is tempting
to show this as " m".
- if "sub" is deleted, we show this as "D sub". What should we
show if "sub/subsub" is deleted? I think this is " m".
It keeps getting more complicated, but I think this is making sense.
Thanks and hope that helps,
Jonathan
