Re: [PATCH] revision walker: include a detached HEAD in --all

From: Santi Béjar <santi@agolina.net>
Date: Jan 16, 2009, 13:46 UTC
Message-ID: <adf1fd3d0901160546o50db0594h7377774fed9fef99@mail.gmail.com>
In-Reply-To: <alpine.DEB.1.00.0901161415230.3586@pacific.mpi-cbg.de>

2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:

Show 18 quoted lines

> Hi,
>
> On Fri, 16 Jan 2009, Santi Béjar wrote:
>
>> 2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> >
>> > Note that this affects creating bundles with --all; I contend that it
>> > is a good change to add the HEAD, so that cloning from such a bundle
>> > will give you a current branch.  However, I had to fix t5701 as it
>> > assumed that --all does not imply HEAD.
>>
>> From the description I understand that it only affects when the HEAD is
>> detached, but in t5701 the HEAD is not detached so nothing should be
>> fixed.
>
> The error in t5701 was that it _wanted_ to test a bundle without a HEAD,
> but it actually created it with --all.  That was implying that --all does
> not mean HEAD

Yes, that is the current behaviour.

> , and I disagree with that.

I know you disagree, but in the commit log you said:

--- [PATCH] revision walker: include a detached HEAD in --all

When HEAD is detached, --all should list it, too, logically, as a detached HEAD is by definition a temporary, unnamed branch. ---

so nothing talks about changing the behaviour when the HEAD is not detached.

But the problem with t5701 is another thing. If you run this:

git init : >file git add . git commit -m1 git bundle create b1.bundle --all HEAD git ls-remote b1.bundle git rev-parse --all HEAD

you will see that the same rev-parse parameters in "git bundle" produce tree lines while with "git rev-parse" only two are produced.

Show 13 quoted lines

>
>> For gc for sure it is a good thing, but I'm not convinced of the others,
>> as a detached HEAD is a very special thing (temporary and unnamed
>> branch).
>
> So?  What does "--all" mean?  All branches or what? :-)
>
> Seriously, I think that --all should imply HEAD at all times, as the only
> time when it makes a difference is when you have that unnamed _branch_
> that is a detached HEAD.
>
> Maybe I would be more amenable to your criticism if you could come up with
> a scenario where implying HEAD with --all is _wrong_.

I don't think it is plainly wrong. I think both makes sense, but I think it is not a good idea to change the behaviour now as some scripts may rely on it.

Santi