On 09/05/2012 10:40 AM, Johannes Sixt wrote: > Am 9/4/2012 10:14, schrieb mhagger@xxxxxxxxxxxx: >> From: Michael Haggerty <mhagger@xxxxxxxxxxxx> >> >> These tests already pass, but make sure they don't break in the >> future. >> >> Signed-off-by: Michael Haggerty <mhagger@xxxxxxxxxxxx> >> --- >> >> It would be great if somebody would check whether these tests pass on >> Windows, and if not, give me a tip about how to fix them. >> >> t/t0000-basic.sh | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh >> index d929578..3c75e97 100755 >> --- a/t/t0000-basic.sh >> +++ b/t/t0000-basic.sh >> @@ -468,6 +468,17 @@ test_expect_success 'real path works on absolute paths' ' >> test "$d/$nopath" = "$(test-path-utils real_path "$d/$nopath")" >> ' >> >> +test_expect_success 'real path removes extra slashes' ' >> + nopath="hopefully-absent-path" && >> + test "/" = "$(test-path-utils real_path "///")" && >> + test "/$nopath" = "$(test-path-utils real_path "///$nopath")" && > > Same here: test-path-utils operates on a DOS-style absolute path. ACK. > Furthermore, as Junio pointed out elsewhere, it is desirable that slashes > (dir-separators) at the beginning are not collapsed if there are exactly > two of them. Three or more can be collapsed to a single slash. The empirical behavior of real_path() on Linux is (assuming "/tmp" exists and "/foo" does not) Before my changes After my changes real_path("/tmp") /tmp /tmp real_path("//tmp") /tmp /tmp real_path("/foo") $(pwd)/foo /foo real_path("//foo") /foo /foo real_path("/tmp/bar") /tmp/bar /tmp/bar real_path("//tmp/bar") /tmp/bar /tmp/bar real_path("/foo/bar") --dies-- --dies-- real_path("//foo/bar") --dies-- --dies-- So I think that my changes makes things strictly better (albeit probably not perfect). real_path() relies on chdir() / getcwd() to canonicalize the path, except for one case: If the specified path is not itself a directory, then it strips off the last component of the name, tries chdir() / getcwd() on the shortened path, then pastes the last component on again. The stripping off is done by find_last_dir_sep(), which literally just looks for the last '/' (or for Windows also '\') in the string. Please note that the pasting back is done using '/' as a separator. So I think that the only problematic case is a path that only includes a single group of slashes, like "//foo" or maybe "c:\\foo", and also is not is_directory() [1]. What should be the algorithm for such cases, and how should it depend on the platform? (And does it really matter? Top-level Linux paths are usually directories. Are Windows-style network shares also considered directories in the sense of is_directory()?) I will make an easy improvement: not to remove *any* slashes when stripping the last path component from the end of the path (letting chdir() deal with them). This results in the same results for Linux and perhaps more hope under Windows: On Linux: "//foo" -> (chdir("//"), "foo") -> ("/", "foo") -> "/foo" On Windows: "//foo" -> (chdir("//"), "foo") -> (?, "foo") -> ? (what is the result of chdir("//") then getcwd()?) On Windows: "c:\foo" -> (chdir("c:\"), "foo") -> (?, "foo") -> ? (what is the result of chdir("c:\") then getcwd()?) >> + # We need an existing top-level directory to use in the >> + # remaining tests. Use the top-level ancestor of $(pwd): >> + d=$(pwd -P | sed -e "s|^\(/[^/]*\)/.*|\1|") && > > Same here: Account for the drive letter-colon. ACK >> + test "$d" = "$(test-path-utils real_path "//$d///")" && >> + test "$d/$nopath" = "$(test-path-utils real_path "//$d///$nopath")" > > Since $d is a DOS-style path on Windows, //$d means something entirely > different than $d. You should split the test in two: One test checks that > slashes at the end or before $nopath are collapsed (this must work on > Windows as well), and another test protected by POSIX prerequisite to > check that slashes at the beginning are collapsed. Done. Thanks again for your help. Michael [1] If there is more than one group of slashes in the name, like "//foo//bar" or "c:\\foo\\bar" then: * All but the last groups of slashes are handled by chdir("//foo/")/getcwd() and presumably de-duplicated or not as appropriate * The extras from the last group of slashes are trailing slashes in the string passed to chdir() and are therefore removed. so everything should be OK. -- Michael Haggerty mhagger@xxxxxxxxxxxx http://softwareswirl.blogspot.com/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html