patchworktree refs: fix case sensitivity for 'head'

>
> 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")) {

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

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

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

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

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

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

> 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

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

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

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

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

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

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