Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-12 Thread Derrick Stolee
On 9/12/2019 10:23 AM, Jeff King wrote: > On Thu, Sep 12, 2019 at 08:23:49AM -0400, Derrick Stolee wrote: > >>> That creates an interesting problem for commits that have _already_ been >>> parsed using the commit graph. Their commit->object.parsed flag is set, >>> their commit->graph_pos is set, b

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-12 Thread Jeff King
On Thu, Sep 12, 2019 at 08:23:49AM -0400, Derrick Stolee wrote: > > That creates an interesting problem for commits that have _already_ been > > parsed using the commit graph. Their commit->object.parsed flag is set, > > their commit->graph_pos is set, but their commit->maybe_tree may still > > be

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-12 Thread Jeff King
On Wed, Sep 11, 2019 at 10:08:48PM -0400, Taylor Blau wrote: > > The test suite passes with my patch both with and without > > GIT_TEST_COMMIT_GRAPH=1. But to my surprise, it also passes if I delete > > the close_commit_graph() line added by 829a321569! > > > > So it's not clear to me whether this

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-12 Thread Jeff King
On Thu, Sep 12, 2019 at 01:06:20PM +0200, SZEDER Gábor wrote: > > > +# - we must use protocol v2, because it handles the "have" negotiation > > > before > > > +#processing the shallow direectives > > s/ee/e/ Thanks, fixed. > We can't simply replace that 'git config' command with 'test_con

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-12 Thread Jeff King
On Wed, Sep 11, 2019 at 10:07:48PM -0400, Taylor Blau wrote: > > The new test in t5500 triggers this segfault, but see the comments there > > for how horribly intimate it has to be with how both upload-pack and > > commit graphs work. > > Thanks for the comment, too. I agree that protocol-level h

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-12 Thread Derrick Stolee
On 9/11/2019 10:07 PM, Taylor Blau wrote:>> >> +# A few subtle things about the request in this test: >> +# >> +# - the server must have commit-graphs present and enabled > > I think "enabled" may be somewhat redundant, at least since some recent > changes to enable this by default. (As an aside,

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-12 Thread Derrick Stolee
On 9/11/2019 8:04 PM, Jeff King wrote: > When the client has asked for certain shallow options like > "deepen-since", we do a custom rev-list walk that pretends to be > shallow. Before doing so, we have to disable the commit-graph, since it > is not compatible with the shallow view of the repositor

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-12 Thread SZEDER Gábor
On Wed, Sep 11, 2019 at 10:07:48PM -0400, Taylor Blau wrote: > > diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh > > index 8210f63d41..7601664b74 100755 > > --- a/t/t5500-fetch-pack.sh > > +++ b/t/t5500-fetch-pack.sh > > +# - we must use protocol v2, because it handles the "have" negot

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-11 Thread Taylor Blau
On Wed, Sep 11, 2019 at 08:18:46PM -0400, Jeff King wrote: > On Wed, Sep 11, 2019 at 08:04:14PM -0400, Jeff King wrote: > > > When the client has asked for certain shallow options like > > "deepen-since", we do a custom rev-list walk that pretends to be > > shallow. Before doing so, we have to disa

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-11 Thread Taylor Blau
Hi Peff, Thanks in advance for digging into all of this. I feel guilty for having found the issue myself, and then gotten a headache for just long enough to have you completely fix the issue by the time that I got back. So, thanks :-). On Wed, Sep 11, 2019 at 08:04:15PM -0400, Jeff King wrote: >

Re: [PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-11 Thread Jeff King
On Wed, Sep 11, 2019 at 08:04:14PM -0400, Jeff King wrote: > When the client has asked for certain shallow options like > "deepen-since", we do a custom rev-list walk that pretends to be > shallow. Before doing so, we have to disable the commit-graph, since it > is not compatible with the shallow

[PATCH] upload-pack: disable commit graph more gently for shallow traversal

2019-09-11 Thread Jeff King
When the client has asked for certain shallow options like "deepen-since", we do a custom rev-list walk that pretends to be shallow. Before doing so, we have to disable the commit-graph, since it is not compatible with the shallow view of the repository. That's handled by 829a321569 (commit-graph: