Branch Name Case Sensitivity

Branch Name Case Sensitivity

[Lee Hopkins]

Hello,

Last week I ran across a potential bug with branch names on case
insensitive file systems, the complete scenario can be found here:

https://groups.google.com/forum/#!topic/msysgit/ugKL-sVMiqI

The tldr is because refs are stored as plain text files except when
packed into packed-refs, Git occasionally cannot tell the difference
between branches whose names only differ in case, and this could
potentially lead to the loss of history.

It sounds like this is a known issue, and after some more digging I
did find some older threads related to this topic, but nothing recent.
So I guess I just wanted to bring this to the attention of the Git
devs and maybe restart some discussions.

Thanks,
-Lee

Re: Branch Name Case Sensitivity

[Junio C Hamano]

Lee Hopkins <leerhop@gmail.com> writes:

> Last week I ran across a potential bug with branch names on case
> insensitive file systems, the complete scenario can be found here:
>
> https://groups.google.com/forum/#!topic/msysgit/ugKL-sVMiqI
>
> The tldr is because refs are stored as plain text files except when
> packed into packed-refs, Git occasionally cannot tell the difference
> between branches whose names only differ in case, and this could
> potentially lead to the loss of history.
>
> It sounds like this is a known issue, and after some more digging I
> did find some older threads related to this topic, but nothing recent.

Yes, it is not limited to branch names but also applies to tags and
filenames in your working tree.

Perhaps git-{branch,tag}.txt and possibly gitrepository-layout.txt
in Documentation/ may need a new "*Note*" section to warn against
this.

Thanks.

Re: Branch Name Case Sensitivity

[Torsten Bögershausen]

On 2014-02-27 20.50, Junio C Hamano wrote:
> Lee Hopkins <leerhop@gmail.com> writes:
> 
>> Last week I ran across a potential bug with branch names on case
>> insensitive file systems, the complete scenario can be found here:
>>
>> https://groups.google.com/forum/#!topic/msysgit/ugKL-sVMiqI
>>
>> The tldr is because refs are stored as plain text files except when
>> packed into packed-refs, Git occasionally cannot tell the difference
>> between branches whose names only differ in case, and this could
>> potentially lead to the loss of history.
>>
>> It sounds like this is a known issue, and after some more digging I
>> did find some older threads related to this topic, but nothing recent.
> 
> Yes, it is not limited to branch names but also applies to tags and
> filenames in your working tree.
> 
> Perhaps git-{branch,tag}.txt and possibly gitrepository-layout.txt
> in Documentation/ may need a new "*Note*" section to warn against
> this.
> 
> Thanks.
There is a possible workaround:
git pack-refs --all --prune

If this can be triggered by a hook, I don't know (I never used a hook)

It uses the C-function pack_refs(flags) in builtin/pack-refs.c
Or we can possibly trigger this function at the the of
"checkout -b" or "fetch" commands ?
Only when core.ignorecase == true ?

Re: Branch Name Case Sensitivity

[Lee Hopkins]

> Perhaps git-{branch,tag}.txt and possibly gitrepository-layout.txt
> in Documentation/ may need a new "*Note*" section to warn against
> this.

A little more documentation never hurt anyone :).

> Or we can possibly trigger this function at the the of
> "checkout -b" or "fetch" commands ?
> Only when core.ignorecase == true ?

This would essentially make git always use packed-refs when
core.ignorecase == true, correct? Are there any downsides to always
using packed-refs?

Thanks,
-Lee

On Thu, Feb 27, 2014 at 3:32 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 2014-02-27 20.50, Junio C Hamano wrote:
>> Lee Hopkins <leerhop@gmail.com> writes:
>>
>>> Last week I ran across a potential bug with branch names on case
>>> insensitive file systems, the complete scenario can be found here:
>>>
>>> https://groups.google.com/forum/#!topic/msysgit/ugKL-sVMiqI
>>>
>>> The tldr is because refs are stored as plain text files except when
>>> packed into packed-refs, Git occasionally cannot tell the difference
>>> between branches whose names only differ in case, and this could
>>> potentially lead to the loss of history.
>>>
>>> It sounds like this is a known issue, and after some more digging I
>>> did find some older threads related to this topic, but nothing recent.
>>
>> Yes, it is not limited to branch names but also applies to tags and
>> filenames in your working tree.
>>
>> Perhaps git-{branch,tag}.txt and possibly gitrepository-layout.txt
>> in Documentation/ may need a new "*Note*" section to warn against
>> this.
>>
>> Thanks.
> There is a possible workaround:
> git pack-refs --all --prune
>
> If this can be triggered by a hook, I don't know (I never used a hook)
>
> It uses the C-function pack_refs(flags) in builtin/pack-refs.c
> Or we can possibly trigger this function at the the of
> "checkout -b" or "fetch" commands ?
> Only when core.ignorecase == true ?
>
>
>
>
>
>
>

Re: Branch Name Case Sensitivity

[Michael Haggerty]

On 02/27/2014 09:37 PM, Lee Hopkins wrote:
>> Perhaps git-{branch,tag}.txt and possibly gitrepository-layout.txt
>> in Documentation/ may need a new "*Note*" section to warn against
>> this.
> 
> A little more documentation never hurt anyone :).
> 
>> Or we can possibly trigger this function at the the of
>> "checkout -b" or "fetch" commands ?
>> Only when core.ignorecase == true ?
> 
> This would essentially make git always use packed-refs when
> core.ignorecase == true, correct? Are there any downsides to always
> using packed-refs?

There are at least two reasons I can think of:

1. Efficiency: any time a reference changes, the whole packed-refs file
would have to be read and written as opposed to a single, small
loose-ref file.

2. Lock contention: two processes can modify loose references at the
same time without contending with each other.  If they always wrote the
packed-refs file, there would be more lock contention (which in the git
world means that one of the processes would fail).

Whether these are concern for a single user using a local git repository
(as opposed to git running on a server) mostly depends on how many
references you have.  With a hundred references you would probably not
notice any difference.  With ten thousand you probably would.  Somewhere
in between lies the pain threshold.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: Branch Name Case Sensitivity

[Karsten Blees]

Am 27.02.2014 21:32, schrieb Torsten Bögershausen:
> On 2014-02-27 20.50, Junio C Hamano wrote:
>> Lee Hopkins <leerhop@gmail.com> writes:
>>
>>> Last week I ran across a potential bug with branch names on case
>>> insensitive file systems, the complete scenario can be found here:
>>>
>>> https://groups.google.com/forum/#!topic/msysgit/ugKL-sVMiqI
>>>
>>> The tldr is because refs are stored as plain text files except when
>>> packed into packed-refs, Git occasionally cannot tell the difference
>>> between branches whose names only differ in case, and this could
>>> potentially lead to the loss of history.
>>>
>>> It sounds like this is a known issue, and after some more digging I
>>> did find some older threads related to this topic, but nothing recent.
>>
>> Yes, it is not limited to branch names but also applies to tags and
>> filenames in your working tree.
>>
>> Perhaps git-{branch,tag}.txt and possibly gitrepository-layout.txt
>> in Documentation/ may need a new "*Note*" section to warn against
>> this.
>>
>> Thanks.
> There is a possible workaround:
> git pack-refs --all --prune
> 

If I understand the issue correctly, the problem is that packed-refs are always case-sensitive, even if core.ignorecase=true. OTOH, checking / updating _unpacked_ refs on a case-insensitive file system is naturally case-insensitive. So wouldn't it be a better workaround to disallow packed refs (i.e. 'git config gc.packrefs false')?

Re: Branch Name Case Sensitivity

[Lee Hopkins]

> If I understand the issue correctly, the problem is that packed-refs are always case-sensitive, even if core.ignorecase=true.
> OTOH, checking / updating _unpacked_ refs on a case-insensitive file system is naturally case-insensitive.
> So wouldn't it be a better workaround to disallow packed refs (i.e. 'git config gc.packrefs false')?

You are correct, the issue boils down to mixing the usage of
packed-refs and loose refs on case insensitive file systems. So either
always using packed-refs or always using loose refs would take care of
the problem. Based Michael Haggerty's response, it seems that always
using loose refs would be a better workaround.

If I understand gc.packrefs = false correctly, it only prevents git gc
from running git pack-refs, so my question would be is there anything
else aside from git gc that would trigger git pack-refs? Are there
significant downsides to always using loose refs? Would checking
core.ignorecase in builtin\pack-refs.c, and exiting if true, be
appropriate?

Thanks,
-Lee

Re: Branch Name Case Sensitivity

[Johannes Sixt]

Am 2/28/2014 0:38, schrieb Lee Hopkins:
>> If I understand the issue correctly, the problem is that packed-refs
>> are always case-sensitive, even if core.ignorecase=true. OTOH,

core.ignorecase is intended to affect filenames of the worktree, not
anything else, BTW.

>> checking / updating _unpacked_ refs on a case-insensitive file system
>> is naturally case-insensitive. So wouldn't it be a better workaround
>> to disallow packed refs (i.e. 'git config gc.packrefs false')?
> 
> You are correct, the issue boils down to mixing the usage of 
> packed-refs and loose refs on case insensitive file systems. So either 
> always using packed-refs or always using loose refs would take care of 
> the problem. Based Michael Haggerty's response, it seems that always 
> using loose refs would be a better workaround.

So, everybody on a case-insensitive file system should pay the price even
if they do not need the "feature"? No way.

If you are on a case-insensitive filesystem, or work on a cross-platform
project, ensure that you avoid ambiguous refs. Problem solved.

-- Hannes

Re: Branch Name Case Sensitivity

[Stephen Leake]

Karsten Blees <karsten.blees@gmail.com> writes:

> If I understand the issue correctly, the problem is that packed-refs
> are always case-sensitive, even if core.ignorecase=true. 

Perhaps that could be changed? if core.ignorecase=true, packed-refs
should be compared with case-insensitive string compares.

-- 
-- Stephe

Re: Branch Name Case Sensitivity

[Michael Haggerty]

On 02/28/2014 12:38 AM, Lee Hopkins wrote:
> [...] Based Michael Haggerty's response, it seems that always
> using loose refs would be a better workaround.

No, I answered the question "what would be the disadvantages of using
only packed refs?".  Now I will answer the question "what would be the
disadvantages of using only loose refs?":

1. Efficiency.  Any time all of the references have to be read, loose
refs are far slower than packed refs.

2. Disk space and inode usage: loose refs consume one inode and one disk
sector (typically 4k) each, whereas packed refs consume only one inode
in total, and many packed refs can fit into each disk sector.

After all, there is a reason that we have both packed refs and loose
refs.  The basic idea is to use packed refs for the bulk of references,
especially "cold" references like tags that only change infrequently,
but to store "hot" references as loose refs so that they can be modified
cheaply.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: Branch Name Case Sensitivity

[Michael Haggerty]

On 02/28/2014 10:11 AM, Stephen Leake wrote:
> Karsten Blees <karsten.blees@gmail.com> writes:
> 
>> If I understand the issue correctly, the problem is that packed-refs
>> are always case-sensitive, even if core.ignorecase=true. 
> 
> Perhaps that could be changed? if core.ignorecase=true, packed-refs
> should be compared with case-insensitive string compares.

I think you are putting too much focus on what the local Git repository
does.  As soon as you pull content from somebody else, you are at the
mercy of the reference names that they have chosen.

In my opinion, a more fruitful approach is to have a pre-receive hook at
your central repository that prevents references that differ in case
only from being pushed in the first place.  As an extra convenience, you
can set your local repos up with a pre-commit hook that does the same
thing, so that developers (usually) see the problem immediately rather
than only when they try to push.

Of course, the pre-receive/pre-commit hooks could be even stricter by,
for example, allowing only lower-case branch names.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: Branch Name Case Sensitivity

[Karsten Blees]

Am 28.02.2014 07:41, schrieb Johannes Sixt:
> Am 2/28/2014 0:38, schrieb Lee Hopkins:
>>> If I understand the issue correctly, the problem is that packed-refs
>>> are always case-sensitive, even if core.ignorecase=true. OTOH,
> 
> core.ignorecase is intended to affect filenames of the worktree, not
> anything else, BTW.
> 

from git-config(1):
"enables various workarounds to enable git to work better on filesystems that are not case sensitive"

It says nothing about work-tree only, so I'd expect it to apply to all git components that store potentially case-sensitive information in file names.

...it also says "better", not "flawlessly" :-)

>>> checking / updating _unpacked_ refs on a case-insensitive file system
>>> is naturally case-insensitive. So wouldn't it be a better workaround
>>> to disallow packed refs (i.e. 'git config gc.packrefs false')?
>>
>> You are correct, the issue boils down to mixing the usage of 
>> packed-refs and loose refs on case insensitive file systems. So either 
>> always using packed-refs or always using loose refs would take care of 
>> the problem. Based Michael Haggerty's response, it seems that always 
>> using loose refs would be a better workaround.
> 
> So, everybody on a case-insensitive file system should pay the price even
> if they do not need the "feature"? No way.
> 
> If you are on a case-insensitive filesystem, or work on a cross-platform
> project, ensure that you avoid ambiguous refs. Problem solved.
> 

So its OK to lose data if you accidentally use an ambiguous ref? I cannot believe you actually meant that.

IMO the proper solution is to teach packed-refs about core.ignorecase. Until that happens, disabling gc.packrefs seems to be a valid workaround for people who have that problem.

Re: Branch Name Case Sensitivity

[Lee Hopkins]

> If you are on a case-insensitive filesystem, or work on a cross-platform
> project, ensure that you avoid ambiguous refs. Problem solved.

I agree this is the best solution, and I personally avoid the use of
ambiguous refs. However, since there is nothing in git stopping the
use of ambiguous refs, there is no way to stop every person who works
on a shared repo from using them.

> So, everybody on a case-insensitive file system should pay the price even
> if they do not need the "feature"? No way.

I would say preventing potential loss of commits is a price worth paying.

> IMO the proper solution is to teach packed-refs about core.ignorecase. Until that happens, disabling gc.packrefs seems to be a valid
> workaround for people who have that problem.

Once again, based on Michael Haggerty's very informative input, maybe
an even better solution would be to add a core.allowambiguousrefs
(default to true) option and when it is false do a case insensitive
comparison during ref creation (branching, tagging).

Thanks,
-Lee

Re: Branch Name Case Sensitivity

[Duy Nguyen]

On Fri, Feb 28, 2014 at 4:13 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
> On 02/28/2014 12:38 AM, Lee Hopkins wrote:
>> [...] Based Michael Haggerty's response, it seems that always
>> using loose refs would be a better workaround.
>
> No, I answered the question "what would be the disadvantages of using
> only packed refs?".  Now I will answer the question "what would be the
> disadvantages of using only loose refs?":
>
> 1. Efficiency.  Any time all of the references have to be read, loose
> refs are far slower than packed refs.
>
> 2. Disk space and inode usage: loose refs consume one inode and one disk
> sector (typically 4k) each, whereas packed refs consume only one inode
> in total, and many packed refs can fit into each disk sector.
>
> After all, there is a reason that we have both packed refs and loose
> refs.  The basic idea is to use packed refs for the bulk of references,
> especially "cold" references like tags that only change infrequently,
> but to store "hot" references as loose refs so that they can be modified
> cheaply.

Could we have a staging place for new refs in between? Case
sensitivity is just another limitation we hit because we rely on
filesystem. We already have problems with having both refs foo and
foo/bar at the same time. Not all repos are super busy and need the
top efficiencies of loose refs.

And about rewriting packed-refs every time, I don't think that's a big
problem for "normal" repos. linux-2.6 index file is 4MB(*) and it's
rewritten on nearly every worktree-related operation and nobody
complains (out loud anyway). Assuming an average ref takes 100 bytes,
that's about 41k refs.

(*) it's 3MB with index-v4 but I don't think v4 is popular
-- 
Duy

Re: Branch Name Case Sensitivity

[Michael Haggerty]

On 02/28/2014 03:31 PM, Duy Nguyen wrote:
> On Fri, Feb 28, 2014 at 4:13 PM, Michael Haggerty <mhagger@alum.mit.edu> wrote:
>> On 02/28/2014 12:38 AM, Lee Hopkins wrote:
>>> [...] Based Michael Haggerty's response, it seems that always
>>> using loose refs would be a better workaround.
>>
>> No, I answered the question "what would be the disadvantages of using
>> only packed refs?".  Now I will answer the question "what would be the
>> disadvantages of using only loose refs?":
>>
>> 1. Efficiency.  Any time all of the references have to be read, loose
>> refs are far slower than packed refs.
>>
>> 2. Disk space and inode usage: loose refs consume one inode and one disk
>> sector (typically 4k) each, whereas packed refs consume only one inode
>> in total, and many packed refs can fit into each disk sector.
>>
>> After all, there is a reason that we have both packed refs and loose
>> refs.  The basic idea is to use packed refs for the bulk of references,
>> especially "cold" references like tags that only change infrequently,
>> but to store "hot" references as loose refs so that they can be modified
>> cheaply.
> 
> Could we have a staging place for new refs in between? Case
> sensitivity is just another limitation we hit because we rely on
> filesystem. We already have problems with having both refs foo and
> foo/bar at the same time. Not all repos are super busy and need the
> top efficiencies of loose refs.

True.  Nor should most people usually need the ability to run multiple
git commands simultaneously.

In fact, I've started working on a pluggable backend for reference
storage.  After that change, it should be easy to experiment with
different combinations of loose-only, packed-only, or other (new)
storage schemes that don't suffer from directory/file conflicts, etc.  I
haven't talked about this work on the list yet because it's still very
young.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: Branch Name Case Sensitivity

[Junio C Hamano]

Karsten Blees <karsten.blees@gmail.com> writes:

>> If you are on a case-insensitive filesystem, or work on a cross-platform
>> project, ensure that you avoid ambiguous refs. Problem solved.
>> 
>
> So its OK to lose data if you accidentally use an ambiguous ref? I
> cannot believe you actually meant that.

I think he meant what he said: "you avoid ambiguous refs".  He did
not say "it is not Git's business to help you doing so".

I think it is prudent to warn in the end-user facing layer (read: do
not touch refs.c to implement something like that) when the user
creates "refs/heads/Next" when there already is "refs/heads/next",
and I further think it would make sense to do so even on case
sensitive platforms.

We warn ambiguous refs across refs hierarchies (e.g. if you have
refs/heads/next and refs/tags/next) with core.warnAmbiguousRefs; I
do not think it is a stretch to either introduce a new configuration
core.warnCaseInsensitiveRefs (auto-detected at the same place as we
auto-detect core.ignorecase) or use the same core.warnAmbiguousRefs
to trigger a warning upon seeing both "refs/heads/next" and
"refs/heads/Next".

Re: Branch Name Case Sensitivity

[Duy Nguyen]

On Sat, Mar 1, 2014 at 1:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Karsten Blees <karsten.blees@gmail.com> writes:
>
>>> If you are on a case-insensitive filesystem, or work on a cross-platform
>>> project, ensure that you avoid ambiguous refs. Problem solved.
>>>
>>
>> So its OK to lose data if you accidentally use an ambiguous ref? I
>> cannot believe you actually meant that.
>
> I think he meant what he said: "you avoid ambiguous refs".  He did
> not say "it is not Git's business to help you doing so".
>
> I think it is prudent to warn in the end-user facing layer (read: do
> not touch refs.c to implement something like that) when the user
> creates "refs/heads/Next" when there already is "refs/heads/next",
> and I further think it would make sense to do so even on case
> sensitive platforms.

That does not help when the user creates "next" and pulls "Next" from
elsewhere, does it?
-- 
Duy

Re: Branch Name Case Sensitivity

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> On Sat, Mar 1, 2014 at 1:58 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Karsten Blees <karsten.blees@gmail.com> writes:
>>
>>>> If you are on a case-insensitive filesystem, or work on a cross-platform
>>>> project, ensure that you avoid ambiguous refs. Problem solved.
>>>>
>>>
>>> So its OK to lose data if you accidentally use an ambiguous ref? I
>>> cannot believe you actually meant that.
>>
>> I think he meant what he said: "you avoid ambiguous refs".  He did
>> not say "it is not Git's business to help you doing so".
>>
>> I think it is prudent to warn in the end-user facing layer (read: do
>> not touch refs.c to implement something like that) when the user
>> creates "refs/heads/Next" when there already is "refs/heads/next",
>> and I further think it would make sense to do so even on case
>> sensitive platforms.
>
> That does not help when the user creates "next" and pulls "Next" from
> elsewhere, does it?

That depends on what the project policy would be.  At that point,
that user needs to talk with the "elsewhere" person and resolve the
issue (if there is one) according to the policy of their project,
and it is not Git's business to _solve_ it for them.  Warning I
suggested was a way to help avoiding without getting in a way of
projects whose policy is to allow these.

Re: Branch Name Case Sensitivity

[Lee Hopkins]

I went ahead and took a stab at a solution. My solution is more
aggressive than a warning, I actually prevent the creation of
ambiguous refs. My changes are also in refs.c, which may not be
appropriate, but it seemed like the natural place.

I have never contributed to Git (in fact this is my first dive into
the source) and my C is a bit rusty, so bear with me, this is just a
suggestion:

---
 refs.c |   31 ++++++++++++++++++++++++-------
 1 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/refs.c b/refs.c
index 89228e2..12ccdac 100644
--- a/refs.c
+++ b/refs.c
@@ -359,14 +359,24 @@ struct string_slice {
  const char *str;
 };

+static int ref_entry_ncmp(const void *key_, const void *ent_, int
(*cmp_fn)(const char *, const char *, size_t))
+{
+    const struct string_slice *key = key_;
+    const struct ref_entry *ent = *(const struct ref_entry * const *)ent_;
+    int cmp = cmp_fn(key->str, ent->name, key->len);
+    if (cmp)
+        return cmp;
+    return '\0' - (unsigned char)ent->name[key->len];
+}
+
 static int ref_entry_cmp_sslice(const void *key_, const void *ent_)
 {
- const struct string_slice *key = key_;
- const struct ref_entry *ent = *(const struct ref_entry * const *)ent_;
- int cmp = strncmp(key->str, ent->name, key->len);
- if (cmp)
- return cmp;
- return '\0' - (unsigned char)ent->name[key->len];
+ return ref_entry_ncmp(key_, ent_, strncmp);
+}
+
+static int ref_entry_casecmp_sslice(const void *key_, const void *ent_)
+{
+    return ref_entry_ncmp(key_, ent_, strncasecmp);
 }

 /*
@@ -378,6 +388,7 @@ static int search_ref_dir(struct ref_dir *dir,
const char *refname, size_t len)
 {
  struct ref_entry **r;
  struct string_slice key;
+    int (*cmp_fn)(const void *, const void *);

  if (refname == NULL || !dir->nr)
  return -1;
@@ -385,8 +396,14 @@ static int search_ref_dir(struct ref_dir *dir,
const char *refname, size_t len)
  sort_ref_dir(dir);
  key.len = len;
  key.str = refname;
+
+    if(ignore_case)
+        cmp_fn = ref_entry_casecmp_sslice;
+    else
+        cmp_fn = ref_entry_cmp_sslice;
+
  r = bsearch(&key, dir->entries, dir->nr, sizeof(*dir->entries),
-    ref_entry_cmp_sslice);
+    cmp_fn);

  if (r == NULL)
  return -1;
--

Re: Branch Name Case Sensitivity

[Torsten Bögershausen]

On 2014-03-01 03.42, Lee Hopkins wrote:
> I went ahead and took a stab at a solution. My solution is more
> aggressive than a warning, I actually prevent the creation of
> ambiguous refs. My changes are also in refs.c, which may not be
> appropriate, but it seemed like the natural place.
> 
> I have never contributed to Git (in fact this is my first dive into
> the source) and my C is a bit rusty, so bear with me, this is just a
> suggestion:
> 
> ---
>  refs.c |   31 ++++++++++++++++++++++++-------
>  1 files changed, 24 insertions(+), 7 deletions(-)
> 
> diff --git a/refs.c b/refs.c
> index 89228e2..12ccdac 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -359,14 +359,24 @@ struct string_slice {
>   const char *str;
>  };
> 
> +static int ref_entry_ncmp(const void *key_, const void *ent_, int
> (*cmp_fn)(const char *, const char *, size_t))
> +{
> +    const struct string_slice *key = key_;
> +    const struct ref_entry *ent = *(const struct ref_entry * const *)ent_;
> +    int cmp = cmp_fn(key->str, ent->name, key->len);
> +    if (cmp)
> +        return cmp;
> +    return '\0' - (unsigned char)ent->name[key->len];
> +}
> +
>  static int ref_entry_cmp_sslice(const void *key_, const void *ent_)
>  {
> - const struct string_slice *key = key_;
> - const struct ref_entry *ent = *(const struct ref_entry * const *)ent_;
> - int cmp = strncmp(key->str, ent->name, key->len);
> - if (cmp)
> - return cmp;
> - return '\0' - (unsigned char)ent->name[key->len];
> + return ref_entry_ncmp(key_, ent_, strncmp);
> +}
> +
> +static int ref_entry_casecmp_sslice(const void *key_, const void *ent_)
> +{
> +    return ref_entry_ncmp(key_, ent_, strncasecmp);
>  }
> 
>  /*
> @@ -378,6 +388,7 @@ static int search_ref_dir(struct ref_dir *dir,
> const char *refname, size_t len)
>  {
>   struct ref_entry **r;
>   struct string_slice key;
> +    int (*cmp_fn)(const void *, const void *);
> 
>   if (refname == NULL || !dir->nr)
>   return -1;
> @@ -385,8 +396,14 @@ static int search_ref_dir(struct ref_dir *dir,
> const char *refname, size_t len)
>   sort_ref_dir(dir);
>   key.len = len;
>   key.str = refname;
> +
> +    if(ignore_case)
Only looking at ignore_case here closes the door for people
who have a branch "foo" and "Foo" at the same time.
(Which means that they are carefully running git pack-refs)
How about something like this:
 +    if (refs_ignore_case < 0)
 +      refs_ignore_case = ignore_case;
 +    if (refs_ignore_case)
(And then we need the diff further down on top of this.)
(And of course Documentation/config.txt)
The main motivation is that you can set refs.ignorecase == true on
e.g. Linux, to prevent to have branches "Foo" and "foo" at the same time,
which gives problems when pulling into e.g. Windows/Mac OS
> +        cmp_fn = ref_entry_casecmp_sslice;
> +    else
> +        cmp_fn = ref_entry_cmp_sslice;
> +
>   r = bsearch(&key, dir->entries, dir->nr, sizeof(*dir->entries),
> -    ref_entry_cmp_sslice);
> +    cmp_fn);
> 
>   if (r == NULL)
>   return -1;
> --




diff --git a/builtin/init-db.c b/builtin/init-db.c
index c7c76bb..dbfc61f 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -288,8 +288,10 @@ static int create_default_files(const char *template_path)
                /* Check if the filesystem is case-insensitive */
                path[len] = 0;
                strcpy(path + len, "CoNfIg");
-               if (!access(path, F_OK))
-                       git_config_set("core.ignorecase", "true");
+               if (!access(path, F_OK)) {
+                       git_config_set("core.ignorecase", "true");
+                       git_config_set("refs.ignorecase", "true");
+               }
                probe_utf8_pathname_composition(path, len);
        }
 
diff --git a/config.c b/config.c
index d969a5a..8f1ec81 100644
--- a/config.c
+++ b/config.c
@@ -698,6 +698,11 @@ static int git_default_core_config(const char *var, const char *value)
                return 0;
        }
 
+       if (!strcmp(var, "refs.ignorecase")) {
+               refs_ignore_case = git_config_bool(var, value);
+               return 0;
+       }
+
        if (!strcmp(var, "core.attributesfile"))
                return git_config_pathname(&git_attributes_file, var, value);
 
diff --git a/environment.c b/environment.c
index 4a3437d..2eced48 100644
--- a/environment.c
+++ b/environment.c
@@ -18,6 +18,7 @@ int check_stat = 1;
 int has_symlinks = 1;
 int minimum_abbrev = 4, default_abbrev = 7;
 int ignore_case;
+int refs_ignore_case = -1;
 int assume_unchanged;
 int prefer_symlink_refs;
 int is_bare_repository_cfg = -1; /* unspecified */

Re: Branch Name Case Sensitivity

[Lee Hopkins]

Incorporating Torsten suggestions and some documentation:

---
 Documentation/config.txt |   12 ++++++++++++
 builtin/init-db.c        |    4 +++-
 config.c                 |    5 +++++
 environment.c            |    1 +
 refs.c                   |   26 +++++++++++++++++++++++---
 5 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 040197b..c0a6c5c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2077,6 +2077,18 @@ receive.shallowupdate::
  If set to true, .git/shallow can be updated when new refs
  require new shallow roots. Otherwise those refs are rejected.

+refs.ignorecase::
+ If true, this option prevents the creation of ref names
+ that differ in case only. For example, if a branch Foo exists,
+ `git checkout -b foo` would fail. This is the case
+ across ref hierarchies, so `git tag foo` would also fail.
+ This option is useful on filesystems that are not case
+ sensitive.
++
+The default is false, except linkgit:git-clone[1] or linkgit:git-init[1]
+will probe and set refs.ignorecase true if appropriate when the repository
+is created. refs.ignorecase will also be true if core.ignorecase is true.
+
 remote.pushdefault::
  The remote to push to by default.  Overrides
  `branch.<name>.remote` for all branches, and is overridden by
diff --git a/builtin/init-db.c b/builtin/init-db.c
index c7c76bb..7c6931b 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -288,8 +288,10 @@ static int create_default_files(const char *template_path)
  /* Check if the filesystem is case-insensitive */
  path[len] = 0;
  strcpy(path + len, "CoNfIg");
- if (!access(path, F_OK))
+ if (!access(path, F_OK)) {
  git_config_set("core.ignorecase", "true");
+ git_config_set("refs.ignorecase", "true");
+ }
  probe_utf8_pathname_composition(path, len);
  }

diff --git a/config.c b/config.c
index 314d8ee..797391a 100644
--- a/config.c
+++ b/config.c
@@ -702,6 +702,11 @@ static int git_default_core_config(const char
*var, const char *value)
  return 0;
  }

+ if (!strcmp(var, "refs.ignorecase")) {
+ refs_ignore_case = git_config_bool(var, value);
+ return 0;
+ }
+
  if (!strcmp(var, "core.attributesfile"))
  return git_config_pathname(&git_attributes_file, var, value);

diff --git a/environment.c b/environment.c
index 4a3437d..2eced48 100644
--- a/environment.c
+++ b/environment.c
@@ -18,6 +18,7 @@ int check_stat = 1;
 int has_symlinks = 1;
 int minimum_abbrev = 4, default_abbrev = 7;
 int ignore_case;
+int refs_ignore_case = -1;
 int assume_unchanged;
 int prefer_symlink_refs;
 int is_bare_repository_cfg = -1; /* unspecified */
diff --git a/refs.c b/refs.c
index 89228e2..1915ec2 100644
--- a/refs.c
+++ b/refs.c
@@ -359,16 +359,26 @@ struct string_slice {
  const char *str;
 };

-static int ref_entry_cmp_sslice(const void *key_, const void *ent_)
+static int ref_entry_ncmp(const void *key_, const void *ent_, int
(*cmp_fn)(const char *, const char *, size_t))
 {
  const struct string_slice *key = key_;
  const struct ref_entry *ent = *(const struct ref_entry * const *)ent_;
- int cmp = strncmp(key->str, ent->name, key->len);
+ int cmp = cmp_fn(key->str, ent->name, key->len);
  if (cmp)
  return cmp;
  return '\0' - (unsigned char)ent->name[key->len];
 }

+static int ref_entry_cmp_sslice(const void *key_, const void *ent_)
+{
+ return ref_entry_ncmp(key_, ent_, strncmp);
+}
+
+static int ref_entry_casecmp_sslice(const void *key_, const void *ent_)
+{
+ return ref_entry_ncmp(key_, ent_, strncasecmp);
+}
+
 /*
  * Return the index of the entry with the given refname from the
  * ref_dir (non-recursively), sorting dir if necessary.  Return -1 if
@@ -378,6 +388,7 @@ static int search_ref_dir(struct ref_dir *dir,
const char *refname, size_t len)
 {
  struct ref_entry **r;
  struct string_slice key;
+ int (*cmp_fn)(const void *, const void *);

  if (refname == NULL || !dir->nr)
  return -1;
@@ -385,8 +396,17 @@ static int search_ref_dir(struct ref_dir *dir,
const char *refname, size_t len)
  sort_ref_dir(dir);
  key.len = len;
  key.str = refname;
+
+ if(refs_ignore_case < 0)
+ refs_ignore_case  = ignore_case;
+
+ if(ignore_case)
+ cmp_fn = ref_entry_casecmp_sslice;
+ else
+ cmp_fn = ref_entry_cmp_sslice;
+
  r = bsearch(&key, dir->entries, dir->nr, sizeof(*dir->entries),
-    ref_entry_cmp_sslice);
+ cmp_fn);

  if (r == NULL)
  return -1;
--

Re: Branch Name Case Sensitivity

[Karsten Blees]

Am 01.03.2014 07:54, schrieb Torsten Bögershausen:
> On 2014-03-01 03.42, Lee Hopkins wrote:
>> +
>> +    if(ignore_case)
> Only looking at ignore_case here closes the door for people
> who have a branch "foo" and "Foo" at the same time.
> (Which means that they are carefully running git pack-refs)
> How about something like this:
>  +    if (refs_ignore_case < 0)
>  +      refs_ignore_case = ignore_case;
>  +    if (refs_ignore_case)

I don't think this distinction is necessary, either you have a case-insensitive file system or you don't. The case that the .git directory is case-sensitive and the worktree directory isn't (or the other way around) is probably so exotic that we can ignore it.

> (And then we need the diff further down on top of this.)
> (And of course Documentation/config.txt)
> The main motivation is that you can set refs.ignorecase == true on
> e.g. Linux, to prevent to have branches "Foo" and "foo" at the same time,
> which gives problems when pulling into e.g. Windows/Mac OS

If you want to prevent problems with Windows/Mac OS, you should set core.ignorecase = true. I don't see why we need yet another config setting for refs (and logs?).

Re: Branch Name Case Sensitivity

[Lee Hopkins]

> I don't think this distinction is necessary, either you have a case-insensitive file system or you don't. The case
> that the .git directory is case-sensitive and the worktree directory isn't (or the other way around) is
> probably so exotic that we can ignore it.

I think Torsten's use case was for someone who is carefully curating
their loose and packed-refs, e.g. gc.packrefs = false. This could be
for backwards compatibility (existing ambiguous refs whose names
cannot be changed for some reason) or simply because they want to.

> If you want to prevent problems with Windows/Mac OS, you should set core.ignorecase = true. I don't see why we need
> yet another config setting for refs (and logs?).

Since refs.ignorecase falls back to core.ignorecase, you could just
set core.ignorecase = true and feel safe when sharing with Windows/Mac
OS. I think having the distinction just makes Git more flexible, OTOH
I can see how having both refs.ignorecase and core.ignorecase could be
confusing and possibly redundant.

Re: Branch Name Case Sensitivity

[Junio C Hamano]

Lee Hopkins <leerhop@gmail.com> writes:

> I went ahead and took a stab at a solution. My solution is more
> aggressive than a warning, I actually prevent the creation of
> ambiguous refs. My changes are also in refs.c, which may not be
> appropriate, but it seemed like the natural place.
>
> I have never contributed to Git (in fact this is my first dive into
> the source) and my C is a bit rusty, so bear with me, this is just a
> suggestion:
>
> ---
>  refs.c |   31 ++++++++++++++++++++++++-------
>  1 files changed, 24 insertions(+), 7 deletions(-)

Starting something like this from forbidding is likely to turn out
to be a very bad idea that can break existing repositories.

A new configuration

	refs.caseInsensitive = {warn|error|allow}

that defaults to "warn" and the user can choose to set to "error" to
forbid, would be more palatable, I would say.

If the variable is not in 'core.' namespace, you should implement
this check at the Porcelain level, allowing lower-level tools like
update-ref as an escape hatch that let users bypass the restriction
to be used to correct breakages; it would mean an unconditional "if
!stricmp(), it is an error" in refs.c will not work well.

I think it might be OK to have

	core.allowCaseInsentitiveRefs = {yes|no|warn}

which defaults to 'warn' (and 'yes' corresponds to 'allow', 'no'
corresponds to 'error', in the previous suggestion), instead. If we
wanted to prevent even lower-level tools like update-ref from
bypassing the check, that is.

Re: Branch Name Case Sensitivity

[Karsten Blees]

Am 03.03.2014 18:51, schrieb Junio C Hamano:
> Lee Hopkins <leerhop@gmail.com> writes:
> 
>> I went ahead and took a stab at a solution. My solution is more
>> aggressive than a warning, I actually prevent the creation of
>> ambiguous refs. My changes are also in refs.c, which may not be
>> appropriate, but it seemed like the natural place.
>>
>> I have never contributed to Git (in fact this is my first dive into
>> the source) and my C is a bit rusty, so bear with me, this is just a
>> suggestion:
>>
>> ---
>>  refs.c |   31 ++++++++++++++++++++++++-------
>>  1 files changed, 24 insertions(+), 7 deletions(-)
> 
> Starting something like this from forbidding is likely to turn out
> to be a very bad idea that can break existing repositories.
> 

Its sure worth considering what should be done with pre-existing duplicates. However, repositories with such refs are already broken on case-insensitive filesystems, and allowing something that's known to be broken is even more dangerous, IMO.

An alternative approach could be to encode upper-case letters in loose refs if core.ignorecase == true (e.g. "Foo" -> "%46oo"). Although this may pose a problem for commands that bypass the refs API / plumbing for whatever reason.

> A new configuration
> 
> 	refs.caseInsensitive = {warn|error|allow}
> 

s/caseInsensitive/caseSensitive/
Its case-sensitive refs that cause trouble, case-insensitive refs would be fine on all platforms.

I still don't see why we need an extra setting for this. The problems are inherently caused by case-insensitive filesystems, and we already have 'core.ignorecase' for that (its even automatically configured). Having an extra setting for refs is somewhat like making 'core.ignorecase' configurable per sub-directory.

> that defaults to "warn" and the user can choose to set to "error" to
> forbid, would be more palatable, I would say.
> 
> If the variable is not in 'core.' namespace, you should implement
> this check at the Porcelain level, allowing lower-level tools like
> update-ref as an escape hatch that let users bypass the restriction
> to be used to correct breakages; it would mean an unconditional "if
> !stricmp(), it is an error" in refs.c will not work well.
> 
> I think it might be OK to have
> 
> 	core.allowCaseInsentitiveRefs = {yes|no|warn}
> 
> which defaults to 'warn' (and 'yes' corresponds to 'allow', 'no'
> corresponds to 'error', in the previous suggestion), instead. If we
> wanted to prevent even lower-level tools like update-ref from
> bypassing the check, that is.
> 

Its the plumbing that's broken, so implementing checks at the porcelain level won't help much. In particular, git-update-ref currently drops branches (or resets them to an earlier state) and messes up reflogs.

Re: Branch Name Case Sensitivity

[Torsten Bögershausen]

On 2014-03-04 14.23, Karsten Blees wrote:
> Am 03.03.2014 18:51, schrieb Junio C Hamano:
>> Lee Hopkins <leerhop@gmail.com> writes:
>>
>>> I went ahead and took a stab at a solution. My solution is more
>>> aggressive than a warning, I actually prevent the creation of
>>> ambiguous refs. My changes are also in refs.c, which may not be
>>> appropriate, but it seemed like the natural place.
>>>
>>> I have never contributed to Git (in fact this is my first dive into
>>> the source) and my C is a bit rusty, so bear with me, this is just a
>>> suggestion:
>>>
>>> ---
>>>  refs.c |   31 ++++++++++++++++++++++++-------
>>>  1 files changed, 24 insertions(+), 7 deletions(-)
>>
>> Starting something like this from forbidding is likely to turn out
>> to be a very bad idea that can break existing repositories.
>>
> 
> Its sure worth considering what should be done with pre-existing duplicates. However, repositories with such refs are already broken on case-insensitive filesystems, and allowing something that's known to be broken is even more dangerous, IMO.
> 
> An alternative approach could be to encode upper-case letters in loose refs if core.ignorecase == true (e.g. "Foo" -> "%46oo"). Although this may pose a problem for commands that bypass the refs API / plumbing for whatever reason.
> 
>> A new configuration
>>
>> 	refs.caseInsensitive = {warn|error|allow}
>>
> 
> s/caseInsensitive/caseSensitive/
> Its case-sensitive refs that cause trouble, case-insensitive refs would be fine on all platforms.
> 
> I still don't see why we need an extra setting for this. The problems are inherently caused by case-insensitive filesystems, and we already have 'core.ignorecase' for that (its even automatically configured). Having an extra setting for refs is somewhat like making 'core.ignorecase' configurable per sub-directory.
I start to agree here.
The case-insensitive file system does not allow branches foo and Foo at the same time,
and the packed refs should simply follow this convention/restriction/behaviour.

(and everything else could and should go into another patch:
 If we ever want Linux to ignore the case in refs,
 to ease the cross-platform development with Windows.
 Or if we allow Windows/Mac OS to handle case insensitive refs (by always packing them)
 to ease the co-working with e.g. Linux.
)

Lee, could you improve your change in refs.c into a real patch, with a commit message?
(And please have a look at the indentation with TABs)

A test case could be good, if time allows I can make a suggestion.

Thanks for all comments
/Torsten
 

Re: Branch Name Case Sensitivity

[Lee Hopkins]

> Lee, could you improve your change in refs.c into a real patch, with a commit message?
> (And please have a look at the indentation with TABs)
>
> A test case could be good, if time allows I can make a suggestion.

I will remove the refs.ignorecase flag and work on a test care or two,
it will have to wait a few days tho.

> (and everything else could and should go into another patch:
>  If we ever want Linux to ignore the case in refs,
>  to ease the cross-platform development with Windows.
>  Or if we allow Windows/Mac OS to handle case insensitive refs (by always packing them)
>  to ease the co-working with e.g. Linux.
> )

I was actually planning on tying to add this to my changes if they
gained any traction. Why is another patch desirable?

> If the variable is not in 'core.' namespace, you should implement
> this check at the Porcelain level, allowing lower-level tools like
> update-ref as an escape hatch that let users bypass the restriction
> to be used to correct breakages; it would mean an unconditional "if
> !stricmp(), it is an error" in refs.c will not work well.
>
> I think it might be OK to have
>
>         core.allowCaseInsentitiveRefs = {yes|no|warn}
>
> which defaults to 'warn' (and 'yes' corresponds to 'allow', 'no'
> corresponds to 'error', in the previous suggestion), instead. If we
> wanted to prevent even lower-level tools like update-ref from
> bypassing the check, that is.

I also would not mind working on either of Junio's suggestions if one
is more desirable than what I already have.

-Lee