{"thread":{"id":"17570","subject":"[PATCH] fix crash in path.c on Windows","startedAt":"2009-02-04T23:00:46Z","lastAt":"2009-02-08T15:50:21Z","messageCount":26,"participants":["René Scharfe","Junio C Hamano","Johannes Sixt","Robin Rosenberg","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103225","messageId":"498A1E1E.8010901@lsrfire.ath.cx","threadId":"17570","inReplyTo":null,"subject":"[PATCH] fix crash in path.c on Windows","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-02-04T23:00:46Z","receivedAt":"2009-02-04T23:00:46Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Use PATH_SEP and is_absolute_path() instead of using Unix conventions\nto enhance portability.  On Windows, the assert() fails almost always\nwithout this change.\n\nAlso convert all backslashes in DOS-style paths to slashes, as that's\nwhat git uses internally.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\nThe patch doesn't fix t1504 on Windows, but it allows one to at least\nrun this test without git crashing multiple times due to the failed\nassertion.  The test case \"first_of_two\" still fails, due to the\nweird translation applied to environment variables like\n$GIT_CEILING_DIRECTORIES.  Is there a method to it?  Here some\nexperiments:\n\n\tset a=\tgetenv(\"a\")\n\t======= ===========\n\tc\tc\n\t/c\tc:/\n\tc/\tc/\n\t/c/\tc:/\n\tc:c\tc:c\n\t/c:c\tc:c\n\tc:/c\tc:/c\n\t/c:/c\tc:/c\n\tc/:/c\tc\\;c:\\\n\t/c:c/\tc:c/\n\t/c/:c\t/c/:c\n\t/c/:/c\tc:\\;c:\\\n\t/c:/c/\tc:/c/\n\t/c/:/c/\tc:\\;c:\\\n\nThe test case can be convinced to pass by replacing \"bar\" with\n\"/bar\", but I'm not sure that's the right fix.  Shouldn't we warn on\ninvalid paths in $GIT_CEILING_DIRECTORIES?\n\n path.c |   13 +++++++++++--\n 1 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex a074aea..4cae7f6 100644\n--- a/path.c\n+++ b/path.c\n@@ -387,7 +387,7 @@ int normalize_absolute_path(char *buf, const char *path)\n \tassert(path);\n \n \twhile (*comp_start) {\n-\t\tassert(*comp_start == '/');\n+\t\tassert(is_absolute_path(comp_start));\n \t\twhile (*++comp_end && *comp_end != '/')\n \t\t\t; /* nothing */\n \t\tcomp_len = comp_end - comp_start;\n@@ -438,11 +438,20 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n \t\treturn -1;\n \n \tfor (colon = ceil = prefix_list; *colon; ceil = colon+1) {\n-\t\tfor (colon = ceil; *colon && *colon != ':'; colon++);\n+\t\tfor (colon = ceil; *colon && *colon != PATH_SEP; colon++);\n \t\tlen = colon - ceil;\n \t\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n \t\t\tcontinue;\n \t\tstrlcpy(buf, ceil, len+1);\n+\n+\t\tif (has_dos_drive_prefix(buf)) {\n+\t\t\tchar *p;\n+\t\t\tfor (p = buf; *p; p++) {\n+\t\t\t\tif (*p == '\\\\')\n+\t\t\t\t\t*p = '/';\n+\t\t\t}\n+\t\t}\n+\n \t\tlen = normalize_absolute_path(buf, buf);\n \t\t/* Strip \"trailing slashes\" from \"/\". */\n \t\tif (len == 1)\n-- \n1.6.1.2\n"},{"id":"103242","messageId":"7vbpthlq0w.fsf@gitster.siamese.dyndns.org","threadId":"17570","inReplyTo":"498A1E1E.8010901@lsrfire.ath.cx","subject":"Re: [PATCH] fix crash in path.c on Windows","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-04T23:51:27Z","receivedAt":"2009-02-04T23:51:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <rene.scharfe@lsrfire.ath.cx> writes:\n\n> @@ -387,7 +387,7 @@ int normalize_absolute_path(char *buf, const char *path)\n>  \tassert(path);\n>  \n>  \twhile (*comp_start) {\n> -\t\tassert(*comp_start == '/');\n> +\t\tassert(is_absolute_path(comp_start));\n>  \t\twhile (*++comp_end && *comp_end != '/')\n>  \t\t\t; /* nothing */\n>  \t\tcomp_len = comp_end - comp_start;\n\nThis change does not make sense to me.  The assert is about the initial\niteration beginning at the \"root\" level, and at the same time previous\niteration ended at dir_sep.  On mingw you would probably need these two as\nseparate tests.  In other words, I would understand if the fix were like\nthis:\n\n\tif (it begins with dos_prefix) {\n        \t/* this is never true outside windows */\n\t\tcopy the dos prefix out and advance comp_start as\n        \tnecessary;\n\t}\n        while (*comp_start) {\n        \tassert(is_dir_sep(*comp_start));\n                while (*++comp_end && !is_dir_sep(*comp_end))\n                \t; /* nothing */\n\t\t...\n\t}\n"},{"id":"103290","messageId":"498A9C01.6020602@viscovery.net","threadId":"17570","inReplyTo":"498A1E1E.8010901@lsrfire.ath.cx","subject":"Re: [PATCH] fix crash in path.c on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-05T07:57:53Z","receivedAt":"2009-02-05T07:57:53Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"René Scharfe schrieb:\n> \tset a=\tgetenv(\"a\")\n> \t======= ===========\n> \tc\tc\n> \t/c\tc:/\n> \tc/\tc/\n> \t/c/\tc:/\n> \tc:c\tc:c\n> \t/c:c\tc:c\n> \tc:/c\tc:/c\n> \t/c:/c\tc:/c\n> \tc/:/c\tc\\;c:\\\n> \t/c:c/\tc:c/\n> \t/c/:c\t/c/:c\n> \t/c/:/c\tc:\\;c:\\\n> \t/c:/c/\tc:/c/\n> \t/c/:/c/\tc:\\;c:\\\n\nBash translates leading single-letter path components to drive prefix\nnotation if it invokes a non-MSYS program; and it also translates ':' to\n';' if the value looks like a path list. Sometimes there is an ambiguity\nand bash guesses wrong.\n\n> @@ -387,7 +387,7 @@ int normalize_absolute_path(char *buf, const char *path)\n>  \tassert(path);\n>  \n>  \twhile (*comp_start) {\n> -\t\tassert(*comp_start == '/');\n> +\t\tassert(is_absolute_path(comp_start));\n>  \t\twhile (*++comp_end && *comp_end != '/')\n>  \t\t\t; /* nothing */\n>  \t\tcomp_len = comp_end - comp_start;\n\nJunio has pointed out your thinko here. Furthermore, *all* uses of '/' in\nthis loop must be replaced by is_dir_sep().\n\nBut I would really appreciate if you could unify normalize_absolute_path()\nwith setup.c's sanitary_path_copy() because they do almost the same thing\n(and the latter already works on Windows).\n\n> @@ -438,11 +438,20 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n>  \t\treturn -1;\n>  \n>  \tfor (colon = ceil = prefix_list; *colon; ceil = colon+1) {\n> -\t\tfor (colon = ceil; *colon && *colon != ':'; colon++);\n> +\t\tfor (colon = ceil; *colon && *colon != PATH_SEP; colon++);\n>  \t\tlen = colon - ceil;\n>  \t\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n>  \t\t\tcontinue;\n>  \t\tstrlcpy(buf, ceil, len+1);\n> +\n> +\t\tif (has_dos_drive_prefix(buf)) {\n> +\t\t\tchar *p;\n> +\t\t\tfor (p = buf; *p; p++) {\n> +\t\t\t\tif (*p == '\\\\')\n> +\t\t\t\t\t*p = '/';\n> +\t\t\t}\n\nIMNSHO this is a kind of normalization and, therefore, must be done by\nnormalize_absolute_path() (sanitary_path_copy() already does this).\n\n> +\t\t}\n> +\n>  \t\tlen = normalize_absolute_path(buf, buf);\n>  \t\t/* Strip \"trailing slashes\" from \"/\". */\n>  \t\tif (len == 1)\n\n-- Hannes\n"},{"id":"103353","messageId":"498B1862.5070703@lsrfire.ath.cx","threadId":"17570","inReplyTo":"498A9C01.6020602@viscovery.net","subject":"Re: [PATCH] fix crash in path.c on Windows","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-02-05T16:48:34Z","receivedAt":"2009-02-05T16:48:34Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Sixt schrieb:\n> René Scharfe schrieb:\n>> \tset a=\tgetenv(\"a\")\n>> \t======= ===========\n>> \tc\tc\n>> \t/c\tc:/\n>> \tc/\tc/\n>> \t/c/\tc:/\n>> \tc:c\tc:c\n>> \t/c:c\tc:c\n>> \tc:/c\tc:/c\n>> \t/c:/c\tc:/c\n>> \tc/:/c\tc\\;c:\\\n>> \t/c:c/\tc:c/\n>> \t/c/:c\t/c/:c\n>> \t/c/:/c\tc:\\;c:\\\n>> \t/c:/c/\tc:/c/\n>> \t/c/:/c/\tc:\\;c:\\\n> \n> Bash translates leading single-letter path components to drive prefix\n> notation if it invokes a non-MSYS program; and it also translates ':' to\n> ';' if the value looks like a path list. Sometimes there is an ambiguity\n> and bash guesses wrong.\n\nSure, but what rules or heuristics does it follow?  Do we need to\npost-process the results or can we simply change the test case in t1504?\n\n>> @@ -387,7 +387,7 @@ int normalize_absolute_path(char *buf, const char *path)\n>>  \tassert(path);\n>>  \n>>  \twhile (*comp_start) {\n>> -\t\tassert(*comp_start == '/');\n>> +\t\tassert(is_absolute_path(comp_start));\n>>  \t\twhile (*++comp_end && *comp_end != '/')\n>>  \t\t\t; /* nothing */\n>>  \t\tcomp_len = comp_end - comp_start;\n> \n> Junio has pointed out your thinko here. Furthermore, *all* uses of '/' in\n> this loop must be replaced by is_dir_sep().\n\nIt's not a thinko because I didn't think there. :)  It's more a case of\nmechanical refactoring resulting in confusing code.  If we inline\nis_absolute_path() manually there, we get the following:\n\n\tassert(*comp_start == '/' || has_dos_drive_prefix(comp_start));\n\nThe loop works because DOS drive prefixes never contain slashes.\n\nIs is_absolute_path() too forgiving on Windows, i.e. should it stop\nclassifying paths starting with a slash as absolute on that platform?\n\n> But I would really appreciate if you could unify normalize_absolute_path()\n> with setup.c's sanitary_path_copy() because they do almost the same thing\n> (and the latter already works on Windows).\n\nGood idea.\n\n>> @@ -438,11 +438,20 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n>>  \t\treturn -1;\n>>  \n>>  \tfor (colon = ceil = prefix_list; *colon; ceil = colon+1) {\n>> -\t\tfor (colon = ceil; *colon && *colon != ':'; colon++);\n>> +\t\tfor (colon = ceil; *colon && *colon != PATH_SEP; colon++);\n>>  \t\tlen = colon - ceil;\n>>  \t\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n>>  \t\t\tcontinue;\n>>  \t\tstrlcpy(buf, ceil, len+1);\n>> +\n>> +\t\tif (has_dos_drive_prefix(buf)) {\n>> +\t\t\tchar *p;\n>> +\t\t\tfor (p = buf; *p; p++) {\n>> +\t\t\t\tif (*p == '\\\\')\n>> +\t\t\t\t\t*p = '/';\n>> +\t\t\t}\n> \n> IMNSHO this is a kind of normalization and, therefore, must be done by\n> normalize_absolute_path() (sanitary_path_copy() already does this).\n\nMakes sense.\n\nRené\n"},{"id":"103354","messageId":"498B1E37.2020302@viscovery.net","threadId":"17570","inReplyTo":"498B1862.5070703@lsrfire.ath.cx","subject":"Re: [PATCH] fix crash in path.c on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-05T17:13:27Z","receivedAt":"2009-02-05T17:13:27Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"René Scharfe schrieb:\n> Johannes Sixt schrieb:\n>> Bash translates leading single-letter path components to drive prefix\n>> notation if it invokes a non-MSYS program; and it also translates ':' to\n>> ';' if the value looks like a path list. Sometimes there is an ambiguity\n>> and bash guesses wrong.\n> \n> Sure, but what rules or heuristics does it follow?  Do we need to\n> post-process the results or can we simply change the test case in t1504?\n\nMingw git assumes that it operates in a typical Windows environment -\ndrive-letter notation for absolute paths, and ';' separates paths in\nPATH-like environment variables. If a test case breaks because bash\nguesses wrongly, and does not transform the environment or command line\nargument correctly, change the test case. (For example if /c:c was meant\nas a two paths, then change it to /foo:bar, perhaps bash gets this right.)\n\nEverything else, I don't know. Audit how sensible the test case is, and\nwhether it addresses a likely real-world case. If we don't care, skip it.\n\nWe try hard not to place Windows specific code outside of compat/.\n\n> Is is_absolute_path() too forgiving on Windows, i.e. should it stop\n> classifying paths starting with a slash as absolute on that platform?\n\nNo. /foo/bar is a valid \"absolute\" path Windows - even though it is\nrelative to the current drive. The point is that it is not relative to the\ncurrent directory.\n\n-- Hannes\n"},{"id":"103373","messageId":"498B3F6B.5080002@lsrfire.ath.cx","threadId":"17570","inReplyTo":"498A1E1E.8010901@lsrfire.ath.cx","subject":"[PATCH] fix t1504 on Windows","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-02-05T19:35:07Z","receivedAt":"2009-02-05T19:35:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"The test t1504 crashes on Windows due to a failed assertion in\nnormalize_absolute_path() because this function expects absolute paths to\nstart with a slash, while on Windows they can start with a drive letter\nor a backslash.\n\nAs suggested by Johannes, fix it by using sanitary_path_copy() instead,\nwhich can handle Windows-style paths just fine.  This patch just exports\nit for usage; a later patch may move it from setup.c to path.c where it\nfits better.\n\nSecondly, use the portability macro PATH_SEP instead of expecting colons\nto be used as path list delimiter.  On Windows, semicolons are used,\nwhile colons separate drive letter and path.\n\nAlso change the test script to help bash recognize the path list variable\nfor test case \"first_of_two\" as needing path conversion.  git expects\nWindows-style paths, which bash happily creates if we turn the second\npath of the list into an absolute one.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n cache.h                 |    1 +\n path.c                  |   11 ++++++-----\n setup.c                 |    2 +-\n t/t1504-ceiling-dirs.sh |    2 +-\n 4 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 45e713e..eeb774a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -626,6 +626,7 @@ const char *make_nonrelative_path(const char *path);\n const char *make_relative_path(const char *abs, const char *base);\n int normalize_absolute_path(char *buf, const char *path);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n+int sanitary_path_copy(char *dst, const char *src);\n \n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\n extern int sha1_object_info(const unsigned char *, unsigned long *);\ndiff --git a/path.c b/path.c\nindex a074aea..ea1913a 100644\n--- a/path.c\n+++ b/path.c\n@@ -438,15 +438,16 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n \t\treturn -1;\n \n \tfor (colon = ceil = prefix_list; *colon; ceil = colon+1) {\n-\t\tfor (colon = ceil; *colon && *colon != ':'; colon++);\n+\t\tfor (colon = ceil; *colon && *colon != PATH_SEP; colon++);\n \t\tlen = colon - ceil;\n \t\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n \t\t\tcontinue;\n \t\tstrlcpy(buf, ceil, len+1);\n-\t\tlen = normalize_absolute_path(buf, buf);\n-\t\t/* Strip \"trailing slashes\" from \"/\". */\n-\t\tif (len == 1)\n-\t\t\tlen = 0;\n+\t\tif (sanitary_path_copy(buf, buf) < 0)\n+\t\t\tcontinue;\n+\t\tlen = strlen(buf);\n+\t\tif (buf[len - 1] == '/')\n+\t\t\tbuf[--len] = '\\0';\n \n \t\tif (!strncmp(path, buf, len) &&\n \t\t    path[len] == '/' &&\ndiff --git a/setup.c b/setup.c\nindex dfda532..4fe438c 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -4,7 +4,7 @@\n static int inside_git_dir = -1;\n static int inside_work_tree = -1;\n \n-static int sanitary_path_copy(char *dst, const char *src)\n+int sanitary_path_copy(char *dst, const char *src)\n {\n \tchar *dst0;\n \ndiff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\nindex 91b704a..9c9c4c9 100755\n--- a/t/t1504-ceiling-dirs.sh\n+++ b/t/t1504-ceiling-dirs.sh\n@@ -96,7 +96,7 @@ test_prefix subdir_ceil_at_subdi_slash \"sub/dir/\"\n GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub\"\n test_fail second_of_two\n \n-GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:bar\"\n+GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:/bar\"\n test_fail first_of_two\n \n GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub:bar\"\n-- \n1.6.1.2\n"},{"id":"103383","messageId":"200902052141.00999.robin.rosenberg.lists@dewire.com","threadId":"17570","inReplyTo":"498B1862.5070703@lsrfire.ath.cx","subject":"Re: [PATCH] fix crash in path.c on Windows","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-02-05T20:41:00Z","receivedAt":"2009-02-05T20:41:00Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"torsdag 05 februari 2009 17:48:34 skrev René Scharfe:\n> Is is_absolute_path() too forgiving on Windows, i.e. should it stop\n> classifying paths starting with a slash as absolute on that platform?\n\n//server/share/file is an absolute path.\n\n-- robin\n"},{"id":"103465","messageId":"498C3328.70804@viscovery.net","threadId":"17570","inReplyTo":"498B3F6B.5080002@lsrfire.ath.cx","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-06T12:55:04Z","receivedAt":"2009-02-06T12:55:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"René Scharfe schrieb:\n> The test t1504 crashes on Windows due to a failed assertion in\n> normalize_absolute_path() because this function expects absolute paths to\n> start with a slash, while on Windows they can start with a drive letter\n> or a backslash.\n...\n> diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\n> index 91b704a..9c9c4c9 100755\n> --- a/t/t1504-ceiling-dirs.sh\n> +++ b/t/t1504-ceiling-dirs.sh\n> @@ -96,7 +96,7 @@ test_prefix subdir_ceil_at_subdi_slash \"sub/dir/\"\n>  GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub\"\n>  test_fail second_of_two\n>  \n> -GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:bar\"\n> +GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:/bar\"\n>  test_fail first_of_two\n>  \n>  GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub:bar\"\n\nI also need this to complete this test:\n\ndiff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\nindex 9c9c4c9..e377d48 100755\n--- a/t/t1504-ceiling-dirs.sh\n+++ b/t/t1504-ceiling-dirs.sh\n@@ -93,13 +93,13 @@ GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/subdi\"\n test_prefix subdir_ceil_at_subdi_slash \"sub/dir/\"\n\n\n-GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub\"\n+GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub\"\n test_fail second_of_two\n\n GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:/bar\"\n test_fail first_of_two\n\n-GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub:bar\"\n+GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub:/bar\"\n test_fail second_of_three\n\n\n\nThat said, I'm in the process of preparing a series that includes your\npatch and that does the proper cleanup and code moving that you again\ndidn't do :-/ But it turns out that this is non-trivial because of bash's\n(MSYS's) I-know-better-what-is-a-path-and-what-not behavior. It will take\nsome time...\n\n-- Hannes\n"},{"id":"103469","messageId":"alpine.DEB.1.00.0902061410210.7377@intel-tinevez-2-302","threadId":"17570","inReplyTo":"498C3328.70804@viscovery.net","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-06T13:11:32Z","receivedAt":"2009-02-06T13:11:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Feb 2009, Johannes Sixt wrote:\n\n> That said, I'm in the process of preparing a series that includes your \n> patch and that does the proper cleanup and code moving that you again \n> didn't do :-/ But it turns out that this is non-trivial because of \n> bash's (MSYS's) I-know-better-what-is-a-path-and-what-not behavior. It \n> will take some time...\n\nPlease be sure to test on msysGit, too, we have some important patches \naffecting the bash I-know-... behavior (IMHO improving things a lot).\n\n(I seem to remember that you use your own MSys environment rather than the \nmsysGit environment.)\n\nCiao,\nDscho\n"},{"id":"103470","messageId":"498C3871.5000507@viscovery.net","threadId":"17570","inReplyTo":"alpine.DEB.1.00.0902061410210.7377@intel-tinevez-2-302","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-06T13:17:37Z","receivedAt":"2009-02-06T13:17:37Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> Please be sure to test on msysGit, too, we have some important patches \n> affecting the bash I-know-... behavior (IMHO improving things a lot).\n\nSure, I'll do.\n\n> (I seem to remember that you use your own MSys environment rather than the \n> msysGit environment.)\n\nI switched to msysGit, the build environment, soon after you published it.\nBut I don't use rxvt or interactive bash.\n\n-- Hannes\n"},{"id":"103472","messageId":"alpine.DEB.1.00.0902061425180.7377@intel-tinevez-2-302","threadId":"17570","inReplyTo":"498C3871.5000507@viscovery.net","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-06T13:26:20Z","receivedAt":"2009-02-06T13:26:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 6 Feb 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> \n> > (I seem to remember that you use your own MSys environment rather than \n> > the msysGit environment.)\n> \n> I switched to msysGit, the build environment, soon after you published \n> it. But I don't use rxvt or interactive bash.\n\nAh.\n\nSo the path mangling only hits you when you run a shell script, such as \nthe tests?\n\nCiao,\nDscho\n"},{"id":"103474","messageId":"498C3CF1.3000208@viscovery.net","threadId":"17570","inReplyTo":"alpine.DEB.1.00.0902061425180.7377@intel-tinevez-2-302","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-02-06T13:36:49Z","receivedAt":"2009-02-06T13:36:49Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> So the path mangling only hits you when you run a shell script, such as \n> the tests?\n\nCorrect.\n\n-- Hannes\n"},{"id":"103533","messageId":"498C70C8.1080009@lsrfire.ath.cx","threadId":"17570","inReplyTo":"498C3328.70804@viscovery.net","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-02-06T17:18:00Z","receivedAt":"2009-02-06T17:18:00Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Sixt schrieb:\n> I also need this to complete this test:\n> \n> diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\n> index 9c9c4c9..e377d48 100755\n> --- a/t/t1504-ceiling-dirs.sh\n> +++ b/t/t1504-ceiling-dirs.sh\n> @@ -93,13 +93,13 @@ GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/subdi\"\n>  test_prefix subdir_ceil_at_subdi_slash \"sub/dir/\"\n> \n> \n> -GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub\"\n> +GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub\"\n>  test_fail second_of_two\n> \n>  GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:/bar\"\n>  test_fail first_of_two\n> \n> -GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub:bar\"\n> +GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub:/bar\"\n>  test_fail second_of_three\n\nI don't, which makes me uneasy -- the tests shouldn't depend on details\nin our setup. :-/\n\n> That said, I'm in the process of preparing a series that includes your\n> patch and that does the proper cleanup and code moving that you again\n> didn't do :-/ But it turns out that this is non-trivial because of bash's\n> (MSYS's) I-know-better-what-is-a-path-and-what-not behavior. It will take\n> some time...\n\nGlad to hear the first part, but what code moving do you mean?  Something\nlike the following?\n\n-- snip! --\nRemove the unused function normalize_absolute_path() and its tests.\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\n---\n cache.h               |    1 -\n path.c                |   53 -------------------------------------------------\n t/t0060-path-utils.sh |   30 ---------------------------\n test-path-utils.c     |    7 ------\n 4 files changed, 0 insertions(+), 91 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex eeb774a..763bce7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -624,7 +624,6 @@ int is_directory(const char *);\n const char *make_absolute_path(const char *path);\n const char *make_nonrelative_path(const char *path);\n const char *make_relative_path(const char *abs, const char *base);\n-int normalize_absolute_path(char *buf, const char *path);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n int sanitary_path_copy(char *dst, const char *src);\n \ndiff --git a/path.c b/path.c\nindex ea1913a..3fb9710 100644\n--- a/path.c\n+++ b/path.c\n@@ -363,59 +363,6 @@ const char *make_relative_path(const char *abs, const char *base)\n }\n \n /*\n- * path = absolute path\n- * buf = buffer of at least max(2, strlen(path)+1) bytes\n- * It is okay if buf == path, but they should not overlap otherwise.\n- *\n- * Performs the following normalizations on path, storing the result in buf:\n- * - Removes trailing slashes.\n- * - Removes empty components.\n- * - Removes \".\" components.\n- * - Removes \"..\" components, and the components the precede them.\n- * \"\" and paths that contain only slashes are normalized to \"/\".\n- * Returns the length of the output.\n- *\n- * Note that this function is purely textual.  It does not follow symlinks,\n- * verify the existence of the path, or make any system calls.\n- */\n-int normalize_absolute_path(char *buf, const char *path)\n-{\n-\tconst char *comp_start = path, *comp_end = path;\n-\tchar *dst = buf;\n-\tint comp_len;\n-\tassert(buf);\n-\tassert(path);\n-\n-\twhile (*comp_start) {\n-\t\tassert(*comp_start == '/');\n-\t\twhile (*++comp_end && *comp_end != '/')\n-\t\t\t; /* nothing */\n-\t\tcomp_len = comp_end - comp_start;\n-\n-\t\tif (!strncmp(\"/\",  comp_start, comp_len) ||\n-\t\t    !strncmp(\"/.\", comp_start, comp_len))\n-\t\t\tgoto next;\n-\n-\t\tif (!strncmp(\"/..\", comp_start, comp_len)) {\n-\t\t\twhile (dst > buf && *--dst != '/')\n-\t\t\t\t; /* nothing */\n-\t\t\tgoto next;\n-\t\t}\n-\n-\t\tmemmove(dst, comp_start, comp_len);\n-\t\tdst += comp_len;\n-\tnext:\n-\t\tcomp_start = comp_end;\n-\t}\n-\n-\tif (dst == buf)\n-\t\t*dst++ = '/';\n-\n-\t*dst = '\\0';\n-\treturn dst - buf;\n-}\n-\n-/*\n  * path = Canonical absolute path\n  * prefix_list = Colon-separated list of absolute paths\n  *\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 6e7501f..9498f72 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -7,41 +7,11 @@ test_description='Test various path utilities'\n \n . ./test-lib.sh\n \n-norm_abs() {\n-\ttest_expect_success \"normalize absolute\" \\\n-\t\"test \\$(test-path-utils normalize_absolute_path '$1') = '$2'\"\n-}\n-\n ancestor() {\n \ttest_expect_success \"longest ancestor\" \\\n \t\"test \\$(test-path-utils longest_ancestor_length '$1' '$2') = '$3'\"\n }\n \n-norm_abs \"\" /\n-norm_abs / /\n-norm_abs // /\n-norm_abs /// /\n-norm_abs /. /\n-norm_abs /./ /\n-norm_abs /./.. /\n-norm_abs /../. /\n-norm_abs /./../.// /\n-norm_abs /dir/.. /\n-norm_abs /dir/sub/../.. /\n-norm_abs /dir /dir\n-norm_abs /dir// /dir\n-norm_abs /./dir /dir\n-norm_abs /dir/. /dir\n-norm_abs /dir///./ /dir\n-norm_abs /dir//sub/.. /dir\n-norm_abs /dir/sub/../ /dir\n-norm_abs //dir/sub/../. /dir\n-norm_abs /dir/s1/../s2/ /dir/s2\n-norm_abs /d1/s1///s2/..//../s3/ /d1/s3\n-norm_abs /d1/s1//../s2/../../d2 /d2\n-norm_abs /d1/.../d2 /d1/.../d2\n-norm_abs /d1/..././../d2 /d1/d2\n-\n ancestor / \"\" -1\n ancestor / / -1\n ancestor /foo \"\" -1\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 2c0f5a3..a470ee4 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -2,13 +2,6 @@\n \n int main(int argc, char **argv)\n {\n-\tif (argc == 3 && !strcmp(argv[1], \"normalize_absolute_path\")) {\n-\t\tchar *buf = xmalloc(PATH_MAX + 1);\n-\t\tint rv = normalize_absolute_path(buf, argv[2]);\n-\t\tassert(strlen(buf) == rv);\n-\t\tputs(buf);\n-\t}\n-\n \tif (argc >= 2 && !strcmp(argv[1], \"make_absolute_path\")) {\n \t\twhile (argc > 2) {\n \t\t\tputs(make_absolute_path(argv[2]));\n-- \n1.6.1.2.350.g88cc\n"},{"id":"103545","messageId":"498C8E43.8010108@kdbg.org","threadId":"17570","inReplyTo":"498C70C8.1080009@lsrfire.ath.cx","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-06T19:23:47Z","receivedAt":"2009-02-06T19:23:47Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"René Scharfe schrieb:\n> Johannes Sixt schrieb:\n>> I also need this to complete this test:\n>>\n>> diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\n>> index 9c9c4c9..e377d48 100755\n>> --- a/t/t1504-ceiling-dirs.sh\n>> +++ b/t/t1504-ceiling-dirs.sh\n>> @@ -93,13 +93,13 @@ GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/subdi\"\n>>  test_prefix subdir_ceil_at_subdi_slash \"sub/dir/\"\n>>\n>>\n>> -GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub\"\n>> +GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub\"\n>>  test_fail second_of_two\n>>\n>>  GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:/bar\"\n>>  test_fail first_of_two\n>>\n>> -GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub:bar\"\n>> +GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub:/bar\"\n>>  test_fail second_of_three\n> \n> I don't, which makes me uneasy -- the tests shouldn't depend on details\n> in our setup. :-/\n\nI updated my msysgit to the current master this moment, and I still need \nthese two changes. Do you use an older msysgit? With 31d5dfeb0 \n(2008-06-08) Steffen changed path mangling so that a text without a \nleading / or . is not converted. That's exactly what we see here.\n\n>> That said, I'm in the process of preparing a series that includes your\n>> patch and that does the proper cleanup and code moving that you again\n>> didn't do :-/ But it turns out that this is non-trivial because of bash's\n>> (MSYS's) I-know-better-what-is-a-path-and-what-not behavior. It will take\n>> some time...\n> \n> Glad to hear the first part, but what code moving do you mean?  Something\n> like the following?\n> \n> -- snip! --\n> Remove the unused function normalize_absolute_path() and its tests.\n\nNo, that's not enough. I mean something more like this (but I'll not \ninclude the diff itself):\n\nJohannes Sixt (4):\n       Make test-path-utils more robust against incorrect use\n       Move sanitary_path_copy to path.c (and rename to\n         normalize_path_copy)\n       Test and fix normalize_path_copy()\n       Remove unused normalize_absolute_path()\n\nRené Scharfe (1):\n       Fix t1504 on Windows\n\n  cache.h                 |    2 +-\n  path.c                  |  124 +++++++++++++++++++++++++------------\n  setup.c                 |   88 +--------------------------\n  t/t0060-path-utils.sh   |   26 ++++----\n  t/t1504-ceiling-dirs.sh |    2 +-\n  test-path-utils.c       |   12 +++-\n  6 files changed, 107 insertions(+), 147 deletions(-)\n\n-- Hannes\n"},{"id":"103560","messageId":"498CAF73.6050409@lsrfire.ath.cx","threadId":"17570","inReplyTo":"498C8E43.8010108@kdbg.org","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-02-06T21:45:23Z","receivedAt":"2009-02-06T21:45:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Sixt schrieb:\n> René Scharfe schrieb:\n>> Johannes Sixt schrieb:\n>>> -GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub\"\n>>> +GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub\"\n>>>  test_fail second_of_two\n>>>\n>>>  GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:/bar\"\n>>>  test_fail first_of_two\n>>>\n>>> -GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub:bar\"\n>>> +GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub:/bar\"\n>>>  test_fail second_of_three\n>>\n>> I don't, which makes me uneasy -- the tests shouldn't depend on details\n>> in our setup. :-/\n> \n> I updated my msysgit to the current master this moment, and I still need\n> these two changes. Do you use an older msysgit? With 31d5dfeb0\n> (2008-06-08) Steffen changed path mangling so that a text without a\n> leading / or . is not converted. That's exactly what we see here.\n\nOh, yes, good idea.  I never updated the msys part of my installation.\n\nMy \"uneasiness\" doesn't mean that I'm against your changes above, in any\ncase.\n\n>>> That said, I'm in the process of preparing a series that includes your\n>>> patch and that does the proper cleanup and code moving that you again\n>>> didn't do :-/ But it turns out that this is non-trivial because of\n>>> bash's\n>>> (MSYS's) I-know-better-what-is-a-path-and-what-not behavior. It will\n>>> take\n>>> some time...\n\n> Johannes Sixt (4):\n>       Make test-path-utils more robust against incorrect use\n>       Move sanitary_path_copy to path.c (and rename to\n>         normalize_path_copy)\n>       Test and fix normalize_path_copy()\n>       Remove unused normalize_absolute_path()\n\nYeah, OK.  I was aiming for a quick short fix, deferring cleanups to\nwhen I have more time.  I wonder what patches one and three do, though.\n\nRené\n"},{"id":"103573","messageId":"498CD4F5.8010102@lsrfire.ath.cx","threadId":"17570","inReplyTo":"498C3328.70804@viscovery.net","subject":"Re: [PATCH] fix t1504 on Windows","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-02-07T00:25:25Z","receivedAt":"2009-02-07T00:25:25Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Sixt schrieb:\n> I also need this to complete this test:\n> \n> diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\n> index 9c9c4c9..e377d48 100755\n> --- a/t/t1504-ceiling-dirs.sh\n> +++ b/t/t1504-ceiling-dirs.sh\n> @@ -93,13 +93,13 @@ GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/subdi\"\n>  test_prefix subdir_ceil_at_subdi_slash \"sub/dir/\"\n> \n> \n> -GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub\"\n> +GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub\"\n>  test_fail second_of_two\n> \n>  GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:/bar\"\n>  test_fail first_of_two\n> \n> -GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub:bar\"\n> +GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub:/bar\"\n>  test_fail second_of_three\n> \n\nJust to confirm: after updating msys, I also need this patch to make\nt1504 pass.\n\nRené\n"},{"id":"103627","messageId":"1234019311-6449-1-git-send-email-j6t@kdbg.org","threadId":"17570","inReplyTo":"498CAF73.6050409@lsrfire.ath.cx","subject":"[PATCH 0/5] Consolidate path normalization functions","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-07T15:08:26Z","receivedAt":"2009-02-07T15:08:26Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"This series:\n\n - moves sanitary_path_copy around, which accounts for the bulk\n   of the change;\n\n - updates longest_ancestor_length(), the core function of\n   GIT_CEILING_DIRECTORIES;\n\n - adjust the test suite, uncovers a buglet in sanitary_path_copy(),\n   and fixes it.\n\n - removes normalize_absolute_path() that duplicated functionality\n   of sanitary_path_copy().\n\nAs a side effect, this fixes GIT_CEILING_DIRECTORIES on Windows, where\nthis feature is currently broken.\n\nJohannes Sixt (4):\n  Make test-path-utils more robust against incorrect use\n  Move sanitary_path_copy() to path.c and rename it to\n    normalize_path_copy()\n  Test and fix normalize_path_copy()\n  Remove unused normalize_absolute_path()\n\nRené Scharfe (1):\n  Fix t1504 on Windows\n\n cache.h                 |    2 +-\n path.c                  |  124 ++++++++++++++++++++++++++-------------\n setup.c                 |   88 +---------------------------\n t/t0060-path-utils.sh   |   33 +++++-----\n t/t1504-ceiling-dirs.sh |    6 +-\n test-path-utils.c       |   14 +++-\n 6 files changed, 115 insertions(+), 152 deletions(-)\n"},{"id":"103625","messageId":"1234019311-6449-2-git-send-email-j6t@kdbg.org","threadId":"17570","inReplyTo":"1234019311-6449-1-git-send-email-j6t@kdbg.org","subject":"[PATCH 1/5] Make test-path-utils more robust against incorrect use","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-07T15:08:27Z","receivedAt":"2009-02-07T15:08:27Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Previously, this test utility happily returned with exit code 0 if garbage\nwas thrown at it. Now it reports failure if an unknown function name was\ngiven on the command line.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n test-path-utils.c |    7 ++++++-\n 1 files changed, 6 insertions(+), 1 deletions(-)\n\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 2c0f5a3..7e6fc8d 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -7,6 +7,7 @@ int main(int argc, char **argv)\n \t\tint rv = normalize_absolute_path(buf, argv[2]);\n \t\tassert(strlen(buf) == rv);\n \t\tputs(buf);\n+\t\treturn 0;\n \t}\n \n \tif (argc >= 2 && !strcmp(argv[1], \"make_absolute_path\")) {\n@@ -15,12 +16,16 @@ int main(int argc, char **argv)\n \t\t\targc--;\n \t\t\targv++;\n \t\t}\n+\t\treturn 0;\n \t}\n \n \tif (argc == 4 && !strcmp(argv[1], \"longest_ancestor_length\")) {\n \t\tint len = longest_ancestor_length(argv[2], argv[3]);\n \t\tprintf(\"%d\\n\", len);\n+\t\treturn 0;\n \t}\n \n-\treturn 0;\n+\tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n+\t\targv[1] ? argv[1] : \"(there was none)\");\n+\treturn 1;\n }\n-- \n1.6.1.297.g9b01e\n"},{"id":"103628","messageId":"1234019311-6449-3-git-send-email-j6t@kdbg.org","threadId":"17570","inReplyTo":"1234019311-6449-2-git-send-email-j6t@kdbg.org","subject":"[PATCH 2/5] Move sanitary_path_copy() to path.c and rename it to normalize_path_copy()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-07T15:08:28Z","receivedAt":"2009-02-07T15:08:28Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"This function and normalize_absolute_path() do almost the same thing. The\nformer already works on Windows, but the latter crashes.\n\nIn subsequent changes we will remove normalize_absolute_path(). Here we\nmake the replacement function reusable. On the way we rename it to reflect\nthat it does some path normalization. Apart from that this is only moving\naround code.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n cache.h |    1 +\n path.c  |   86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n setup.c |   88 +--------------------------------------------------------------\n 3 files changed, 88 insertions(+), 87 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 2d889de..82fbb80 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -628,6 +628,7 @@ const char *make_absolute_path(const char *path);\n const char *make_nonrelative_path(const char *path);\n const char *make_relative_path(const char *abs, const char *base);\n int normalize_absolute_path(char *buf, const char *path);\n+int normalize_path_copy(char *dst, const char *src);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n \n /* Read and unpack a sha1 file into memory, write memory to a sha1 file */\ndiff --git a/path.c b/path.c\nindex 108d9e9..d0a3519 100644\n--- a/path.c\n+++ b/path.c\n@@ -415,6 +415,92 @@ int normalize_absolute_path(char *buf, const char *path)\n \treturn dst - buf;\n }\n \n+int normalize_path_copy(char *dst, const char *src)\n+{\n+\tchar *dst0;\n+\n+\tif (has_dos_drive_prefix(src)) {\n+\t\t*dst++ = *src++;\n+\t\t*dst++ = *src++;\n+\t}\n+\tdst0 = dst;\n+\n+\tif (is_dir_sep(*src)) {\n+\t\t*dst++ = '/';\n+\t\twhile (is_dir_sep(*src))\n+\t\t\tsrc++;\n+\t}\n+\n+\tfor (;;) {\n+\t\tchar c = *src;\n+\n+\t\t/*\n+\t\t * A path component that begins with . could be\n+\t\t * special:\n+\t\t * (1) \".\" and ends   -- ignore and terminate.\n+\t\t * (2) \"./\"           -- ignore them, eat slash and continue.\n+\t\t * (3) \"..\" and ends  -- strip one and terminate.\n+\t\t * (4) \"../\"          -- strip one, eat slash and continue.\n+\t\t */\n+\t\tif (c == '.') {\n+\t\t\tif (!src[1]) {\n+\t\t\t\t/* (1) */\n+\t\t\t\tsrc++;\n+\t\t\t} else if (is_dir_sep(src[1])) {\n+\t\t\t\t/* (2) */\n+\t\t\t\tsrc += 2;\n+\t\t\t\twhile (is_dir_sep(*src))\n+\t\t\t\t\tsrc++;\n+\t\t\t\tcontinue;\n+\t\t\t} else if (src[1] == '.') {\n+\t\t\t\tif (!src[2]) {\n+\t\t\t\t\t/* (3) */\n+\t\t\t\t\tsrc += 2;\n+\t\t\t\t\tgoto up_one;\n+\t\t\t\t} else if (is_dir_sep(src[2])) {\n+\t\t\t\t\t/* (4) */\n+\t\t\t\t\tsrc += 3;\n+\t\t\t\t\twhile (is_dir_sep(*src))\n+\t\t\t\t\t\tsrc++;\n+\t\t\t\t\tgoto up_one;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\n+\t\t/* copy up to the next '/', and eat all '/' */\n+\t\twhile ((c = *src++) != '\\0' && !is_dir_sep(c))\n+\t\t\t*dst++ = c;\n+\t\tif (is_dir_sep(c)) {\n+\t\t\t*dst++ = '/';\n+\t\t\twhile (is_dir_sep(c))\n+\t\t\t\tc = *src++;\n+\t\t\tsrc--;\n+\t\t} else if (!c)\n+\t\t\tbreak;\n+\t\tcontinue;\n+\n+\tup_one:\n+\t\t/*\n+\t\t * dst0..dst is prefix portion, and dst[-1] is '/';\n+\t\t * go up one level.\n+\t\t */\n+\t\tdst -= 2; /* go past trailing '/' if any */\n+\t\tif (dst < dst0)\n+\t\t\treturn -1;\n+\t\twhile (1) {\n+\t\t\tif (dst <= dst0)\n+\t\t\t\tbreak;\n+\t\t\tc = *dst--;\n+\t\t\tif (c == '/') {\t/* MinGW: cannot be '\\\\' anymore */\n+\t\t\t\tdst += 2;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\t*dst = '\\0';\n+\treturn 0;\n+}\n+\n /*\n  * path = Canonical absolute path\n  * prefix_list = Colon-separated list of absolute paths\ndiff --git a/setup.c b/setup.c\nindex dfda532..6c2deda 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -4,92 +4,6 @@\n static int inside_git_dir = -1;\n static int inside_work_tree = -1;\n \n-static int sanitary_path_copy(char *dst, const char *src)\n-{\n-\tchar *dst0;\n-\n-\tif (has_dos_drive_prefix(src)) {\n-\t\t*dst++ = *src++;\n-\t\t*dst++ = *src++;\n-\t}\n-\tdst0 = dst;\n-\n-\tif (is_dir_sep(*src)) {\n-\t\t*dst++ = '/';\n-\t\twhile (is_dir_sep(*src))\n-\t\t\tsrc++;\n-\t}\n-\n-\tfor (;;) {\n-\t\tchar c = *src;\n-\n-\t\t/*\n-\t\t * A path component that begins with . could be\n-\t\t * special:\n-\t\t * (1) \".\" and ends   -- ignore and terminate.\n-\t\t * (2) \"./\"           -- ignore them, eat slash and continue.\n-\t\t * (3) \"..\" and ends  -- strip one and terminate.\n-\t\t * (4) \"../\"          -- strip one, eat slash and continue.\n-\t\t */\n-\t\tif (c == '.') {\n-\t\t\tif (!src[1]) {\n-\t\t\t\t/* (1) */\n-\t\t\t\tsrc++;\n-\t\t\t} else if (is_dir_sep(src[1])) {\n-\t\t\t\t/* (2) */\n-\t\t\t\tsrc += 2;\n-\t\t\t\twhile (is_dir_sep(*src))\n-\t\t\t\t\tsrc++;\n-\t\t\t\tcontinue;\n-\t\t\t} else if (src[1] == '.') {\n-\t\t\t\tif (!src[2]) {\n-\t\t\t\t\t/* (3) */\n-\t\t\t\t\tsrc += 2;\n-\t\t\t\t\tgoto up_one;\n-\t\t\t\t} else if (is_dir_sep(src[2])) {\n-\t\t\t\t\t/* (4) */\n-\t\t\t\t\tsrc += 3;\n-\t\t\t\t\twhile (is_dir_sep(*src))\n-\t\t\t\t\t\tsrc++;\n-\t\t\t\t\tgoto up_one;\n-\t\t\t\t}\n-\t\t\t}\n-\t\t}\n-\n-\t\t/* copy up to the next '/', and eat all '/' */\n-\t\twhile ((c = *src++) != '\\0' && !is_dir_sep(c))\n-\t\t\t*dst++ = c;\n-\t\tif (is_dir_sep(c)) {\n-\t\t\t*dst++ = '/';\n-\t\t\twhile (is_dir_sep(c))\n-\t\t\t\tc = *src++;\n-\t\t\tsrc--;\n-\t\t} else if (!c)\n-\t\t\tbreak;\n-\t\tcontinue;\n-\n-\tup_one:\n-\t\t/*\n-\t\t * dst0..dst is prefix portion, and dst[-1] is '/';\n-\t\t * go up one level.\n-\t\t */\n-\t\tdst -= 2; /* go past trailing '/' if any */\n-\t\tif (dst < dst0)\n-\t\t\treturn -1;\n-\t\twhile (1) {\n-\t\t\tif (dst <= dst0)\n-\t\t\t\tbreak;\n-\t\t\tc = *dst--;\n-\t\t\tif (c == '/') {\t/* MinGW: cannot be '\\\\' anymore */\n-\t\t\t\tdst += 2;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n-\t}\n-\t*dst = '\\0';\n-\treturn 0;\n-}\n-\n const char *prefix_path(const char *prefix, int len, const char *path)\n {\n \tconst char *orig = path;\n@@ -101,7 +15,7 @@ const char *prefix_path(const char *prefix, int len, const char *path)\n \t\t\tmemcpy(sanitized, prefix, len);\n \t\tstrcpy(sanitized + len, path);\n \t}\n-\tif (sanitary_path_copy(sanitized, sanitized))\n+\tif (normalize_path_copy(sanitized, sanitized))\n \t\tgoto error_out;\n \tif (is_absolute_path(orig)) {\n \t\tconst char *work_tree = get_git_work_tree();\n-- \n1.6.1.297.g9b01e\n"},{"id":"103629","messageId":"1234019311-6449-4-git-send-email-j6t@kdbg.org","threadId":"17570","inReplyTo":"1234019311-6449-3-git-send-email-j6t@kdbg.org","subject":"[PATCH 3/5] Fix GIT_CEILING_DIRECTORIES on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-07T15:08:29Z","receivedAt":"2009-02-07T15:08:29Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: René Scharfe <rene.scharfe@lsrfire.ath.cx>\n\nUsing git with GIT_CEILING_DIRECTORIES crashed on Windows due to a failed\nassertion in normalize_absolute_path(): This function expects absolute\npaths to start with a slash, while on Windows they can start with a drive\nletter or a backslash.\n\nThis fixes it by using the alternative, normalize_path_copy() instead,\nwhich can handle Windows-style paths just fine.\n\nSecondly, the portability macro PATH_SEP is used instead of expecting\ncolons to be used as path list delimiter.\n\nThe test script t1504 is also changed to help MSYS's bash recognize some\nprogram arguments as path list. (MSYS's bash must translate POSIX-style\npath lists to Windows-style path lists, and the heuristic did not catch\nsome cases.)\n\nSigned-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n\tI have adjusted the patch to fit into the series.\n\tI modified the commit message to pretend that the goal was\n\tto fix a feature instead of the test case ;) Hope that is\n\tfine with you.\n\n\t-- Hannes\n\n path.c                  |   11 ++++++-----\n t/t1504-ceiling-dirs.sh |    6 +++---\n 2 files changed, 9 insertions(+), 8 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex d0a3519..dc3807a 100644\n--- a/path.c\n+++ b/path.c\n@@ -524,15 +524,16 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n \t\treturn -1;\n \n \tfor (colon = ceil = prefix_list; *colon; ceil = colon+1) {\n-\t\tfor (colon = ceil; *colon && *colon != ':'; colon++);\n+\t\tfor (colon = ceil; *colon && *colon != PATH_SEP; colon++);\n \t\tlen = colon - ceil;\n \t\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n \t\t\tcontinue;\n \t\tstrlcpy(buf, ceil, len+1);\n-\t\tlen = normalize_absolute_path(buf, buf);\n-\t\t/* Strip \"trailing slashes\" from \"/\". */\n-\t\tif (len == 1)\n-\t\t\tlen = 0;\n+\t\tif (normalize_path_copy(buf, buf) < 0)\n+\t\t\tcontinue;\n+\t\tlen = strlen(buf);\n+\t\tif (len > 0 && buf[len-1] == '/')\n+\t\t\tbuf[--len] = '\\0';\n \n \t\tif (!strncmp(path, buf, len) &&\n \t\t    path[len] == '/' &&\ndiff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\nindex 91b704a..e377d48 100755\n--- a/t/t1504-ceiling-dirs.sh\n+++ b/t/t1504-ceiling-dirs.sh\n@@ -93,13 +93,13 @@ GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/subdi\"\n test_prefix subdir_ceil_at_subdi_slash \"sub/dir/\"\n \n \n-GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub\"\n+GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub\"\n test_fail second_of_two\n \n-GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:bar\"\n+GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub:/bar\"\n test_fail first_of_two\n \n-GIT_CEILING_DIRECTORIES=\"foo:$TRASH_ROOT/sub:bar\"\n+GIT_CEILING_DIRECTORIES=\"/foo:$TRASH_ROOT/sub:/bar\"\n test_fail second_of_three\n \n \n-- \n1.6.1.297.g9b01e\n"},{"id":"103626","messageId":"1234019311-6449-5-git-send-email-j6t@kdbg.org","threadId":"17570","inReplyTo":"1234019311-6449-4-git-send-email-j6t@kdbg.org","subject":"[PATCH 4/5] Test and fix normalize_path_copy()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-07T15:08:30Z","receivedAt":"2009-02-07T15:08:30Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"This changes the test-path-utils utility to invoke normalize_path_copy()\ninstead of normalize_absolute_path() because the latter is about to be\nremoved.\n\nThe test cases in t0060 are adjusted in two regards:\n\n- normalize_path_copy() more often leaves a trailing slash in the result.\n  This has no negative side effects because the new user of this function,\n  longest_ancester_length(), already accounts for this behavior.\n\n- The function can fail.\n\nThe tests uncover a flaw in normalize_path_copy(): If there are\nsufficiently many '..' path components so that the root is reached, such as\nin \"/d1/s1/../../d2\", then the leading slash was lost. This manifested\nitself that (assuming there is a repository at /tmp/foo)\n\n  $ git add /d1/../tmp/foo/some-file\n\nreported 'pathspec is outside repository'. This is now fixed.\n\nMoreover, the test case descriptions of t0060 now include the test data and\nexpected outcome.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n path.c                |   16 +++++-----------\n t/t0060-path-utils.sh |   33 +++++++++++++++++----------------\n test-path-utils.c     |    7 ++++---\n 3 files changed, 26 insertions(+), 30 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex dc3807a..16628b2 100644\n--- a/path.c\n+++ b/path.c\n@@ -484,18 +484,12 @@ int normalize_path_copy(char *dst, const char *src)\n \t\t * dst0..dst is prefix portion, and dst[-1] is '/';\n \t\t * go up one level.\n \t\t */\n-\t\tdst -= 2; /* go past trailing '/' if any */\n-\t\tif (dst < dst0)\n+\t\tdst--;\t/* go to trailing '/' */\n+\t\tif (dst <= dst0)\n \t\t\treturn -1;\n-\t\twhile (1) {\n-\t\t\tif (dst <= dst0)\n-\t\t\t\tbreak;\n-\t\t\tc = *dst--;\n-\t\t\tif (c == '/') {\t/* MinGW: cannot be '\\\\' anymore */\n-\t\t\t\tdst += 2;\n-\t\t\t\tbreak;\n-\t\t\t}\n-\t\t}\n+\t\t/* Windows: dst[-1] cannot be backslash anymore */\n+\t\twhile (dst0 < dst && dst[-1] != '/')\n+\t\t\tdst--;\n \t}\n \t*dst = '\\0';\n \treturn 0;\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 6e7501f..4ed1f0b 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -8,36 +8,37 @@ test_description='Test various path utilities'\n . ./test-lib.sh\n \n norm_abs() {\n-\ttest_expect_success \"normalize absolute\" \\\n-\t\"test \\$(test-path-utils normalize_absolute_path '$1') = '$2'\"\n+\ttest_expect_success \"normalize absolute: $1 => $2\" \\\n+\t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$2'\"\n }\n \n ancestor() {\n-\ttest_expect_success \"longest ancestor\" \\\n-\t\"test \\$(test-path-utils longest_ancestor_length '$1' '$2') = '$3'\"\n+\ttest_expect_success \"longest ancestor: $1 $2 => $3\" \\\n+\t\"test \\\"\\$(test-path-utils longest_ancestor_length '$1' '$2')\\\" = '$3'\"\n }\n \n-norm_abs \"\" /\n+norm_abs \"\" \"\"\n norm_abs / /\n norm_abs // /\n norm_abs /// /\n norm_abs /. /\n norm_abs /./ /\n-norm_abs /./.. /\n-norm_abs /../. /\n-norm_abs /./../.// /\n+norm_abs /./.. ++failed++\n+norm_abs /../. ++failed++\n+norm_abs /./../.// ++failed++\n norm_abs /dir/.. /\n norm_abs /dir/sub/../.. /\n+norm_abs /dir/sub/../../.. ++failed++\n norm_abs /dir /dir\n-norm_abs /dir// /dir\n+norm_abs /dir// /dir/\n norm_abs /./dir /dir\n-norm_abs /dir/. /dir\n-norm_abs /dir///./ /dir\n-norm_abs /dir//sub/.. /dir\n-norm_abs /dir/sub/../ /dir\n-norm_abs //dir/sub/../. /dir\n-norm_abs /dir/s1/../s2/ /dir/s2\n-norm_abs /d1/s1///s2/..//../s3/ /d1/s3\n+norm_abs /dir/. /dir/\n+norm_abs /dir///./ /dir/\n+norm_abs /dir//sub/.. /dir/\n+norm_abs /dir/sub/../ /dir/\n+norm_abs //dir/sub/../. /dir/\n+norm_abs /dir/s1/../s2/ /dir/s2/\n+norm_abs /d1/s1///s2/..//../s3/ /d1/s3/\n norm_abs /d1/s1//../s2/../../d2 /d2\n norm_abs /d1/.../d2 /d1/.../d2\n norm_abs /d1/..././../d2 /d1/d2\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 7e6fc8d..5168a8e 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -2,10 +2,11 @@\n \n int main(int argc, char **argv)\n {\n-\tif (argc == 3 && !strcmp(argv[1], \"normalize_absolute_path\")) {\n+\tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n \t\tchar *buf = xmalloc(PATH_MAX + 1);\n-\t\tint rv = normalize_absolute_path(buf, argv[2]);\n-\t\tassert(strlen(buf) == rv);\n+\t\tint rv = normalize_path_copy(buf, argv[2]);\n+\t\tif (rv)\n+\t\t\tbuf = \"++failed++\";\n \t\tputs(buf);\n \t\treturn 0;\n \t}\n-- \n1.6.1.297.g9b01e\n"},{"id":"103630","messageId":"1234019311-6449-6-git-send-email-j6t@kdbg.org","threadId":"17570","inReplyTo":"1234019311-6449-5-git-send-email-j6t@kdbg.org","subject":"[PATCH 5/5] Remove unused normalize_absolute_path()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-07T15:08:31Z","receivedAt":"2009-02-07T15:08:31Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"This function is now superseded by normalize_path_copy().\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n cache.h |    1 -\n path.c  |   51 ++++++---------------------------------------------\n 2 files changed, 6 insertions(+), 46 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 82fbb80..8dcc53c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -627,7 +627,6 @@ int is_directory(const char *);\n const char *make_absolute_path(const char *path);\n const char *make_nonrelative_path(const char *path);\n const char *make_relative_path(const char *abs, const char *base);\n-int normalize_absolute_path(char *buf, const char *path);\n int normalize_path_copy(char *dst, const char *src);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n \ndiff --git a/path.c b/path.c\nindex 16628b2..dd22370 100644\n--- a/path.c\n+++ b/path.c\n@@ -363,58 +363,19 @@ const char *make_relative_path(const char *abs, const char *base)\n }\n \n /*\n- * path = absolute path\n- * buf = buffer of at least max(2, strlen(path)+1) bytes\n- * It is okay if buf == path, but they should not overlap otherwise.\n+ * It is okay if dst == src, but they should not overlap otherwise.\n  *\n- * Performs the following normalizations on path, storing the result in buf:\n- * - Removes trailing slashes.\n- * - Removes empty components.\n+ * Performs the following normalizations on src, storing the result in dst:\n+ * - Ensures that components are separated by '/' (Windows only)\n+ * - Squashes sequences of '/'.\n  * - Removes \".\" components.\n  * - Removes \"..\" components, and the components the precede them.\n- * \"\" and paths that contain only slashes are normalized to \"/\".\n- * Returns the length of the output.\n+ * Returns failure (non-zero) if a \"..\" component appears as first path\n+ * component anytime during the normalization. Otherwise, returns success (0).\n  *\n  * Note that this function is purely textual.  It does not follow symlinks,\n  * verify the existence of the path, or make any system calls.\n  */\n-int normalize_absolute_path(char *buf, const char *path)\n-{\n-\tconst char *comp_start = path, *comp_end = path;\n-\tchar *dst = buf;\n-\tint comp_len;\n-\tassert(buf);\n-\tassert(path);\n-\n-\twhile (*comp_start) {\n-\t\tassert(*comp_start == '/');\n-\t\twhile (*++comp_end && *comp_end != '/')\n-\t\t\t; /* nothing */\n-\t\tcomp_len = comp_end - comp_start;\n-\n-\t\tif (!strncmp(\"/\",  comp_start, comp_len) ||\n-\t\t    !strncmp(\"/.\", comp_start, comp_len))\n-\t\t\tgoto next;\n-\n-\t\tif (!strncmp(\"/..\", comp_start, comp_len)) {\n-\t\t\twhile (dst > buf && *--dst != '/')\n-\t\t\t\t; /* nothing */\n-\t\t\tgoto next;\n-\t\t}\n-\n-\t\tmemmove(dst, comp_start, comp_len);\n-\t\tdst += comp_len;\n-\tnext:\n-\t\tcomp_start = comp_end;\n-\t}\n-\n-\tif (dst == buf)\n-\t\t*dst++ = '/';\n-\n-\t*dst = '\\0';\n-\treturn dst - buf;\n-}\n-\n int normalize_path_copy(char *dst, const char *src)\n {\n \tchar *dst0;\n-- \n1.6.1.297.g9b01e\n"},{"id":"103673","messageId":"200902080108.50550.robin.rosenberg.lists@dewire.com","threadId":"17570","inReplyTo":"1234019311-6449-5-git-send-email-j6t@kdbg.org","subject":"Re: [PATCH 4/5] Test and fix normalize_path_copy()","fromName":"Robin Rosenberg","fromEmail":"robin.rosenberg.lists@dewire.com","sentAt":"2009-02-08T00:08:50Z","receivedAt":"2009-02-08T00:08:50Z","isPatch":true,"sender":{"key":"robin.rosenberg@dewire.com","avatar":"https://avatars.githubusercontent.com/u/46357?v=4"},"body":"\nFor Windows leading // is special indicating a UNC\npath and should not be changed to /.\n\n//foo/bar is different from /foo/bar\n\n-- robin\n"},{"id":"103701","messageId":"200902080952.53478.j6t@kdbg.org","threadId":"17570","inReplyTo":"200902080108.50550.robin.rosenberg.lists@dewire.com","subject":"Re: [PATCH 4/5] Test and fix normalize_path_copy()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-08T08:52:53Z","receivedAt":"2009-02-08T08:52:53Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Sonntag, 8. Februar 2009, Robin Rosenberg wrote:\n> For Windows leading // is special indicating a UNC\n> path and should not be changed to /.\n>\n> //foo/bar is different from /foo/bar\n\nIndeed. But stripping the double-dash would bite you only if you are using \nGIT_CEILING_DIRECTORIES or if you tend to call eg. git add with absolute \npaths; and that only if your repository lives in a UNC share, which is a \nrather odd use-case.\n\nIn particular, it does *not* affect fetch and push etc. so that you can have a \nremote repository on an UNC share.\n\n-- Hannes\n"},{"id":"103753","messageId":"498EF028.3040702@lsrfire.ath.cx","threadId":"17570","inReplyTo":"1234019311-6449-5-git-send-email-j6t@kdbg.org","subject":"Re: [PATCH 4/5] Test and fix normalize_path_copy()","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-02-08T14:46:00Z","receivedAt":"2009-02-08T14:46:00Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Johannes Sixt schrieb:\n> Moreover, the test case descriptions of t0060 now include the test data and\n> expected outcome.\n\nThe test still fails in a lot of cases for me, because \"/\" is translated to\nthe installation path of msys.  E.g. with the patch at the end, which shows\nthe results in addition to input and expected output, I get several cases\nlike this (the first test call checks the return code):\n\n* FAIL 2: normalize absolute: '/' => '/'\n\ttest 0 = 0 && test 'C:/Program Files (x86)/msysgit/msysgit' = '/'\n\n* FAIL 32: longest ancestor: '/foo' '/' => '0'\n\ttest 0 = 0 && test '38' = '0'\n\nI'm not sure what to do about it.  Should test-path-utils translate it back\ninto \"root is / is root\"?  Should the test script look up \"/\" at the top\nand just append this value to all paths?  Unpretty.\n\n\nAnd then there's this:\n\n* FAIL 7: normalize absolute: '/./..' => '++failed++'\n\ttest 0 = 0 && test 'C:/Program Files (x86)/msysgit/msysgit' = '++failed++'\n*   ok 8: normalize absolute: '/../.' => '++failed++'\n\nA printf() added to the top of normalize_path_copy() tells me that you can\nadd as many \"/..\" as you want after a path starting with \"/.\", it will\nalways translate into the install directory of msys regardless.\n\n\nAnyway, here's the patch mentioned above:\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 4ed1f0b..d5a38a6 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -8,13 +8,17 @@ test_description='Test various path utilities'\n . ./test-lib.sh\n \n norm_abs() {\n-\ttest_expect_success \"normalize absolute: $1 => $2\" \\\n-\t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$2'\"\n+\tresult=$(test-path-utils normalize_path_copy \"$1\")\n+\trc=$?\n+\ttest_expect_success \"normalize absolute: '$1' => '$2'\" \\\n+\t\"test $rc = 0 && test '$result' = '$2'\"\n }\n \n ancestor() {\n-\ttest_expect_success \"longest ancestor: $1 $2 => $3\" \\\n-\t\"test \\\"\\$(test-path-utils longest_ancestor_length '$1' '$2')\\\" = '$3'\"\n+\tresult=$(test-path-utils longest_ancestor_length \"$1\" \"$2\")\n+\trc=$?\n+\ttest_expect_success \"longest ancestor: '$1' '$2' => '$3'\" \\\n+\t\"test $rc = 0 && test '$result' = '$3'\"\n }\n \n norm_abs \"\" \"\"\n"},{"id":"103757","messageId":"200902081650.22068.j6t@kdbg.org","threadId":"17570","inReplyTo":"498EF028.3040702@lsrfire.ath.cx","subject":"Re: [PATCH 4/5] Test and fix normalize_path_copy()","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-02-08T15:50:21Z","receivedAt":"2009-02-08T15:50:21Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Sonntag, 8. Februar 2009, René Scharfe wrote:\n> Johannes Sixt schrieb:\n> > Moreover, the test case descriptions of t0060 now include the test data\n> > and expected outcome.\n>\n> The test still fails in a lot of cases for me, because \"/\" is translated to\n> the installation path of msys.\n\nTo fix t0060 on Windows was not even on the radar of this series.\n\nThat said, I'm slowly working on a series that fixes or skips many tests on \nWindows. I currently have the patch below in my tree.\n\n-- Hannes\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 4ed1f0b..dd2d241 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -7,41 +7,89 @@ test_description='Test various path utilities'\n \n . ./test-lib.sh\n \n-norm_abs() {\n-\ttest_expect_success \"normalize absolute: $1 => $2\" \\\n+norm_path() {\n+\ttest_expect_success \"normalize path: $1 => $2\" \\\n \t\"test \\\"\\$(test-path-utils normalize_path_copy '$1')\\\" = '$2'\"\n }\n+xnorm_path() { say fix this: \"$*\"; }\n+\n+# On Windows, we are using MSYS's bash, which mangles the paths.\n+# Absolute paths are anchored at the MSYS installation directory,\n+# which means that the path / accounts for this many characters:\n+rootoff=$(test-path-utils normalize_path_copy / | wc -c)\n+# Account for the trailing LF:\n+if test \"$rootoff\" = 2; then\n+\trootoff=\t# we are on Unix\n+else\n+\trootoff=$(($rootoff-1))\n+fi\n \n ancestor() {\n-\ttest_expect_success \"longest ancestor: $1 $2 => $3\" \\\n-\t\"test \\\"\\$(test-path-utils longest_ancestor_length '$1' '$2')\\\" = '$3'\"\n+\t# We do some math with the expected ancestor length.\n+\texpected=$3\n+\tif test -n \"$rootoff\" && ! test \"$expected\" = -1; then\n+\t\texpected=$(($expected+$rootoff))\n+\tfi\n+\ttest_expect_success \"longest ancestor: $1 $2 => $expected\" \\\n+\t\"actual=\\$(test-path-utils longest_ancestor_length '$1' '$2') &&\n+\t test \\\"\\$actual\\\" = '$expected'\"\n }\n \n-norm_abs \"\" \"\"\n-norm_abs / /\n-norm_abs // /\n-norm_abs /// /\n-norm_abs /. /\n-norm_abs /./ /\n-norm_abs /./.. ++failed++\n-norm_abs /../. ++failed++\n-norm_abs /./../.// ++failed++\n-norm_abs /dir/.. /\n-norm_abs /dir/sub/../.. /\n-norm_abs /dir/sub/../../.. ++failed++\n-norm_abs /dir /dir\n-norm_abs /dir// /dir/\n-norm_abs /./dir /dir\n-norm_abs /dir/. /dir/\n-norm_abs /dir///./ /dir/\n-norm_abs /dir//sub/.. /dir/\n-norm_abs /dir/sub/../ /dir/\n-norm_abs //dir/sub/../. /dir/\n-norm_abs /dir/s1/../s2/ /dir/s2/\n-norm_abs /d1/s1///s2/..//../s3/ /d1/s3/\n-norm_abs /d1/s1//../s2/../../d2 /d2\n-norm_abs /d1/.../d2 /d1/.../d2\n-norm_abs /d1/..././../d2 /d1/d2\n+do_abs=t\n+case $(uname -s) in\n+*MINGW*) do_abs=\n+esac\n+\n+norm_path \"\" \"\"\n+norm_path . \"\"\n+norm_path ./ \"\"\n+norm_path ./. \"\"\n+norm_path .. ++failed++\n+norm_path ../sub ++failed++\n+norm_path dir/.. \"\"\n+norm_path dir/sub/../.. \"\"\n+norm_path dir/sub/../../.. ++failed++\n+norm_path dir dir\n+norm_path dir// dir/\n+norm_path ./dir dir\n+norm_path dir/. dir/\n+norm_path dir///./ dir/\n+norm_path dir//sub/.. dir/\n+norm_path dir/sub/../ dir/\n+norm_path dir/sub/../. dir/\n+norm_path dir/s1/../s2/ dir/s2/\n+norm_path d1/s1///s2/..//../s3/ d1/s3/\n+norm_path d1/s1//../s2/../../d2 d2\n+norm_path d1/.../d2 d1/.../d2\n+norm_path d1/..././../d2 d1/d2\n+\n+if test -n \"$do_abs\"; then\n+norm_path \"\" \"\"\n+norm_path / /\n+norm_path // /\n+norm_path /// /\n+norm_path /. /\n+norm_path /./ /\n+norm_path /./.. ++failed++\n+norm_path /../. ++failed++\n+norm_path /./../.// ++failed++\n+norm_path /dir/.. /\n+norm_path /dir/sub/../.. /\n+norm_path /dir/sub/../../.. ++failed++\n+norm_path /dir /dir\n+norm_path /dir// /dir/\n+norm_path /./dir /dir\n+norm_path /dir/. /dir/\n+norm_path /dir///./ /dir/\n+norm_path /dir//sub/.. /dir/\n+norm_path /dir/sub/../ /dir/\n+norm_path //dir/sub/../. /dir/\n+norm_path /dir/s1/../s2/ /dir/s2/\n+norm_path /d1/s1///s2/..//../s3/ /d1/s3/\n+norm_path /d1/s1//../s2/../../d2 /d2\n+norm_path /d1/.../d2 /d1/.../d2\n+norm_path /d1/..././../d2 /d1/d2\n+fi\n \n ancestor / \"\" -1\n ancestor / / -1\n@@ -80,9 +128,9 @@ ancestor /foo/bar /:/foo:/bar/ 4\n ancestor /foo/bar /foo:/:/bar/ 4\n ancestor /foo/bar /:/bar/:/fo 0\n ancestor /foo/bar /:/bar/ 0\n-ancestor /foo/bar :://foo/. 4\n-ancestor /foo/bar :://foo/.:: 4\n-ancestor /foo/bar //foo/./::/bar 4\n-ancestor /foo/bar ::/bar -1\n+ancestor /foo/bar .:/foo/. 4\n+ancestor /foo/bar .:/foo/.:.: 4\n+ancestor /foo/bar /foo/./:.:/bar 4\n+ancestor /foo/bar .:/bar -1\n \n test_done\n"}]}