On Thu, 16 Oct 2008, Junio C Hamano wrote:
> Sorry for bringing up a potentially old issue, but I do not think the test
> added by 9ed36cf (branch: optionally setup branch.*.merge from upstream
> local branches, 2008-02-19) is correct. It does this:
>
> test_expect_success \
> 'checkout w/--track from non-branch HEAD fails' '
> git checkout -b delete-me master &&
> rm .git/refs/heads/delete-me &&
> test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
> test_must_fail git checkout --track -b track'
>
> It creates branch 'delete-me' at the tip of 'master' to check it out, and
> then it manually deletes the branch. It expects "checkout --track -b track"
> to fail because the current branch is broken and there is no way to set up
> a tracking information for the new branch.
>
> But I think this is bogus. The checkout fails not because of lack of
> trackability (due to broken current _branch_), but because there is no
> current _commit_ to branch from.
>
> Jay CC'ed because 9ed36cf is his; Daniel CC'ed as branch.c was
> originally his.
>
> Here is an attempt to fix the test, which then revealed that the feature
> the commit introduced is broken in the tip of 'maint'. Back when 9ed36cf
> was written, test_must_fail was unavailable, and test_expect_failure meant
> something different, so transplanting this on top of that old commit won't
> reveal the breakage, but I strongly suspect that the feature was broken
> from the very beginning.
>
> The patch to branch.c is a quick fix for this issue. The resulting code
> passes all the tests, but I am not very proud of hardcoding the "HEAD" in
> the code. There must be a better way to do this.
I agree with the change to the test. I think it would be better to
hard-code "refs/heads/" instead of "HEAD", and I feel like we must have a
"is this ref name a branch?" function, if only because someone could stick
"refs/tags/foo" in HEAD, and we should still say it's not something you
could track, despite it being something different from "HEAD".
> branch.c | 4 +++-
> t/t7201-co.sh | 11 +++++------
> 2 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git c/branch.c w/branch.c
> index b1e59f2..6a75057 100644
> --- c/branch.c
> +++ w/branch.c
> @@ -129,7 +129,9 @@ void create_branch(const char *head,
> die("Cannot setup tracking information; starting point is not a branch.");
> break;
> case 1:
> - /* Unique completion -- good */
> + /* Unique completion -- good, only if it is a real ref */
> + if (track == BRANCH_TRACK_EXPLICIT && !strcmp(real_ref, "HEAD"))
> + die("Cannot setup tracking information; starting point is not a branch.");
> break;
> default:
> die("Ambiguous object name: '%s'.", start_name);
> diff --git c/t/t7201-co.sh w/t/t7201-co.sh
> index fbec70d..da1fbf8 100755
> --- c/t/t7201-co.sh
> +++ w/t/t7201-co.sh
> @@ -330,12 +330,11 @@ test_expect_success \
> test "$(git config branch.track2.merge)"
> git config branch.autosetupmerge false'
>
> -test_expect_success \
> - 'checkout w/--track from non-branch HEAD fails' '
> - git checkout -b delete-me master &&
> - rm .git/refs/heads/delete-me &&
> - test refs/heads/delete-me = "$(git symbolic-ref HEAD)" &&
> - test_must_fail git checkout --track -b track'
> +test_expect_success 'checkout w/--track from non-branch HEAD fails' '
> + git checkout master^0 &&
> + test_must_fail git symbolic-ref HEAD &&
> + test_must_fail git checkout --track -b track
> +'
>
> test_expect_success 'checkout an unmerged path should fail' '
> rm -f .git/index &&
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
