Re: [PATCH 2/3] teach sha1_name to look in graveyard reflogs
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 19, 2012, 22:39 UTC
- Message-ID
- <7vtxx3tlyb.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <20120719213326.GB20385@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 40 quoted lines
> The previous commit introduced graveyard reflogs, where the
> reflog for a deleted branch "foo" appears in
> "logs/graveyard/refs/heads/foo~".
>
> This patch teaches dwim_log to search for these logs if the
> ref does not exist, and teaches read_ref_at to fall back to
> them when the literal reflog does not exist. This allows
> "deleted@{1}" to refer to the final commit of a deleted
> branch (either to view or to re-create the branch). You can
> also go further back, or refer to the deleted reflog entries
> by time. Accessing deleted@{0} will yield the null sha1.
>
> Similarly, for_each_reflog_ent learns to fallback to
> graveyard refs, which allows the reflog walker to work.
> However, this is slightly less friendly, as the revision
> parser expects the matching ref to exist before it realizes
> that we are interested in the reflog. Therefore you must use
> "git log -g deleted@{1}" insted of "git log -g deleted" to
> walk a deleted reflog.
>
> In both cases, we also tighten up the mode-checking when
> opening the reflogs. dwim_log checks that the entry we found
> is a regular file (not a directory) to avoid D/F confusion
> (e.g., you ask for "foo" but "foo/bar" exists and we find
> the "foo" but it is a directory).
>
> However, read_ref_at and for_each_reflog_ent did not do this
> check, and relied on earlier parts of the code to have
> verified the log they are about to open. This meant that
> even before this patch, a race condition in changing refs
> between dwim_log and the actual read could cause bizarre
> errors (e.g., read_ref_at would open and try to mmap a
> directory). This patch makes it even easier to trigger those
> conditions (because the ref namespace and the fallback
> graveyard namespace can have D/F ambiguity for a certain
> path). To solve this, we check the mode of the file we open
> and treat it as if it did not exist if it is not a regular
> file (this is the same way dwim_log handles it).
>
> Signed-off-by: Jeff King <peff@peff.net>This may or may not be related, but I vaguely recall that "log -g" traversal hack had a corner case where the walking stops prematurely upon seeing a gap (or creation/deletion that has 0{40})? Do you recall if we have ever dealt with that?
The patch seems fine from a cursory look. Thanks.