worktree refs: fix case sensitivity for 'head'

From: Michael Rappazzo <[email protected]>

On a worktree which is not the primary, using the symbolic-ref 'head' was incorrectly pointing to the main worktree's HEAD. The same was true for any other case of the word 'Head'.

Signed-off-by Michael Rappazzo <[email protected]>

---
 refs.c                   | 8 ++++----
 t/t1415-worktree-refs.sh | 9 +++++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

refs.c +0 -0

index f9936355cd..963e786458 100644

refs.c +0 -0

                        warning(_("ignoring dangling symref %s"), fullref.buf);
                } else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
                        warning(_("ignoring broken ref %s"), fullref.buf);

Quoted 13 lines · Michael Rappazzo via GitGitGadget

From: Michael Rappazzo <[email protected]>

On a worktree which is not the primary, using the symbolic-ref 'head' was incorrectly pointing to the main worktree's HEAD. The same was true for any other case of the word 'Head'.

Signed-off-by Michael Rappazzo <[email protected]>

---
 refs.c                   | 8 ++++----
 t/t1415-worktree-refs.sh | 9 +++++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

refs.c int expand_ref(const char *str, int len, struct object_id *oid, char **ref) · line 582

Quoted 10 lines

refs.c +1 -1

index f9936355cd..963e786458 100644
@@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
                                *ref = xstrdup(r);
                        if (!warn_ambiguous_refs)
                                break;
-               } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
+               } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf, "HEAD")) {

This is not going to work. How about ~40 other "strcmp.*HEAD" instances? All refs are case-sensitive and this probably will not change even when we introduce new ref backends.

refs.c int expand_ref(const char *str, int len, struct object_id *oid, char **ref) · line 585

Quoted 4 lines

refs.c +0 -0

                        warning(_("ignoring dangling symref %s"), fullref.buf);
                } else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
                        warning(_("ignoring broken ref %s"), fullref.buf);

--
Duy

Quoted 16 lines · Duy Nguyen

Quoted 13 lines · Michael Rappazzo via GitGitGadget

From: Michael Rappazzo <[email protected]>

On a worktree which is not the primary, using the symbolic-ref 'head' was incorrectly pointing to the main worktree's HEAD. The same was true for any other case of the word 'Head'.

Signed-off-by Michael Rappazzo <[email protected]>

---
 refs.c                   | 8 ++++----
 t/t1415-worktree-refs.sh | 9 +++++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

Quoted 14 lines

refs.c int expand_ref(const char *str, int len, struct object_id *oid, char **ref) · line 582

Quoted 10 lines

refs.c +1 -1

index f9936355cd..963e786458 100644
@@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
                                *ref = xstrdup(r);
                        if (!warn_ambiguous_refs)
                                break;
-               } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
+               } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf, "HEAD")) {

This is not going to work. How about ~40 other "strcmp.*HEAD" instances? All refs are case-sensitive and this probably will not change even when we introduce new ref backends.

The current situation is definitely a problem. If I am in a worktree, using "head" should be the same as "HEAD".

I am not sure if you mean that the fix is too narrow or too wide. Maybe it is only necessary in 'is_per_worktree_ref'. On the other side of the coin, I could change every strcmp to strcasecmp where the comparison is against "HEAD".

Quoted 2 lines

The current situation is definitely a problem. If I am in a worktree, using "head" should be the same as "HEAD".

Quoted 33 lines · Mike Rappazzo

Quoted 30 lines · Duy Nguyen

Quoted 18 lines · Michael Rappazzo via GitGitGadget

From: Michael Rappazzo <[email protected]>

On a worktree which is not the primary, using the symbolic-ref 'head' was incorrectly pointing to the main worktree's HEAD. The same was true for any other case of the word 'Head'.

Signed-off-by Michael Rappazzo <[email protected]>

---
 refs.c                   | 8 ++++----
 t/t1415-worktree-refs.sh | 9 +++++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

refs.c +0 -0

index f9936355cd..963e786458 100644

refs.c int expand_ref(const char *str, int len, struct object_id *oid, char **ref) · line 582

Quoted 5 lines

refs.c +1 -1

@@ -579,7 +579,7 @@ int expand_ref(const char *str, int len, struct object_id *oid, char **ref)
                                *ref = xstrdup(r);
                        if (!warn_ambiguous_refs)
                                break;
-               } else if ((flag & REF_ISSYMREF) && strcmp(fullref.buf, "HEAD")) {
+               } else if ((flag & REF_ISSYMREF) && strcasecmp(fullref.buf, "HEAD")) {

This is not going to work. How about ~40 other "strcmp.*HEAD" instances? All refs are case-sensitive and this probably will not change even when we introduce new ref backends.

Quoted 2 lines

The current situation is definitely a problem. If I am in a worktree, using "head" should be the same as "HEAD".

No "head" is not the same as "HEAD". It does not matter if you're in a worktree or not.

Quoted 5 lines

I am not sure if you mean that the fix is too narrow or too wide. Maybe it is only necessary in 'is_per_worktree_ref'. On the other side of the coin, I could change every strcmp to strcasecmp where the comparison is against "HEAD".

If you make "head" work like "HEAD", then it should work for _all_ commands, not just worktree, and "MASTER" should match "refs/heads/master" and so on. I don't think it's as simple as changing strcmp to strcasecmp. You would need to make ref management case-insensitive (and make sure if still is case-sensitive if configured so). I don't think anybody has managed that.

--
Duy

Quoted 3 lines

Quoted 3 lines

The current situation is definitely a problem. If I am in a worktree, using "head" should be the same as "HEAD".

By any chance, is your file system case insensitive? That is usually the source of confusion for these discussions.

Maybe in worktree code we have a spillover between path resolution and ref namespace?

Quoted 6 lines · Stefan Beller

Quoted 3 lines

Quoted 3 lines

The current situation is definitely a problem. If I am in a worktree, using "head" should be the same as "HEAD".

By any chance, is your file system case insensitive? That is usually the source of confusion for these discussions.

This behavior is the same for MacOS (High Sierra) and Windows 7. I assume other derivatives of those act the same.

On CentOS "head" is an ambiguous ref. If Windows and Mac resulted in an ambiguous ref, that would also be OK, but as it is now, they return the result of "HEAD" on the primary worktree.

If you make "head" work like "HEAD", then it should work for _all_ commands, not just worktree, and "MASTER" should match "refs/heads/master" and so on. I don't think it's as simple as changing strcmp to strcasecmp. You would need to make ref management case-insensitive (and make sure if still is case-sensitive if configured so). I don't think anybody has managed that.

Quoted 6 lines · Stefan Beller

Quoted 3 lines

Quoted 3 lines

The current situation is definitely a problem. If I am in a worktree, using "head" should be the same as "HEAD".

By any chance, is your file system case insensitive? That is usually the source of confusion for these discussions.

This behavior is the same for MacOS (High Sierra) and Windows 7. I assume other derivatives of those act the same.

On CentOS "head" is an ambiguous ref. If Windows and Mac resulted in an ambiguous ref, that would also be OK, but as it is now, they return the result of "HEAD" on the primary worktree.

Quoted 3 lines

Maybe in worktree code we have a spillover between path resolution and ref namespace?

Quoted 9 lines · Mike Rappazzo

Quoted 6 lines · Stefan Beller

Quoted 3 lines

Quoted 3 lines

The current situation is definitely a problem. If I am in a worktree, using "head" should be the same as "HEAD".

By any chance, is your file system case insensitive? That is usually the source of confusion for these discussions.

Quoted 7 lines

This behavior is the same for MacOS (High Sierra) and Windows 7. I assume other derivatives of those act the same.

On CentOS "head" is an ambiguous ref. If Windows and Mac resulted in an ambiguous ref, that would also be OK, but as it is now, they return the result of "HEAD" on the primary worktree.

Because refs are *not* case sensitive, and we know that "HEAD" should be per-worktree, it gets checked in the per-worktree refs section. But lowercase head is known to not be a per-worktree ref, so we then ask the main worktree about head. Since you happen to be on a case insensitive file system, it then finds refs/HEAD in the main refs worktree, and returns that.

I don't understand why the CentOS shows it as ambiguous, unless you actually happen to have a ref named head. (possibly a branch?)

I suspect we could improve things by attempting to figure out if our file system is case insensitive and warn users. However, I recall patches which tried this, and no suitable method was found. Partly because it's not just "case" that is the only problem. There might be things like unicode characters which don't get properly encoded, etc.

The best solution would be to get a non-filesystem backed ref storage working that could be used in place of the filesystem.

Pull-Request: https://github.com/gitgitgadget/git/pull/100

Even with a new ref storage, I'm pretty sure pseudo refs like HEAD, FETCH_HEAD... will forever be backed by filesystem. HEAD for example is part of the repository signature and must exist as a file. We could also lookup pseudo refs with readdir() instead of lstat(). On case-preserving-and-insensitive filesystems, we can reject "head" this way. But that comes with a high cost.
--
Duy

Quoted 8 lines · Duy Nguyen

Even with a new ref storage, I'm pretty sure pseudo refs like HEAD, FETCH_HEAD... will forever be backed by filesystem. HEAD for example is part of the repository signature and must exist as a file. We could also lookup pseudo refs with readdir() instead of lstat(). On case-preserving-and-insensitive filesystems, we can reject "head" this way. But that comes with a high cost.
--
Duy

Once other refs are backed by something that doesn't depend on filesystem case sensitivity, you could enforce that we only accept call-caps HEAD as a psuedo ref, and always look up other spellings in the other refs backend, though, right?

So, yea the actual file may not
be case sensitive, but we would never create refs/head anymore for any reason, so there would be no ambiguity if reading the refs/head vs refs/HEAD on a case insensitive file system, since refs/head would no longer be a legitimate ref stored as a file if you used a different refs backend.

Basically, we'd be looking up HEAD by checking the file, but we'd stop looking up head, hEAd, etc in the files, and instead use whatever other refs backend for non-pseudo refs. Thus, it wouldn't matter, since we'd never actually lookup the other spellings of HEAD as a file. Wouldn't that solve the ambiguity, at least once a repository has fully switched to some alternative refs backend for non-pseudo refs? (Unless I mis-understand and refs/head could be an added pseudo ref?)

Jake

Quoted 11 lines · Jacob Keller

Quoted 8 lines · Duy Nguyen

Even with a new ref storage, I'm pretty sure pseudo refs like HEAD, FETCH_HEAD... will forever be backed by filesystem. HEAD for example is part of the repository signature and must exist as a file. We could also lookup pseudo refs with readdir() instead of lstat(). On case-preserving-and-insensitive filesystems, we can reject "head" this way. But that comes with a high cost.
--
Duy

Quoted 4 lines

Once other refs are backed by something that doesn't depend on filesystem case sensitivity, you could enforce that we only accept call-caps HEAD as a psuedo ref, and always look up other spellings in the other refs backend, though, right?

Hmm.. yes. I don't know off hand if we have any pseudo refs in lowercase. Unlikely so yes this should work.

Quoted 11 lines · Jacob Keller

Quoted 8 lines · Duy Nguyen

Even with a new ref storage, I'm pretty sure pseudo refs like HEAD, FETCH_HEAD... will forever be backed by filesystem. HEAD for example is part of the repository signature and must exist as a file. We could also lookup pseudo refs with readdir() instead of lstat(). On case-preserving-and-insensitive filesystems, we can reject "head" this way. But that comes with a high cost.
--
Duy

Quoted 4 lines

Once other refs are backed by something that doesn't depend on filesystem case sensitivity, you could enforce that we only accept call-caps HEAD as a psuedo ref, and always look up other spellings in the other refs backend, though, right?

Hmm.. yes. I don't know off hand if we have any pseudo refs in lowercase. Unlikely so yes this should work.

Quoted 7 lines

So, yea the actual file may not
be case sensitive, but we would never create refs/head anymore for any reason, so there would be no ambiguity if reading the refs/head vs refs/HEAD on a case insensitive file system, since refs/head would no longer be a legitimate ref stored as a file if you used a different refs backend.

Quoted 8 lines

Basically, we'd be looking up HEAD by checking the file, but we'd stop looking up head, hEAd, etc in the files, and instead use whatever other refs backend for non-pseudo refs. Thus, it wouldn't matter, since we'd never actually lookup the other spellings of HEAD as a file. Wouldn't that solve the ambiguity, at least once a repository has fully switched to some alternative refs backend for non-pseudo refs? (Unless I mis-understand and refs/head could be an added pseudo ref?)

No I think "pseudo refs" are those outside "refs" directory only. So "refs/head" would be a "normal" ref.

Quoted 1 line

Jake

--
Duy

Quoted 13 lines · Duy Nguyen

Quoted 11 lines · Jacob Keller

Quoted 8 lines · Duy Nguyen

Even with a new ref storage, I'm pretty sure pseudo refs like HEAD, FETCH_HEAD... will forever be backed by filesystem. HEAD for example is part of the repository signature and must exist as a file. We could also lookup pseudo refs with readdir() instead of lstat(). On case-preserving-and-insensitive filesystems, we can reject "head" this way. But that comes with a high cost.
--
Duy

Quoted 7 lines

Quoted 4 lines

Once other refs are backed by something that doesn't depend on filesystem case sensitivity, you could enforce that we only accept call-caps HEAD as a psuedo ref, and always look up other spellings in the other refs backend, though, right?

Hmm.. yes. I don't know off hand if we have any pseudo refs in lowercase. Unlikely so yes this should work.

One thing we could do _today_ without waiting for a new refs backend is, avoid looking up pseudo refs if the given ref name is not all-caps. So "head" (or hEAd) can match refs/head, refs/tags/head, refs/heads/head but never $GIT_DIR/HEAD. And yes I checked the code, pseudo refs must be all-caps.
--
Duy