os: in RemoveAll, try Remove first

Otherwise we can fail to remove a unreadable empty directory.

Fixes #29178

Change-Id: I43d5c89fce57a86626abe2a1c2bbf145716e087b
Reviewed-on: https://go-review.googlesource.com/c/153720
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/os/removeall_at.go b/src/os/removeall_at.go
index c42319a831282bcdb6b0461b8eb42a9a0fae0a03..f0fed6dc33f4cdedc8c3f04542a6ab5a06ba24b2 100644 (file)
--- a/src/os/removeall_at.go
+++ b/src/os/removeall_at.go
@@ -25,6 +25,12 @@ func RemoveAll(path string) error {
                return &PathError{"RemoveAll", path, syscall.EINVAL}
        }
 
+       // Simple case: if Remove works, we're done.
+       err := Remove(path)
+       if err == nil || IsNotExist(err) {
+               return nil
+       }
+
        // RemoveAll recurses by deleting the path base from
        // its parent directory
        parentDir, base := splitPath(path)

diff --git a/src/os/removeall_test.go b/src/os/removeall_test.go
index 1c9f16322547ae19389a852521fc702e09dbd019..0f7dce078a6b081769a4db3936b898e9beaaeca3 100644 (file)
--- a/src/os/removeall_test.go
+++ b/src/os/removeall_test.go
@@ -264,3 +264,31 @@ func TestRemoveAllDotDot(t *testing.T) {
                }
        }
 }
+
+// Issue #29178.
+func TestRemoveReadOnlyDir(t *testing.T) {
+       t.Parallel()
+
+       tempDir, err := ioutil.TempDir("", "TestRemoveReadOnlyDir-")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer RemoveAll(tempDir)
+
+       subdir := filepath.Join(tempDir, "x")
+       if err := Mkdir(subdir, 0); err != nil {
+               t.Fatal(err)
+       }
+
+       // If an error occurs make it more likely that removing the
+       // temporary directory will succeed.
+       defer Chmod(subdir, 0777)
+
+       if err := RemoveAll(subdir); err != nil {
+               t.Fatal(err)
+       }
+
+       if _, err := Stat(subdir); err == nil {
+               t.Error("subdirectory was not removed")
+       }
+}