Bug with branches/merges in submodules

Bug with branches/merges in submodules

[Mel Dafert]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

Hello,
I ran into a bug where commits are omitted from `git submodule summary`
and friends when the submodule contains merge commits.
Originally using 2.30.2, I can also reproduce this with a fresh build on the
`next` branch (see attached bugreport).
I would assume that this is not intentional, however I cannot find any relevant
information on this.
Feel free to contact me for extra details.
Best regards,
Mel

[-- Attachment #2: git-bugreport-2021-07-07-1419.txt --]
[-- Type: text/plain, Size: 2209 bytes --]

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue)
Set up a submodule that has branches/merge commits:
```bash
# create repository "child"
mkdir child-repo
cd child-repo
git init
touch initial.txt
git add initial.txt
git commit -m "initial child"
cd ..
# create repository "parent"
mkdir parent
cd parent
git init
# add submodule "child" to repository "parent"
git submodule add ../child-repo/ child
git commit -am "initial parent"
cd child
# make two commits in separate branches in "child"
git switch -c "secondary"
touch s1.txt
git add s1.txt
git commit -m "s1"
git switch master
touch m1.txt
git add m1.txt
git commit -m "m1"
# merge branch "secondary" into "master" in "child" - this creates a merge commit
git merge secondary --no-edit
cd ..
```
Run `git diff --submodule=log` inside the "parent" repository.

What did you expect to happen? (Expected behavior)
`git diff --submodule=log` should show all three commits added to the "child"
submodule:
```
Submodule child XXXXXXX..YYYYYYY
> Merge branch 'secondary'
> m1
> s1
```

What happened instead? (Actual behavior)
`git diff --submodule=log` only shows commits from one ancestor of the merge
commit:
```
Submodule child XXXXXXX..YYYYYYY
> Merge branch 'secondary'
> m1
```

What's different between what you expected and what actually happened?
All the commits added to the "secondary" branch are missing in
`git diff --submodule=log`.

Anything else you want to add:
The commit range shown by `git diff --submodule=log` is correct:
`cd child; git log XXXXXXX..YYYYYYY` shows the correct list of commits.

This bug also affects `git submodule summary`, and `git show --submodule=log`
after the changes have been committed.


[System Info]
git version:
git version 2.32.0.262.g8c582fcd64
cpu: x86_64
built from commit: 8c582fcd643d12802e0a6d7e307890aa5b6b26e1
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.11.0-22-generic #23-Ubuntu SMP Thu Jun 17 00:34:23 UTC 2021 x86_64
compiler info: gnuc: 10.3
libc info: glibc: 2.33
$SHELL (typically, interactive shell): /bin/bash


[Enabled Hooks]

Re: Bug with branches/merges in submodules

[Philippe Blain]

Hi Mel!

Thanks for writing in! Responses inline below.

Le 2021-07-07 à 09:13, Mel Dafert a écrit :
> Hello,
> I ran into a bug where commits are omitted from `git submodule summary`
> and friends when the submodule contains merge commits.
> Originally using 2.30.2, I can also reproduce this with a fresh build on the
> `next` branch (see attached bugreport).
> I would assume that this is not intentional, however I cannot find any relevant
> information on this.

You are right that the doc seems to be quiet about that [1], [2].


> Feel free to contact me for extra details.
> Best regards,
> Mel
> 

> Thank you for filling out a Git bug report!
> Please answer the following questions to help us understand your issue.
> 
> What did you do before the bug happened? (Steps to reproduce your issue)
> Set up a submodule that has branches/merge commits:
> ```bash
> # create repository "child"
> mkdir child-repo
> cd child-repo
> git init
> touch initial.txt
> git add initial.txt
> git commit -m "initial child"
> cd ..
> # create repository "parent"
> mkdir parent
> cd parent
> git init
> # add submodule "child" to repository "parent"
> git submodule add ../child-repo/ child
> git commit -am "initial parent"
> cd child
> # make two commits in separate branches in "child"
> git switch -c "secondary"
> touch s1.txt
> git add s1.txt
> git commit -m "s1"
> git switch master
> touch m1.txt
> git add m1.txt
> git commit -m "m1"
> # merge branch "secondary" into "master" in "child" - this creates a merge commit
> git merge secondary --no-edit
> cd ..
> ```
> Run `git diff --submodule=log` inside the "parent" repository.
> 
> What did you expect to happen? (Expected behavior)
> `git diff --submodule=log` should show all three commits added to the "child"
> submodule:
> ```
> Submodule child XXXXXXX..YYYYYYY
>> Merge branch 'secondary'
>> m1
>> s1
> ```
> 
> What happened instead? (Actual behavior)
> `git diff --submodule=log` only shows commits from one ancestor of the merge
> commit:
> ```
> Submodule child XXXXXXX..YYYYYYY
>> Merge branch 'secondary'
>> m1
> ```
> 
> What's different between what you expected and what actually happened?
> All the commits added to the "secondary" branch are missing in
> `git diff --submodule=log`.
> 
> Anything else you want to add:
> The commit range shown by `git diff --submodule=log` is correct:
> `cd child; git log XXXXXXX..YYYYYYY` shows the correct list of commits.
> 
> This bug also affects `git submodule summary`, and `git show --submodule=log`
> after the changes have been committed.

Thanks for the reproducer. The behaviour for 'git log/show/diff' is due this line [3]
and the behaviour for 'git submodule summary' to these lines [4] [5].

For 'git diff' and friends, it goes back to the addition of the '--submodule=log'
option in 752c0c2492 (Add the --submodule option to the diff option family, 2009-10-19).
(authored CC'ed). The use of '--first-parent' was discussed on the list
when this was implemented [6]. I did not read the whole thing.

For 'git submodule summary', it goes back to the addition of the subcommand
in 1cb639e6b0 (git-submodule summary: show commit summary, 2008-03-11). (author also CC'ed).
The justification of the use of '--first-parent' was not really discussed
as far as I could tell [7].


I personnally think it would be a good addition to be able to choose
if yes or no '--first-parent' should be used.

Cheers,

Philippe.


[1] https://git-scm.com/docs/git-submodule#Documentation/git-submodule.txt-summary--cached--files-n--summary-limitltngtcommit--ltpathgt82308203
[2] https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---submoduleltformatgt
[3] https://github.com/git/git/blob/d486ca60a51c9cb1fe068803c3f540724e95e83a/submodule.c#L453
[4] https://github.com/git/git/blob/d486ca60a51c9cb1fe068803c3f540724e95e83a/builtin/submodule--helper.c#L1020-L1021
[5] https://github.com/git/git/blob/d486ca60a51c9cb1fe068803c3f540724e95e83a/builtin/submodule--helper.c#L1117-L1118
[6] https://lore.kernel.org/git/67a884457aeaead275612be10902a80726b2a7db.1254668669u.git.johannes.schindelin@gmx.de/t/#u
[7] https://lore.kernel.org/git/?q=f%3Ayin+s%3A%280+submodule+summary+NOT%3ARe+%29+

Re: Bug with branches/merges in submodules

[Mel Dafert]

Hello Philippe,
Thank you for your reply, and the points to the mailing list.

>Thanks for the reproducer. The behaviour for 'git log/show/diff' is due this line
>and the behaviour for 'git submodule summary' to these lines.
>
>For 'git diff' and friends, it goes back to the addition of the '--submodule=log'
>option in 752c0c2492 (Add the --submodule option to the diff option family, 2009-10-19).
>(authored CC'ed). The use of '--first-parent' was discussed on the list
>when this was implemented. I did not read the whole thing.
>
>For 'git submodule summary', it goes back to the addition of the subcommand
>in 1cb639e6b0 (git-submodule summary: show commit summary, 2008-03-11). (author also CC'ed).
>The justification of the use of '--first-parent' was not really discussed
>as far as I could tell.
>
>
>I personnally think it would be a good addition to be able to choose
>if yes or no '--first-parent' should be used

The discussion you found [1] also suggested that as a future option.

In the original implementation, the first mention is [2], where the length of the
message is discussed - I (and later discussions) assume that this just
happened to be the preference of the author.

I would thus like to correct myself, and refile this as a feature request - it would
be very helpful to me to have this as an option, possibly even with one that can be set in the config, similar to
'diff.submodule=log'.

I would be open to implementing this - I might need some mentoring, however, as
this would be my first time contributing (and reading the git codebase).

Regards,
Mel

[1] https://lore.kernel.org/git/67a884457aeaead275612be10902a80726b2a7db.1254668669u.git.johannes.schindelin@gmx.de/t/#u
[2] https://lore.kernel.org/git/46dff0320803061750x70d059a2yaf1e5751e9c62150@mail.gmail.com/

Re: Bug with branches/merges in submodules

[Philippe Blain]

Hi Mel,

Le 2021-07-14 à 07:07, Mel Dafert a écrit :
> Hello Philippe,
> Thank you for your reply, and the points to the mailing list.
> 
> 
> I would thus like to correct myself, and refile this as a feature request - it would
> be very helpful to me to have this as an option, possibly even with one that can be set in the config, similar to
> 'diff.submodule=log'.
> 
> I would be open to implementing this - I might need some mentoring, however, as
> this would be my first time contributing (and reading the git codebase).

That would great! The page "Hacking Git" [1] has several pointers to interesting
ressources to learn about the code and the contribution process. One of these is
the "My First Contribution" tutorial. [2]

Cheers,

Philippe.

[1] https://git.github.io/Hacking-Git/
[2] https://git-scm.com/docs/MyFirstContribution