From: Jeff King <peff@peff.net>
To: Michael Haggerty <mhagger@alum.mit.edu>
Cc: "Junio C Hamano" <gitster@pobox.com>, "Brodie Rao" <brodie@sf.io>,
git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH v3 3/5] refs: teach for_each_ref a flag to avoid recursion
Date: Fri, 10 Jan 2014 04:15:46 -0500 [thread overview]
Message-ID: <20140110091546.GA17443@sigill.intra.peff.net> (raw)
In-Reply-To: <52CFB66D.5070800@alum.mit.edu>
On Fri, Jan 10, 2014 at 09:59:25AM +0100, Michael Haggerty wrote:
> > 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.
> 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
next prev parent reply other threads:[~2014-01-10 9:16 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-01-07 3:32 [PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false Brodie Rao
2014-01-07 3:35 ` Brodie Rao
2014-01-07 17:13 ` Jeff King
2014-01-07 17:51 ` Junio C Hamano
2014-01-07 17:52 ` Jeff King
2014-01-07 19:38 ` Junio C Hamano
2014-01-07 19:58 ` Jeff King
2014-01-07 20:31 ` Junio C Hamano
2014-01-07 22:08 ` Jeff King
2014-01-07 22:10 ` [PATCH 1/4] cat-file: refactor error handling of batch_objects Jeff King
2014-01-07 22:10 ` [PATCH 2/4] cat-file: fix a minor memory leak in batch_objects Jeff King
2014-01-07 22:10 ` [PATCH 3/4] cat-file: restore ambiguity warning flag " Jeff King
2014-01-07 22:11 ` [PATCH 4/4] revision: turn off object/refname ambiguity check for --stdin Jeff King
2014-01-07 23:56 ` [PATCH v2] speeding up 40-hex ambiguity check Jeff King
2014-01-07 23:57 ` [PATCH v2 1/5] cat-file: refactor error handling of batch_objects Jeff King
2014-01-07 23:57 ` [PATCH v2 2/5] cat-file: fix a minor memory leak in batch_objects Jeff King
2014-01-07 23:58 ` [PATCH v2 3/5] refs: teach for_each_ref a flag to avoid recursion Jeff King
2014-01-08 3:47 ` [PATCH v3 " Jeff King
2014-01-08 10:23 ` Jeff King
2014-01-08 11:29 ` Michael Haggerty
2014-01-09 21:49 ` Jeff King
2014-01-10 8:59 ` Michael Haggerty
2014-01-10 9:15 ` Jeff King [this message]
2014-01-09 17:51 ` Junio C Hamano
2014-01-09 21:55 ` Jeff King
2014-01-07 23:59 ` [PATCH v2 4/5] get_sha1: speed up ambiguous 40-hex test Jeff King
2014-01-08 16:09 ` Michael Haggerty
2014-01-09 18:25 ` Junio C Hamano
2014-01-10 9:41 ` Jeff King
2014-01-14 9:50 ` Jeff King
2014-01-14 11:34 ` Michael Haggerty
2014-01-08 0:00 ` [PATCH v2 5/5] get_sha1: drop object/refname ambiguity flag Jeff King
2014-01-08 16:34 ` Michael Haggerty
2014-01-07 6:45 ` [PATCH] sha1_name: don't resolve refs when core.warnambiguousrefs is false Duy Nguyen
2014-01-07 17:24 ` Junio C Hamano
2014-01-07 19:23 ` Brodie Rao
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140110091546.GA17443@sigill.intra.peff.net \
--to=peff@peff.net \
--cc=brodie@sf.io \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=mhagger@alum.mit.edu \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).