[PATCH 0/1] worktree refs: fix case sensitivity for 'head'

[PATCH 0/1] worktree refs: fix case sensitivity for 'head'

[Michael Rappazzo via GitGitGadget]

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 casing of the word 'Head'.

Signed-off-by: Michael Rappazzo rappazzo@gmail.com [rappazzo@gmail.com]

Michael Rappazzo (1):
  worktree refs: fix case sensitivity for 'head'

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


base-commit: 5d826e972970a784bd7a7bdf587512510097b8c7
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-100%2Frappazzo%2Fhead_case_sensitivity_on_worktree-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-100/rappazzo/head_case_sensitivity_on_worktree-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/100
-- 
gitgitgadget

[PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Michael Rappazzo via GitGitGadget]

From: Michael Rappazzo <rappazzo@gmail.com>

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 <rappazzo@gmail.com>
---
 refs.c                   | 8 ++++----
 t/t1415-worktree-refs.sh | 9 +++++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index f9936355cd..963e786458 100644
--- a/refs.c
+++ b/refs.c
@@ -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")) {
 			warning(_("ignoring dangling symref %s"), fullref.buf);
 		} else if ((flag & REF_ISBROKEN) && strchr(fullref.buf, '/')) {
 			warning(_("ignoring broken ref %s"), fullref.buf);
@@ -627,7 +627,7 @@ int dwim_log(const char *str, int len, struct object_id *oid, char **log)
 
 static int is_per_worktree_ref(const char *refname)
 {
-	return !strcmp(refname, "HEAD") ||
+	return !strcasecmp(refname, "HEAD") ||
 		starts_with(refname, "refs/worktree/") ||
 		starts_with(refname, "refs/bisect/") ||
 		starts_with(refname, "refs/rewritten/");
@@ -847,7 +847,7 @@ int should_autocreate_reflog(const char *refname)
 		return starts_with(refname, "refs/heads/") ||
 			starts_with(refname, "refs/remotes/") ||
 			starts_with(refname, "refs/notes/") ||
-			!strcmp(refname, "HEAD");
+			!strcasecmp(refname, "HEAD");
 	default:
 		return 0;
 	}
@@ -855,7 +855,7 @@ int should_autocreate_reflog(const char *refname)
 
 int is_branch(const char *refname)
 {
-	return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
+	return !strcasecmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
 }
 
 struct read_ref_at_cb {
diff --git a/t/t1415-worktree-refs.sh b/t/t1415-worktree-refs.sh
index b664e51250..e7f8a129fd 100755
--- a/t/t1415-worktree-refs.sh
+++ b/t/t1415-worktree-refs.sh
@@ -76,4 +76,13 @@ test_expect_success 'reflog of worktrees/xx/HEAD' '
 	test_cmp expected actual.wt2
 '
 
+test_expect_success 'head, Head, and HEAD are the same in worktree' '
+	test_cmp_rev worktree/foo initial &&
+	git -C wt1 rev-parse HEAD >uc_ref.wt1 &&
+	git -C wt1 rev-parse Head >mc_ref.wt1 &&
+	git -C wt1 rev-parse head >lc_ref.wt1 &&
+	test_cmp uc_ref.wt1 lc_ref.wt1 &&
+	test_cmp uc_ref.wt1 mc_ref.wt1
+'
+
 test_done
-- 
gitgitgadget

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Duy Nguyen]

On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Michael Rappazzo <rappazzo@gmail.com>
>
> 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 <rappazzo@gmail.com>
> ---
>  refs.c                   | 8 ++++----
>  t/t1415-worktree-refs.sh | 9 +++++++++
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index f9936355cd..963e786458 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -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.

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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Mike Rappazzo]

On Thu, Dec 13, 2018 at 3:23 PM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Michael Rappazzo <rappazzo@gmail.com>
> >
> > 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 <rappazzo@gmail.com>
> > ---
> >  refs.c                   | 8 ++++----
> >  t/t1415-worktree-refs.sh | 9 +++++++++
> >  2 files changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/refs.c b/refs.c
> > index f9936355cd..963e786458 100644
> > --- a/refs.c
> > +++ b/refs.c
> > @@ -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".


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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Duy Nguyen]

On Thu, Dec 13, 2018 at 9:34 PM Mike Rappazzo <rappazzo@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 3:23 PM Duy Nguyen <pclouds@gmail.com> wrote:
> >
> > On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> > >
> > > From: Michael Rappazzo <rappazzo@gmail.com>
> > >
> > > 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 <rappazzo@gmail.com>
> > > ---
> > >  refs.c                   | 8 ++++----
> > >  t/t1415-worktree-refs.sh | 9 +++++++++
> > >  2 files changed, 13 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/refs.c b/refs.c
> > > index f9936355cd..963e786458 100644
> > > --- a/refs.c
> > > +++ b/refs.c
> > > @@ -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".

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

> 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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Stefan Beller]

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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Mike Rappazzo]

On Thu, Dec 13, 2018 at 3:48 PM Stefan Beller <sbeller@google.com> wrote:
>
> > > 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.

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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]

On Thu, Dec 13, 2018 at 04:14:28PM -0500, Mike Rappazzo wrote:
> On Thu, Dec 13, 2018 at 3:48 PM Stefan Beller <sbeller@google.com> wrote:
> >
> > > > 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.

I'm pretty sure that we want HEAD to be only written as "HEAD". It's
known that systems with case-insensitive file systems sometimes allow
"head" instead of "HEAD" because the ref is written in the file system.

I think the improvement we'd want here is to reject HEAD being written
as "head" on those systems, but in a global way that affects all uses of
HEAD.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Jacob Keller]

On Thu, Dec 13, 2018 at 1:16 PM Mike Rappazzo <rappazzo@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 3:48 PM Stefan Beller <sbeller@google.com> wrote:
> >
> > > > 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.
>

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.

Thanks,
Jake

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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Duy Nguyen]

On Fri, Dec 14, 2018 at 7:50 AM Jacob Keller <jacob.keller@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 1:16 PM Mike Rappazzo <rappazzo@gmail.com> wrote:
> >
> > On Thu, Dec 13, 2018 at 3:48 PM Stefan Beller <sbeller@google.com> wrote:
> > >
> > > > > 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.
> >
>
> 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 think it's just our default answer when we can't decide

$ git rev-parse head
head
fatal: ambiguous argument 'head': unknown revision or path not in the
working tree.
Use '--' to separate paths from revisions, like this:
'git <command> [<revision>...] -- [<file>...]'
$ git rev-parse head --
fatal: bad revision 'head'

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

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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Jacob Keller]

On Thu, Dec 13, 2018 at 11:38 PM Duy Nguyen <pclouds@gmail.com> wrote:
> 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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Duy Nguyen]

On Fri, Dec 14, 2018 at 6:22 PM Jacob Keller <jacob.keller@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 11:38 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > 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?

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

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

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

> Jake



-- 
Duy

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Duy Nguyen]

On Fri, Dec 14, 2018 at 6:38 PM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Fri, Dec 14, 2018 at 6:22 PM Jacob Keller <jacob.keller@gmail.com> wrote:
> >
> > On Thu, Dec 13, 2018 at 11:38 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > > 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?
>
> 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

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Jacob Keller]

On Fri, Dec 14, 2018 at 9:47 AM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Fri, Dec 14, 2018 at 6:38 PM Duy Nguyen <pclouds@gmail.com> wrote:
> >
> > On Fri, Dec 14, 2018 at 6:22 PM Jacob Keller <jacob.keller@gmail.com> wrote:
> > >
> > > On Thu, Dec 13, 2018 at 11:38 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > > > 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?
> >
> > 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

Right, I think that's a good start, at least for these pseudo refs. It
doesn't solve the more general case of refs mismatching, but it
prevents the obvious one where case actually matters, by preventing
head from looking up as HEAD.

Thanks,
Jake

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Jacob Keller]

On Fri, Dec 14, 2018 at 9:38 AM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Fri, Dec 14, 2018 at 6:22 PM Jacob Keller <jacob.keller@gmail.com> wrote:
> >
> > On Thu, Dec 13, 2018 at 11:38 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > > 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?
>
> Hmm.. yes. I don't know off hand if we have any pseudo refs in
> lowercase. Unlikely so yes this should work.
>

I think even if we had lowercase pseudo refs, as long as we know which
identifiers represent pseudo refs, and we don't have two variants
which match if compared case insensitively, we shouldn't have
ambiguity, since we'd distinguish whether to check a pseudo ref spot
before we actually check the file system.

> > 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?)
>
> No I think "pseudo refs" are those outside "refs" directory only. So
> "refs/head" would be a "normal" ref.
>

Right, I was a bit confused pre-coffee and forgot why a ref was a pseudo ref.

> > Jake
>
>
>
> --
> Duy

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Mike Rappazzo]

On Thu, Dec 13, 2018 at 3:43 PM Duy Nguyen <pclouds@gmail.com> wrote:
>
> On Thu, Dec 13, 2018 at 9:34 PM Mike Rappazzo <rappazzo@gmail.com> wrote:
> >
> > On Thu, Dec 13, 2018 at 3:23 PM Duy Nguyen <pclouds@gmail.com> wrote:
> > >
> > > On Thu, Dec 13, 2018 at 8:56 PM Michael Rappazzo via GitGitGadget
> > > <gitgitgadget@gmail.com> wrote:
> > > >
> > > > From: Michael Rappazzo <rappazzo@gmail.com>
> > > >
> > > > 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 <rappazzo@gmail.com>
> > > > ---
> > > >  refs.c                   | 8 ++++----
> > > >  t/t1415-worktree-refs.sh | 9 +++++++++
> > > >  2 files changed, 13 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/refs.c b/refs.c
> > > > index f9936355cd..963e786458 100644
> > > > --- a/refs.c
> > > > +++ b/refs.c
> > > > @@ -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".
>
> No "head" is not the same as "HEAD". It does not matter if you're in a
> worktree or not.

I was not aware of a difference.  Is that spelled out in the docs
somewhere?  It seems like a bad idea to have a magical symbolic ref
that _sometimes_ gives you a different answer depending on casing.
What should "head" do in a worktree?  Is it supposed to mean the HEAD
of the primary worktree?

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

I am all for making "head" work in all cases, not just worktree.  I
don't think that this situation applies to non-magical refs
(branches/tags).

> --
> Duy

Re: [PATCH 1/1] worktree refs: fix case sensitivity for 'head'

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

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

And it is unclear why anybody would even want to do so.

Thanks for a doze of sanity.

Re: [PATCH 0/1] worktree refs: fix case sensitivity for 'head'

[Johannes Schindelin]

Hi Michael,

On Thu, 13 Dec 2018, Michael Rappazzo via GitGitGadget wrote:

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

What a nice thing that the 100th GitGitGadget Pull Request is celebrated
by a new GitGitGadget user.

Pleased,
Johannes