From: Jeff King <peff@peff.net>
Date: Sat, 27 Feb 2016 22:08:16 GMT
Subject: Re: [PATCH] fetch-pack: fix unadvertised requests validation
Message-ID: <20160227220816.GA17475@sigill.intra.peff.net>
In-Reply-To: <CABaesJ+yNJ6_z=sFc+bDEPqDDsw9fkB5bYgxJaAkAMVqHNWwrQ@mail.gmail.com>

On Sat, Feb 27, 2016 at 05:28:55PM -0300, Gabriel Souza Franco wrote:

> On Sat, Feb 27, 2016 at 4:19 PM, Jeff King <peff@peff.net> wrote:
> > On Sat, Feb 27, 2016 at 02:07:12PM -0500, Jeff King wrote:
> >
> >> We expect whoever creates the "sought" list to fill in the name and sha1
> >> as appropriate. If that is not happening in some code path, then yeah,
> >> filter_refs() will not work as intended. But I think the solution there
> >> would be to fix the caller to set up the "struct ref" more completely.
> >>
> >> Gabriel, did this come from a bug you noticed in practice, or was it
> >> just an intended cleanup?
> 
> I was experimenting with uploadpack.hiderefs and uploadpack.allowTipSHA1InWant
> and couldn't get
> 
>         git fetch-pack $remote <sha1>
> 
> to work, and I traced the failure until that check. Reading more, I now see
> that currently it requires
> 
>        git fetch-pack $remote "<sha1> <sha1>"
> 
> to do what I want.

Ah, that makes sense. I do think the "<sha1> <sha1>" syntax is a bit
weird, and I think mostly because the pure-object fetch came much later
in git's life; at this point hardly anybody uses a manual fetch-pack.

It would probably make sense to "<sha1>" to set up the ref correctly.

> > I double-checked that the code for git-fetch does so. It's in
> > get_fetch_map()
> [...]
> 
> git fetch-pack doesn't use these code paths. I'll send a new patch
> shortly to allow
> bare sha1's in fetch-pack.

Right. Sounds like a good plan.

-Peff