Creating remote branch called HEAD corrupts remote clones

Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

Hi,

On Friday we had an issue where a developer pushed a branch called HEAD to 
the remote server. The result was that other developers could not pull or 
push. I have not been able to reproduce the exact issue locally, but this 
script shows that the bob clone behaves oddly on each pull. That is a 
symptom we saw on Friday. However, bob is still able to push, which we were 
not able to. That point could be something to do with how the kde git 
infrastructure is configured.

mkdir remote
cd remote/
git init --bare
cd ../
git clone remote/ alice
cd alice/
echo test >> file
git add file
git commit -am w
git push origin master
echo test >> file
git commit -am w
git branch HEAD
git push origin HEAD
git push
cd ..
git clone remote bob
cd bob/
git pull --rebase
echo test >> file
git commit -am w
git push
git pull
git pull
git pull

There were also messages like this:

$ git pull
remote: Counting objects: 5, done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From /home/kde-devel/dev/src/playground/git/tmp/remote
 + 1434cd2...dd30974 HEAD       -> origin/HEAD  (forced update)
error: Ref refs/remotes/origin/master is at 
dd3097498a6c1c5bc73ad1f2ff3b7969a6f6d059 but expected 
1434cd2bb9823d2d2b1548c75fdd4ff8b1feddc1
 ! 1434cd2..2fb560d  master     -> origin/master  (unable to update local 
ref)

The HEAD branch was created accidentally and the issue was resolved by doing 
a git push origin -f :refs/heads/HEAD. Again though, git push -f is not 
something all developers are allowed to do on the kde git infrastructure, so 
until that was done, the repo was corrupt for everyone.

Shouldn't git forbit the creation of a branch called HEAD? Hopefully the 
provided script can lead to the actual issue that caused the corruption of 
our repo.

Thanks,

Steve.


_______________________________________________
KDE PIM mailing list kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

Stephen Kelly wrote:

> 
> Hi,
> 
> On Friday we had an issue where a developer pushed a branch called HEAD to
> the remote server. The result was that other developers could not pull or
> push. 

Does anyone have any thoughts/response on this?

Why does git not have a bug tracker?

Steve.

Re: Creating remote branch called HEAD corrupts remote clones

[Thomas Rast]

Stephen Kelly wrote:
> Why does git not have a bug tracker?

http://thread.gmane.org/gmane.comp.version-control.git/136500


-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

Ok so there was some movement with the result that no one uses what was set up.

Presumably because git developers don't want a bug tracker.

So how can I ensure that this particular issue doesn't get lost? Is
there no way except hope that people get involved in fixing it
straight away, and fix it straight away before they forget about it?

We worked around this on the KDE side by forbidding pushing any ref
with the name HEAD to the remote, but it's still a git bug.

On 1/20/11, Thomas Rast <trast@student.ethz.ch> wrote:
> Stephen Kelly wrote:
>> Why does git not have a bug tracker?
>
> http://thread.gmane.org/gmane.comp.version-control.git/136500
>
>
> --
> Thomas Rast
> trast@{inf,student}.ethz.ch
>

Re: Creating remote branch called HEAD corrupts remote clones

[Erik Faye-Lund]

On Thu, Jan 20, 2011 at 4:05 PM, Stephen Kelly <steveire@gmail.com> wrote:
> Ok so there was some movement with the result that no one uses what was set up.
>
> Presumably because git developers don't want a bug tracker.
>
> So how can I ensure that this particular issue doesn't get lost? Is
> there no way except hope that people get involved in fixing it
> straight away, and fix it straight away before they forget about it?
>

You could always fix it yourself, and submit a patch.

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

On Thu, Jan 20, 2011 at 4:41 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Thu, Jan 20, 2011 at 4:05 PM, Stephen Kelly <steveire@gmail.com> wrote:
>> Ok so there was some movement with the result that no one uses what was set up.
>>
>> Presumably because git developers don't want a bug tracker.
>>
>> So how can I ensure that this particular issue doesn't get lost? Is
>> there no way except hope that people get involved in fixing it
>> straight away, and fix it straight away before they forget about it?
>>
>
> You could always fix it yourself, and submit a patch.
>

Correct. That would be a big rampup though and I might give up, move
on or forget and then no one would do it because everyone has
forgotten about it. I mean that in the general sense of any bug that
gets posted to this mailing list.

But I'm not trying to change how git people work. I'm just trying to
discover what the states of a bug in git should be understood to be
after it is emailed to this list and before it's in master. So far it
seems there is only one intermediate state: "Limbo, maybe forgotten.
You should email the list again"

Thanks,

Steve.

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

Hi,

On Thu, Jan 20, 2011 at 1:14 PM, Stephen Kelly <steveire@gmail.com> wrote:
> Stephen Kelly wrote:
>> On Friday we had an issue where a developer pushed a branch called HEAD to
>> the remote server. The result was that other developers could not pull or
>> push.
>
> Does anyone have any thoughts/response on this?

Can you list a series of steps to reproduce this?

> Why does git not have a bug tracker?

Because it's not needed. If you have an issue, post the issue as you
would file a bug:
 Which version of git?
 Which kind of network transport was used?
 Is this reproducible?

Chances are, if this is reproducible in the latest version, someone
would fix it soon enough. If not, and it's important to you, you would
ping back. If other people find this issue, they would send another
email.

In fact, if you really want to help, you could clone the latest
'master' to see if this still happening, narrow down the steps needed
to reproduce this, write a test to trigger it and send a patch.
Certainly, a test case that constantly fails would be a constant
remainder that there is a bug.

Cheers.

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Wesley J. Landaker]

On Thursday, January 20, 2011 10:32:40 Felipe Contreras wrote:
> Hi,
> 
> On Thu, Jan 20, 2011 at 1:14 PM, Stephen Kelly <steveire@gmail.com> wrote:
> > Stephen Kelly wrote:
> >> On Friday we had an issue where a developer pushed a branch called
> >> HEAD to the remote server. The result was that other developers could
> >> not pull or push.
[...]
>  Which version of git?
>  Which kind of network transport was used?
>  Is this reproducible?

FWIW, here is a quick demonstration of at least one problem with having a 
branch called HEAD. You can make it and push it fine, but when cloning, you
don't get it.

#!/bin/bash
git init --bare origin.git
git clone origin.git wc1
cd wc1
git commit --allow-empty -m "Initial rev"
git checkout -b HEAD
git commit --allow-empty -m "Make HEAD branch"
git push --all
cd ..
git clone origin.git wc2
diff -u <(cd wc1; git branch -a) <(cd wc2; git branch -a)
diff -u <(cd wc1; git log --all) <(cd wc2; git log --all)

If I do the following from wc2 I can get the branch manually:

git pull origin refs/heads/HEAD:HEAD

I haven't played with it enough to see what other problems might arise.

Re: Creating remote branch called HEAD corrupts remote clones

[Junio C Hamano]

Stephen Kelly <steveire@gmail.com> writes:

> There were also messages like this:
>
> $ git pull
> remote: Counting objects: 5, done.
> remote: Total 3 (delta 0), reused 0 (delta 0)
> Unpacking objects: 100% (3/3), done.
> From /home/kde-devel/dev/src/playground/git/tmp/remote
>  + 1434cd2...dd30974 HEAD       -> origin/HEAD  (forced update)

Stephen, thanks, I think you have found a bug, not in this step but one
step before this 'git pull', where Bob pushes to the remote for the first
time after making a commit.

This issue is inherent in the way how the 'separate remotes' layout
introduced in 1.6.0 arranges the remote tracking mappings.

The refs/remotes/origin/HEAD in Bob's repository is supposed to be a
symbolic ref that points at the primary branch of the 'origin' remote
(typically its master), e.g. "ref: refs/remotes/origin/master".  But in
general, local 'refs/remotes/origin/X' for any value of X is to copy
'refs/heads/X' from the 'origin'.

Oops.  If the origin repository has 'refs/heads/HEAD', these rules
obviously conflict with each other.

In this particular case, Bob pushes the change to his refs/heads/master to
the remote to update its refs/heads/master.  The push at the same time
tries to pretend that it fetched from the remote to update Bob's tracking
branches, so refs/remotes/origin/master in Bob's repository is also
updated to point at this commit.

However, because Bob's refs/remotes/origin/HEAD is a symbolic ref that
points at his refs/remotes/origin/master, its value is also updated by
this push.

Bob's next fetch from remote will then notice that remotes/origin/HEAD he
has is different from refs/heads/HEAD the remote has, and tries to update
it, which would obviously a non-fast-forward.

I personally think it is reasonable to forbid HEAD or anything all caps
that ends with "_HEAD" as branch names.  Opinions?

Re: Creating remote branch called HEAD corrupts remote clones

[Jeff King]

On Thu, Jan 20, 2011 at 11:53:16AM -0800, Junio C Hamano wrote:

> The refs/remotes/origin/HEAD in Bob's repository is supposed to be a
> symbolic ref that points at the primary branch of the 'origin' remote
> (typically its master), e.g. "ref: refs/remotes/origin/master".  But in
> general, local 'refs/remotes/origin/X' for any value of X is to copy
> 'refs/heads/X' from the 'origin'.
> 
> Oops.  If the origin repository has 'refs/heads/HEAD', these rules
> obviously conflict with each other.
>
> [...]
>
> I personally think it is reasonable to forbid HEAD or anything all caps
> that ends with "_HEAD" as branch names.  Opinions?

Hmm. It seems like the symbolic ref is the culprit, not just HEAD. The
HEAD thing is the most likely, of course, but I could do something like:

  git symbolic-ref refs/remotes/origin/convenient-alias \
                   refs/remotes/origin/some-name-you-dont-like

which is basically the same as the HEAD case (except that the
"convenient alias" for HEAD is "origin" and not
"origin/convenient-alias" due to the lookup table in dwim_ref).

Now imagine the remote creates a branch called convenient-alias. When I
fetch, am I corrupting my local tracking branches by falsely equating
the two? And/or when I push, am I then corrupting the remote?

So I wonder if the safety valve here should be about symbolic refs, and
not about the special name HEAD. Maybe we should not follow symbolic
refs during fetch. So if we are fetching the refspec "foo:bar", and the
RHS "bar" is a symref, we should _not_ follow it, but instead just
overwrite the symref with a regular ref.

For pushing, one rule could be to allow pushing from a named symref, but
not allow the matching rules to use a symref as a source. So I could do:

  git push origin convenient-alias:new-name

but

  git push origin

would never overwrite upstream's convenient-alias.

I dunno. That's just off the top of my head, so maybe I'm missing some
corner cases. I would be tempted to put the push rule into receive-pack,
so it could look at the local refs, but I don't think receive-pack has
any way of knowing what is a symref and what is not on the pushing end.

-Peff

Re: Creating remote branch called HEAD corrupts remote clones

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Hmm. It seems like the symbolic ref is the culprit, not just HEAD. The
> HEAD thing is the most likely, of course, but I could do something like:
>
>   git symbolic-ref refs/remotes/origin/convenient-alias \
>                    refs/remotes/origin/some-name-you-dont-like

Isn't it already wrong to do the above locally, in the sense that it is
equally wrong to do this?

    git update-ref refs/remotes/origin/no-such-thing-exists-over-there 
    	refs/heads/master

I agree that the symbolic ref is the real source of confusion in Stephen's
case (I admit that I was scratching my head chasing a non-existent bug in
transport_update_tracking_ref() before I realized what was happening), but
the usual (and the only) way for refs/remotes hierarchy to get a symbolic
ref is via a normal clone, and HEAD is the only name that can cause this
conflict.  Creating a branch called HEAD (or FETCH_HEAD for that matter)
is infinitely more likely to be a mistake than being clever, I think.

> ... Maybe we should not follow symbolic
> refs during fetch. So if we are fetching the refspec "foo:bar", and the
> RHS "bar" is a symref, we should _not_ follow it, but instead just
> overwrite the symref with a regular ref.
> 
> For pushing, one rule could be to allow pushing from a named symref, but
> not allow the matching rules to use a symref as a source....

I personally like this line of thought, especially as a thought experiment
to see what corner cases we could find, but I doubt I will be able to say
we covered all the corner cases with confidence without thinking long and
very hard.  For now, I do not find this issue worth spending that kind of
deep thinking, especially when a lot simpler and easier to explain
workaround is available, but others may disagree and perfect your idea.

Re: Creating remote branch called HEAD corrupts remote clones

[Jeff King]

On Thu, Jan 20, 2011 at 01:43:17PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > Hmm. It seems like the symbolic ref is the culprit, not just HEAD. The
> > HEAD thing is the most likely, of course, but I could do something like:
> >
> >   git symbolic-ref refs/remotes/origin/convenient-alias \
> >                    refs/remotes/origin/some-name-you-dont-like
> 
> Isn't it already wrong to do the above locally, in the sense that it is
> equally wrong to do this?

Probably. My argument in favor would be "that is what we are doing with
remotes/*/HEAD", but I think the logic is somewhat circular there.

Thinking on it more, yeah, if you want "convenient-alias", you are
probably better off to do:

  git symbolic-ref refs/convenient-alias refs/remotes/origin/whatever

>     git update-ref refs/remotes/origin/no-such-thing-exists-over-there 
>     	refs/heads/master

Actually, don't we end up with that in the case of upstream deleting a
ref? Which isn't to say it isn't a stupid thing to do, but that we can
and do get into that state.

Thinking on it even more, I don't think we can cover all of the weird
"you have a ref named $foo and they suddenly created a ref named $foo"
push corner cases. There is no substitution for actual communication and
organization of ref names if you are going to be pushing along with
other people into a central repo. Because fundamentally the ref name is
the unique identifier, and matching refs during push tries to reconcile
matches based on those identifiers.

> I personally like this line of thought, especially as a thought experiment
> to see what corner cases we could find, but I doubt I will be able to say
> we covered all the corner cases with confidence without thinking long and
> very hard.  For now, I do not find this issue worth spending that kind of
> deep thinking, especially when a lot simpler and easier to explain
> workaround is available, but others may disagree and perfect your idea.

Yeah, after reading your response and considering a bit, I think the
simple "don't make HEAD" thing (or at least "don't pull or push HEAD")
is a sane workaround.

-Peff

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

Hi,

On Thu, Jan 20, 2011 at 11:54 PM, Jeff King <peff@peff.net> wrote:
> On Thu, Jan 20, 2011 at 01:43:17PM -0800, Junio C Hamano wrote:
>> I personally like this line of thought, especially as a thought experiment
>> to see what corner cases we could find, but I doubt I will be able to say
>> we covered all the corner cases with confidence without thinking long and
>> very hard.  For now, I do not find this issue worth spending that kind of
>> deep thinking, especially when a lot simpler and easier to explain
>> workaround is available, but others may disagree and perfect your idea.
>
> Yeah, after reading your response and considering a bit, I think the
> simple "don't make HEAD" thing (or at least "don't pull or push HEAD")
> is a sane workaround.

I don't fully understand the issue, so excuse me if this is totally
wrong, but wouldn't a rule like 'you can't create a branch for which
there's already a symbolic ref' do the trick?

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> I don't fully understand the issue, so excuse me if this is totally
> wrong, but wouldn't a rule like 'you can't create a branch for which
> there's already a symbolic ref' do the trick?

But whose symbolic ref are you checking against?  Your own, or ones in
somebody else's repository that you haven't recently updated from?

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

On Fri, Jan 21, 2011 at 7:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> I don't fully understand the issue, so excuse me if this is totally
>> wrong, but wouldn't a rule like 'you can't create a branch for which
>> there's already a symbolic ref' do the trick?
>
> But whose symbolic ref are you checking against?  Your own, or ones in
> somebody else's repository that you haven't recently updated from?

The local ones. That means that somebody can't create a 'HEAD' branch
locally, and can't push a 'HEAD' branch either, as the remote server
would already have a 'HEAD' symbolic link. And actually, if for some
reason I have a FOO_HEAD, and I fetch a branch called bob/FOO_HEAD,
obviously the local symbolic ref without namespace should take
precedence.

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

bump.

I don't think this issue was fixed, was it?

(no need to put kdepim back in the cc list)

On Sat, Jan 22, 2011 at 1:46 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Fri, Jan 21, 2011 at 7:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> I don't fully understand the issue, so excuse me if this is totally
>>> wrong, but wouldn't a rule like 'you can't create a branch for which
>>> there's already a symbolic ref' do the trick?
>>
>> But whose symbolic ref are you checking against?  Your own, or ones in
>> somebody else's repository that you haven't recently updated from?
>
> The local ones. That means that somebody can't create a 'HEAD' branch
> locally, and can't push a 'HEAD' branch either, as the remote server
> would already have a 'HEAD' symbolic link. And actually, if for some
> reason I have a FOO_HEAD, and I fetch a branch called bob/FOO_HEAD,
> obviously the local symbolic ref without namespace should take
> precedence.
>
> --
> Felipe Contreras
>

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

Can git have a bug tracker please?

This is another reminder to fix this bug which is otherwise untrackable.

Thanks,

Steve.

On Sun, Feb 20, 2011 at 2:17 PM, Stephen Kelly <steveire@gmail.com> wrote:
> bump.
>
> I don't think this issue was fixed, was it?
>
> (no need to put kdepim back in the cc list)
>
> On Sat, Jan 22, 2011 at 1:46 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Fri, Jan 21, 2011 at 7:37 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> I don't fully understand the issue, so excuse me if this is totally
>>>> wrong, but wouldn't a rule like 'you can't create a branch for which
>>>> there's already a symbolic ref' do the trick?
>>>
>>> But whose symbolic ref are you checking against?  Your own, or ones in
>>> somebody else's repository that you haven't recently updated from?
>>
>> The local ones. That means that somebody can't create a 'HEAD' branch
>> locally, and can't push a 'HEAD' branch either, as the remote server
>> would already have a 'HEAD' symbolic link. And actually, if for some
>> reason I have a FOO_HEAD, and I fetch a branch called bob/FOO_HEAD,
>> obviously the local symbolic ref without namespace should take
>> precedence.
>>
>> --
>> Felipe Contreras
>>
>

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

On Tue, Apr 26, 2011 at 3:09 PM, Stephen Kelly <steveire@gmail.com> wrote:
> Can git have a bug tracker please?

So that you would feel comfortable that there would be a bug report
gathering dust? Or that it's closed as invalid for lack of
information?

> This is another reminder to fix this bug which is otherwise untrackable.

Let's imagine you are posting this to bugzilla: first question?
How do you reproduce this?

But I already asked you this[1], and you didn't reply. What should one
assume but that you don't care enough to help get this fixed.

[1] http://article.gmane.org/gmane.comp.version-control.git/165320

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

On Tue, Apr 26, 2011 at 8:18 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Tue, Apr 26, 2011 at 3:09 PM, Stephen Kelly <steveire@gmail.com> wrote:
>> Can git have a bug tracker please?
>
> So that you would feel comfortable that there would be a bug report
> gathering dust? Or that it's closed as invalid for lack of
> information?

If you believe that it is a foregone conclusion that that is the fate
of all bug trackers, and that that's a reasonable reason for git not
to have one, then you have had very different experiences to me.

I don't think there's more I can say than that.

>
>> This is another reminder to fix this bug which is otherwise untrackable.
>
> Let's imagine you are posting this to bugzilla: first question?
> How do you reproduce this?

My initial mail illustrated the problem as best I could:

http://thread.gmane.org/gmane.comp.kde.devel.pim/29534

>
> But I already asked you this[1], and you didn't reply. What should one
> assume but that you don't care enough to help get this fixed.
>
> [1] http://article.gmane.org/gmane.comp.version-control.git/165320

Someone else replied. Isn't that enough?

Other git developers confirmed it's probably an issue. Isn't that enough?

http://thread.gmane.org/gmane.comp.kde.devel.pim/29534/focus=165326

Anyway, we've had a work around in place since January. From the git
POV, this just falls through the cracks. Consider the bug marked as
can not reproduce/needs info/whatever you prefer. I'm outie.

Steve.

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

On Wed, Apr 27, 2011 at 12:18 PM, Stephen Kelly <steveire@gmail.com> wrote:
> On Tue, Apr 26, 2011 at 8:18 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Tue, Apr 26, 2011 at 3:09 PM, Stephen Kelly <steveire@gmail.com> wrote:
>>> Can git have a bug tracker please?
>>
>> So that you would feel comfortable that there would be a bug report
>> gathering dust? Or that it's closed as invalid for lack of
>> information?
>
> If you believe that it is a foregone conclusion that that is the fate
> of all bug trackers, and that that's a reasonable reason for git not
> to have one, then you have had very different experiences to me.
>
> I don't think there's more I can say than that.
>
>>
>>> This is another reminder to fix this bug which is otherwise untrackable.
>>
>> Let's imagine you are posting this to bugzilla: first question?
>> How do you reproduce this?
>
> My initial mail illustrated the problem as best I could:
>
> http://thread.gmane.org/gmane.comp.kde.devel.pim/29534

No problems here:

Initialized empty Git repository in /tmp/remote/
Cloning into alice...
done.
warning: You appear to have cloned an empty repository.
[master (root-commit) 6983153] w
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 file
Counting objects: 3, done.
Writing objects: 100% (3/3), 213 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /tmp/remote/
 * [new branch]      master -> master
[master 116a225] w
 1 files changed, 1 insertions(+), 0 deletions(-)
Counting objects: 5, done.
Writing objects: 100% (3/3), 242 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /tmp/remote/
 * [new branch]      HEAD -> HEAD
Everything up-to-date
Cloning into bob...
done.
From /tmp/remote
   6983153..116a225  HEAD       -> origin/HEAD
Current branch master is up to date.
[master 0951422] w
 1 files changed, 1 insertions(+), 0 deletions(-)
Counting objects: 5, done.
Writing objects: 100% (3/3), 242 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /tmp/remote
   6983153..0951422  HEAD -> master
From /tmp/remote
 + 0951422...116a225 HEAD       -> origin/HEAD  (forced update)
Already up-to-date.
From /tmp/remote
 + 116a225...0951422 master     -> origin/master  (forced update)
Already up-to-date.
From /tmp/remote
 + 0951422...116a225 HEAD       -> origin/HEAD  (forced update)
Already up-to-date.

>> But I already asked you this[1], and you didn't reply. What should one
>> assume but that you don't care enough to help get this fixed.
>>
>> [1] http://article.gmane.org/gmane.comp.version-control.git/165320
>
> Someone else replied. Isn't that enough?

With a different issue that's not really important.

> Other git developers confirmed it's probably an issue. Isn't that enough?
>
> http://thread.gmane.org/gmane.comp.kde.devel.pim/29534/focus=165326

_probably_, many things have happened since then.

Is it still an issue? Doesn't seem so.

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

On Wed, Apr 27, 2011 at 11:48 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> No problems here:

I had another go.

mkdir remote
cd remote/
git init --bare
cd ../
git clone remote/ alice
cd alice/
echo test >> file
git add file
git commit -am w
git push origin master
echo test >> file
git commit -am w
git branch HEAD
git push origin HEAD
git push
cd ..
git clone remote bob
cd bob/
git branch
git pull --rebase
echo test >> file
git commit -am w
git push
git pull
git pull
git pull
echo test >> file
git commit -am w
git push
cd ../alice
git branch
git status
echo test >> file
git commit -am w
git push
echo test2 >> file
git commit -am w
git push
git pull
echo test3 >> file
git commit -am w
git status
git push
gitk


stephen@bishop:/tmp/git$ mkdir remote

stephen@bishop:/tmp/git$ cd remote/

stephen@bishop:/tmp/git/remote$ git init --bare
Initialized empty Git repository in /tmp/git/remote/

stephen@bishop:/tmp/git/remote$ cd ../

stephen@bishop:/tmp/git$ git clone remote/ alice
Cloning into alice...
done.
warning: You appear to have cloned an empty repository.

stephen@bishop:/tmp/git$ cd alice/

stephen@bishop:/tmp/git/alice$ echo test >> file

stephen@bishop:/tmp/git/alice$ git add file

stephen@bishop:/tmp/git/alice$ git commit -am w
[master (root-commit) 072df32] w
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 file

stephen@bishop:/tmp/git/alice{master}$ git push origin master
Counting objects: 3, done.
Writing objects: 100% (3/3), 210 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /tmp/git/remote/
 * [new branch]      master -> master

stephen@bishop:/tmp/git/alice{master}$ echo test >> file

stephen@bishop:/tmp/git/alice{master}$ git commit -am w
[master b39d099] w
 1 files changed, 1 insertions(+), 0 deletions(-)

stephen@bishop:/tmp/git/alice{master}$ git branch HEAD

stephen@bishop:/tmp/git/alice{master}$ git push origin HEAD
Counting objects: 5, done.
Writing objects: 100% (3/3), 242 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /tmp/git/remote/
 * [new branch]      HEAD -> HEAD

stephen@bishop:/tmp/git/alice{master}$ git push
Everything up-to-date

stephen@bishop:/tmp/git/alice{master}$ cd ..

stephen@bishop:/tmp/git$ git clone remote bob
Cloning into bob...
done.

stephen@bishop:/tmp/git$ cd bob/

stephen@bishop:/tmp/git/bob{master}$ git pull --rebase
From /tmp/git/remote
   072df32..b39d099  HEAD       -> origin/HEAD
Current branch master is up to date.

stephen@bishop:/tmp/git/bob{master}$ echo test >> file

stephen@bishop:/tmp/git/bob{master}$ git commit -am w
[master b39d099] w
 1 files changed, 1 insertions(+), 0 deletions(-)

stephen@bishop:/tmp/git/bob{master}$ git push
Total 0 (delta 0), reused 0 (delta 0)
To /tmp/git/remote
   072df32..b39d099  HEAD -> master

stephen@bishop:/tmp/git/bob{master}$ git pull
Current branch master is up to date.

stephen@bishop:/tmp/git/bob{master}$ git pull
Current branch master is up to date.

stephen@bishop:/tmp/git/bob{master}$ git pull
Current branch master is up to date.

stephen@bishop:/tmp/git/bob{master}$ echo test >> file

stephen@bishop:/tmp/git/bob{master}$ git commit -am w
[master 47699a9] w
 1 files changed, 1 insertions(+), 0 deletions(-)

stephen@bishop:/tmp/git/bob{master}$ git push
Counting objects: 5, done.
Writing objects: 100% (3/3), 240 bytes, done.
Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
To /tmp/git/remote
   b39d099..47699a9  HEAD -> master

stephen@bishop:/tmp/git/bob{master}$ cd ../alice

stephen@bishop:/tmp/git/alice{master}$ git branch
  HEAD
* master

stephen@bishop:/tmp/git/alice{master}$ git status
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
# On branch master
# Your branch is ahead of 'origin/master' by 1 commit.
#
nothing to commit (working directory clean)

stephen@bishop:/tmp/git/alice{master}$ echo test >> file

stephen@bishop:/tmp/git/alice{master}$ git commit -am w
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
[master 7e83bed] w
 1 files changed, 1 insertions(+), 0 deletions(-)

stephen@bishop:/tmp/git/alice{master}$ git push
Everything up-to-date

stephen@bishop:/tmp/git/alice{master}$ echo test2 >> file

stephen@bishop:/tmp/git/alice{master}$ git commit -am w
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
[master b4f5b5b] w
 1 files changed, 1 insertions(+), 0 deletions(-)

stephen@bishop:/tmp/git/alice{master}$ git push
Everything up-to-date

stephen@bishop:/tmp/git/alice{master}$ git pull
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
remote: Counting objects: 5, done.
remote: Total 3 (delta 0), reused 0 (delta 0)
Unpacking objects: 100% (3/3), done.
From /tmp/git/remote
   072df32..47699a9  master     -> origin/master
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
First, rewinding head to replay your work on top of it...
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
Applying: w
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.

stephen@bishop:/tmp/git/alice{master}$ echo test3 >> file

stephen@bishop:/tmp/git/alice{master}$ git commit -am w
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
[master cc2088e] w
 1 files changed, 1 insertions(+), 0 deletions(-)

stephen@bishop:/tmp/git/alice{master}$ git status
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
# On branch master
# Your branch is ahead of 'origin/master' by 2 commits.
#
nothing to commit (working directory clean)

stephen@bishop:/tmp/git/alice{master}$ git push
Everything up-to-date

stephen@bishop:/tmp/git/alice{master}$

stephen@bishop:/tmp/git/alice{master}$ git --version
git version 1.7.4.1

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

On Wed, Apr 27, 2011 at 2:29 PM, Stephen Kelly <steveire@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 11:48 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> No problems here:
>
> I had another go.

And is that the expected behavior or not? BTW. I used 1.7.5.

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

It is not expected.

Alices repo is fubar'd. gitk doesn't work. The info about master being
ahead of remote etc is wrong or git push tells me it worked, though it
doesn't seem to.



stephen@bishop:/tmp/git/alice{master}$ git status
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
# On branch master
# Your branch is ahead of 'origin/master' by 2 commits.
#
nothing to commit (working directory clean)

stephen@bishop:/tmp/git/alice{master}$ git push
Everything up-to-date

stephen@bishop:/tmp/git/alice{master}$ git status
warning: refname 'HEAD' is ambiguous.
warning: refname 'HEAD' is ambiguous.
# On branch master
# Your branch is ahead of 'origin/master' by 2 commits.
#
nothing to commit (working directory clean)


On Wed, Apr 27, 2011 at 1:32 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 2:29 PM, Stephen Kelly <steveire@gmail.com> wrote:
>> On Wed, Apr 27, 2011 at 11:48 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> No problems here:
>>
>> I had another go.
>
> And is that the expected behavior or not? BTW. I used 1.7.5.
>
> --
> Felipe Contreras
>

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

On Wed, Apr 27, 2011 at 2:37 PM, Stephen Kelly <steveire@gmail.com> wrote:
> It is not expected.
>
> Alices repo is fubar'd. gitk doesn't work. The info about master being
> ahead of remote etc is wrong or git push tells me it worked, though it
> doesn't seem to.

gitk --all works fine, and gitk show a precise warning explaining the problem.

Also, the 'git push' worked fine. Perhaps what you didn't expect is
that when push.default=current, instead of pushing the current branch,
the 'HEAD' branch is being pushed.

So the test can be simplified to:

mkdir remote
cd remote/
git init --bare
cd ../
git clone remote/ alice
cd alice/
echo test >> file
git add file
git commit -am w
git push origin master
echo test >> file
git commit -am w
git branch HEAD
git push origin HEAD
git -c push.default=current push
git diff master origin/master

And the diff should be empty. With that in mind, it should be easy to
create a test script that does something similar, and add it to the
suite.

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Erik Faye-Lund]

On Wed, Apr 27, 2011 at 1:29 PM, Stephen Kelly <steveire@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 11:48 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> No problems here:
>
> I had another go.
>
> mkdir remote
> cd remote/
> git init --bare
> cd ../
> git clone remote/ alice
> cd alice/
> echo test >> file
> git add file
> git commit -am w
> git push origin master
> echo test >> file
> git commit -am w
> git branch HEAD

I'll stop you here. You reproduce the issue a lot simpler:

git init foo &&
cd foo &&
echo "foo" > bar &&
git add bar &&
git commit -m. &&
git branch HEAD &&
gitk

No need to involve remote branches. While remote branches makes the
issue worse, because you can get in a situation where gitk doesn't
when someone else made a nasty branch, and you fetched it.

The real problem is that "git rev-parse HEAD" outputs "warning:
refname 'HEAD' is ambiguous." to stderr (even if stderr is a non-tty),
and gitk does not like that.

This can be fixed by either doing "git -c core.warnambiguousrefs=0
rev-parse HEAD", which strikes me as ugly, or by making sure that we
don't issue this warning when not attached to a tty:

diff --git a/sha1_name.c b/sha1_name.c
index faea58d..c7e855e 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -391,7 +391,7 @@ static int get_sha1_basic(const char *str, int
len, unsigned char *sha1)
 	if (!refs_found)
 		return -1;

-	if (warn_ambiguous_refs && refs_found > 1)
+	if (warn_ambiguous_refs && refs_found > 1 && isatty(2))
 		warning(warn_msg, len, str);

 	if (reflog_len) {

Re: Creating remote branch called HEAD corrupts remote clones

[Erik Faye-Lund]

On Wed, Apr 27, 2011 at 2:21 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 1:29 PM, Stephen Kelly <steveire@gmail.com> wrote:
>> On Wed, Apr 27, 2011 at 11:48 AM, Felipe Contreras
>> <felipe.contreras@gmail.com> wrote:
>>> No problems here:
>>
>> I had another go.
>>
>> mkdir remote
>> cd remote/
>> git init --bare
>> cd ../
>> git clone remote/ alice
>> cd alice/
>> echo test >> file
>> git add file
>> git commit -am w
>> git push origin master
>> echo test >> file
>> git commit -am w
>> git branch HEAD
>
> I'll stop you here. You reproduce the issue a lot simpler:
>
> git init foo &&
> cd foo &&
> echo "foo" > bar &&
> git add bar &&
> git commit -m. &&
> git branch HEAD &&
> gitk
>
> No need to involve remote branches. While remote branches makes the
> issue worse, because you can get in a situation where gitk doesn't
> when someone else made a nasty branch, and you fetched it.
>
> The real problem is that "git rev-parse HEAD" outputs "warning:
> refname 'HEAD' is ambiguous." to stderr (even if stderr is a non-tty),
> and gitk does not like that.
>
> This can be fixed by either doing "git -c core.warnambiguousrefs=0
> rev-parse HEAD", which strikes me as ugly, or by making sure that we
> don't issue this warning when not attached to a tty:

Of course, a third (and probably even better) option is to make gitk
warn about the ambiguous refname (like other commands will), but not
treat it as a fatal problem. But I'm not motivated enough to give that
solution a stab myself.

Not outputting that warning might be a regression for other users of
rev-parse (and/or the underlying mechanics).

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

On Wed, Apr 27, 2011 at 2:49 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 2:21 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> On Wed, Apr 27, 2011 at 1:29 PM, Stephen Kelly <steveire@gmail.com> wrote:
>>> On Wed, Apr 27, 2011 at 11:48 AM, Felipe Contreras
>>> <felipe.contreras@gmail.com> wrote:
>>>> No problems here:
>>>
>>> I had another go.
>>>
>>> mkdir remote
>>> cd remote/
>>> git init --bare
>>> cd ../
>>> git clone remote/ alice
>>> cd alice/
>>> echo test >> file
>>> git add file
>>> git commit -am w
>>> git push origin master
>>> echo test >> file
>>> git commit -am w
>>> git branch HEAD
>>
>> I'll stop you here. You reproduce the issue a lot simpler:
>>
>> git init foo &&
>> cd foo &&
>> echo "foo" > bar &&
>> git add bar &&
>> git commit -m. &&
>> git branch HEAD &&
>> gitk
>>
>> No need to involve remote branches. While remote branches makes the
>> issue worse, because you can get in a situation where gitk doesn't
>> when someone else made a nasty branch, and you fetched it.
>>
>> The real problem is that "git rev-parse HEAD" outputs "warning:
>> refname 'HEAD' is ambiguous." to stderr (even if stderr is a non-tty),
>> and gitk does not like that.
>>
>> This can be fixed by either doing "git -c core.warnambiguousrefs=0
>> rev-parse HEAD", which strikes me as ugly, or by making sure that we
>> don't issue this warning when not attached to a tty:
>
> Of course, a third (and probably even better) option is to make gitk
> warn about the ambiguous refname (like other commands will), but not
> treat it as a fatal problem. But I'm not motivated enough to give that
> solution a stab myself.
>
> Not outputting that warning might be a regression for other users of
> rev-parse (and/or the underlying mechanics).
>

Ok, if you can't see in the code why a branch called HEAD might
corrupt the remote and I can't demonstrate it with a testcase, maybe
it's not an issue anymore, I don't know.

Hopefully the relevant people saw the side issues brought up such as
this ambiguous ref issue. After all, there's no other way to track
those issues.

Thanks for the investigation and help,

Steve.

Re: Creating remote branch called HEAD corrupts remote clones

[Erik Faye-Lund]

On Mon, May 2, 2011 at 9:26 PM, Stephen Kelly <steveire@gmail.com> wrote:
> On Wed, Apr 27, 2011 at 2:49 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> On Wed, Apr 27, 2011 at 2:21 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> On Wed, Apr 27, 2011 at 1:29 PM, Stephen Kelly <steveire@gmail.com> wrote:
>>>> On Wed, Apr 27, 2011 at 11:48 AM, Felipe Contreras
>>>> <felipe.contreras@gmail.com> wrote:
>>>>> No problems here:
>>>>
>>>> I had another go.
>>>>
>>>> mkdir remote
>>>> cd remote/
>>>> git init --bare
>>>> cd ../
>>>> git clone remote/ alice
>>>> cd alice/
>>>> echo test >> file
>>>> git add file
>>>> git commit -am w
>>>> git push origin master
>>>> echo test >> file
>>>> git commit -am w
>>>> git branch HEAD
>>>
>>> I'll stop you here. You reproduce the issue a lot simpler:
>>>
>>> git init foo &&
>>> cd foo &&
>>> echo "foo" > bar &&
>>> git add bar &&
>>> git commit -m. &&
>>> git branch HEAD &&
>>> gitk
>>>
>>> No need to involve remote branches. While remote branches makes the
>>> issue worse, because you can get in a situation where gitk doesn't
>>> when someone else made a nasty branch, and you fetched it.
>>>
>>> The real problem is that "git rev-parse HEAD" outputs "warning:
>>> refname 'HEAD' is ambiguous." to stderr (even if stderr is a non-tty),
>>> and gitk does not like that.
>>>
>>> This can be fixed by either doing "git -c core.warnambiguousrefs=0
>>> rev-parse HEAD", which strikes me as ugly, or by making sure that we
>>> don't issue this warning when not attached to a tty:
>>
>> Of course, a third (and probably even better) option is to make gitk
>> warn about the ambiguous refname (like other commands will), but not
>> treat it as a fatal problem. But I'm not motivated enough to give that
>> solution a stab myself.
>>
>> Not outputting that warning might be a regression for other users of
>> rev-parse (and/or the underlying mechanics).
>>
>
> Ok, if you can't see in the code why a branch called HEAD might
> corrupt the remote and I can't demonstrate it with a testcase, maybe
> it's not an issue anymore, I don't know.
>

No, it's still an issue, and I believe I pin-pointed it in my first
mail. You can try out the patch I sent, and see if that helps in your
case. If it does, I think it'd make sense to do something (preferably
a bit more robust) with it.

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

On Mon, May 2, 2011 at 10:43 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
> On Mon, May 2, 2011 at 9:26 PM, Stephen Kelly <steveire@gmail.com> wrote:
>> On Wed, Apr 27, 2011 at 2:49 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> On Wed, Apr 27, 2011 at 2:21 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>>> On Wed, Apr 27, 2011 at 1:29 PM, Stephen Kelly <steveire@gmail.com> wrote:
>>>>> On Wed, Apr 27, 2011 at 11:48 AM, Felipe Contreras
>>>>> <felipe.contreras@gmail.com> wrote:
>>>>>> No problems here:
>>>>>
>>>>> I had another go.
>>>>>
>>>>> mkdir remote
>>>>> cd remote/
>>>>> git init --bare
>>>>> cd ../
>>>>> git clone remote/ alice
>>>>> cd alice/
>>>>> echo test >> file
>>>>> git add file
>>>>> git commit -am w
>>>>> git push origin master
>>>>> echo test >> file
>>>>> git commit -am w
>>>>> git branch HEAD
>>>>
>>>> I'll stop you here. You reproduce the issue a lot simpler:
>>>>
>>>> git init foo &&
>>>> cd foo &&
>>>> echo "foo" > bar &&
>>>> git add bar &&
>>>> git commit -m. &&
>>>> git branch HEAD &&
>>>> gitk
>>>>
>>>> No need to involve remote branches. While remote branches makes the
>>>> issue worse, because you can get in a situation where gitk doesn't
>>>> when someone else made a nasty branch, and you fetched it.
>>>>
>>>> The real problem is that "git rev-parse HEAD" outputs "warning:
>>>> refname 'HEAD' is ambiguous." to stderr (even if stderr is a non-tty),
>>>> and gitk does not like that.
>>>>
>>>> This can be fixed by either doing "git -c core.warnambiguousrefs=0
>>>> rev-parse HEAD", which strikes me as ugly, or by making sure that we
>>>> don't issue this warning when not attached to a tty:
>>>
>>> Of course, a third (and probably even better) option is to make gitk
>>> warn about the ambiguous refname (like other commands will), but not
>>> treat it as a fatal problem. But I'm not motivated enough to give that
>>> solution a stab myself.
>>>
>>> Not outputting that warning might be a regression for other users of
>>> rev-parse (and/or the underlying mechanics).
>>>
>>
>> Ok, if you can't see in the code why a branch called HEAD might
>> corrupt the remote and I can't demonstrate it with a testcase, maybe
>> it's not an issue anymore, I don't know.
>>
>
> No, it's still an issue, and I believe I pin-pointed it in my first
> mail. You can try out the patch I sent, and see if that helps in your
> case. If it does, I think it'd make sense to do something (preferably
> a bit more robust) with it.

Yes, I think your patch should be applied regardless, as that solves
_one_ issue.

But there are other issues.

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Stephen Kelly]

On Tue, May 3, 2011 at 7:54 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Mon, May 2, 2011 at 10:43 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> No, it's still an issue, and I believe I pin-pointed it in my first
>> mail. You can try out the patch I sent, and see if that helps in your
>> case. If it does, I think it'd make sense to do something (preferably
>> a bit more robust) with it.

I don't have a build of git at the moment to test it as I'm using
distro packages again. The only test case I have is the alice and bob
stuff already posted, so if your patch fixes that for you that's good
enough from my POV.

>
> Yes, I think your patch should be applied regardless, as that solves
> _one_ issue.
>
> But there are other issues.
>

All the best,

Steve.

Re: Creating remote branch called HEAD corrupts remote clones

[Felipe Contreras]

On Tue, May 3, 2011 at 9:08 PM, Stephen Kelly <steveire@gmail.com> wrote:
> On Tue, May 3, 2011 at 7:54 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> On Mon, May 2, 2011 at 10:43 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>>> No, it's still an issue, and I believe I pin-pointed it in my first
>>> mail. You can try out the patch I sent, and see if that helps in your
>>> case. If it does, I think it'd make sense to do something (preferably
>>> a bit more robust) with it.
>
> I don't have a build of git at the moment to test it as I'm using
> distro packages again. The only test case I have is the alice and bob
> stuff already posted, so if your patch fixes that for you that's good
> enough from my POV.

As I said, 'gitk --all' works fine, the patch would fix 'gitk'.

-- 
Felipe Contreras

Re: Creating remote branch called HEAD corrupts remote clones

[Erik Faye-Lund]

On Tue, May 3, 2011 at 7:54 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Mon, May 2, 2011 at 10:43 PM, Erik Faye-Lund <kusmabite@gmail.com> wrote:
>> On Mon, May 2, 2011 at 9:26 PM, Stephen Kelly <steveire@gmail.com> wrote:
>>> Ok, if you can't see in the code why a branch called HEAD might
>>> corrupt the remote and I can't demonstrate it with a testcase, maybe
>>> it's not an issue anymore, I don't know.
>>
>> No, it's still an issue, and I believe I pin-pointed it in my first
>> mail. You can try out the patch I sent, and see if that helps in your
>> case. If it does, I think it'd make sense to do something (preferably
>> a bit more robust) with it.
>
> Yes, I think your patch should be applied regardless, as that solves
> _one_ issue.

OK, I'll send out an RFC with some discussion on the alternatives a bit later.

> But there are other issues.

I guess the root of the problem(s) is that there's no way to
disambiguate 'HEAD'. One solution could be to say that 'HEAD' never is
ambiguous, but it feels a little inconsistent... Thoughts, anyone?

[PATCH] only warn about ambiguous refs if stderr is a tty

[Erik Faye-Lund]

If there's a branch (either local or remote) called 'HEAD'
commands that take a ref currently emits a warning, no matter
if the output is going to a TTY or not.

Fix this by making sure we only output this warning when stderr
is a TTY. Other git commands or scripts should not care about
this ambiguity.

This fix prevents gitk from barfing when given no arguments and
there's a branch called 'HEAD'.

Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com>

---

In 2f8acdb ('core.warnambiguousrefs: warns when "name" is used
and both "name" branch and tag exists.'), a check for collisions
of refs was introduced. It does not seem to me like the intention
was to check HEAD for ambiguty (because the commit talks about
branches and tags), but it does.

Because HEAD cannot be disambiguated like branches and tags can,
this can lead to an annoying warning, or even an error in the case
of gitk.

A branch called HEAD can be 'injected' into another user's repo
through remotes, and this can cause annoyance (and in the case of
gitk, brokenness) just by pulling the wrong remote. Yuck.

The particular problem of gitk can be fixed by making gitk able
to parse the warning, and probably forwarding it to the user.
This strikes me as The Right Thing To Do(tm), but is outside of my
gitk and TCL/TK skills.

Alternatively, gitk could state that it doesn't care about
ambiguous refs, by calling 'git -c core.warnambiguousrefs=0
show-ref <ref>'.

One question is if ANY warnings should be output to stderr if it's
not a TTY. My guess is that there probably are some classes of
warnings that should, but the vast majority should probably not.

Perhaps it's better to make warning() filter the output if stderr
is not a tty instead, and make the places that needs to warn just
do fprintf(stderr, ...) instead? That's one huge hammer, though.

Another question is if we should come up with a way of
disambiguating HEAD. Perhaps having something like 'refs/HEAD'
will do?

So, to recap: The way I see it, these are our options:

 1) Discard this specific warning when stderr isn't a TTY (i.e
    what this patch does)
 2) Discard all warnings when stderr isn't a TTY
 3) Make gitk understand and forward warnings to the user
 4) Have gitk explicitly ignore ambiuous refs
 5) Come up with a way to disambiguate HEAD, and use that instead
    by default
 6) Force HEAD to never be ambiguous
 7) Leave things as they are

I think 3) + 5) might be the most sane solution. That way we
inform the user that there's an ambiguity if he or she runs
'gitk HEAD' (so he or she has a chance the chance to correct it),
but the correct HEAD is chosen (without any annoying warnings) if
the user didn't specify a ref.

This combination also relies on us NOT doing 1), 2) or 4); i.e the
warning must still be output to reach the user.

Thoughs?

 sha1_name.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index faea58d..c7e855e 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -391,7 +391,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	if (!refs_found)
 		return -1;
 
-	if (warn_ambiguous_refs && refs_found > 1)
+	if (warn_ambiguous_refs && refs_found > 1 && isatty(2))
 		warning(warn_msg, len, str);
 
 	if (reflog_len) {
-- 
1.7.5.3775.ga8770a

Re: [PATCH] only warn about ambiguous refs if stderr is a tty

[Jeff King]

On Mon, May 09, 2011 at 09:51:18AM +0200, Erik Faye-Lund wrote:

> If there's a branch (either local or remote) called 'HEAD'
> commands that take a ref currently emits a warning, no matter
> if the output is going to a TTY or not.
> 
> Fix this by making sure we only output this warning when stderr
> is a TTY. Other git commands or scripts should not care about
> this ambiguity.
> 
> This fix prevents gitk from barfing when given no arguments and
> there's a branch called 'HEAD'.

This feels wrong. Gitk should not care about messages on stderr, for
exactly the reason that they may be harmless warnings (if anything, it
should show them to the user in a dialog).

My understanding is that this is a tcl thing, but I just think it's
insane.

> In 2f8acdb ('core.warnambiguousrefs: warns when "name" is used
> and both "name" branch and tag exists.'), a check for collisions
> of refs was introduced. It does not seem to me like the intention
> was to check HEAD for ambiguty (because the commit talks about
> branches and tags), but it does.
> 
> Because HEAD cannot be disambiguated like branches and tags can,
> this can lead to an annoying warning, or even an error in the case
> of gitk.

This is a separate issue, isn't it? Gitk should probably handle
ambiguous ref warnings better, no matter what the name.  And if
ambiguous HEAD warnings are considered too annoying, they should be
squelched for everyone. I don't personally have an opinion on the
latter, though.

> A branch called HEAD can be 'injected' into another user's repo
> through remotes, and this can cause annoyance (and in the case of
> gitk, brokenness) just by pulling the wrong remote. Yuck.

Can you give an example? If I am fetching your refs into
refs/remotes/$remote/*, how does that create an ambiguity?

> The particular problem of gitk can be fixed by making gitk able
> to parse the warning, and probably forwarding it to the user.
> This strikes me as The Right Thing To Do(tm), but is outside of my
> gitk and TCL/TK skills.

Agreed. And also outside my tcl skills. :)

> One question is if ANY warnings should be output to stderr if it's
> not a TTY. My guess is that there probably are some classes of
> warnings that should, but the vast majority should probably not.

I disagree. If I do:

  git foo 2>errors

I would certainly expect any relevant errors to end up in that file. As
for why I would do that, two cases I can think of offhand are:

  1. Test scripts, which use this extensively.

  2. Sometimes cron jobs will capture chatty output in a file and show
     it only in the case of some error condition.

> Another question is if we should come up with a way of
> disambiguating HEAD. Perhaps having something like 'refs/HEAD'
> will do?

Yeah, if we disambiguate, I would be tempted to say that "HEAD" always
unambiguously refers to "HEAD". And "refs/HEAD" should already
work, no?

> So, to recap: The way I see it, these are our options:
> 
>  1) Discard this specific warning when stderr isn't a TTY (i.e
>     what this patch does)
>  2) Discard all warnings when stderr isn't a TTY
>  3) Make gitk understand and forward warnings to the user
>  4) Have gitk explicitly ignore ambiuous refs
>  5) Come up with a way to disambiguate HEAD, and use that instead
>     by default
>  6) Force HEAD to never be ambiguous
>  7) Leave things as they are

> 
> I think 3) + 5) might be the most sane solution.

Agreed.

> diff --git a/sha1_name.c b/sha1_name.c
> index faea58d..c7e855e 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -391,7 +391,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>  	if (!refs_found)
>  		return -1;
>  
> -	if (warn_ambiguous_refs && refs_found > 1)
> +	if (warn_ambiguous_refs && refs_found > 1 && isatty(2))
>  		warning(warn_msg, len, str);
>  

I think I have made it clear that I am not in favor of this approach,
but if we were to do it, it is too late to be calling isatty(2) here.
You need to also check pager_in_use(), as we may have redirected stderr
into the pager's pipe.

-Peff

Re: [PATCH] only warn about ambiguous refs if stderr is a tty

[Erik Faye-Lund]

On Mon, May 9, 2011 at 10:03 AM, Jeff King <peff@peff.net> wrote:
> On Mon, May 09, 2011 at 09:51:18AM +0200, Erik Faye-Lund wrote:
>
>> If there's a branch (either local or remote) called 'HEAD'
>> commands that take a ref currently emits a warning, no matter
>> if the output is going to a TTY or not.
>>
>> Fix this by making sure we only output this warning when stderr
>> is a TTY. Other git commands or scripts should not care about
>> this ambiguity.
>>
>> This fix prevents gitk from barfing when given no arguments and
>> there's a branch called 'HEAD'.
>
> This feels wrong. Gitk should not care about messages on stderr, for
> exactly the reason that they may be harmless warnings (if anything, it
> should show them to the user in a dialog).

I agree.

>> In 2f8acdb ('core.warnambiguousrefs: warns when "name" is used
>> and both "name" branch and tag exists.'), a check for collisions
>> of refs was introduced. It does not seem to me like the intention
>> was to check HEAD for ambiguty (because the commit talks about
>> branches and tags), but it does.
>>
>> Because HEAD cannot be disambiguated like branches and tags can,
>> this can lead to an annoying warning, or even an error in the case
>> of gitk.
>
> This is a separate issue, isn't it? Gitk should probably handle
> ambiguous ref warnings better, no matter what the name.  And if
> ambiguous HEAD warnings are considered too annoying, they should be
> squelched for everyone. I don't personally have an opinion on the
> latter, though.
>

I agree; it's possible to squelch them already with the
core.warnambiguousrefs config, so people who want to live with
branched called 'HEAD' can already get around it.

>> A branch called HEAD can be 'injected' into another user's repo
>> through remotes, and this can cause annoyance (and in the case of
>> gitk, brokenness) just by pulling the wrong remote. Yuck.
>
> Can you give an example? If I am fetching your refs into
> refs/remotes/$remote/*, how does that create an ambiguity?
>

Actually, this is just something I read out of Stephen's report and
was too lazy to double check. It's not possible to do, because
refs/remotes/* does not seem to be checked for ambiguity. Thanks for
setting me straight :)

>> One question is if ANY warnings should be output to stderr if it's
>> not a TTY. My guess is that there probably are some classes of
>> warnings that should, but the vast majority should probably not.
>
> I disagree. If I do:
>
>  git foo 2>errors
>
> I would certainly expect any relevant errors to end up in that file. As
> for why I would do that, two cases I can think of offhand are:
>
>  1. Test scripts, which use this extensively.
>
>  2. Sometimes cron jobs will capture chatty output in a file and show
>     it only in the case of some error condition.
>

I was talking about warnings, not errors. But I can also see that one
would sometimes want warnings even when not connected to a tty, but
perhaps only when -v is specified?

>> Another question is if we should come up with a way of
>> disambiguating HEAD. Perhaps having something like 'refs/HEAD'
>> will do?
>
> Yeah, if we disambiguate, I would be tempted to say that "HEAD" always
> unambiguously refers to "HEAD".

While that would touch less code, my gut tells me it's a bit more
fragile. But perhaps you're right; I can't come up with any real
arguments (i.e use cases that I care about) on top of my head.

> And "refs/HEAD" should already work, no?

No:
$ git init foo
$ cd foo/
$ echo "foo" > bar
$ git add bar
$ git commit -m.
[master (root-commit) fc0cbef] .
warning: LF will be replaced by CRLF in bar.
The file will have its original line endings in your working directory.
 1 files changed, 1 insertions(+), 0 deletions(-)
 create mode 100644 bar
$ git show refs/HEAD
fatal: ambiguous argument 'refs/HEAD': unknown revision or path not in
the working tree.
Use '--' to separate paths from revisions

>> diff --git a/sha1_name.c b/sha1_name.c
>> index faea58d..c7e855e 100644
>> --- a/sha1_name.c
>> +++ b/sha1_name.c
>> @@ -391,7 +391,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>>       if (!refs_found)
>>               return -1;
>>
>> -     if (warn_ambiguous_refs && refs_found > 1)
>> +     if (warn_ambiguous_refs && refs_found > 1 && isatty(2))
>>               warning(warn_msg, len, str);
>>
>
> I think I have made it clear that I am not in favor of this approach,
> but if we were to do it, it is too late to be calling isatty(2) here.
> You need to also check pager_in_use(), as we may have redirected stderr
> into the pager's pipe.

Good point. I doubt I'll update the patch in this direction though,
since I agree it's not the right approach.

Re: [PATCH] only warn about ambiguous refs if stderr is a tty

[Jeff King]

On Mon, May 09, 2011 at 10:41:02AM +0200, Erik Faye-Lund wrote:

> > I disagree. If I do:
> >
> >  git foo 2>errors
> >
> > I would certainly expect any relevant errors to end up in that file. As
> > for why I would do that, two cases I can think of offhand are:
> >
> >  1. Test scripts, which use this extensively.
> >
> >  2. Sometimes cron jobs will capture chatty output in a file and show
> >     it only in the case of some error condition.
> >
> 
> I was talking about warnings, not errors. But I can also see that one
> would sometimes want warnings even when not connected to a tty, but
> perhaps only when -v is specified?

I know. I meant a script like this:

  cat >>foo.sh <<'EOF'
  # go to branch in question
  git checkout "$1"

  # note some point of interest
  sha1=`git rev-parse "$2"`

  # do some script-specific inspection of $sha1, and
  # merge if it looks OK
  if test -z "$(git log ..$sha1 -- some-path)"; then
    git merge $sha1 || exit 1
  fi
  EOF

It may produce some chatty output (like "switched to branch..."). So I
redirect it to a file, and if everything is successful, that output is
uninteresting. But if it fails, then I want to see everything. So I do
something like:

  if ! foo.sh master topic >output.tmp 2>&1; then
    cat output.tmp
    exit 1
  fi

If the merge fails, it will produce an error message. But I _also_ want
to see any warnings that were generated by it and earlier commands, like
rev-parse (e.g., an ambiguous ref warning might help us understand why
the merge failed).

Obviously this is a pretty trivial example that I cooked up for this
email. But the concept of stash-stderr-and-report-on-error is a pretty
common pattern for cron jobs.

> > Yeah, if we disambiguate, I would be tempted to say that "HEAD" always
> > unambiguously refers to "HEAD".
> 
> While that would touch less code, my gut tells me it's a bit more
> fragile. But perhaps you're right; I can't come up with any real
> arguments (i.e use cases that I care about) on top of my head.

Honestly, I'm kind of surprised it's not that way already. It would make
sense to me that "upper" levels would take precedence over lower levels,
but that ambiguity would occur within a level. So if I say "foo", we
would look for:

  1. $GIT_DIR/foo, with no ambiguity

  2. $GIT_DIR/refs/foo, with no ambiguity

  3. $GIT_DIR/refs/tags/foo
     $GIT_DIR/refs/heads/foo
     $GIT_DIR/refs/remotes/foo

     And note any ambiguity between those three.

Which is not very different than what we do today, except that things
like HEAD and FETCH_HEAD would always be unambiguously about the
top-level.

> > And "refs/HEAD" should already work, no?
> 
> No:
> $ git init foo
> $ cd foo/
> $ echo "foo" > bar
> $ git add bar
> $ git commit -m.
> [master (root-commit) fc0cbef] .
> warning: LF will be replaced by CRLF in bar.
> The file will have its original line endings in your working directory.
>  1 files changed, 1 insertions(+), 0 deletions(-)
>  create mode 100644 bar
> $ git show refs/HEAD
> fatal: ambiguous argument 'refs/HEAD': unknown revision or path not in
> the working tree.
> Use '--' to separate paths from revisions

Of course, because there is no refs/HEAD at all. I meant "if you have
ambiguity between $GIT_DIR/HEAD and $GIT_DIR/refs/HEAD", then saying
"refs/HEAD" should disambiguate already. In your example, there is no
ambiguity.

What I failed to notice is that the likely disambiguator is actually
"refs/heads/HEAD" if you erroneously made a branch.

Try this:

  # A repo with two commits
  git init repo && cd repo &&
  echo content >file &&
  git add file &&
  git commit -m one &&
  echo content >>file &&
  git commit -a -m two &&

  # And an ambiguously named ref called HEAD, pointing to "one";
  # our real HEAD is still pointing to "two"
  git branch HEAD HEAD^ &&

  # This should warn of ambiguity, but show "two"
  git log -1 --oneline HEAD

  # And this should not be ambiguous at all, and show "one"
  git log -1 --oneline refs/heads/HEAD

  # You can even do the same thing with refs/HEAD if you want, but
  # you have to use plumbing to get such a ref.
  git branch -d HEAD
  git update-ref refs/HEAD HEAD^

  # same as before, ambiguous "two"
  git log -1 --oneline HEAD

  # or we can use refs/HEAD to get "one"
  git log -1 --oneline refs/HEAD

So most of that makes sense to me. We choose $GIT_DIR/HEAD over other
options, and you can specifically refer to something further down by
its fully-qualified name.

The only thing that I think we might want to change is that "HEAD" is
considered ambiguous with "refs/heads/HEAD". On the other hand, it seems
a little insane to name your branch that, given that it has a
well-established meaning in git. I admit I haven't been following this
thread too closely. What is the reason not to tell the user "sorry, that
is an insane branch name. Accept the ambiguity warning, or choose a
different name"?

-Peff

Re: [PATCH] only warn about ambiguous refs if stderr is a tty

[Erik Faye-Lund]

On Mon, May 9, 2011 at 12:32 PM, Jeff King <peff@peff.net> wrote:
> On Mon, May 09, 2011 at 10:41:02AM +0200, Erik Faye-Lund wrote:
>> I was talking about warnings, not errors. But I can also see that one
>> would sometimes want warnings even when not connected to a tty, but
>> perhaps only when -v is specified?
>
> I know. I meant a script like this:
>
>  cat >>foo.sh <<'EOF'
>  # go to branch in question
>  git checkout "$1"
>
>  # note some point of interest
>  sha1=`git rev-parse "$2"`
>
>  # do some script-specific inspection of $sha1, and
>  # merge if it looks OK
>  if test -z "$(git log ..$sha1 -- some-path)"; then
>    git merge $sha1 || exit 1
>  fi
>  EOF
>
> It may produce some chatty output (like "switched to branch..."). So I
> redirect it to a file, and if everything is successful, that output is
> uninteresting. But if it fails, then I want to see everything. So I do
> something like:
>
>  if ! foo.sh master topic >output.tmp 2>&1; then
>    cat output.tmp
>    exit 1
>  fi
>
> If the merge fails, it will produce an error message. But I _also_ want
> to see any warnings that were generated by it and earlier commands, like
> rev-parse (e.g., an ambiguous ref warning might help us understand why
> the merge failed).

Yeah, I understood that part. My point was that once the output is
wanted for diagnostics, you probably also want verbose output. And
warnings should probably always be output if we're verbose.

But I have no strong feelings about this, so it's probably better to
leave it alone.

>> > Yeah, if we disambiguate, I would be tempted to say that "HEAD" always
>> > unambiguously refers to "HEAD".
>>
>> While that would touch less code, my gut tells me it's a bit more
>> fragile. But perhaps you're right; I can't come up with any real
>> arguments (i.e use cases that I care about) on top of my head.
>
> Honestly, I'm kind of surprised it's not that way already. It would make
> sense to me that "upper" levels would take precedence over lower levels,
> but that ambiguity would occur within a level. So if I say "foo", we
> would look for:
>
>  1. $GIT_DIR/foo, with no ambiguity
>
>  2. $GIT_DIR/refs/foo, with no ambiguity
>
>  3. $GIT_DIR/refs/tags/foo
>     $GIT_DIR/refs/heads/foo
>     $GIT_DIR/refs/remotes/foo
>
>     And note any ambiguity between those three.
>
> Which is not very different than what we do today, except that things
> like HEAD and FETCH_HEAD would always be unambiguously about the
> top-level.
>

I think that would make sense.

>> > And "refs/HEAD" should already work, no?
>>
>> No:
>> $ git init foo
>> $ cd foo/
>> $ echo "foo" > bar
>> $ git add bar
>> $ git commit -m.
>> [master (root-commit) fc0cbef] .
>> warning: LF will be replaced by CRLF in bar.
>> The file will have its original line endings in your working directory.
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>  create mode 100644 bar
>> $ git show refs/HEAD
>> fatal: ambiguous argument 'refs/HEAD': unknown revision or path not in
>> the working tree.
>> Use '--' to separate paths from revisions
>
> Of course, because there is no refs/HEAD at all. I meant "if you have
> ambiguity between $GIT_DIR/HEAD and $GIT_DIR/refs/HEAD", then saying
> "refs/HEAD" should disambiguate already. In your example, there is no
> ambiguity.

I meant that "refs/HEAD" could be an non-ambiguous alias for HEAD, but
it's probably easier to just say that 'HEAD' isn't ambiguous. Your
suggestion of only checking for ambiguousness on the same level is IMO
an elegant way of doing this.

> What I failed to notice is that the likely disambiguator is actually
> "refs/heads/HEAD" if you erroneously made a branch.
>
> Try this:
>
>  # A repo with two commits
>  git init repo && cd repo &&
>  echo content >file &&
>  git add file &&
>  git commit -m one &&
>  echo content >>file &&
>  git commit -a -m two &&
>
>  # And an ambiguously named ref called HEAD, pointing to "one";
>  # our real HEAD is still pointing to "two"
>  git branch HEAD HEAD^ &&
>
>  # This should warn of ambiguity, but show "two"
>  git log -1 --oneline HEAD
>
>  # And this should not be ambiguous at all, and show "one"
>  git log -1 --oneline refs/heads/HEAD
>
>  # You can even do the same thing with refs/HEAD if you want, but
>  # you have to use plumbing to get such a ref.
>  git branch -d HEAD
>  git update-ref refs/HEAD HEAD^
>
>  # same as before, ambiguous "two"
>  git log -1 --oneline HEAD
>
>  # or we can use refs/HEAD to get "one"
>  git log -1 --oneline refs/HEAD
>
> So most of that makes sense to me. We choose $GIT_DIR/HEAD over other
> options, and you can specifically refer to something further down by
> its fully-qualified name.
>
> The only thing that I think we might want to change is that "HEAD" is
> considered ambiguous with "refs/heads/HEAD". On the other hand, it seems
> a little insane to name your branch that, given that it has a
> well-established meaning in git.

I agree. There could be a remote chance that you can get a branch
called 'HEAD' from some foreign vcs or something, though. But I don't
think it's very likely, and the problem will also go away if we go
with your approach mentioned above.

> I admit I haven't been following this
> thread too closely. What is the reason not to tell the user "sorry, that
> is an insane branch name. Accept the ambiguity warning, or choose a
> different name"?

I think having the ambiguity warning in itself isn't the problem, it's
gitk not swallowing it that is.

The reporter also had some problems pushing with a branch named 'HEAD'
in his repo, but I didn't look into that part at all.

Re: [PATCH] only warn about ambiguous refs if stderr is a tty

[Jeff King]

On Mon, May 09, 2011 at 02:37:48PM +0200, Erik Faye-Lund wrote:

> Yeah, I understood that part. My point was that once the output is
> wanted for diagnostics, you probably also want verbose output. And
> warnings should probably always be output if we're verbose.

Ah, I see. I think my main concern is that the behavior you proposed
would simply be surprising to people used to normal unix conventions.
But it sounds like we both agree that isn't the right direction anyway.

> > Of course, because there is no refs/HEAD at all. I meant "if you have
> > ambiguity between $GIT_DIR/HEAD and $GIT_DIR/refs/HEAD", then saying
> > "refs/HEAD" should disambiguate already. In your example, there is no
> > ambiguity.
> 
> I meant that "refs/HEAD" could be an non-ambiguous alias for HEAD, but
> it's probably easier to just say that 'HEAD' isn't ambiguous. Your
> suggestion of only checking for ambiguousness on the same level is IMO
> an elegant way of doing this.

OK, I see what you meant. But "refs/HEAD" cannot be a shortcut for
"HEAD", as it means something totally different. You can have "HEAD",
"refs/HEAD", "refs/heads/HEAD" all co-existing.

> I agree. There could be a remote chance that you can get a branch
> called 'HEAD' from some foreign vcs or something, though. But I don't
> think it's very likely, and the problem will also go away if we go
> with your approach mentioned above.

Thinking on it more, I think warning is probably the only sane thing to
do there. Having a branch with that name is just going to be confusing
in the long run, and the sooner we start making the user aware of the
situation, the better.

> > I admit I haven't been following this
> > thread too closely. What is the reason not to tell the user "sorry, that
> > is an insane branch name. Accept the ambiguity warning, or choose a
> > different name"?
> 
> I think having the ambiguity warning in itself isn't the problem, it's
> gitk not swallowing it that is.

Agreed.

> The reporter also had some problems pushing with a branch named 'HEAD'
> in his repo, but I didn't look into that part at all.

I expect that would be a separate issue entirely (if it were fetching, I
wouldn't be surprised if it was the "fake" refs/remotes/*/HEAD symref we
create getting in the way).

-Peff

Re: [PATCH] only warn about ambiguous refs if stderr is a tty

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Thinking on it more, I think warning is probably the only sane thing to
> do there. Having a branch with that name is just going to be confusing
> in the long run, and the sooner we start making the user aware of the
> situation, the better.
> ...
>> I think having the ambiguity warning in itself isn't the problem, it's
>> gitk not swallowing it that is.
>
> Agreed.

I agree with both of the above.  It seems that the only thing we would
need is to do (3) and nothing else in Erik's original list?

>> So, to recap: The way I see it, these are our options:
>> 
>>  1) Discard this specific warning when stderr isn't a TTY (i.e
>>     what this patch does)
>>  2) Discard all warnings when stderr isn't a TTY
>>  3) Make gitk understand and forward warnings to the user
>>  4) Have gitk explicitly ignore ambiuous refs
>>  5) Come up with a way to disambiguate HEAD, and use that instead
>>     by default
>>  6) Force HEAD to never be ambiguous
>>  7) Leave things as they are

Re: [PATCH] only warn about ambiguous refs if stderr is a tty

[Jeff King]

On Mon, May 09, 2011 at 09:33:07AM -0700, Junio C Hamano wrote:

> >> I think having the ambiguity warning in itself isn't the problem, it's
> >> gitk not swallowing it that is.
> >
> > Agreed.
> 
> I agree with both of the above.  It seems that the only thing we would
> need is to do (3) and nothing else in Erik's original list?
> [...]
> >>  3) Make gitk understand and forward warnings to the user

Yeah, I think so. Now we just need a volunteer who wants to write some
tcl.  :)

-Peff