On Tue, Jul 31, 2018 at 04:23:43PM -0700, Jonathan Tan wrote: > > > Because transport_fetch_refs() filters the refs sent to the transport, > > > it cannot just report the transport's result directly, but first needs > > > to readd the excluded refs, pretending that they are fetched. However, > > > this results in a wrong result if the transport did not report the refs > > > that they have fetched in "fetched_refs" - the excluded refs would be > > > added and reported, presenting an incomplete picture to the caller. > > > > This part leaves me confused. If we are not fetching them, then why do > > we need to pretend that they are fetched? > > The short answer is that we need: > (1) the complete list of refs that was passed to > transport_fetch_refs(), > (2) with shallow information (REF_STATUS_REJECT_SHALLOW set if > relevant), and > (3) with updated OIDs if ref-in-want was used. > > The fetched_refs out param already fulfils (2) and (3), and this patch > makes it fulfil (1). As for calling them fetched_refs, perhaps that is a > misnomer, but they do appear in FETCH_HEAD even though they are not > truly fetched. Thanks for this explanation. It does make more sense to me now, and I agree that a lot of my confusion was from calling it "fetched_refs" (and the comment saying "reported as fetched, but not actually fetched"). > Which raises the question...if completeness is so important, why not > reuse the input list of refs and document that transport_fetch_refs() > can mutate the input list? You ask the same question below, so I'll put > the answer after quoting your paragraph. > [...] Thanks, the answer here was enlightening as well. I see you posted a patch to go back to mutating the list, and that seems reasonable to me. I'm fine with a separate "out" list, too. Its purpose and expectations just need to be reflected in the name (and possibly in a comment). -Peff
