Re: [PATCH] fetch: print an error when declining to request an unadvertised object

On Fri, 2017-02-10 at 10:36 -0800, Junio C Hamano wrote:

quoted

There is this piece of code near the end of builtin/fetch-pack.c:

[...]

that happens before the command shows the list of fetched refs, and
this code is prepared to inspect what happend to the requests it (in
response to the user request) made to the underlying fetch
machinery, and issue the error message.
If you change your command line to "git fetch-pack REMOTE SHA1", you
do see an error from the above.

Yes, "error: no such remote ref NNNN", which at least makes clear that
the operation didn't work, though it would be nice to give a more
specific error message.

This all happens in transport.c::fetch_refs_via_pack().
I think that function is a much better place to error or die than
filter_refs().

I confirmed that checking the sought refs there works.  However, in
filter_refs, it's easy to give a more specific error message that the
server doesn't allow requests for unadvertised objects, and that code
works for "git fetch-pack" too.  To do the same in fetch_refs_via_pack,
we'd have to duplicate a few lines of code from filter_refs and expose
the allow_unadvertised_object_request variable, or just set a flag on
the "struct ref" in filter_refs and check it in fetch_refs_via_pack.

What do you think?  Do you not care about having a more specific error,
in which case I can copy the code from builtin/fetch-pack.c to
fetch_refs_via_pack?  Or shall I add code to filter_refs to set a flag
and add code to builtin/fetch-pack.c and fetch_refs_via_pack to check
the flag?  Or what?

Matt