Re: [PATCH 2/3] teach sha1_name to look in graveyard reflogs
Jeff King writes: > But what _should_ it show for such an entry? There is no commit to show > in the reflog walker, but it would still be nice to say "BTW, there was > a deletion even here". Obviously just skipping it and showing the next > entry would be better than the current behavior of stopping the > traversal, but I feel like there must be some better behavior. Like showing an entry that says "Ref deleted here", which should be easy to do by creating a phoney commit object and inserting it to the queue the reflog walker uses, I would guess. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] teach sha1_name to look in graveyard reflogs
On Thu, Jul 19, 2012 at 03:39:24PM -0700, Junio C Hamano wrote:
> > 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.
>
> 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?
>From my tests, I think it is probably still broken (if you do a delete,
create, delete sequence on a branch and then walk the reflog, it stops
prematurely at the 0{40} sha1).
But what _should_ it show for such an entry? There is no commit to show
in the reflog walker, but it would still be nice to say "BTW, there was
a deletion even here". Obviously just skipping it and showing the next
entry would be better than the current behavior of stopping the
traversal, but I feel like there must be some better behavior.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] teach sha1_name to look in graveyard reflogs
Jeff King writes:
> 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
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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
