]> Cypherpunks repositories - gostls13.git/commitdiff
path/filepath: make Abs("") return working directory on Windows
authorDmitri Shuralyov <dmitri@shuralyov.com>
Fri, 13 Apr 2018 21:59:16 +0000 (17:59 -0400)
committerBrad Fitzpatrick <bradfitz@golang.org>
Mon, 14 May 2018 04:41:33 +0000 (04:41 +0000)
The current Abs docs say:

// If the path is not absolute it will be joined with the current
// working directory to turn it into an absolute path.

The empty string is not an absolute path, so the docs suggest that the
empty string should be joined with the current working directory to
turn it into an absolute path. This was already the case on all
platforms other than Windows. Per the decision in issue #24441,
this change makes it work on Windows too.

Since the empty string is not a valid path for the purposes of calling
os.Stat on it, we can't simply add the empty string test case to
absTests, which TestAbs uses. It would error when trying to do:

info, err := os.Stat(path)

I didn't find a good way to modify TestAbs to handle this situation
without significantly complicating its code and compromising the test.
So, a separate test is created for testing Abs on empty string input.

Fixes #24441.

Change-Id: I11d8ae2f6e6e358f3e996372ee2a0449093898d2
Reviewed-on: https://go-review.googlesource.com/112935
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

src/path/filepath/path_test.go
src/path/filepath/path_windows.go

index 6e8d1cb4324619cc87f88497391d8b12f4acbb6d..dde087253dda34ff1d2c57f29768855820e52946 100644 (file)
@@ -1069,6 +1069,47 @@ func TestAbs(t *testing.T) {
        }
 }
 
+// Empty path needs to be special-cased on Windows. See golang.org/issue/24441.
+// We test it separately from all other absTests because the empty string is not
+// a valid path, so it can't be used with os.Stat.
+func TestAbsEmptyString(t *testing.T) {
+       root, err := ioutil.TempDir("", "TestAbsEmptyString")
+       if err != nil {
+               t.Fatal("TempDir failed: ", err)
+       }
+       defer os.RemoveAll(root)
+
+       wd, err := os.Getwd()
+       if err != nil {
+               t.Fatal("getwd failed: ", err)
+       }
+       err = os.Chdir(root)
+       if err != nil {
+               t.Fatal("chdir failed: ", err)
+       }
+       defer os.Chdir(wd)
+
+       info, err := os.Stat(root)
+       if err != nil {
+               t.Fatalf("%s: %s", root, err)
+       }
+
+       abspath, err := filepath.Abs("")
+       if err != nil {
+               t.Fatalf(`Abs("") error: %v`, err)
+       }
+       absinfo, err := os.Stat(abspath)
+       if err != nil || !os.SameFile(absinfo, info) {
+               t.Errorf(`Abs("")=%q, not the same file`, abspath)
+       }
+       if !filepath.IsAbs(abspath) {
+               t.Errorf(`Abs("")=%q, not an absolute path`, abspath)
+       }
+       if filepath.IsAbs(abspath) && abspath != filepath.Clean(abspath) {
+               t.Errorf(`Abs("")=%q, isn't clean`, abspath)
+       }
+}
+
 type RelTests struct {
        root, path, want string
 }
index 03542559f8b3ca9c5268c97c9c2f722e9c5f51cc..409e8d6466a9506abc84aa9cb5dae34525f6de6f 100644 (file)
@@ -107,6 +107,12 @@ func splitList(path string) []string {
 }
 
 func abs(path string) (string, error) {
+       if path == "" {
+               // syscall.FullPath returns an error on empty path, because it's not a valid path.
+               // To implement Abs behavior of returning working directory on empty string input,
+               // special-case empty path by changing it to "." path. See golang.org/issue/24441.
+               path = "."
+       }
        fullPath, err := syscall.FullPath(path)
        if err != nil {
                return "", err