Re: [PATCH v2 6/9] bundle: avoid using the rev_info flag leak_pending

From: Jeff King <peff@peff.net>
Date: Jan 10, 2018, 08:18 UTC
Message-ID: <20180110081835.GF16315@sigill.intra.peff.net>
In-Reply-To: <5fe3b4c8-e48c-1922-f564-e75c5a354cef@web.de>

On Mon, Dec 25, 2017 at 06:46:14PM +0100, René Scharfe wrote:

Show 10 quoted lines

> The leak_pending flag is so awkward to use that multiple comments had to
> be added around each occurrence.  We use it for remembering the
> prerequisites for the bundle.  That is easy, though: We have the
> ref_list named "prerequisites" in the header for just that purpose.
> 
> Use this original list of prerequisites to check if all of them are
> present and to clear their commit marks afterward.  The two new loops
> are intentionally kept similar to the first one in the function.
> Calling parse_object() a second time is expected be quick and successful
> in each case -- any errors should have been handled in the first round.

There are a few code paths which unset the "parsed" flag, and could cause us to actually re-load an object. I doubt we could trigger any here. And AFAICT this is what the original code was doing inside the helper function anyway.

I do wonder what this is supposed to do for refs that point to non-commits (both before and after your patch). Are we failing to clear the flags on them?

I guess not, because we do not pass "--objects" to the traversal in the first place, so we would never visit them (though that also makes me wonder if we do the wrong thing in the verification step for such a ref).

-Peff