[PATCH 0/6] Extensions of core.ignorecase=true support

The following patch series extends the core.ignorecase=true support to
handle case insensitive comparisons for the .gitignore file, git status,
and git ls-files.  git add and git fast-import will fold the case of the
file being added, matching that of an already added directory entry.  Case
folding is also applied to git fast-import for renames, copies, and deletes.

The most notable benefit, IMO, is that the case of directories in the
worktree does not matter if, and only if, the directory exists already in
the index with some different case variant.  This helps applications on
Windows that change the case even of directories in unpredictable ways.
Joshua mentioned Perforce as the primary example.

Concerning the implementation, Joshua explained when he initially submitted
the series to the msysgit mailing list:

 git status and add both use an update made to name-hash.c where 
 directories, specifically names with a trailing slash, can be looked up 
 in a case insensitive manner. After trying a myriad of solutions, this 
 seemed to be the cleanest. Does anyone see a problem with embedding the 
 directory names in the same hash as the file names? I couldn't find one, 
 especially since I append a slash to each directory name.

 The git add path case folding functionality is a somewhat radical 
 departure from what Git does now. It is described in detail in patch 5. 
 Does anyone have any concerns?

I support the idea of this patch, and I can confirm that it works: I've
used this series in production both with core.ignorecase set to true and
to false, and in the former case, with directories and files with case
different from the index.

Joshua Jensen (6):
  Add string comparison functions that respect the ignore_case
    variable.
  Case insensitivity support for .gitignore via core.ignorecase
  Add case insensitivity support for directories when using git status
  Add case insensitivity support when using git ls-files
  Support case folding for git add when core.ignorecase=true
  Support case folding in git fast-import when core.ignorecase=true

 dir.c         |  105 ++++++++++++++++++++++++++++++++++++++++++++++----------
 dir.h         |    4 ++
 fast-import.c |    7 ++--
 name-hash.c   |   72 ++++++++++++++++++++++++++++++++++++++-
 read-cache.c  |   23 ++++++++++++
 5 files changed, 188 insertions(+), 23 deletions(-)

From: Joshua Jensen <redacted>

Multiple locations within this patch series alter a case sensitive
string comparison call such as strcmp() to be a call to a string
comparison call that selects case comparison based on the global
ignore_case variable. Behaviorally, when core.ignorecase=false, the
*_icase() versions are functionally equivalent to their C runtime
counterparts.  When core.ignorecase=true, the *_icase() versions perform
a case insensitive comparison.

Like Linus' earlier ignorecase patch, these may ignore filename
conventions on certain file systems. By isolating filename comparisons
to certain functions, support for those filename conventions may be more
easily met.

Signed-off-by: Joshua Jensen <redacted>
Signed-off-by: Johannes Sixt <redacted>
---
 dir.c |   16 ++++++++++++++++
 dir.h |    4 ++++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/dir.c b/dir.c
index 133f472..4d001fd 100644
--- a/dir.c
+++ b/dir.c

@@ -18,6 +18,22 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, in
 	int check_only, const struct path_simplify *simplify);
 static int get_dtype(struct dirent *de, const char *path, int len);
 
+/* helper string functions with support for the ignore_case flag */
+int strcmp_icase(const char *a, const char *b)
+{
+	return ignore_case ? strcasecmp(a, b) : strcmp(a, b);
+}
+
+int strncmp_icase(const char *a, const char *b, size_t count)
+{
+	return ignore_case ? strncasecmp(a, b, count) : strncmp(a, b, count);
+}
+
+int fnmatch_icase(const char *pattern, const char *string, int flags)
+{
+	return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));
+}
+
 static int common_prefix(const char **pathspec)
 {
 	const char *path, *slash, *next;

diff --git a/dir.h b/dir.h
index 278d84c..b3e2104 100644
--- a/dir.h
+++ b/dir.h

@@ -101,4 +101,8 @@ extern int remove_dir_recursively(struct strbuf *path, int flag);
 /* tries to remove the path with empty directories along it, ignores ENOENT */
 extern int remove_path(const char *path);
 
+extern int strcmp_icase(const char *a, const char *b);
+extern int strncmp_icase(const char *a, const char *b, size_t count);
+extern int fnmatch_icase(const char *pattern, const char *string, int flags);
+
 #endif

-- 
1.7.1.402.gf1eeb

From: Joshua Jensen <redacted>

This is especially beneficial when using Windows and Perforce and the
git-p4 bridge. Internally, Perforce preserves a given file's full path
including its case at the time it was added to the Perforce repository.
When syncing a file down via Perforce, missing directories are created,
if necessary, using the case as stored with the filename. Unfortunately,
two files in the same directory can have differing cases for their
respective paths, such as /diRa/file1.c and /DirA/file2.c. Depending on
sync order, DirA/ may get created instead of diRa/.

It is possible to handle directory names in a case insensitive manner
without this patch, but it is highly inconvenient, requiring each
character to be specified like so: [Bb][Uu][Ii][Ll][Dd]. With this patch, the
gitignore exclusions honor the core.ignorecase=true configuration
setting and make the process less error prone. The above is specified
like so: Build

Signed-off-by: Joshua Jensen <redacted>
Signed-off-by: Johannes Sixt <redacted>
---
 dir.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/dir.c b/dir.c
index 4d001fd..be21c20 100644
--- a/dir.c
+++ b/dir.c

@@ -390,14 +390,14 @@ int excluded_from_list(const char *pathname,
 			if (x->flags & EXC_FLAG_NODIR) {
 				/* match basename */
 				if (x->flags & EXC_FLAG_NOWILDCARD) {
-					if (!strcmp(exclude, basename))
+					if (!strcmp_icase(exclude, basename))
 						return to_exclude;
 				} else if (x->flags & EXC_FLAG_ENDSWITH) {
 					if (x->patternlen - 1 <= pathlen &&
-					    !strcmp(exclude + 1, pathname + pathlen - x->patternlen + 1))
+					    !strcmp_icase(exclude + 1, pathname + pathlen - x->patternlen + 1))
 						return to_exclude;
 				} else {
-					if (fnmatch(exclude, basename, 0) == 0)
+					if (fnmatch_icase(exclude, basename, 0) == 0)
 						return to_exclude;
 				}
 			}

@@ -412,14 +412,14 @@ int excluded_from_list(const char *pathname,
 
 				if (pathlen < baselen ||
 				    (baselen && pathname[baselen-1] != '/') ||
-				    strncmp(pathname, x->base, baselen))
+				    strncmp_icase(pathname, x->base, baselen))
 				    continue;
 
 				if (x->flags & EXC_FLAG_NOWILDCARD) {
-					if (!strcmp(exclude, pathname + baselen))
+					if (!strcmp_icase(exclude, pathname + baselen))
 						return to_exclude;
 				} else {
-					if (fnmatch(exclude, pathname+baselen,
+					if (fnmatch_icase(exclude, pathname+baselen,
 						    FNM_PATHNAME) == 0)
 					    return to_exclude;
 				}

-- 
1.7.1.402.gf1eeb

From: Joshua Jensen <redacted>

When core.ignorecase=true, imported file paths will be folded to match
existing directory case.

Signed-off-by: Joshua Jensen <redacted>
Signed-off-by: Johannes Sixt <redacted>
---
 fast-import.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 1e5d66e..82df00e 100644
--- a/fast-import.c
+++ b/fast-import.c

@@ -156,6 +156,7 @@ Format of STDIN stream:
 #include "csum-file.h"
 #include "quote.h"
 #include "exec_cmd.h"
+#include "dir.h"
 
 #define PACK_ID_BITS 16
 #define MAX_PACK_ID ((1<<PACK_ID_BITS)-1)

@@ -1461,7 +1462,7 @@ static int tree_content_set(
 
 	for (i = 0; i < t->entry_count; i++) {
 		e = t->entries[i];
-		if (e->name->str_len == n && !strncmp(p, e->name->str_dat, n)) {
+		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
 			if (!slash1) {
 				if (!S_ISDIR(mode)
 						&& e->versions[1].mode == mode

@@ -1527,7 +1528,7 @@ static int tree_content_remove(
 
 	for (i = 0; i < t->entry_count; i++) {
 		e = t->entries[i];
-		if (e->name->str_len == n && !strncmp(p, e->name->str_dat, n)) {
+		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
 			if (!slash1 || !S_ISDIR(e->versions[1].mode))
 				goto del_entry;
 			if (!e->tree)

@@ -1577,7 +1578,7 @@ static int tree_content_get(
 
 	for (i = 0; i < t->entry_count; i++) {
 		e = t->entries[i];
-		if (e->name->str_len == n && !strncmp(p, e->name->str_dat, n)) {
+		if (e->name->str_len == n && !strncmp_icase(p, e->name->str_dat, n)) {
 			if (!slash1) {
 				memcpy(leaf, e, sizeof(*leaf));
 				if (e->tree && is_null_sha1(e->versions[1].sha1))

-- 
1.7.1.402.gf1eeb

From: Joshua Jensen <redacted>

When MyDir/ABC/filea.txt is added to Git, the disk directory MyDir/ABC/
is renamed to mydir/aBc/, and then mydir/aBc/fileb.txt is added, the
index will contain MyDir/ABC/filea.txt and mydir/aBc/fileb.txt. Although
the earlier portions of this patch series account for those differences
in case, this patch makes the pathing consistent by folding the case of
newly added files against the first file added with that path.

In read-cache.c's add_to_index(), the index_name_exists() support used
for git status's case insensitive directory lookups is used to find the
proper directory case according to what the user already checked in.
That is, MyDir/ABC/'s case is used to alter the stored path for
fileb.txt to MyDir/ABC/fileb.txt (instead of mydir/aBc/fileb.txt).

This is especially important when cloning a repository to a case
sensitive file system. MyDir/ABC/ and mydir/aBc/ exist in the same
directory on a Windows machine, but on Linux, the files exist in two
separate directories. The update to add_to_index(), in effect, treats a
Windows file system as case sensitive by making path case consistent.

Signed-off-by: Joshua Jensen <redacted>
Signed-off-by: Johannes Sixt <redacted>
---
 read-cache.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index f1f789b..ae1366a 100644
--- a/read-cache.c
+++ b/read-cache.c

@@ -608,6 +608,29 @@ int add_to_index(struct index_state *istate, const char *path, struct stat *st,
 		ce->ce_mode = ce_mode_from_stat(ent, st_mode);
 	}
 
+	/* When core.ignorecase=true, determine if a directory of the same name but differing
+	 * case already exists within the Git repository.  If it does, ensure the directory
+	 * case of the file being added to the repository matches (is folded into) the existing
+	 * entry's directory case.
+	 */
+	if (ignore_case) {
+		const char *startPtr = ce->name;
+		const char *ptr = startPtr;
+		while (*ptr) {
+			while (*ptr && *ptr != '/')
+				++ptr;
+			if (*ptr == '/') {
+				struct cache_entry *foundce;
+				++ptr;
+				foundce = index_name_exists(&the_index, ce->name, ptr - ce->name, ignore_case);
+				if (foundce) {
+					memcpy((void*)startPtr, foundce->name + (startPtr - ce->name), ptr - startPtr);
+					startPtr = ptr;
+				}
+			}
+		}
+	}
+
 	alias = index_name_exists(istate, ce->name, ce_namelen(ce), ignore_case);
 	if (alias && !ce_stage(alias) && !ie_match_stat(istate, alias, st, ce_option)) {
 		/* Nothing changed, really */

-- 
1.7.1.402.gf1eeb

From: Joshua Jensen <redacted>

When mydir/filea.txt is added, mydir/ is renamed to MyDir/, and
MyDir/fileb.txt is added, running git ls-files mydir only shows
mydir/filea.txt. Running git ls-files MyDir shows MyDir/fileb.txt.
Running git ls-files mYdIR shows nothing.

With this patch running git ls-files for mydir, MyDir, and mYdIR shows
mydir/filea.txt and MyDir/fileb.txt.

Wildcards are not handled case insensitively in this patch. Example:
MyDir/aBc/file.txt is added. git ls-files MyDir/a* works fine, but git
ls-files mydir/a* does not.

Signed-off-by: Joshua Jensen <redacted>
Signed-off-by: Johannes Sixt <redacted>
---
 dir.c |   38 ++++++++++++++++++++++++++------------
 1 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/dir.c b/dir.c
index f80e091..af5f54f 100644
--- a/dir.c
+++ b/dir.c

@@ -107,16 +107,30 @@ static int match_one(const char *match, const char *name, int namelen)
 	if (!*match)
 		return MATCHED_RECURSIVELY;
 
-	for (;;) {
-		unsigned char c1 = *match;
-		unsigned char c2 = *name;
-		if (c1 == '\0' || is_glob_special(c1))
-			break;
-		if (c1 != c2)
-			return 0;
-		match++;
-		name++;
-		namelen--;
+	if (ignore_case) {
+		for (;;) {
+			unsigned char c1 = tolower(*match);
+			unsigned char c2 = tolower(*name);
+			if (c1 == '\0' || is_glob_special(c1))
+				break;
+			if (c1 != c2)
+				return 0;
+			match++;
+			name++;
+			namelen--;
+		}
+	} else {
+		for (;;) {
+			unsigned char c1 = *match;
+			unsigned char c2 = *name;
+			if (c1 == '\0' || is_glob_special(c1))
+				break;
+			if (c1 != c2)
+				return 0;
+			match++;
+			name++;
+			namelen--;
+		}
 	}
 
 

@@ -125,8 +139,8 @@ static int match_one(const char *match, const char *name, int namelen)
 	 * we need to match by fnmatch
 	 */
 	matchlen = strlen(match);
-	if (strncmp(match, name, matchlen))
-		return !fnmatch(match, name, 0) ? MATCHED_FNMATCH : 0;
+	if (strncmp_icase(match, name, matchlen))
+		return !fnmatch_icase(match, name, 0) ? MATCHED_FNMATCH : 0;
 
 	if (namelen == matchlen)
 		return MATCHED_EXACTLY;

-- 
1.7.1.402.gf1eeb

From: Joshua Jensen <redacted>

When using a case preserving but case insensitive file system, directory
case can differ but still refer to the same physical directory.  git
status reports the directory with the alternate case as an Untracked
file.  (That is, when mydir/filea.txt is added to the repository and
then the directory on disk is renamed from mydir/ to MyDir/, git status
shows MyDir/ as being untracked.)

Support has been added in name-hash.c for hashing directories with a
terminating slash into the name hash. When index_name_exists() is called
with a directory (a name with a terminating slash), the name is not
found via the normal cache_name_compare() call, but it is found in the
slow_same_name() function.

Additionally, in dir.c, directory_exists_in_index_icase() allows newly
added directories deeper in the directory chain to be identified.

Ultimately, it would be better if the file list was read in case
insensitive alphabetical order from disk, but this change seems to
suffice for now.

The end result is the directory is looked up in a case insensitive
manner and does not show in the Untracked files list.

Signed-off-by: Joshua Jensen <redacted>
Signed-off-by: Johannes Sixt <redacted>
---
 dir.c       |   39 +++++++++++++++++++++++++++++++-
 name-hash.c |   72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/dir.c b/dir.c
index be21c20..f80e091 100644
--- a/dir.c
+++ b/dir.c

@@ -485,6 +485,38 @@ enum exist_status {
 };
 
 /*
+ * Do not use the alphabetically stored index to look up
+ * the directory name; instead, use the case insensitive
+ * name hash.
+ */
+static enum exist_status directory_exists_in_index_icase(const char *dirname, int len)
+{
+	struct cache_entry *ce = index_name_exists(&the_index, dirname, len + 1, ignore_case);
+	if (!ce)
+		return index_nonexistent;
+
+	unsigned char endchar = ce->name[len];
+
+	/*
+	 * The cache_entry structure returned will contain this dirname
+	 * and possibly additional path components.
+	 */
+	if (endchar == '/')
+		return index_directory;
+
+	/*
+	 * If there are no additional path components, then this cache_entry
+	 * represents a submodule.  Submodules, despite being directories,
+	 * are stored in the cache without a closing slash.
+	 */
+	if (!endchar && S_ISGITLINK(ce->ce_mode))
+		return index_gitdir;
+
+	/* This should never be hit, but it exists just in case. */
+	return index_nonexistent;
+}
+
+/*
  * The index sorts alphabetically by entry name, which
  * means that a gitlink sorts as '\0' at the end, while
  * a directory (which is defined not as an entry, but as

@@ -493,7 +525,12 @@ enum exist_status {
  */
 static enum exist_status directory_exists_in_index(const char *dirname, int len)
 {
-	int pos = cache_name_pos(dirname, len);
+	int pos;
+
+	if (ignore_case)
+		return directory_exists_in_index_icase(dirname, len);
+
+	pos = cache_name_pos(dirname, len);
 	if (pos < 0)
 		pos = -pos-1;
 	while (pos < active_nr) {

diff --git a/name-hash.c b/name-hash.c
index 0031d78..c6b6a3f 100644
--- a/name-hash.c
+++ b/name-hash.c

@@ -32,6 +32,42 @@ static unsigned int hash_name(const char *name, int namelen)
 	return hash;
 }
 
+static void hash_index_entry_directories(struct index_state *istate, struct cache_entry *ce)
+{
+	/*
+	 * Throw each directory component in the hash for quick lookup
+	 * during a git status. Directory components are stored with their
+	 * closing slash.  Despite submodules being a directory, they never
+	 * reach this point, because they are stored without a closing slash
+	 * in the cache.
+	 *
+	 * Note that the cache_entry stored with the directory does not
+	 * represent the directory itself.  It is a pointer to an existing
+	 * filename, and its only purpose is to represent existence of the
+	 * directory in the cache.  It is very possible multiple directory
+	 * hash entries may point to the same cache_entry.
+	 */
+	unsigned int hash;
+	void **pos;
+
+	const char *ptr = ce->name;
+	while (*ptr) {
+		while (*ptr && *ptr != '/')
+			++ptr;
+		if (*ptr == '/') {
+			++ptr;
+			hash = hash_name(ce->name, ptr - ce->name);
+			if (!lookup_hash(hash, &istate->name_hash)) {
+				pos = insert_hash(hash, ce, &istate->name_hash);
+				if (pos) {
+					ce->next = *pos;
+					*pos = ce;
+				}
+			}
+		}
+	}
+}
+
 static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)
 {
 	void **pos;

@@ -47,6 +83,9 @@ static void hash_index_entry(struct index_state *istate, struct cache_entry *ce)
 		ce->next = *pos;
 		*pos = ce;
 	}
+
+	if (ignore_case)
+		hash_index_entry_directories(istate, ce);
 }
 
 static void lazy_init_name_hash(struct index_state *istate)

@@ -97,7 +136,21 @@ static int same_name(const struct cache_entry *ce, const char *name, int namelen
 	if (len == namelen && !cache_name_compare(name, namelen, ce->name, len))
 		return 1;
 
-	return icase && slow_same_name(name, namelen, ce->name, len);
+	if (!icase)
+		return 0;
+
+	/*
+	 * If the entry we're comparing is a filename (no trailing slash), then compare
+	 * the lengths exactly.
+	 */
+	if (name[namelen - 1] != '/')
+		return slow_same_name(name, namelen, ce->name, len);
+
+	/*
+	 * For a directory, we point to an arbitrary cache_entry filename.  Just
+	 * make sure the directory portion matches.
+	 */
+	return slow_same_name(name, namelen, ce->name, namelen < len ? namelen : len);
 }
 
 struct cache_entry *index_name_exists(struct index_state *istate, const char *name, int namelen, int icase)

@@ -115,5 +168,22 @@ struct cache_entry *index_name_exists(struct index_state *istate, const char *na
 		}
 		ce = ce->next;
 	}
+
+	/*
+	 * Might be a submodule.  Despite submodules being directories,
+	 * they are stored in the name hash without a closing slash.
+	 * When ignore_case is 1, directories are stored in the name hash
+	 * with their closing slash.
+	 *
+	 * The side effect of this storage technique is we have need to
+	 * remove the slash from name and perform the lookup again without
+	 * the slash.  If a match is made, S_ISGITLINK(ce->mode) will be
+	 * true.
+	 */
+	if (icase && name[namelen - 1] == '/') {
+		ce = index_name_exists(istate, name, namelen - 1, icase);
+		if (ce && S_ISGITLINK(ce->ce_mode))
+			return ce;
+	}
 	return NULL;
 }

-- 
1.7.1.402.gf1eeb

While I tend to agree with case-insensitive searches, I would tend to
question the use of a non-case-preserving / last-use methodology
reminiscent of the days of DOS. I was never terribly fond of DOS nor
of Windows for Workgroups, and this change smacks of that. That said,
as an algorithm it is one legitimate option, I just would not tend to
use it.

Have you thought of other approaches that are case-preserving /
case-insensitive (modern Windows implementations and Mac OS X) or of
case-sensitive (UNIX)? What about giving the user the choice?

Looking back at how flexible and simple folks here on this list made
the EOL support (the best I have seen to date) what if you followed a
similar approach and provided the flexibility to the users by giving
the options to control the behavior themselves, then fall back to
reasonable defaults if left unspecified?

I would imagine having two properties would be somewhat more useful:

core.casepreserving=true|false
core.caseinsensitive=true|false

For Unix-centric environments the defaults would be true & false,
respectively. For mixed Windows/Unix environments users would
configure it as true & true. A combination of false & true,
respectively, would be appropriate for DOS users, but this may be
another option for mixed environments. A combination of false & false
would be impossible by definition.

What this would mean is that searches in the purely Unix world would
store and match sensitively; but in such an environment there may be
issues should any Windows users attempt to work with the repository if
two files having a similar names existed. For Windows users the last
read operation would overwrite the first (not a good thing).

For mixed configurations there are two options, the Windows/Mac
option, and the DOS option.

The nice thing about case-insensitivity is that when requesting a file
by name any capitalization can be used. The bad thing, well come to
think of it, there is none that I can think of, but for those more
religious than I about Unix they may cite otherwise.

-Bob

On Mon, Aug 16, 2010 at 3:38 PM, Johannes Sixt [off-list ref] wrote:

The following patch series extends the core.ignorecase=true support to
handle case insensitive comparisons for the .gitignore file, git status,
and git ls-files.  git add and git fast-import will fold the case of the
file being added, matching that of an already added directory entry.  Case
folding is also applied to git fast-import for renames, copies, and deletes.

The most notable benefit, IMO, is that the case of directories in the
worktree does not matter if, and only if, the directory exists already in
the index with some different case variant.  This helps applications on
Windows that change the case even of directories in unpredictable ways.
Joshua mentioned Perforce as the primary example.

Concerning the implementation, Joshua explained when he initially submitted
the series to the msysgit mailing list:

 git status and add both use an update made to name-hash.c where
 directories, specifically names with a trailing slash, can be looked up
 in a case insensitive manner. After trying a myriad of solutions, this
 seemed to be the cleanest. Does anyone see a problem with embedding the
 directory names in the same hash as the file names? I couldn't find one,
 especially since I append a slash to each directory name.

 The git add path case folding functionality is a somewhat radical
 departure from what Git does now. It is described in detail in patch 5.
 Does anyone have any concerns?

I support the idea of this patch, and I can confirm that it works: I've
used this series in production both with core.ignorecase set to true and
to false, and in the former case, with directories and files with case
different from the index.

Joshua Jensen (6):
 Add string comparison functions that respect the ignore_case
   variable.
 Case insensitivity support for .gitignore via core.ignorecase
 Add case insensitivity support for directories when using git status
 Add case insensitivity support when using git ls-files
 Support case folding for git add when core.ignorecase=true
 Support case folding in git fast-import when core.ignorecase=true

 dir.c         |  105 ++++++++++++++++++++++++++++++++++++++++++++++----------
 dir.h         |    4 ++
 fast-import.c |    7 ++--
 name-hash.c   |   72 ++++++++++++++++++++++++++++++++++++++-
 read-cache.c  |   23 ++++++++++++
 5 files changed, 188 insertions(+), 23 deletions(-)

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

On Dienstag, 17. August 2010, Robert Buck wrote:

While I tend to agree with case-insensitive searches, I would tend to
question the use of a non-case-preserving / last-use methodology
reminiscent of the days of DOS.

There is no "last-use" involved. Everything's rather "first-use", i.e., 
case-preserving.

The nice thing about case-insensitivity is that when requesting a file
by name any capitalization can be used. The bad thing, well come to
think of it, there is none that I can think of, but for those more
religious than I about Unix they may cite otherwise.

What do you mean by "requesting a file"?

core.ignorecase is purely about the worktree and the transition of files from 
the worktree to the index. It is *not* involved when files are moved from the 
index or the repository to the worktree. In particular, it is not used when 
you give a pathspec to limit 'git log' results. (Joshua proposed a change 
where core.ignorecase would also kick in in this case as well, but this 
change is not included in this series, and I would not agree to it.)

-- Hannes

So apparently when core.ignorecase=true, this really means
casepreserving=false, casesensitive=true. Yet when
core.ignorecase=false it actually means casepreserving=true,
casesensitive=true. That's what I infer from the git-config
documentation. Would you agree?

On Tue, Aug 17, 2010 at 5:20 PM, Johannes Sixt [off-list ref] wrote:

On Dienstag, 17. August 2010, Robert Buck wrote:

quoted

While I tend to agree with case-insensitive searches, I would tend to
question the use of a non-case-preserving / last-use methodology
reminiscent of the days of DOS.

There is no "last-use" involved. Everything's rather "first-use", i.e.,
case-preserving.

quoted

The nice thing about case-insensitivity is that when requesting a file
by name any capitalization can be used. The bad thing, well come to
think of it, there is none that I can think of, but for those more
religious than I about Unix they may cite otherwise.

What do you mean by "requesting a file"?

core.ignorecase is purely about the worktree and the transition of files from
the worktree to the index. It is *not* involved when files are moved from the
index or the repository to the worktree. In particular, it is not used when
you give a pathspec to limit 'git log' results. (Joshua proposed a change
where core.ignorecase would also kick in in this case as well, but this
change is not included in this series, and I would not agree to it.)

So what I am hearing is that unless one sets core.ignorecase, in mixed
environments you are in for a world of hurt; you'd end up with Foo and
foo from the Unix side of the house, and on Macs or Windows the last
file materialized from the index or repository into the working
directory would clobber the first one materialized, potentially
introducing relatively quiescent bugs that could make their way into
production environments.

On Mon, Aug 16, 2010 at 19:38, Johannes Sixt [off-list ref] wrote:

+       return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));

On Solaris 10:

dir.c: In function `fnmatch_icase':
dir.c:34: error: `FNM_CASEFOLD' undeclared (first use in this function)
dir.c:34: error: (Each undeclared identifier is reported only once
dir.c:34: error: for each function it appears in.)

On Wed, Aug 18, 2010 at 12:52, Ævar Arnfjörð Bjarmason [off-list ref] wrote:

On Mon, Aug 16, 2010 at 19:38, Johannes Sixt [off-list ref] wrote:

quoted

+       return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));

On Solaris 10:

dir.c: In function `fnmatch_icase':
dir.c:34: error: `FNM_CASEFOLD' undeclared (first use in this function)
dir.c:34: error: (Each undeclared identifier is reported only once
dir.c:34: error: for each function it appears in.)

Actually, reading the fnmatch manpage it's not just Solaris, but all
non-GNU systems:

       FNM_CASEFOLD
              If this flag (a GNU extension) is set, the pattern is
matched case-insensitively.

  ----- Original Message -----
From: Ævar Arnfjörð Bjarmason
Date: 8/18/2010 6:53 AM

On Wed, Aug 18, 2010 at 12:52, Ævar Arnfjörð Bjarmason[off-list ref]  wrote:

quoted

On Mon, Aug 16, 2010 at 19:38, Johannes Sixt[off-list ref]  wrote:

quoted

+       return fnmatch(pattern, string, flags | (ignore_case ? FNM_CASEFOLD : 0));

On Solaris 10:

dir.c: In function `fnmatch_icase':
dir.c:34: error: `FNM_CASEFOLD' undeclared (first use in this function)
dir.c:34: error: (Each undeclared identifier is reported only once
dir.c:34: error: for each function it appears in.)

Actually, reading the fnmatch manpage it's not just Solaris, but all
non-GNU systems:

        FNM_CASEFOLD - If this flag (a GNU extension) is set, the pattern is matched case-insensitively

Well, that's no good.  :(

Thanks for the research.  It helps tremendously.

One easy way out of this situation would be to duplicate the GNU 
fnmatch() into fnmatch_icase().  I have not looked at the source code, 
so it may not be possible.  If it can be copied in, does anyone object?

I'll also look for a non-GNU function that may work.

Josh

On Wed, Aug 18, 2010 at 15:52, Joshua Jensen [off-list ref] wrote:

 ----- Original Message -----
From: Ævar Arnfjörð Bjarmason
Date: 8/18/2010 6:53 AM

quoted

On Wed, Aug 18, 2010 at 12:52, Ævar Arnfjörð Bjarmason[off-list ref]
 wrote:

quoted

On Mon, Aug 16, 2010 at 19:38, Johannes Sixt[off-list ref]  wrote:

quoted

+       return fnmatch(pattern, string, flags | (ignore_case ?
FNM_CASEFOLD : 0));

On Solaris 10:

dir.c: In function `fnmatch_icase':
dir.c:34: error: `FNM_CASEFOLD' undeclared (first use in this function)
dir.c:34: error: (Each undeclared identifier is reported only once
dir.c:34: error: for each function it appears in.)

Actually, reading the fnmatch manpage it's not just Solaris, but all
non-GNU systems:

       FNM_CASEFOLD - If this flag (a GNU extension) is set, the pattern
is matched case-insensitively

Well, that's no good.  :(

Thanks for the research.  It helps tremendously.

One easy way out of this situation would be to duplicate the GNU fnmatch()
into fnmatch_icase().  I have not looked at the source code, so it may not
be possible.  If it can be copied in, does anyone object?

I'll also look for a non-GNU function that may work.

According to some further research at least FreeBSD and NetBSD have
copied this GNU extension. You may find their versions easier to
integrate.

On Mittwoch, 18. August 2010, Robert Buck wrote:

So apparently when core.ignorecase=true, this really means
casepreserving=false, casesensitive=true.

No. When an entry enters the index from the worktree, an existing entry is 
reused and the index entry's case is preserved, where existence is determined 
in a case-insensitive manner[*]; if the entry is new, the case is preserved.

In the oppsite direction, case preservation depends entirely on the 
capabilities of the file system.

[*] The new part of this series is that this case-insensitive existence test 
happens on for the entire path, and not just the file name part.

So what I am hearing is that unless one sets core.ignorecase, in mixed
environments you are in for a world of hurt; you'd end up with Foo and
foo from the Unix side of the house, and on Macs or Windows the last
file materialized from the index or repository into the working
directory would clobber the first one materialized,

core.ignorecase is not designed to help this case. It is a "Doctor, it hurts 
when I poke myself in the eye" problem. Don't have both foo and Foo in your 
repository if you go cross-platform. The git repository format is not 
designed to treat them as identical, and no configuration option alters this.

-- Hannes

On Mittwoch, 18. August 2010, Ævar Arnfjörð Bjarmason wrote:

According to some further research at least FreeBSD and NetBSD have
copied this GNU extension. You may find their versions easier to
integrate.

We already have a GNU fnmatch in compat/fnmatch.

-- Hannes

On Wed, Aug 18, 2010 at 18:32, Johannes Sixt [off-list ref] wrote:

On Mittwoch, 18. August 2010, Ævar Arnfjörð Bjarmason wrote:

quoted

According to some further research at least FreeBSD and NetBSD have
copied this GNU extension. You may find their versions easier to
integrate.

We already have a GNU fnmatch in compat/fnmatch.

I didn't spot that. But until now it's only been used for Windows,
there isn't a NO_* flag you can set to compile it in.

Johannes Sixt [off-list ref] writes:

I support the idea of this patch, and I can confirm that it works: I've
used this series in production both with core.ignorecase set to true and
to false, and in the former case, with directories and files with case
different from the index.

Thanks

On Wed, Aug 18, 2010 at 18:32, Johannes Sixt [off-list ref] wrote:

On Mittwoch, 18. August 2010, Ævar Arnfjörð Bjarmason wrote:

quoted

According to some further research at least FreeBSD and NetBSD have
copied this GNU extension. You may find their versions easier to
integrate.

We already have a GNU fnmatch in compat/fnmatch.

Do you have any plan to deal with this? I currently have this
monkeypatch to build on Solaris:

    diff --git a/Makefile b/Makefile
    index 62d526a..079fae5 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -863,2 +863,4 @@ endif
     ifeq ($(uname_S),SunOS)
    +       COMPAT_OBJS = compat/fnmatch/fnmatch.o
    +       COMPAT_CFLAGS = -Icompat -Icompat/fnmatch
            NEEDS_SOCKET = YesPlease

One way to deal with it would be a new NONGNU_FNMATCH=UnfortunatelyYes
flag, or the fnmatch_icase() suggestion above which we could bundle
and always use. But having next build on systems without GNU
extensions would be preferrable.

  ----- Original Message -----
From: Ævar Arnfjörð Bjarmason
Date: 8/29/2010 1:39 PM

On Wed, Aug 18, 2010 at 18:32, Johannes Sixt[off-list ref]  wrote:

quoted

On Mittwoch, 18. August 2010, Ævar Arnfjörð Bjarmason wrote:

quoted

According to some further research at least FreeBSD and NetBSD have
copied this GNU extension. You may find their versions easier to
integrate.

We already have a GNU fnmatch in compat/fnmatch.

Do you have any plan to deal with this? I currently have this
monkeypatch to build on Solaris:

     diff --git a/Makefile b/Makefile
     index 62d526a..079fae5 100644
     --- a/Makefile
     +++ b/Makefile
     @@ -863,2 +863,4 @@ endif
      ifeq ($(uname_S),SunOS)
     +       COMPAT_OBJS = compat/fnmatch/fnmatch.o
     +       COMPAT_CFLAGS = -Icompat -Icompat/fnmatch
             NEEDS_SOCKET = YesPlease

One way to deal with it would be a new NONGNU_FNMATCH=UnfortunatelyYes
flag, or the fnmatch_icase() suggestion above which we could bundle
and always use. But having next build on systems without GNU
extensions would be preferrable.

I am going to deal with this, but I haven't been around.  I hope for 
some time this week.

Short of duplicating fnmatch's code and renaming the function, I am not 
sure how to make this play nice on all systems.  You added COMPAT_OBJS 
above, but I think there is no linker guarantee it will pick up 
compat/fnmatch/fnmatch.o over the C runtime version?  Perhaps the 
makefile is architected to do so.

The safest alternative is to allocate character buffers, lowercase the 
filename and match arguments into those buffers, and pass them off to 
fnmatch without any special flags.  I don't like the idea of a double 
memory allocation/free combo per each call to this function, but it 
would work.  Is anyone opposed to this approach?

Josh

On Mon, Aug 30, 2010 at 14:42, Joshua Jensen [off-list ref] wrote:

 ----- Original Message -----
From: Ævar Arnfjörð Bjarmason
Date: 8/29/2010 1:39 PM

quoted

On Wed, Aug 18, 2010 at 18:32, Johannes Sixt[off-list ref]  wrote:

quoted

On Mittwoch, 18. August 2010, Ævar Arnfjörð Bjarmason wrote:

quoted

According to some further research at least FreeBSD and NetBSD have
copied this GNU extension. You may find their versions easier to
integrate.

We already have a GNU fnmatch in compat/fnmatch.

Do you have any plan to deal with this? I currently have this
monkeypatch to build on Solaris:

    diff --git a/Makefile b/Makefile
    index 62d526a..079fae5 100644
    --- a/Makefile
    +++ b/Makefile
    @@ -863,2 +863,4 @@ endif
     ifeq ($(uname_S),SunOS)
    +       COMPAT_OBJS = compat/fnmatch/fnmatch.o
    +       COMPAT_CFLAGS = -Icompat -Icompat/fnmatch
            NEEDS_SOCKET = YesPlease

One way to deal with it would be a new NONGNU_FNMATCH=UnfortunatelyYes
flag, or the fnmatch_icase() suggestion above which we could bundle
and always use. But having next build on systems without GNU
extensions would be preferrable.

I am going to deal with this, but I haven't been around.  I hope for some
time this week.

Sure, no rush. I was just wondering whether you had some plan for it,
or whether I should submit a patch to use the fallback on
e.g. Solaris.

Short of duplicating fnmatch's code and renaming the function, I am not sure
how to make this play nice on all systems.

I don't think duplicating the GNU (or *BSD) version into a
compat/fnmatch.c would be a bad thing. See e.g. compat/snprintf.c.

You added COMPAT_OBJS above, but I think there is no linker
guarantee it will pick up compat/fnmatch/fnmatch.o over the C
runtime version?  Perhaps the makefile is architected to do so.

It's probably just an artifact of how the Solaris linker works, it
doesn't go through the trouble of undefining an existing fnmatch
symbol or anything.

The safest alternative is to allocate character buffers, lowercase the
filename and match arguments into those buffers, and pass them off to
fnmatch without any special flags.  I don't like the idea of a double memory
allocation/free combo per each call to this function, but it would work.  Is
anyone opposed to this approach?

Just using the GNU extension and providing a fallback is probably
cleaner and easier to maintain.

Joshua Jensen wrote:

I don't like the idea of
a double memory allocation/free combo per each call to this
function, but it would work.

Maybe the caller can provide a (strbuf) buffer?

Ævar Arnfjörð Bjarmason wrote:

I don't think duplicating the GNU (or *BSD) version into a
compat/fnmatch.c would be a bad thing. See e.g. compat/snprintf.c.

Maybe meanwhile someone (I guess that means "I") can ask around about
FNM_CASEFOLD going into posix.  I like to imagine the compat/
directory shrinking over time.

quoted

You added COMPAT_OBJS above, but I think there is no linker
guarantee it will pick up compat/fnmatch/fnmatch.o over the C
runtime version?  Perhaps the makefile is architected to do so.

It's probably just an artifact of how the Solaris linker works

Isn't the portable way to do something like

 #define fnmatch gitfnmatch

in fnmatch.h?

Meanwhile I guess that no other system header includes fnmatch.h, but
if that were to happen, there would be preprocessor symbol conflicts.
So on systems with fnmatch, ideally one would want to do something
like this:

 #include <fnmatch.h>
 #ifdef FNMATCH_LACKS_CASEFOLD
 # undef FNM_PATHNAME
 # define FNM_PATHNAME	(1 << 0)
 # define FNM_CASEFOLD	(1 << 4)
 # define fnmatch gitfnmatch
 extern int fnmatch(const char *pattern, const char *name, int flags);
 #endif

On Montag, 30. August 2010, Joshua Jensen wrote:

From: Ævar Arnfjörð Bjarmason

quoted

     +       COMPAT_OBJS = compat/fnmatch/fnmatch.o
     +       COMPAT_CFLAGS = -Icompat -Icompat/fnmatch

...

You added COMPAT_OBJS
above, but I think there is no linker guarantee it will pick up
compat/fnmatch/fnmatch.o over the C runtime version?

The way linkers work ensures that git code picks up the compat version when 
defined this way.

-- Hannes

On Mon, Aug 30, 2010 at 18:40, Johannes Sixt [off-list ref] wrote:

On Montag, 30. August 2010, Joshua Jensen wrote:

quoted

From: Ævar Arnfjörð Bjarmason

quoted

     +       COMPAT_OBJS = compat/fnmatch/fnmatch.o
     +       COMPAT_CFLAGS = -Icompat -Icompat/fnmatch

...

quoted

You added COMPAT_OBJS
above, but I think there is no linker guarantee it will pick up
compat/fnmatch/fnmatch.o over the C runtime version?

The way linkers work ensures that git code picks up the compat version when
defined this way.

So why does e.g. compat/snprintf.c need to "#undef snprintf" before
defining it again?

On Montag, 30. August 2010, Ævar Arnfjörð Bjarmason wrote:

So why does e.g. compat/snprintf.c need to "#undef snprintf" before
defining it again?

Because the implementation of the compat snprintf uses the system's snprintf.

There are other cases, like e.g., pread, where it would not be necessary to 
#define pread git_pread.

-- Hannes