Re: [PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion

From: Jeff King <peff@peff.net>
Date: Jan 10, 2014, 09:15 UTC
Message-ID: <20140110091546.GA17443@sigill.intra.peff.net>
In-Reply-To: <52CFB66D.5070800@alum.mit.edu>

On Fri, Jan 10, 2014 at 09:59:25AM +0100, Michael Haggerty wrote:

Show 12 quoted lines

> > However, is it safe to prime only part of the loose ref namespace?
> [...]
> 
> prime_ref_dir() is called by do_for_each_entry(), which all the
> iteration functions pass through.  It is always called before the
> iteration starts, and it primes only the subtree of the refs hierarchy
> that is being iterated over.  For example, if iterating over
> "refs/heads" then it only primes references with that prefix.
> 
> This is OK, because if later somebody iterates over a broader part of
> the refs hierarchy (say, "refs"), then priming is done again, including
> re-checking the packed refs.

Ah, right. This is the part I was forgetting: the next for_each_ref will re-prime with the expanded view. Thanks for a dose of sanity.

I'll fix that in my re-roll.

Show 7 quoted lines

> It would also be possible to swing in the other direction.  I don't
> remember a particular reason why I left the DO_FOR_EACH_INCLUDE_BROKEN
> handling at the do_for_each_ref() level rather than handling it at the
> do_for_each_entry() level.  But now that you are passing the flags
> parameter all the way down the call stack, it wouldn't cost anything to
> support both of the DO_FOR_EACH flags everywhere and just document it
> that way.

I think it was simply that it was an option that the traversal did not need to know about (just like the "trim" option), so you kept it as encapsulated as possible. I think I'll introduce it as a separate flag namespace, as discussed in the previous email. It is the same amount of refactoring work to merge them later as it is now, if we so choose.

-Peff