Re: [PATCH v2 6/9] Documentation: add Packfile URIs design doc

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 25, 2020, 19:09 UTC
Message-ID: <20201125190957.1113461-1-jonathantanmy@google.com>
In-Reply-To: <87zh35okzy.fsf@evledraar.gmail.com>

Show 15 quoted lines

> On Wed, Jun 10 2020, Jonathan Tan wrote:
> 
> > +This is the implementation: a feature, marked experimental, that allows the
> > +server to be configured by one or more `uploadpack.blobPackfileUri=<sha1>
> > +<uri>` entries. Whenever the list of objects to be sent is assembled, all such
> > +blobs are excluded, replaced with URIs. The client will download those URIs,
> > +expecting them to each point to packfiles containing single blobs.
> 
> I was poking at this recently to see whether I could change it into the
> more dumb method I noted in
> https://public-inbox.org/git/87k1hv6eel.fsf@evledraar.gmail.com/
> 
> As an aside on a protocol level could that be supported with this
> current verb by having the client say "packfile-uris=early" or something
> like that instead of "packfile-uris"?

Hmm...would the advantage of this be that the client can subsequently report any OIDs it finds as "want"s?

I guess the protocol could be extended to support "packfile-uris" at any negotiation step.

Show 5 quoted lines

> The server advertising the same,
> and the client then just requesting packfile-uris before ls-refs or
> whatever? The server would need to be stateful about what's requested
> when and serve up something different than the current
> one-blob-per-pack.

Statefulness will be difficult. Right now, protocol v2 is stateless, and updating it to be stateful will be difficult, I believe - at least for HTTP, the statelessness design has been long there and other implementations of Git or systems that use Git might have already made that assumption (it is stateless both for protocol v0 and v2).

As for serving more than one blob per pack, the current protocol and implementation already allows this. You can see a demonstration by cloning the following repository, which supports it on the server side:

  GIT_TRACE_PACKET=1 git -c fetch.uriprotocols=https clone \
    https://chromium.googlesource.com/chromium/src/base

> Any pointers to where/how to implement that would be
> welcome, I got lost in the non-linearity of the
> connect.c/fetch-pack.c/upload-pack.c code yesterday.

upload_pack_v2() in upload-pack.c and do_fetch_pack_v2() in fetch-pack.c have the state machines of the server and client side respectively - I think those would be the first places to look.

> But I'm mainly replying here to ask if it's intentional that clients are
> tolerant of the server sending whatever it pleases in the supposedly
> "single blob" packs. As demonstrated by the tests passing with this
> patch:

[snip test]

Yes, it has the same tolerance w.r.t. the packfile URI packs as w.r.t. the inline packfile that gets sent.

Show 6 quoted lines

> As you may guess from the "shattered" I was trying to find if the
> particulars around the partial fsck allowed me to exploit this somehow,
> I haven't found a way to do that, just be annoying by sending the client
> more than they asked for, but I could also do that with the normal
> dialog. Just wondering if the client should be opening the pack and
> barfing if it has more than one object, or not care.

Ah yes, as you said, it's the same as with the normal dialog.