To answer Junio's questions in [1], I think it's best to include the full patch set that I'm developing, so here it is. The original patch is now patch 3 of this set. [1] https://public-inbox.org/git/xmqq8t3pnphe.fsf@xxxxxxxxxxxxxxxxxxxxxxxxx/ Rearranging Junio's questions: > ... ah, do you mean that this is not a new feature, but is a bugfix > for some callers that are not calling get-remote-refs before calling > fetch-refs, and the bit is to work around the fact that some > transport not just can function without get-remote-refs first but do > not want to call it? Yes, it is the bugfix you describe, except that the bug coincidentally does not cause any bad behavior. fetch-object.c indeed does not call get-remote-refs before fetch-refs, but it calls transport_set_option(), which so happens to do what we need (call set_helper_option()). However, we need it now, because ... > But this I do not quite understand. It looks saying "when asked to > fetch, if the transport does not allow us to do so without first > getting the advertisement, lazily do that", and that may be a good > thing to do, but then aren't the current set of callers already > calling transport-get-remote-refs elsewhere before they call > transport-fetch-refs? IOW, I would have expected to see a matching > removal, or at least a code that turns an unconditional call to > get-remote-refs to a conditional one that is done only for the > transport that lacks the capability, or something along that line. ... this "matching removal" you are talking about is in the subsequent patch 4. And there is no transport_set_option() to save us this time, so we really do need this bugfix. > IOW, I am a bit confused by this comment (copied from an earlier part) > > > + /** > > + * This transport supports the fetch() function being called > > + * without get_refs_list() first being called. > > + */ > > Shouldn't it read more like "this transport does not want its > get-refs-list called when fetch-refs is done"? > > I dunno. I'm not sure I understand - transports generally don't care if get-refs-list is called after fetch-refs. Also, this already happens when fetching with tag following from a server that does not support tag following, using a transport that supports reuse. Jonathan Tan (4): transport: allow skipping of ref listing transport: do not list refs if possible transport: list refs before fetch if necessary fetch: do not list refs if fetching only hashes builtin/fetch.c | 32 +++++++++++++++++----- fetch-pack.c | 2 +- t/t5551-http-fetch-smart.sh | 15 +++++++++++ t/t5702-protocol-v2.sh | 18 +++++++++++++ transport-helper.c | 1 + transport-internal.h | 6 +++++ transport.c | 54 ++++++++++++++++++++++++++++++++----- 7 files changed, 115 insertions(+), 13 deletions(-) -- 2.19.0.605.g01d371f741-goog
