Re: git checkout does not warn about tags without corresponding commits

[Henrik Austad <henrik@austad.us>]

On Saturday 03 January 2009 12:36:04 Junio C Hamano wrote:
> Henrik Austad <henrik@austad.us> writes:
> > On Friday 02 January 2009 22:44:50 Junio C Hamano wrote:
> >> Henrik Austad <henrik@austad.us> writes:
> >> > I recently tried to do a checkout of (what I thought was the first)
> >> > inux kernel in the linux git repo.
> >> >
> >> > git checkout -b 2.6.11 v2.6.11
> >>
> >> This should have barfed, and indeed I think it is a regression around
> >> v1.5.5.  v1.5.4 and older git definitely fails to check out a tree
> >> object like that.
> >
> > You're right, I bisected it down to commit
> > 782c2d65c24066a5d83453efb52763bc34c10f81
>
> I am not surprised.
>
> That one discarded an implementation of "git checkout" in Bourne shell,
> with a complete reimplementation in C.
>
> I haven't looked at the code very closely, but I think this should fix
> it.  Thorough reviewing (not just running the test suite) is much
> appreciated.
>
> -- >8 --
> Subject: git-checkout: do not allow switching to a tree-ish
>
> "git checkout -b newbranch $commit^{tree}" mistakenly created a new branch
> rooted at the current HEAD, because in that case, the two structure fields
> used to see if the command was invoked without any argument (hence it
> needs to default to checking out the HEAD), were populated incorrectly.
>
> Upon seeing a command line argument that we took as a rev, we should store
> that string in new.name, even if that does not name a commit.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin-checkout.c               |    2 +-
>  t/t2011-checkout-invalid-head.sh |    4 ++++
>  2 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git c/builtin-checkout.c w/builtin-checkout.c
> index c2c0561..b5dd9c0 100644
> --- c/builtin-checkout.c
> +++ w/builtin-checkout.c
> @@ -681,8 +681,8 @@ int cmd_checkout(int argc, const char **argv, const
> char *prefix) argv++;
>  		argc--;
>
> +		new.name = arg;
>  		if ((new.commit = lookup_commit_reference_gently(rev, 1))) {
> -			new.name = arg;
>  			setup_branch_path(&new);
>  			if (resolve_ref(new.path, rev, 1, NULL))
>  				new.commit = lookup_commit_reference(rev);


this fixed my problem when i hacked this into master (git apply failed on the 
patch, so I did a manual patch. Moving that line did the trick

Acked-by/Signed-off-by Henrik Austad <henrik@austad.us>


> diff --git c/t/t2011-checkout-invalid-head.sh
> w/t/t2011-checkout-invalid-head.sh index 764bb0a..798790d 100755
> --- c/t/t2011-checkout-invalid-head.sh
> +++ w/t/t2011-checkout-invalid-head.sh
> @@ -15,4 +15,8 @@ test_expect_success 'checkout master from invalid HEAD' '
>  	git checkout master --
>  '
>
> +test_expect_success 'checkout should not start branch from a tree' '
> +	test_must_fail git checkout -b newbranch master^{tree}
> +'
> +
>  test_done
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
 -> henrik