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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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;
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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;
--
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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 tbo...@web.de wrote:
 On 2014-02-27 20.50, Junio C Hamano wrote:
 Lee Hopkins leer...@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 ?







--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html