git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Philippe Blain <levraiphilippeblain@gmail.com>
To: Mel Dafert <mel@dafert.at>, git@vger.kernel.org
Cc: Johannes Schindelin <johannes.schindelin@gmx.de>,
	Ping Yin <pkufranky@gmail.com>
Subject: Re: Bug with branches/merges in submodules
Date: Fri, 9 Jul 2021 17:18:31 -0400	[thread overview]
Message-ID: <b519a79a-5e35-bb40-71d3-0fb3c65320d7@gmail.com> (raw)
In-Reply-To: <E9E32A45-DA88-47CB-B7F9-F01F9BEC394C@dafert.at>

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+

  reply	other threads:[~2021-07-09 21:18 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-07 13:13 Bug with branches/merges in submodules Mel Dafert
2021-07-09 21:18 ` Philippe Blain [this message]
2021-07-14 11:07   ` Mel Dafert
2021-07-14 12:25     ` Philippe Blain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b519a79a-5e35-bb40-71d3-0fb3c65320d7@gmail.com \
    --to=levraiphilippeblain@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=johannes.schindelin@gmx.de \
    --cc=mel@dafert.at \
    --cc=pkufranky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).