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

["Santi Béjar" <santi@agolina.net>]

2009/1/16 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> 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.


>
>> 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