Re: [PATCH] fetch-pack: unify ref in and out param
- From
Jeff King <peff@peff.net>
- Date
- Aug 2, 2018, 16:40 UTC
- Message-ID
- <20180802164026.GB15984@sigill.intra.peff.net>
- In-Reply-To
- <20180801201320.201133-1-jonathantanmy@google.com>
On Wed, Aug 01, 2018 at 01:13:20PM -0700, Jonathan Tan wrote:
Show 20 quoted lines
> When a user fetches:
> - at least one up-to-date ref and at least one non-up-to-date ref,
> - using HTTP with protocol v0 (or something else that uses the fetch
> command of a remote helper)
> some refs might not be updated after the fetch.
>
> This bug was introduced in commit 989b8c4452 ("fetch-pack: put shallow
> info in output parameter", 2018-06-28) which allowed transports to
> report the refs that they have fetched in a new out-parameter
> "fetched_refs". If they do so, transport_fetch_refs() makes this
> information available to its caller.
>
> Users of "fetched_refs" rely on the following 3 properties:
> (1) it is the complete list of refs that was passed to
> transport_fetch_refs(),
> (2) it has shallow information (REF_STATUS_REJECT_SHALLOW set if
> relevant), and
> (3) it has updated OIDs if ref-in-want was used (introduced after
> 989b8c4452).
> [...]Thanks, this is a very clear and well-organized commit message. It answers my questions, and I agree with the general notion of "we can figure out the right API for ref patterns later" approach.
Show 9 quoted lines
> builtin/clone.c | 4 ++-- > builtin/fetch.c | 28 ++++------------------------ > fetch-object.c | 2 +- > fetch-pack.c | 30 +++++++++++++++--------------- > t/t5551-http-fetch-smart.sh | 18 ++++++++++++++++++ > transport-helper.c | 6 ++---- > transport-internal.h | 9 +-------- > transport.c | 34 ++++++---------------------------- > transport.h | 3 +--
The patch itself looks sane to me, and obviously fixes the problem. I cannot offhand think of any reason that munging the existing list would be a problem (though it has been a while since I have dealt with this code, so take that with the appropriate grain of salt).
-Peff