threads / patch / 34924

v2, 3 partstest: use unambigous leading path (/foo) for mingw

Subject: [PATCH v2 1/3] test: use unambigous leading path (/foo) for mingw

## tl;dr

8 messages between Sep 13, 2013 and Sep 17, 2013. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Jiang Xin· Sep 13, 2013, 05:08 UTC · lore

[PATCH v2 0/3] fixes for relative_path

Updates since v1:
* New patch 1/3 on t0060, which use umambigous leading path (/foo).
* Call tolower instead of strncasecmp in patch 2/3.
* Rename simple_relative_path to remove_leading_path in patch 3/3.
Jiang Xin (3):
  test: use unambigous leading path (/foo) for mingw
  relative_path should honor dos_drive_prefix
  Use simpler relative_path when set_git_dir
 cache.h               |  1 +
 path.c                | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++
 setup.c               |  5 +---
 t/t0060-path-utils.sh | 60 +++++++++++++++++++++++++----------------------
 4 files changed, 99 insertions(+), 32 deletions(-)
-- 
1.8.4.459.gd80d422
Jiang Xin· Sep 13, 2013, 05:08 UTC · re: Jiang Xin · lore

In test cases for relative_path, path with one leading character (such as /a, /x) may be recogonized as "a:/" or "x:/" if there is such doc drive on MINGW platform. Use an umambigous leading path "/foo" instead.

Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 t/t0060-path-utils.sh | 56 +++++++++++++++++++++++++--------------------------
 1 file changed, 28 insertions(+), 28 deletions(-)
Show changes to t/t0060-path-utils.sh +28 −28
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 3a48de2..82a6f21 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -190,33 +190,33 @@ test_expect_success SYMLINKS 'real path works on symlinks' '
 	test "$sym" = "$(test-path-utils real_path "$dir2/syml")"
 '
 
-relative_path /a/b/c/	/a/b/		c/
-relative_path /a/b/c/	/a/b		c/
-relative_path /a//b//c/	//a/b//		c/	POSIX
-relative_path /a/b	/a/b		./
-relative_path /a/b/	/a/b		./
-relative_path /a	/a/b		../
-relative_path /		/a/b/		../../
-relative_path /a/c	/a/b/		../c
-relative_path /a/c	/a/b		../c
-relative_path /x/y	/a/b/		../../x/y
-relative_path /a/b	"<empty>"	/a/b
-relative_path /a/b 	"<null>"	/a/b
-relative_path a/b/c/	a/b/		c/
-relative_path a/b/c/	a/b		c/
-relative_path a/b//c	a//b		c
-relative_path a/b/	a/b/		./
-relative_path a/b/	a/b		./
-relative_path a		a/b		../
-relative_path x/y	a/b		../../x/y
-relative_path a/c	a/b		../c
-relative_path a/b	"<empty>"	a/b
-relative_path a/b 	"<null>"	a/b
-relative_path "<empty>"	/a/b		./
-relative_path "<empty>"	"<empty>"	./
-relative_path "<empty>"	"<null>"	./
-relative_path "<null>"	"<empty>"	./
-relative_path "<null>"	"<null>"	./
-relative_path "<null>"	/a/b		./
+relative_path /foo/a/b/c/	/foo/a/b/	c/
+relative_path /foo/a/b/c/	/foo/a/b	c/
+relative_path /foo/a//b//c/	//foo/a/b//	c/		POSIX
+relative_path /foo/a/b		/foo/a/b	./
+relative_path /foo/a/b/		/foo/a/b	./
+relative_path /foo/a		/foo/a/b	../
+relative_path /			/foo/a/b/	../../../
+relative_path /foo/a/c		/foo/a/b/	../c
+relative_path /foo/a/c		/foo/a/b	../c
+relative_path /foo/x/y		/foo/a/b/	../../x/y
+relative_path /foo/a/b		"<empty>"	/foo/a/b
+relative_path /foo/a/b 		"<null>"	/foo/a/b
+relative_path foo/a/b/c/	foo/a/b/	c/
+relative_path foo/a/b/c/	foo/a/b		c/
+relative_path foo/a/b//c	foo/a//b	c
+relative_path foo/a/b/		foo/a/b/	./
+relative_path foo/a/b/		foo/a/b		./
+relative_path foo/a		foo/a/b		../
+relative_path foo/x/y		foo/a/b		../../x/y
+relative_path foo/a/c		foo/a/b		../c
+relative_path foo/a/b		"<empty>"	foo/a/b
+relative_path foo/a/b 		"<null>"	foo/a/b
+relative_path "<empty>"		/foo/a/b	./
+relative_path "<empty>"		"<empty>"	./
+relative_path "<empty>"		"<null>"	./
+relative_path "<null>"		"<empty>"	./
+relative_path "<null>"		"<null>"	./
+relative_path "<null>"		/foo/a/b	./
 
 test_done
-- 
1.8.4.459.gd80d422
Junio C Hamano· Sep 13, 2013, 19:51 UTC · re: Jiang Xin · lore

Re: [PATCH v2 1/3] test: use unambigous leading path (/foo) for mingw

Jiang Xin <worldhello.net@gmail.com> writes:
> In test cases for relative_path, path with one leading character
> (such as /a, /x) may be recogonized as "a:/" or "x:/" if there is
> such doc drive on MINGW platform. Use an umambigous leading path
> "/foo" instead.
"DOS drive", you mean?
Are they really spelled as /a or /x (not e.g. //a or something)?
Just double-checking.
Show 71 quoted lines
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ---
>  t/t0060-path-utils.sh | 56 +++++++++++++++++++++++++--------------------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 3a48de2..82a6f21 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -190,33 +190,33 @@ test_expect_success SYMLINKS 'real path works on symlinks' '
>  	test "$sym" = "$(test-path-utils real_path "$dir2/syml")"
>  '
>  
> -relative_path /a/b/c/	/a/b/		c/
> -relative_path /a/b/c/	/a/b		c/
> -relative_path /a//b//c/	//a/b//		c/	POSIX
> -relative_path /a/b	/a/b		./
> -relative_path /a/b/	/a/b		./
> -relative_path /a	/a/b		../
> -relative_path /		/a/b/		../../
> -relative_path /a/c	/a/b/		../c
> -relative_path /a/c	/a/b		../c
> -relative_path /x/y	/a/b/		../../x/y
> -relative_path /a/b	"<empty>"	/a/b
> -relative_path /a/b 	"<null>"	/a/b
> -relative_path a/b/c/	a/b/		c/
> -relative_path a/b/c/	a/b		c/
> -relative_path a/b//c	a//b		c
> -relative_path a/b/	a/b/		./
> -relative_path a/b/	a/b		./
> -relative_path a		a/b		../
> -relative_path x/y	a/b		../../x/y
> -relative_path a/c	a/b		../c
> -relative_path a/b	"<empty>"	a/b
> -relative_path a/b 	"<null>"	a/b
> -relative_path "<empty>"	/a/b		./
> -relative_path "<empty>"	"<empty>"	./
> -relative_path "<empty>"	"<null>"	./
> -relative_path "<null>"	"<empty>"	./
> -relative_path "<null>"	"<null>"	./
> -relative_path "<null>"	/a/b		./
> +relative_path /foo/a/b/c/	/foo/a/b/	c/
> +relative_path /foo/a/b/c/	/foo/a/b	c/
> +relative_path /foo/a//b//c/	//foo/a/b//	c/		POSIX
> +relative_path /foo/a/b		/foo/a/b	./
> +relative_path /foo/a/b/		/foo/a/b	./
> +relative_path /foo/a		/foo/a/b	../
> +relative_path /			/foo/a/b/	../../../
> +relative_path /foo/a/c		/foo/a/b/	../c
> +relative_path /foo/a/c		/foo/a/b	../c
> +relative_path /foo/x/y		/foo/a/b/	../../x/y
> +relative_path /foo/a/b		"<empty>"	/foo/a/b
> +relative_path /foo/a/b 		"<null>"	/foo/a/b
> +relative_path foo/a/b/c/	foo/a/b/	c/
> +relative_path foo/a/b/c/	foo/a/b		c/
> +relative_path foo/a/b//c	foo/a//b	c
> +relative_path foo/a/b/		foo/a/b/	./
> +relative_path foo/a/b/		foo/a/b		./
> +relative_path foo/a		foo/a/b		../
> +relative_path foo/x/y		foo/a/b		../../x/y
> +relative_path foo/a/c		foo/a/b		../c
> +relative_path foo/a/b		"<empty>"	foo/a/b
> +relative_path foo/a/b 		"<null>"	foo/a/b
> +relative_path "<empty>"		/foo/a/b	./
> +relative_path "<empty>"		"<empty>"	./
> +relative_path "<empty>"		"<null>"	./
> +relative_path "<null>"		"<empty>"	./
> +relative_path "<null>"		"<null>"	./
> +relative_path "<null>"		/foo/a/b	./
>  
>  test_done
Torsten Bögershausen· Sep 14, 2013, 06:52 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 1/3] test: use unambigous leading path (/foo) for mingw

On 2013-09-13 21.51, Junio C Hamano wrote:
Show 12 quoted lines
> Jiang Xin <worldhello.net@gmail.com> writes:
> 
>> In test cases for relative_path, path with one leading character
>> (such as /a, /x) may be recogonized as "a:/" or "x:/" if there is
>> such doc drive on MINGW platform. Use an umambigous leading path
>> "/foo" instead.
> 
> "DOS drive", you mean?
> 
> Are they really spelled as /a or /x (not e.g. //a or something)?
> 
> Just double-checking.

Yes, there is a directoctory structure in / like this:

/usr /bin /lib Then we have the drive letters mapped to single letters: /c/Documents and Settings /c/temp

As an alternative c:/temp can be used or the DOS style "c:\temp"

And the // or "\\" is used for the UNC names (Universal Name Convention) //Servername/ShareName/Directory

/Torsten
Junio C Hamano· Sep 17, 2013, 16:19 UTC · re: Torsten Bögershausen · lore

Re: [PATCH v2 1/3] test: use unambigous leading path (/foo) for mingw

Torsten Bögershausen <tboegi@web.de> writes:
Show 9 quoted lines
> Yes,
> there is a directoctory structure in / like this:
>
> /usr
> /bin
> /lib
> Then we have the drive letters mapped to single letters:
> /c/Documents and Settings
> /c/temp
Thanks.
Jiang Xin· Sep 13, 2013, 05:08 UTC · re: Jiang Xin · lore

[PATCH v2 2/3] relative_path should honor dos_drive_prefix

Tvangeste found that the "relative_path" function could not work properly on Windows if "in" and "prefix" have dos driver prefix. ($gmane/234434)

e.g., When execute: test-path-utils relative_path "C:/a/b" "D:/x/y", should return "C:/a/b", but returns "../../C:/a/b", which is wrong.

So make relative_path honor dos_drive_prefix, and add test cases for it in t0060.

Reported-by: Tvangeste <i.4m.l33t@yandex.ru>
Helped-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 path.c                | 20 ++++++++++++++++++++
 t/t0060-path-utils.sh |  4 ++++
 2 files changed, 24 insertions(+)
Show changes to 2 files +24 −0

path.c, t/t0060-path-utils.sh

diff --git a/path.c b/path.c
index 9fd28bcd..65d376d 100644
--- a/path.c
+++ b/path.c
@@ -434,6 +434,16 @@ int adjust_shared_perm(const char *path)
 	return 0;
 }
 
+static int have_same_root(const char *path1, const char *path2)
+{
+	int is_abs1, is_abs2;
+
+	is_abs1 = is_absolute_path(path1);
+	is_abs2 = is_absolute_path(path2);
+	return (is_abs1 && is_abs2 && tolower(path1[0]) == tolower(path2[0])) ||
+	       (!is_abs1 && !is_abs2);
+}
+
 /*
  * Give path as relative to prefix.
  *
@@ -454,6 +464,16 @@ const char *relative_path(const char *in, const char *prefix,
 	else if (!prefix_len)
 		return in;
 
+	if (have_same_root(in, prefix)) {
+		/* bypass dos_drive, for "c:" is identical to "C:" */
+		if (has_dos_drive_prefix(in)) {
+			i = 2;
+			j = 2;
+		}
+	} else {
+		return in;
+	}
+
 	while (i < prefix_len && j < in_len && prefix[i] == in[j]) {
 		if (is_dir_sep(prefix[i])) {
 			while (is_dir_sep(prefix[i]))
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 82a6f21..0187d11 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -210,6 +210,10 @@ relative_path foo/a/b/		foo/a/b		./
 relative_path foo/a		foo/a/b		../
 relative_path foo/x/y		foo/a/b		../../x/y
 relative_path foo/a/c		foo/a/b		../c
+relative_path foo/a/b		/foo/x/y	foo/a/b
+relative_path /foo/a/b		foo/x/y		/foo/a/b
+relative_path d:/a/b		D:/a/c		../b		MINGW
+relative_path C:/a/b		D:/a/c		C:/a/b		MINGW
 relative_path foo/a/b		"<empty>"	foo/a/b
 relative_path foo/a/b 		"<null>"	foo/a/b
 relative_path "<empty>"		/foo/a/b	./
-- 
1.8.4.459.gd80d422
Torsten Bögershausen· Sep 14, 2013, 06:11 UTC · re: Jiang Xin · lore

Re: [PATCH v2 2/3] relative_path should honor dos_drive_prefix

On 13.09.13 07:08, Jiang Xin wrote:
Show 36 quoted lines
> Tvangeste found that the "relative_path" function could not work
> properly on Windows if "in" and "prefix" have dos driver prefix.
> ($gmane/234434)
>
> e.g., When execute: test-path-utils relative_path "C:/a/b" "D:/x/y",
> should return "C:/a/b", but returns "../../C:/a/b", which is wrong.
>
> So make relative_path honor dos_drive_prefix, and add test cases
> for it in t0060.
>
> Reported-by: Tvangeste <i.4m.l33t@yandex.ru>
> Helped-by: Johannes Sixt <j6t@kdbg.org>
> Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
> ---
>  path.c                | 20 ++++++++++++++++++++
>  t/t0060-path-utils.sh |  4 ++++
>  2 files changed, 24 insertions(+)
>
> diff --git a/path.c b/path.c
> index 9fd28bcd..65d376d 100644
> --- a/path.c
> +++ b/path.c
> @@ -434,6 +434,16 @@ int adjust_shared_perm(const char *path)
>  	return 0;
>  }
>  
> +static int have_same_root(const char *path1, const char *path2)
> +{
> +	int is_abs1, is_abs2;
> +
> +	is_abs1 = is_absolute_path(path1);
> +	is_abs2 = is_absolute_path(path2);
> +	return (is_abs1 && is_abs2 && tolower(path1[0]) == tolower(path2[0])) ||
> +	       (!is_abs1 && !is_abs2);
> +}
> +

I think the name of the fuction is somewhat misleading, as we are not sure if they really have the same root. And that is investigated further down.

may_have_same_root() could be a better name.
[snip]
Show 15 quoted lines
>  	while (i < prefix_len && j < in_len && prefix[i] == in[j]) {
>  		if (is_dir_sep(prefix[i])) {
>  			while (is_dir_sep(prefix[i]))
> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
> index 82a6f21..0187d11 100755
> --- a/t/t0060-path-utils.sh
> +++ b/t/t0060-path-utils.sh
> @@ -210,6 +210,10 @@ relative_path foo/a/b/		foo/a/b		./
>  relative_path foo/a		foo/a/b		../
>  relative_path foo/x/y		foo/a/b		../../x/y
>  relative_path foo/a/c		foo/a/b		../c
> +relative_path foo/a/b		/foo/x/y	foo/a/b
> +relative_path /foo/a/b		foo/x/y		/foo/a/b
> +relative_path d:/a/b		D:/a/c		../b		MINGW
> +relative_path C:/a/b		D:/a/c		C:/a/b		MINGW

Side question: What happens if we feed in a relative path with a dos drive? like "c:foo" which is different from c:/foo.

Jiang Xin· Sep 13, 2013, 05:08 UTC · re: Jiang Xin · lore

[PATCH v2 3/3] Use simpler relative_path when set_git_dir

Using a relative_path as git_dir first appears in v1.5.6-1-g044bbbc. It will make git_dir shorter only if git_dir is inside work_tree, and this will increase performance. But my last refactor effort on relative_path function (commit v1.8.3-rc2-12-ge02ca72) changed that. Always use relative_path as git_dir may bring troubles like $gmane/234434.

Because new relative_path is a combination of original relative_path from path.c and original path_relative from quote.c, so in order to restore the origin implementation, save the original relative_path as remove_leading_path, and call it in setup.c.

Suggested-by: Karsten Blees <karsten.blees@gmail.com>
Signed-off-by: Jiang Xin <worldhello.net@gmail.com>
---
 cache.h |  1 +
 path.c  | 45 +++++++++++++++++++++++++++++++++++++++++++++
 setup.c |  5 +----
 3 files changed, 47 insertions(+), 4 deletions(-)
Show changes to 3 files +47 −4

cache.h, path.c, setup.c

diff --git a/cache.h b/cache.h
index a47b9c0..73fa334 100644
--- a/cache.h
+++ b/cache.h
@@ -747,6 +747,7 @@ int is_directory(const char *);
 const char *real_path(const char *path);
 const char *real_path_if_valid(const char *path);
 const char *absolute_path(const char *path);
+const char *remove_leading_path(const char *in, const char *prefix);
 const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
 int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);
 int normalize_path_copy(char *dst, const char *src);
diff --git a/path.c b/path.c
index 65d376d..24594c4 100644
--- a/path.c
+++ b/path.c
@@ -551,6 +551,51 @@ const char *relative_path(const char *in, const char *prefix,
 }
 
 /*
+ * A simpler implementation of relative_path
+ *
+ * Get relative path by removing "prefix" from "in". This function
+ * first appears in v1.5.6-1-g044bbbc, and makes git_dir shorter
+ * to increase performance when traversing the path to work_tree.
+ */
+const char *remove_leading_path(const char *in, const char *prefix)
+{
+	static char buf[PATH_MAX + 1];
+	int i = 0, j = 0;
+
+	if (!prefix || !prefix[0])
+		return in;
+	while (prefix[i]) {
+		if (is_dir_sep(prefix[i])) {
+			if (!is_dir_sep(in[j]))
+				return in;
+			while (is_dir_sep(prefix[i]))
+				i++;
+			while (is_dir_sep(in[j]))
+				j++;
+			continue;
+		} else if (in[j] != prefix[i]) {
+			return in;
+		}
+		i++;
+		j++;
+	}
+	if (
+	    /* "/foo" is a prefix of "/foo" */
+	    in[j] &&
+	    /* "/foo" is not a prefix of "/foobar" */
+	    !is_dir_sep(prefix[i-1]) && !is_dir_sep(in[j])
+	   )
+		return in;
+	while (is_dir_sep(in[j]))
+		j++;
+	if (!in[j])
+		strcpy(buf, ".");
+	else
+		strcpy(buf, in + j);
+	return buf;
+}
+
+/*
  * It is okay if dst == src, but they should not overlap otherwise.
  *
  * Performs the following normalizations on src, storing the result in dst:
diff --git a/setup.c b/setup.c
index f08dd64..dbf4138 100644
--- a/setup.c
+++ b/setup.c
@@ -227,7 +227,6 @@ int is_inside_work_tree(void)
 
 void setup_work_tree(void)
 {
-	struct strbuf sb = STRBUF_INIT;
 	const char *work_tree, *git_dir;
 	static int initialized = 0;
 
@@ -247,10 +246,8 @@ void setup_work_tree(void)
 	if (getenv(GIT_WORK_TREE_ENVIRONMENT))
 		setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
 
-	set_git_dir(relative_path(git_dir, work_tree, &sb));
+	set_git_dir(remove_leading_path(git_dir, work_tree));
 	initialized = 1;
-
-	strbuf_release(&sb);
 }
 
 static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
-- 
1.8.4.459.gd80d422

← back to recent threads