From: Derrick Stolee <stolee@gmail•com>
To: Taylor Blau <me@ttaylorr•com>, Jeff King <peff@peff•net>
Cc: Thomas Braun <thomas.braun@virtuell-zuhause•de>,
GIT Mailing-list <git@vger•kernel.org>,
Derrick Stolee <dstolee@microsoft•com>
Subject: Re: 2.29.0.rc0.windows.1: Duplicate commit id error message when fetching
Date: Fri, 9 Oct 2020 14:33:02 -0400 [thread overview]
Message-ID: <4c138121-ef58-c870-60b2-8140e6e0cbee@gmail.com> (raw)
In-Reply-To: <20201009182833.GA437455@nand.local>
On 10/9/2020 2:28 PM, Taylor Blau wrote:
> On Fri, Oct 09, 2020 at 01:55:06PM -0400, Jeff King wrote:
>> On Fri, Oct 09, 2020 at 01:46:07PM -0400, Derrick Stolee wrote:
>>
>>>> Can you reproduce it if you do
>>>>
>>>> git config core.commitGraph false
>>>> git config fetch.writeCommitGraph true
>>>> ?
>>>
>>> I _can_ repro it in this case! I think there must be something
>>> very interesting going on where the commit-graph is parsed in
>>> _some_ places, but not in others. This is something that I can
>>> really start to dig into.
>>
>> Here's a much more minimal reproduction:
>>
>> git init repo
>> cd repo
>>
>> git commit --allow-empty -m one
>> git rev-parse HEAD |
>> git -c core.commitGraph=false \
>> commit-graph write --split=no-merge --stdin-commits
>> git rev-parse HEAD |
>> git -c core.commitGraph=false \
>> commit-graph write --split=no-merge --stdin-commits
>>
>> git commit --allow-empty -m two
>> git rev-parse HEAD |
>> git commit-graph write --split --stdin-commits
>>
>> The final write will die() with the "unexpected duplicate" message.
>
> Makes sense; the second commit-graph write won't know that 'one' is
> already in the graph because 'core.commitGraph' prevents
> 'prepare_commit_graph()' from actually loading the graph (actually
> loading the graph would be enough to stop the second write from
> occurring at all.)
Right. We aren't parsing from the commit-graph, so we don't see
that these commits are already in the file.
> I'm of two minds about what we could be doing here:
>
> - On the one hand we could be ignoring `core.commitGraph` setting
> during commit-graph writes and forcibly loading the commit-graph
> anyway.
>
> - But on the other hand, writing a commit graph with `core.commitGraph` set
> to false makes no sense. So, I'd almost rather have us die()
> immediately if core.commitGraph is set to false.
I agree that we should just give up, but die() would not be correct.
We should just "return 0", possibly with a warning.
> I think I'd advocate for the latter, along with Stolee's patch to not
> die in the case of duplicate commits in multiple layers of the graph.
If we agree that writing a commit-graph makes no sense if the feature
is disabled, then I can include a patch that has a test similar to
Peff's and that change.
Thanks,
-Stolee
next prev parent reply other threads:[~2020-10-09 18:35 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-07 20:28 2.29.0.rc0.windows.1: Duplicate commit id error message when fetching Thomas Braun
2020-10-07 21:06 ` Jeff King
2020-10-08 9:52 ` Thomas Braun
2020-10-08 12:06 ` Jeff King
2020-10-08 12:50 ` Derrick Stolee
2020-10-08 13:22 ` Derrick Stolee
2020-10-09 15:29 ` Thomas Braun
2020-10-09 16:49 ` Derrick Stolee
2020-10-09 17:12 ` Thomas Braun
2020-10-09 17:46 ` Derrick Stolee
2020-10-09 17:55 ` Jeff King
2020-10-09 18:28 ` Taylor Blau
2020-10-09 18:33 ` Derrick Stolee [this message]
2020-10-09 18:37 ` Taylor Blau
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=4c138121-ef58-c870-60b2-8140e6e0cbee@gmail.com \
--to=stolee@gmail$(echo .)com \
--cc=dstolee@microsoft$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=me@ttaylorr$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=thomas.braun@virtuell-zuhause$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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