{"thread":{"id":"31462","subject":"[PATCH v2 0/8] Fix some bugs in abspath.c","startedAt":"2012-09-06T22:40:56Z","lastAt":"2012-09-10T11:52:50Z","messageCount":14,"participants":["Michael Haggerty","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":2,"patchTotal":8},"messages":[{"id":"198486","messageId":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":null,"subject":"[PATCH v2 0/8] Fix some bugs in abspath.c","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:40:56Z","receivedAt":"2012-09-06T22:40:56Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Second version of patches, incorporating gratefully received advice\nfrom Hannes:\n\n* Moved existing tests of real_path() from t0000 to t0060, and add new\n  tests to t0060 instead of t0000.\n\n* Adjusted the regexp used to construct a top-level directory that is\n  needed for tests.\n\n* Skip some tests when the POSIX prerequisite is not satisfied.\n\nMichael Haggerty (8):\n  t0060: move tests of real_path() from t0000 to here\n  t0060: verify that absolute_path() fails if passed the empty string\n  absolute_path(): reject the empty string\n  t0060: verify that real_path() fails if passed the empty string\n  real_path(): reject the empty string\n  t0060: verify that real_path() works correctly with absolute paths\n  real_path(): properly handle nonexistent top-level paths\n  t0060: verify that real_path() removes extra slashes\n\n abspath.c             |  9 +++++++--\n t/t0000-basic.sh      | 18 -----------------\n t/t0060-path-utils.sh | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 62 insertions(+), 20 deletions(-)\n\n-- \n1.7.11.3\n"},{"id":"198487","messageId":"1346971264-23744-2-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 1/8] t0060: move tests of real_path() from t0000 to here","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:40:57Z","receivedAt":"2012-09-06T22:40:57Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Suggested by: Johannes Sixt <j6t@kdbg.org>\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n t/t0000-basic.sh      | 18 ------------------\n t/t0060-path-utils.sh | 19 +++++++++++++++++++\n 2 files changed, 19 insertions(+), 18 deletions(-)\n\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex ccb5435..ae6a3f0 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -450,24 +450,6 @@ test_expect_success 'update-index D/F conflict' '\n \ttest $numpath0 = 1\n '\n \n-test_expect_success SYMLINKS 'real path works as expected' '\n-\tmkdir first &&\n-\tln -s ../.git first/.git &&\n-\tmkdir second &&\n-\tln -s ../first second/other &&\n-\tmkdir third &&\n-\tdir=\"$(cd .git; pwd -P)\" &&\n-\tdir2=third/../second/other/.git &&\n-\ttest \"$dir\" = \"$(test-path-utils real_path $dir2)\" &&\n-\tfile=\"$dir\"/index &&\n-\ttest \"$file\" = \"$(test-path-utils real_path $dir2/index)\" &&\n-\tbasename=blub &&\n-\ttest \"$dir/$basename\" = \"$(cd .git && test-path-utils real_path \"$basename\")\" &&\n-\tln -s ../first/file .git/syml &&\n-\tsym=\"$(cd first; pwd -P)\"/file &&\n-\ttest \"$sym\" = \"$(test-path-utils real_path \"$dir2/syml\")\"\n-'\n-\n test_expect_success 'very long name in the index handled sanely' '\n \n \ta=a && # 1\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 53cf1f8..c8db144 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -139,4 +139,23 @@ test_expect_success 'strip_path_suffix' '\n \ttest c:/msysgit = $(test-path-utils strip_path_suffix \\\n \t\tc:/msysgit/libexec//git-core libexec/git-core)\n '\n+\n+test_expect_success SYMLINKS 'real path works as expected' '\n+\tmkdir first &&\n+\tln -s ../.git first/.git &&\n+\tmkdir second &&\n+\tln -s ../first second/other &&\n+\tmkdir third &&\n+\tdir=\"$(cd .git; pwd -P)\" &&\n+\tdir2=third/../second/other/.git &&\n+\ttest \"$dir\" = \"$(test-path-utils real_path $dir2)\" &&\n+\tfile=\"$dir\"/index &&\n+\ttest \"$file\" = \"$(test-path-utils real_path $dir2/index)\" &&\n+\tbasename=blub &&\n+\ttest \"$dir/$basename\" = \"$(cd .git && test-path-utils real_path \"$basename\")\" &&\n+\tln -s ../first/file .git/syml &&\n+\tsym=\"$(cd first; pwd -P)\"/file &&\n+\ttest \"$sym\" = \"$(test-path-utils real_path \"$dir2/syml\")\"\n+'\n+\n test_done\n-- \n1.7.11.3\n"},{"id":"198489","messageId":"1346971264-23744-3-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 2/8] t0060: verify that absolute_path() fails if passed the empty string","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:40:58Z","receivedAt":"2012-09-06T22:40:58Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It doesn't, so mark the test as failing.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n t/t0060-path-utils.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex c8db144..d91e516 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -140,6 +140,10 @@ test_expect_success 'strip_path_suffix' '\n \t\tc:/msysgit/libexec//git-core libexec/git-core)\n '\n \n+test_expect_failure 'absolute path rejects the empty string' '\n+\ttest_must_fail test-path-utils absolute_path \"\"\n+'\n+\n test_expect_success SYMLINKS 'real path works as expected' '\n \tmkdir first &&\n \tln -s ../.git first/.git &&\n-- \n1.7.11.3\n"},{"id":"198488","messageId":"1346971264-23744-4-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 3/8] absolute_path(): reject the empty string","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:40:59Z","receivedAt":"2012-09-06T22:40:59Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n abspath.c             | 4 +++-\n t/t0060-path-utils.sh | 2 +-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex f04ac18..5d62430 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -123,7 +123,9 @@ const char *absolute_path(const char *path)\n {\n \tstatic char buf[PATH_MAX + 1];\n \n-\tif (is_absolute_path(path)) {\n+\tif (!*path) {\n+\t\tdie(\"The empty string is not a valid path\");\n+\t} else if (is_absolute_path(path)) {\n \t\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n \t\t\tdie(\"Too long path: %.*s\", 60, path);\n \t} else {\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex d91e516..924aa60 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -140,7 +140,7 @@ test_expect_success 'strip_path_suffix' '\n \t\tc:/msysgit/libexec//git-core libexec/git-core)\n '\n \n-test_expect_failure 'absolute path rejects the empty string' '\n+test_expect_success 'absolute path rejects the empty string' '\n \ttest_must_fail test-path-utils absolute_path \"\"\n '\n \n-- \n1.7.11.3\n"},{"id":"198490","messageId":"1346971264-23744-5-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 4/8] t0060: verify that real_path() fails if passed the empty string","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:41:00Z","receivedAt":"2012-09-06T22:41:00Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It doesn't, so mark the test as failing.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n t/t0060-path-utils.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 924aa60..1118056 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -144,6 +144,10 @@ test_expect_success 'absolute path rejects the empty string' '\n \ttest_must_fail test-path-utils absolute_path \"\"\n '\n \n+test_expect_failure 'real path rejects the empty string' '\n+\ttest_must_fail test-path-utils real_path \"\"\n+'\n+\n test_expect_success SYMLINKS 'real path works as expected' '\n \tmkdir first &&\n \tln -s ../.git first/.git &&\n-- \n1.7.11.3\n"},{"id":"198492","messageId":"1346971264-23744-6-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 5/8] real_path(): reject the empty string","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:41:01Z","receivedAt":"2012-09-06T22:41:01Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n abspath.c             | 3 +++\n t/t0060-path-utils.sh | 2 +-\n 2 files changed, 4 insertions(+), 1 deletion(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 5d62430..3e8325c 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -35,6 +35,9 @@ const char *real_path(const char *path)\n \tif (path == buf || path == next_buf)\n \t\treturn path;\n \n+\tif (!*path)\n+\t\tdie(\"The empty string is not a valid path\");\n+\n \tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n \t\tdie (\"Too long path: %.*s\", 60, path);\n \ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 1118056..fab5ea2 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -144,7 +144,7 @@ test_expect_success 'absolute path rejects the empty string' '\n \ttest_must_fail test-path-utils absolute_path \"\"\n '\n \n-test_expect_failure 'real path rejects the empty string' '\n+test_expect_success 'real path rejects the empty string' '\n \ttest_must_fail test-path-utils real_path \"\"\n '\n \n-- \n1.7.11.3\n"},{"id":"198494","messageId":"1346971264-23744-7-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 6/8] t0060: verify that real_path() works correctly with absolute paths","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:41:02Z","receivedAt":"2012-09-06T22:41:02Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"There is currently a bug: if passed an absolute top-level path that\ndoesn't exist (e.g., \"/foo\") it incorrectly interprets the path as a\nrelative path (e.g., returns \"$(pwd)/foo\").  So mark the test as\nfailing.\n\nThese tests are skipped on Windows because test-path-utils operates on\na DOS-style absolute path even if a POSIX style absolute path is\npassed as argument.\n\nAdjusted for Windows by: Johannes Sixt <j6t@kdbg.org>\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n t/t0060-path-utils.sh | 12 +++++++++++-\n 1 file changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex fab5ea2..3121691 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -148,7 +148,17 @@ test_expect_success 'real path rejects the empty string' '\n \ttest_must_fail test-path-utils real_path \"\"\n '\n \n-test_expect_success SYMLINKS 'real path works as expected' '\n+test_expect_failure POSIX 'real path works on absolute paths' '\n+\tnopath=\"hopefully-absent-path\" &&\n+\ttest \"/\" = \"$(test-path-utils real_path \"/\")\" &&\n+\ttest \"/$nopath\" = \"$(test-path-utils real_path \"/$nopath\")\" &&\n+\t# Find an existing top-level directory for the remaining tests:\n+\td=$(pwd -P | sed -e \"s|^\\([^/]*/[^/]*\\)/.*|\\1|\") &&\n+\ttest \"$d\" = \"$(test-path-utils real_path \"$d\")\" &&\n+\ttest \"$d/$nopath\" = \"$(test-path-utils real_path \"$d/$nopath\")\"\n+'\n+\n+test_expect_success SYMLINKS 'real path works on symlinks' '\n \tmkdir first &&\n \tln -s ../.git first/.git &&\n \tmkdir second &&\n-- \n1.7.11.3\n"},{"id":"198493","messageId":"1346971264-23744-8-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 7/8] real_path(): properly handle nonexistent top-level paths","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:41:03Z","receivedAt":"2012-09-06T22:41:03Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The change has two points:\n\n1. Do not strip off a leading slash, because that erroneously turns an\n   absolute path into a relative path.\n\n2. Do not remove slashes from groups of multiple slashes; instead let\n   chdir() handle them.  It could be, for example, that it wants to\n   leave leading double-slashes alone.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n abspath.c             | 2 +-\n t/t0060-path-utils.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 3e8325c..05f2d79 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -45,8 +45,8 @@ const char *real_path(const char *path)\n \t\tif (!is_directory(buf)) {\n \t\t\tchar *last_slash = find_last_dir_sep(buf);\n \t\t\tif (last_slash) {\n-\t\t\t\t*last_slash = '\\0';\n \t\t\t\tlast_elem = xstrdup(last_slash + 1);\n+\t\t\t\tlast_slash[1] = '\\0';\n \t\t\t} else {\n \t\t\t\tlast_elem = xstrdup(buf);\n \t\t\t\t*buf = '\\0';\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 3121691..30361f9 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -148,7 +148,7 @@ test_expect_success 'real path rejects the empty string' '\n \ttest_must_fail test-path-utils real_path \"\"\n '\n \n-test_expect_failure POSIX 'real path works on absolute paths' '\n+test_expect_success POSIX 'real path works on absolute paths' '\n \tnopath=\"hopefully-absent-path\" &&\n \ttest \"/\" = \"$(test-path-utils real_path \"/\")\" &&\n \ttest \"/$nopath\" = \"$(test-path-utils real_path \"/$nopath\")\" &&\n-- \n1.7.11.3\n"},{"id":"198491","messageId":"1346971264-23744-9-git-send-email-mhagger@alum.mit.edu","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v2 8/8] t0060: verify that real_path() removes extra slashes","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-06T22:41:04Z","receivedAt":"2012-09-06T22:41:04Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Adjusted for Windows by: Johannes Sixt <j6t@kdbg.org>\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n t/t0060-path-utils.sh | 18 ++++++++++++++++++\n 1 file changed, 18 insertions(+)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 30361f9..e40f764 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -158,6 +158,24 @@ test_expect_success POSIX 'real path works on absolute paths' '\n \ttest \"$d/$nopath\" = \"$(test-path-utils real_path \"$d/$nopath\")\"\n '\n \n+test_expect_success POSIX 'real path removes extra leading slashes' '\n+\tnopath=\"hopefully-absent-path\" &&\n+\ttest \"/\" = \"$(test-path-utils real_path \"///\")\" &&\n+\ttest \"/$nopath\" = \"$(test-path-utils real_path \"///$nopath\")\" &&\n+\t# Find an existing top-level directory for the remaining tests:\n+\td=$(pwd -P | sed -e \"s|^\\([^/]*/[^/]*\\)/.*|\\1|\") &&\n+\ttest \"$d\" = \"$(test-path-utils real_path \"//$d\")\" &&\n+\ttest \"$d/$nopath\" = \"$(test-path-utils real_path \"//$d/$nopath\")\"\n+'\n+\n+test_expect_success 'real path removes other extra slashes' '\n+\tnopath=\"hopefully-absent-path\" &&\n+\t# Find an existing top-level directory for the remaining tests:\n+\td=$(pwd -P | sed -e \"s|^\\([^/]*/[^/]*\\)/.*|\\1|\") &&\n+\ttest \"$d\" = \"$(test-path-utils real_path \"$d///\")\" &&\n+\ttest \"$d/$nopath\" = \"$(test-path-utils real_path \"$d///$nopath\")\"\n+'\n+\n test_expect_success SYMLINKS 'real path works on symlinks' '\n \tmkdir first &&\n \tln -s ../.git first/.git &&\n-- \n1.7.11.3\n"},{"id":"198496","messageId":"7vy5kmzr3e.fsf@alter.siamese.dyndns.org","threadId":"31462","inReplyTo":"1346971264-23744-4-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v2 3/8] absolute_path(): reject the empty string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-06T23:09:09Z","receivedAt":"2012-09-06T23:09:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n> ---\n\nI think I asked why this matters (iow, why it is the right thing to\ndo to reject an empty string, instead of treating it as \"the current\ndirectory\") in the previous round.  I would have expected to find\nthe answer be above the S-o-b line here.\n\nThe same comment applies to the other patch for real_path().\n\n>  abspath.c             | 4 +++-\n>  t/t0060-path-utils.sh | 2 +-\n>  2 files changed, 4 insertions(+), 2 deletions(-)\n>\n> diff --git a/abspath.c b/abspath.c\n> index f04ac18..5d62430 100644\n> --- a/abspath.c\n> +++ b/abspath.c\n> @@ -123,7 +123,9 @@ const char *absolute_path(const char *path)\n>  {\n>  \tstatic char buf[PATH_MAX + 1];\n>  \n> -\tif (is_absolute_path(path)) {\n> +\tif (!*path) {\n> +\t\tdie(\"The empty string is not a valid path\");\n> +\t} else if (is_absolute_path(path)) {\n\n>  \t\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n>  \t\t\tdie(\"Too long path: %.*s\", 60, path);\n>  \t} else {\n> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n> index d91e516..924aa60 100755\n> --- a/t/t0060-path-utils.sh\n> +++ b/t/t0060-path-utils.sh\n> @@ -140,7 +140,7 @@ test_expect_success 'strip_path_suffix' '\n>  \t\tc:/msysgit/libexec//git-core libexec/git-core)\n>  '\n>  \n> -test_expect_failure 'absolute path rejects the empty string' '\n> +test_expect_success 'absolute path rejects the empty string' '\n>  \ttest_must_fail test-path-utils absolute_path \"\"\n>  '\n"},{"id":"198592","messageId":"504C1F86.4060608@alum.mit.edu","threadId":"31462","inReplyTo":"7vy5kmzr3e.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2 3/8] absolute_path(): reject the empty string","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-09T04:48:06Z","receivedAt":"2012-09-09T04:48:06Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 09/07/2012 01:09 AM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n>> ---\n> \n> I think I asked why this matters (iow, why it is the right thing to\n> do to reject an empty string, instead of treating it as \"the current\n> directory\") in the previous round.  I would have expected to find\n> the answer be above the S-o-b line here.\n\nThe reasons that the change is desirable:\n\n1. The empty string is not a legitimate path according to POSIX; e.g.,\nsee Linux's path_resolution(7):\n\n   Empty pathname\n       In the original UNIX, the empty pathname referred to the current\n       directory.   Nowadays  POSIX  decrees  that  an  empty  pathname\n       must not be resolved successfully.  Linux returns ENOENT in this\n       case.\n\nAccordingly, comparable standard functions like realpath(3) reject the\nempty string.\n\n2. The functions did not handle the empty path consistently with the way\nthey handled other paths (namely, the return value contained a trailing\nslash).\n\n3. This unusual behavior was undocumented.\n\nThe above points let me to the conclusion that the anomalous handling of\nthe empty string was a bug in the implementation rather than an intended\nbehavior.  Moreover, a quick check of callers didn't turn up any that\nseemed to rely on the strange behavior.\n\nDo you want a re-roll with this verbiage added to the commit messages of\nthe two relevant commits?\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"198594","messageId":"7vbohfu6yg.fsf@alter.siamese.dyndns.org","threadId":"31462","inReplyTo":"504C1F86.4060608@alum.mit.edu","subject":"Re: [PATCH v2 3/8] absolute_path(): reject the empty string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-09-09T04:59:51Z","receivedAt":"2012-09-09T04:59:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> On 09/07/2012 01:09 AM, Junio C Hamano wrote:\n>> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>> \n>>> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n>>> ---\n>> \n>> I think I asked why this matters (iow, why it is the right thing to\n>> do to reject an empty string, instead of treating it as \"the current\n>> directory\") in the previous round.  I would have expected to find\n>> the answer be above the S-o-b line here.\n>\n> The reasons that the change is desirable:\n>\n> 1. The empty string is not a legitimate path according to POSIX; e.g.,\n> see Linux's path_resolution(7):\n>\n>    Empty pathname\n>        In the original UNIX, the empty pathname referred to the current\n>        directory.   Nowadays  POSIX  decrees  that  an  empty  pathname\n>        must not be resolved successfully.  Linux returns ENOENT in this\n>        case.\n>\n> Accordingly, comparable standard functions like realpath(3) reject the\n> empty string.\n>\n> 2. The functions did not handle the empty path consistently with the way\n> they handled other paths (namely, the return value contained a trailing\n> slash).\n>\n> 3. This unusual behavior was undocumented.\n>\n> The above points let me to the conclusion that the anomalous handling of\n> the empty string was a bug in the implementation rather than an intended\n> behavior.  Moreover, a quick check of callers didn't turn up any that\n> seemed to rely on the strange behavior.\n>\n> Do you want a re-roll with this verbiage added to the commit messages of\n> the two relevant commits?\n\nWhat the function used to do does not really matter when we are\ntrying to see if the behaviour of the new implementation makes\nsense, even though it is a good supporting argument to help us judge\nthat this change will not cause regressions to existing callers.\n\nA two sentence paragraph, with \"realpath(3) from POSIX.1 rejects an\nempty string as input\" as the primary justification, with \"no\nexisting callers passes an empty string and expects to get the\ncurrent directory\" as a supporting argument, should suffice, I would\nthink.\n"},{"id":"198632","messageId":"504CB8DC.90202@kdbg.org","threadId":"31462","inReplyTo":"1346971264-23744-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH 9/8] t0060: split absolute path test in two to exercise some of it on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-09-09T15:42:20Z","receivedAt":"2012-09-09T15:42:20Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Only the first half of the test works only on POSIX, the second half\npasses on Windows as well.\n\nA later test \"real path removes other extra slashes\" looks very similar,\nbut it does not make sense to split it in the same way: When two slashes\nare prepended in front of an absolute DOS-style path on Windows, the\nmeaning of the path is changed (//server/share style), so that the test\ncannot pass on Windows.\n\nSigned-off-by: Johannes Sixt <j6t@kdbg.org>\n---\n The series passes for me as is, but one test needs POSIX only in\n the first half. This patch splits it in two.\n\n t/t0060-path-utils.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex e40f764..4ef2345 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -148,10 +148,14 @@ test_expect_success 'real path rejects the empty string' '\n \ttest_must_fail test-path-utils real_path \"\"\n '\n \n-test_expect_success POSIX 'real path works on absolute paths' '\n+test_expect_success POSIX 'real path works on absolute paths 1' '\n \tnopath=\"hopefully-absent-path\" &&\n \ttest \"/\" = \"$(test-path-utils real_path \"/\")\" &&\n-\ttest \"/$nopath\" = \"$(test-path-utils real_path \"/$nopath\")\" &&\n+\ttest \"/$nopath\" = \"$(test-path-utils real_path \"/$nopath\")\"\n+'\n+\n+test_expect_success 'real path works on absolute paths 2' '\n+\tnopath=\"hopefully-absent-path\" &&\n \t# Find an existing top-level directory for the remaining tests:\n \td=$(pwd -P | sed -e \"s|^\\([^/]*/[^/]*\\)/.*|\\1|\") &&\n \ttest \"$d\" = \"$(test-path-utils real_path \"$d\")\" &&\n-- \n1.7.12.1493.g247bc0e\n"},{"id":"198668","messageId":"504DD492.8030800@alum.mit.edu","threadId":"31462","inReplyTo":"504CB8DC.90202@kdbg.org","subject":"Re: [PATCH 9/8] t0060: split absolute path test in two to exercise some of it on Windows","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-09-10T11:52:50Z","receivedAt":"2012-09-10T11:52:50Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Looks fine to me.  Thanks!\n\nMichael\n\nOn 09/09/2012 05:42 PM, Johannes Sixt wrote:\n> Only the first half of the test works only on POSIX, the second half\n> passes on Windows as well.\n> \n> A later test \"real path removes other extra slashes\" looks very similar,\n> but it does not make sense to split it in the same way: When two slashes\n> are prepended in front of an absolute DOS-style path on Windows, the\n> meaning of the path is changed (//server/share style), so that the test\n> cannot pass on Windows.\n> \n> Signed-off-by: Johannes Sixt <j6t@kdbg.org>\n> ---\n>  The series passes for me as is, but one test needs POSIX only in\n>  the first half. This patch splits it in two.\n> \n>  t/t0060-path-utils.sh | 8 ++++++--\n>  1 file changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\n> index e40f764..4ef2345 100755\n> --- a/t/t0060-path-utils.sh\n> +++ b/t/t0060-path-utils.sh\n> @@ -148,10 +148,14 @@ test_expect_success 'real path rejects the empty string' '\n>  \ttest_must_fail test-path-utils real_path \"\"\n>  '\n>  \n> -test_expect_success POSIX 'real path works on absolute paths' '\n> +test_expect_success POSIX 'real path works on absolute paths 1' '\n>  \tnopath=\"hopefully-absent-path\" &&\n>  \ttest \"/\" = \"$(test-path-utils real_path \"/\")\" &&\n> -\ttest \"/$nopath\" = \"$(test-path-utils real_path \"/$nopath\")\" &&\n> +\ttest \"/$nopath\" = \"$(test-path-utils real_path \"/$nopath\")\"\n> +'\n> +\n> +test_expect_success 'real path works on absolute paths 2' '\n> +\tnopath=\"hopefully-absent-path\" &&\n>  \t# Find an existing top-level directory for the remaining tests:\n>  \td=$(pwd -P | sed -e \"s|^\\([^/]*/[^/]*\\)/.*|\\1|\") &&\n>  \ttest \"$d\" = \"$(test-path-utils real_path \"$d\")\" &&\n> \n\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}