Re: Branch Name Case Sensitivity
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
Am 03.03.2014 18:51, schrieb Junio C Hamano:
Lee Hopkins leer...@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.
--
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
On 2014-03-04 14.23, Karsten Blees wrote:
Am 03.03.2014 18:51, schrieb Junio C Hamano:
Lee Hopkins leer...@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
--
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
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?). -- 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
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 leer...@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.
--
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
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
Karsten Blees karsten.bl...@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 -- 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
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 mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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
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. -- 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
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
On Fri, Feb 28, 2014 at 4:13 PM, Michael Haggerty mhag...@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 -- 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
On 02/28/2014 03:31 PM, Duy Nguyen wrote: On Fri, Feb 28, 2014 at 4:13 PM, Michael Haggerty mhag...@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 mhag...@alum.mit.edu http://softwareswirl.blogspot.com/ -- 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
Karsten Blees karsten.bl...@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. -- 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
On Sat, Mar 1, 2014 at 1:58 AM, Junio C Hamano gits...@pobox.com wrote: Karsten Blees karsten.bl...@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 -- 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
Duy Nguyen pclo...@gmail.com writes: On Sat, Mar 1, 2014 at 1:58 AM, Junio C Hamano gits...@pobox.com wrote: Karsten Blees karsten.bl...@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. -- 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
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
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 */
--
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 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.
--
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
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
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
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
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
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
Am 27.02.2014 21:32, schrieb Torsten Bögershausen:
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 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')?
--
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
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
Re: Branch Name Case Sensitivity
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 -- 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
