Branch Name Case Sensitivity

Post by Lee Hopkins
Last week I ran across a potential bug with branch names on case
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.

Torsten Bögershausen

2014-02-27 20:32:01 UTC

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 ?

Lee Hopkins

2014-02-27 20:37:49 UTC

Post by Junio C Hamano
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 :).

Post by Junio C Hamano
Or we can possibly trigger this function at the the of
"checkout -b" or "fetch" commands ?
Only when core.ignorecase =3D=3D true ?

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

Thanks,
-Lee

Post by Lee Hopkins
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 rece=

nt.

Post by Junio C Hamano
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.

git pack-refs --all --prune
If this can be triggered by a hook, I don't know (I never used a hook=

)

Post by Junio C Hamano
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 =3D=3D true ?

Michael Haggerty

2014-02-27 21:00:43 UTC

Post by Junio C Hamano
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 :).

Post by Junio C Hamano
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
***@alum.mit.edu
http://softwareswirl.blogspot.com/

Karsten Blees

2014-02-27 22:24:29 UTC

nt.

git pack-refs --all --prune
=20

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

Lee Hopkins

2014-02-27 23:38:04 UTC

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

Johannes Sixt

2014-02-28 06:41:52 UTC

Post by Karsten Blees
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.

Post by Karsten Blees
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')?

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

Karsten Blees

2014-02-28 13:56:45 UTC

Post by Johannes Sixt

Post by Karsten Blees
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" :-)

Post by Johannes Sixt

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.

Lee Hopkins

2014-02-28 14:10:56 UTC

Post by Johannes Sixt
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.

Post by Johannes Sixt
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.

Post by Johannes Sixt
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

Junio C Hamano

2014-02-28 18:58:10 UTC

Post by Johannes Sixt
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".

Duy Nguyen

2014-02-28 23:22:09 UTC

Post by Johannes Sixt
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.

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

--
Duy

Junio C Hamano

2014-02-28 23:28:58 UTC

Post by Duy Nguyen

Post by Johannes Sixt
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.

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.

Lee Hopkins

2014-03-01 02:42:12 UTC

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

Torsten Bögershausen

2014-03-01 06:54:46 UTC

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

Post by Lee Hopkins
+ 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 */

Lee Hopkins

2014-03-01 19:38:47 UTC

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

Karsten Blees

2014-03-03 10:03:33 UTC

Post by Lee Hopkins
+
+ 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)
+ if (refs_ignore_case < 0)
+ refs_ignore_case =3D ignore_case;
+ if (refs_ignore_case)

I don't think this distinction is necessary, either you have a case-ins=
ensitive 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 aroun=
d) is probably so exotic that we can ignore it.

Post by Torsten BÃ¶gershausen
(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 =3D=3D true o=

Post by Torsten BÃ¶gershausen
e.g. Linux, to prevent to have branches "Foo" and "foo" at the same t=

ime,

Post by Torsten BÃ¶gershausen
which gives problems when pulling into e.g. Windows/Mac OS

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

Lee Hopkins

2014-03-03 14:21:10 UTC

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.

Junio C Hamano

2014-03-03 17:51:52 UTC

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.

Karsten Blees

2014-03-04 13:23:05 UTC

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.

Post by Junio C Hamano
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.

Post by Junio C Hamano
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.

Torsten Bögershausen

2014-03-04 20:37:38 UTC

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.

Post by Junio C Hamano
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

Lee Hopkins

2014-03-05 14:02:48 UTC

Post by Torsten BÃ¶gershausen
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.

Post by Torsten BÃ¶gershausen
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?

Post by Torsten BÃ¶gershausen
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

Michael Haggerty

2014-02-28 09:13:39 UTC

[...] 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
***@alum.mit.edu
http://softwareswirl.blogspot.com/

Duy Nguyen

2014-02-28 14:31:29 UTC

Post by Michael Haggerty

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

Michael Haggerty

2014-02-28 14:45:25 UTC

Post by Duy Nguyen

Post by Michael Haggerty

[...] Based Michael Haggerty's response, it seems that always
using loose refs would be a better workaround.

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
***@alum.mit.edu
http://softwareswirl.blogspot.com/

Stephen Leake

2014-02-28 09:11:00 UTC

Post by Karsten Blees
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

Michael Haggerty

2014-02-28 09:49:52 UTC