Make abspath() aware of case-insensitive filesystems

It is completely legitimate these days to call git add with absolute paths. Of course, on a case-insensitive file system, users rightfully expect the argument to be handled case-insensitively, too. This patch makes it so.

Git for Windows carried this patch for over one and a half years already, I think it is time to get it into git.git.

Johannes Schindelin (1):
abspath_part_inside_repo: respect core.fileMode

 setup.c        | 6 +++---
 t/t3700-add.sh | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

base-commit: b21ebb671bb7dea8d342225f0d66c41f4e54d5ca
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-104%2Fdscho%2Fcase-insensitive-abspath-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-104/dscho/case-insensitive-abspath-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/104

gitgitgadget

From: Johannes Schindelin <[email protected]>

If the file system is case-insensitive, we really must be careful to ignore differences in case only.

This fixes https://github.com/git-for-windows/git/issues/735

---
 setup.c        | 6 +++---
 t/t3700-add.sh | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

index 1be5037f12..291bfb2128 100644
@@ -39,7 +39,7 @@ static int abspath_part_inside_repo(char *path)
 	off = offset_1st_component(path);
 
 	/* check if work tree is already the prefix */
-	if (wtlen <= len && !strncmp(path, work_tree, wtlen)) {
+	if (wtlen <= len && !fspathncmp(path, work_tree, wtlen)) {
 		if (path[wtlen] == '/') {
 			memmove(path, path + wtlen + 1, len - wtlen);
 			return 0;

@@ -59,7 +59,7 @@ static int abspath_part_inside_repo(char *path)
 		path++;
 		if (*path == '/') {
 			*path = '\0';
-			if (strcmp(real_path(path0), work_tree) == 0) {
+			if (fspathcmp(real_path(path0), work_tree) == 0) {
 				memmove(path0, path + 1, len - (path - path0));
 				return 0;
 			}

@@ -68,7 +68,7 @@ static int abspath_part_inside_repo(char *path)
 	}
 
 	/* check whole path */
-	if (strcmp(real_path(path0), work_tree) == 0) {
+	if (fspathcmp(real_path(path0), work_tree) == 0) {
 		*path0 = '\0';
 		return 0;
 	}

index 37729ba258..8ee4fc70ad 100755
@@ -402,4 +402,11 @@ test_expect_success 'all statuses changed in folder if . is given' '
 	test $(git ls-files --stage | grep ^100755 | wc -l) -eq 0
 '
 
+test_expect_success MINGW 'path is case-insensitive' '
+	path="$(pwd)/BLUB" &&
+	touch "$path" &&
+	downcased="$(echo "$path" | tr A-Z a-z)" &&
+	git add "$downcased"
+'
+
 test_done

gitgitgadget

CASE_INSENSITIVE_FS might be a better prereq

Carlo

Hi Carlo,

You're right, the path that I downcase in the test contains an upper case "BLUB" component.

Will fix,
Johannes

It is completely legitimate these days to call git add with absolute paths. Of course, on a case-insensitive file system, users rightfully expect the argument to be handled case-insensitively, too. This patch makes it so.

Git for Windows carried this patch for over one and a half years already, I think it is time to get it into git.git.

Change since v1:

* Replaced MINGW prerequisite in the test by CASE_INSENSITIVE_FS.

Johannes Schindelin (1):
abspath_part_inside_repo: respect core.fileMode

 setup.c        | 6 +++---
 t/t3700-add.sh | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

base-commit: b21ebb671bb7dea8d342225f0d66c41f4e54d5ca
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-104%2Fdscho%2Fcase-insensitive-abspath-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-104/dscho/case-insensitive-abspath-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/104

Range-diff vs v1:

1: 4fb5de504e = 1: 3eaec10c46 abspath_part_inside_repo: respect core.fileMode

gitgitgadget

From: Johannes Schindelin <[email protected]>

If the file system is case-insensitive, we really must be careful to ignore differences in case only.

This fixes https://github.com/git-for-windows/git/issues/735

---
 setup.c        | 6 +++---
 t/t3700-add.sh | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

index 1be5037f12..291bfb2128 100644
@@ -39,7 +39,7 @@ static int abspath_part_inside_repo(char *path)
 	off = offset_1st_component(path);
 
 	/* check if work tree is already the prefix */
-	if (wtlen <= len && !strncmp(path, work_tree, wtlen)) {
+	if (wtlen <= len && !fspathncmp(path, work_tree, wtlen)) {
 		if (path[wtlen] == '/') {
 			memmove(path, path + wtlen + 1, len - wtlen);
 			return 0;

@@ -59,7 +59,7 @@ static int abspath_part_inside_repo(char *path)
 		path++;
 		if (*path == '/') {
 			*path = '\0';
-			if (strcmp(real_path(path0), work_tree) == 0) {
+			if (fspathcmp(real_path(path0), work_tree) == 0) {
 				memmove(path0, path + 1, len - (path - path0));
 				return 0;
 			}

@@ -68,7 +68,7 @@ static int abspath_part_inside_repo(char *path)
 	}
 
 	/* check whole path */
-	if (strcmp(real_path(path0), work_tree) == 0) {
+	if (fspathcmp(real_path(path0), work_tree) == 0) {
 		*path0 = '\0';
 		return 0;
 	}

index 37729ba258..8ee4fc70ad 100755
@@ -402,4 +402,11 @@ test_expect_success 'all statuses changed in folder if . is given' '
 	test $(git ls-files --stage | grep ^100755 | wc -l) -eq 0
 '
 
+test_expect_success MINGW 'path is case-insensitive' '
+	path="$(pwd)/BLUB" &&
+	touch "$path" &&
+	downcased="$(echo "$path" | tr A-Z a-z)" &&
+	git add "$downcased"
+'
+
 test_done

gitgitgadget

Should it be
s/respect core.fileMode/respect core.ignoreCase/

in the header line ?

Hi Torsten,

Yep. Fixed.
Dscho

It is completely legitimate these days to call git add with absolute paths. Of course, on a case-insensitive file system, users rightfully expect the argument to be handled case-insensitively, too. This patch makes it so.

Git for Windows carried this patch for over one and a half years already, I think it is time to get it into git.git.

Change since v2:

* Replaced MINGW prerequisite in the test by CASE_INSENSITIVE_FS. v1 was sent out without a change by mistake. Sorry.

Johannes Schindelin (1):
abspath_part_inside_repo: respect core.fileMode

 setup.c        | 6 +++---
 t/t3700-add.sh | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

base-commit: b21ebb671bb7dea8d342225f0d66c41f4e54d5ca
Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-104%2Fdscho%2Fcase-insensitive-abspath-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-104/dscho/case-insensitive-abspath-v3
Pull-Request: https://github.com/gitgitgadget/git/pull/104

Range-diff vs v2:

1: 3eaec10c46 ! 1: b935e11d21 abspath_part_inside_repo: respect core.fileMode

     @@ -47,7 +47,7 @@
       	test $(git ls-files --stage | grep ^100755 | wc -l) -eq 0
       '
       
     -+test_expect_success MINGW 'path is case-insensitive' '
     ++test_expect_success CASE_INSENSITIVE_FS 'path is case-insensitive' '
      +	path="$(pwd)/BLUB" &&
      +	touch "$path" &&
      +	downcased="$(echo "$path" | tr A-Z a-z)" &&

gitgitgadget

From: Johannes Schindelin <[email protected]>

If the file system is case-insensitive, we really must be careful to ignore differences in case only.

This fixes https://github.com/git-for-windows/git/issues/735

---
 setup.c        | 6 +++---
 t/t3700-add.sh | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

index 1be5037f12..291bfb2128 100644
@@ -39,7 +39,7 @@ static int abspath_part_inside_repo(char *path)
 	off = offset_1st_component(path);
 
 	/* check if work tree is already the prefix */
-	if (wtlen <= len && !strncmp(path, work_tree, wtlen)) {
+	if (wtlen <= len && !fspathncmp(path, work_tree, wtlen)) {
 		if (path[wtlen] == '/') {
 			memmove(path, path + wtlen + 1, len - wtlen);
 			return 0;

@@ -59,7 +59,7 @@ static int abspath_part_inside_repo(char *path)
 		path++;
 		if (*path == '/') {
 			*path = '\0';
-			if (strcmp(real_path(path0), work_tree) == 0) {
+			if (fspathcmp(real_path(path0), work_tree) == 0) {
 				memmove(path0, path + 1, len - (path - path0));
 				return 0;
 			}

@@ -68,7 +68,7 @@ static int abspath_part_inside_repo(char *path)
 	}
 
 	/* check whole path */
-	if (strcmp(real_path(path0), work_tree) == 0) {
+	if (fspathcmp(real_path(path0), work_tree) == 0) {
 		*path0 = '\0';
 		return 0;
 	}

index 37729ba258..be582a513b 100755
@@ -402,4 +402,11 @@ test_expect_success 'all statuses changed in folder if . is given' '
 	test $(git ls-files --stage | grep ^100755 | wc -l) -eq 0
 '
 
+test_expect_success CASE_INSENSITIVE_FS 'path is case-insensitive' '
+	path="$(pwd)/BLUB" &&
+	touch "$path" &&
+	downcased="$(echo "$path" | tr A-Z a-z)" &&
+	git add "$downcased"
+'
+
 test_done

gitgitgadget

"Johannes Schindelin via GitGitGadget" <[email protected]>

Needless to say, the last part of the above is a mere thetorical question, and I am not questioning the established behaviour or suggesting to "improve" it. On a case insensitive filesystem, we trust what readdir() gave us (but match them with pathspec case insensitively) for a new path that is not in the index. When we update the contents of a path that is already in the index, we preserve the case in the index, even when readdir() reports the same path in different case (iow, we trust the case in the index more than what readdir() gives us)..

What I am wondering in the above is if we should document that in the test, perhaps with a simple

	git ls-files blub >actual &&
	echo BLUB >expect &&
	test_cmp expect actual

or something like that.

Hi Junio,

first of all, my apologies for the incorrect commit message (you missed it, too, though): it is not about respecting core.fileMode, but about core.ignoreCase.

Actually, I would interpret the description in our documentation to mean that Git does not want to trust what readdir() gave us but heed what the user said, i.e. when BLUB is added as git add blub, Git really should think that it is blub, not BLUB (i.e. the exact opposite of what you suggest). From Documentation/config/core.txt's section about core.ignoreCase:

For example, if a directory listing finds "makefile" when Git expects "Makefile", Git will assume it is really the same file, and continue to remember it as "Makefile".

Ciao,
Dscho

It is completely legitimate these days to call git add with absolute paths. Of course, on a case-insensitive file system, users rightfully expect the argument to be handled case-insensitively, too. This patch makes it so.

Git for Windows carried this patch for over one and a half years already, I think it is time to get it into git.git.

Changes since v3:

* Fixed the oneline (it talked about core.fileMode when it really wanted to talk about core.ignoreCase!).

Change since v2:

* Replaced MINGW prerequisite in the test by CASE_INSENSITIVE_FS. v1 was sent out without a change by mistake. Sorry.

Johannes Schindelin (1):
abspath_part_inside_repo: respect core.ignoreCase

 setup.c        | 6 +++---
 t/t3700-add.sh | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

base-commit: 77556354bb7ac50450e3b28999e3576969869068
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-104%2Fdscho%2Fcase-insensitive-abspath-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-104/dscho/case-insensitive-abspath-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/104

Range-diff vs v3:

1: b935e11d21 ! 1: 3fb927fc77 abspath_part_inside_repo: respect core.fileMode
@@ -1,6 +1,6 @@
Author: Johannes Schindelin <[email protected]>

- abspath_part_inside_repo: respect core.fileMode
+ abspath_part_inside_repo: respect core.ignoreCase

If the file system is case-insensitive, we really must be careful to ignore differences in case only.

gitgitgadget

From: Johannes Schindelin <[email protected]>

If the file system is case-insensitive, we really must be careful to ignore differences in case only.

This fixes https://github.com/git-for-windows/git/issues/735

---
 setup.c        | 6 +++---
 t/t3700-add.sh | 7 +++++++
 2 files changed, 10 insertions(+), 3 deletions(-)

index 1be5037f12..291bfb2128 100644
@@ -39,7 +39,7 @@ static int abspath_part_inside_repo(char *path)
 	off = offset_1st_component(path);
 
 	/* check if work tree is already the prefix */
-	if (wtlen <= len && !strncmp(path, work_tree, wtlen)) {
+	if (wtlen <= len && !fspathncmp(path, work_tree, wtlen)) {
 		if (path[wtlen] == '/') {
 			memmove(path, path + wtlen + 1, len - wtlen);
 			return 0;

@@ -59,7 +59,7 @@ static int abspath_part_inside_repo(char *path)
 		path++;
 		if (*path == '/') {
 			*path = '\0';
-			if (strcmp(real_path(path0), work_tree) == 0) {
+			if (fspathcmp(real_path(path0), work_tree) == 0) {
 				memmove(path0, path + 1, len - (path - path0));
 				return 0;
 			}

@@ -68,7 +68,7 @@ static int abspath_part_inside_repo(char *path)
 	}
 
 	/* check whole path */
-	if (strcmp(real_path(path0), work_tree) == 0) {
+	if (fspathcmp(real_path(path0), work_tree) == 0) {
 		*path0 = '\0';
 		return 0;
 	}

index 37729ba258..be582a513b 100755
@@ -402,4 +402,11 @@ test_expect_success 'all statuses changed in folder if . is given' '
 	test $(git ls-files --stage | grep ^100755 | wc -l) -eq 0
 '
 
+test_expect_success CASE_INSENSITIVE_FS 'path is case-insensitive' '
+	path="$(pwd)/BLUB" &&
+	touch "$path" &&
+	downcased="$(echo "$path" | tr A-Z a-z)" &&
+	git add "$downcased"
+'
+
 test_done

gitgitgadget