Stefan Beller wrote:
> When a nested submodule has untracked files, it would be reported as
> "modified submodule" in the superproject, because submodules are not
> parsed correctly in is_submodule_modified as they are bucketed into
> the modified pile as "they are not an untracked file".
>
> Signed-off-by: Stefan Beller <sbel...@google.com>
> ---
> submodule.c | 16 ++++++++++++++--
> t/t3600-rm.sh | 2 +-
> t/t7506-status-submodule.sh | 2 +-
> 3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/submodule.c b/submodule.c
> index 467f1de763..ec7e9b1b06 100644
> --- a/submodule.c
> +++ b/submodule.c
> @@ -1078,8 +1078,20 @@ 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;
> +
> + /* regular unmerged and renamed files */
> + if (buf.buf[0] == 'u' ||
> + buf.buf[0] == '1' ||
> + buf.buf[0] == '2') {
> + if (buf.buf[5] == 'S') {
Can this overflow the buffer? Submodule state is supposed to be 4
characters, so could do
/*
* T XY SSSS:
* T = line type, XY = status, SSSS = submodule state
*/
if (buf.len < 1 + 1 + 2 + 1 + 4)
die("BUG: invalid status --porcelain=2 line %s",
buf.buf);
if (buf.buf[5] == 'S' && buf.buf[8] == 'U')
/* untracked file */
dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
if (memcmp(buf.buf + 5, "S..U", 4))
/* other change */
dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
}
> + /* nested submodule handling */
> + if (buf.buf[6] == 'C' || buf.buf[7] == 'M')
> + dirty_submodule |=
> DIRTY_SUBMODULE_MODIFIED;
> + if (buf.buf[8] == 'U')
> + dirty_submodule |=
> DIRTY_SUBMODULE_UNTRACKED;
> + } else
> + dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
> + }
I get lost in these cases. What does it mean if we see S..., for
example?
As an example, if I am understanding correctly, before this patch, if
I have a submodule-in-submodule:
$ find . -name .git
.git
sub/.git
sub/subsub/.git
and I put a stray file in the sub-sub module:
$ echo stray-file >sub/subsub/x.o
then status --porcelain=2 tells me that sub is modified:
$ git status --porcelain=2
1 .M S.M. [...] sub
What should it say after the patch? Is the XY field ".." or ".M"?
Some tests covering these cases with --porcelain=2 and a brief mention
in documentation may help.
Thanks,
Jonathan
diff --git i/submodule.c w/submodule.c
index ec7e9b1b06..aec1b2cdca 100644
--- i/submodule.c
+++ w/submodule.c
@@ -1083,13 +1083,18 @@ unsigned is_submodule_modified(const char *path, int
ignore_untracked)
if (buf.buf[0] == 'u' ||
buf.buf[0] == '1' ||
buf.buf[0] == '2') {
- if (buf.buf[5] == 'S') {
- /* nested submodule handling */
- if (buf.buf[6] == 'C' || buf.buf[7] == 'M')
- dirty_submodule |=
DIRTY_SUBMODULE_MODIFIED;
- if (buf.buf[8] == 'U')
- dirty_submodule |=
DIRTY_SUBMODULE_UNTRACKED;
- } else
+ /*
+ * T XY SSSS:
+ * T = line type, XY = status, SSSS = submodule state
+ */
+ if (buf.len < 1 + 1 + 2 + 1 + 4)
+ die("BUG: invalid status --porcelain=2 line %s",
+ buf.buf);
+ if (buf.buf[5] == 'S' && buf.buf[8] == 'U')
+ /* untracked file */
+ dirty_submodule |= DIRTY_SUBMODULE_UNTRACKED;
+ if (memcmp(buf.buf + 5, "S..U", 4))
+ /* other change */
dirty_submodule |= DIRTY_SUBMODULE_MODIFIED;
}
