Re: False positive from orphaned_commit_warning() ?

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 25, 2012, 22:05 UTC
Message-ID: <7v629bbio9.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20120725215730.GA30966@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 12 quoted lines

> On Wed, Jul 25, 2012 at 02:52:54PM -0700, Junio C Hamano wrote:
>
>> Paul Gortmaker <paul.gortmaker@windriver.com> writes:
>> 
>> > Has anyone else noticed false positives coming from the
>> > orphan check?
>> 
>> Thanks.  This should fix it.
>
> I've just been hunting the same bug and came up with the same answer.
> Here's a commit message. Feel free to apply or steal text for your
> commit.

Heh, let's try not to waste duplicated efforts by being silent next time, OK? Winning such a race by 5 minutes does not buy us much.

I wish we had some type safe way to say "This uint and the other uint are to hold different kinds of flag bits; do not mix them by bitwise operators".

Thanks.

Show 17 quoted lines

> -- >8 --
> Subject: [PATCH] checkout: don't confuse ref and object flags
>
> When we are leaving a detached HEAD, we do a revision
> traversal to check whether we are orphaning any commits,
> marking the commit we're leaving as the start of the
> traversal, and all existing refs as uninteresting.
>
> Prior to commit 468224e5, we did so by calling for_each_ref,
> and feeding each resulting refname to setup_revisions.
> Commit 468224e5 refactored this to simply mark the pending
> objects, saving an extra lookup.
>
> However, it confused the "flags" parameter to the
> each_ref_fn clalback, which is about the flags we found
> while looking up the ref (e.g., REF_ISSYMREF) with the
> object flag (UNINTERESTING), leading to unpredictable

s/UNINTERESTING/SEEN/; I think.

What was happening was that the remotes/origin/HEAD symref happened to point at the same commit as "master", and ^master that was in the pending array was not transferred to the commit list used by the revision traversal.

What's interesting still is that

	git checkout master~
        git checkout master

does not exhibit this problem in the same repository.

Show 19 quoted lines

> results, as we were setting random flag bits on objects in
> the traversal.
> ---
>  builtin/checkout.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/checkout.c b/builtin/checkout.c
> index a76899d..f855489 100644
> --- a/builtin/checkout.c
> +++ b/builtin/checkout.c
> @@ -592,7 +592,7 @@ static int add_pending_uninteresting_ref(const char *refname,
>  					 const unsigned char *sha1,
>  					 int flags, void *cb_data)
>  {
> -	add_pending_sha1(cb_data, refname, sha1, flags | UNINTERESTING);
> +	add_pending_sha1(cb_data, refname, sha1, UNINTERESTING);
>  	return 0;
>  }
>