os: fix Remove for file with read only attribute on Windows

Include integration test. Confirmed that without the fix, the test case
TestDeleteReadOnly fails.

This permits to revert "cmd/go: reset read-only flag during TestIssue10952"
This reverts commit 3b7841b3aff9204f054ffabbe4dd39d3e3dd3e91.

Fixes #9606

Change-Id: Ib55c151a8cf1a1da02ab18c34a9b58f615c34254
Reviewed-on: https://go-review.googlesource.com/18235
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index ac82b2ffeb467b8333c013526691a3d746f49e05..b667396761937444c76b8356fa03b0f150940cd1 100644 (file)
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -580,32 +580,6 @@ func (tg *testgoData) cleanup() {
        }
 }
 
-// resetReadOnlyFlagAll resets windows read-only flag
-// set on path and any children it contains.
-// The flag is set by git and has to be removed.
-// os.Remove refuses to remove files with read-only flag set.
-func (tg *testgoData) resetReadOnlyFlagAll(path string) {
-       fi, err := os.Stat(path)
-       if err != nil {
-               tg.t.Fatalf("resetReadOnlyFlagAll(%q) failed: %v", path, err)
-       }
-       if !fi.IsDir() {
-               err := os.Chmod(path, 0666)
-               if err != nil {
-                       tg.t.Fatalf("resetReadOnlyFlagAll(%q) failed: %v", path, err)
-               }
-       }
-       fd, err := os.Open(path)
-       if err != nil {
-               tg.t.Fatalf("resetReadOnlyFlagAll(%q) failed: %v", path, err)
-       }
-       defer fd.Close()
-       names, _ := fd.Readdirnames(-1)
-       for _, name := range names {
-               tg.resetReadOnlyFlagAll(path + string(filepath.Separator) + name)
-       }
-}
-
 // failSSH puts an ssh executable in the PATH that always fails.
 // This is to stub out uses of ssh by go get.
 func (tg *testgoData) failSSH() {
@@ -1192,7 +1166,6 @@ func TestIssue10952(t *testing.T) {
        const importPath = "github.com/zombiezen/go-get-issue-10952"
        tg.run("get", "-d", "-u", importPath)
        repoDir := tg.path("src/" + importPath)
-       defer tg.resetReadOnlyFlagAll(repoDir)
        tg.runGit(repoDir, "remote", "set-url", "origin", "https://"+importPath+".git")
        tg.run("get", "-d", "-u", importPath)
 }
@@ -1216,7 +1189,6 @@ func TestGetGitDefaultBranch(t *testing.T) {
 
        tg.run("get", "-d", importPath)
        repoDir := tg.path("src/" + importPath)
-       defer tg.resetReadOnlyFlagAll(repoDir)
        tg.runGit(repoDir, "branch", "--contains", "HEAD")
        tg.grepStdout(`\* another-branch`, "not on correct default branch")
 

diff --git a/src/os/file_windows.go b/src/os/file_windows.go
index 08aff83a77510c912597c99382edefdb4b2a8e06..f470fc4315cc2b987fab3518beeb88226bf12cf2 100644 (file)
--- a/src/os/file_windows.go
+++ b/src/os/file_windows.go
@@ -474,6 +474,12 @@ func Remove(name string) error {
                } else {
                        if a&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
                                e = e1
+                       } else if a&syscall.FILE_ATTRIBUTE_READONLY != 0 {
+                               if e1 = syscall.SetFileAttributes(p, a&^syscall.FILE_ATTRIBUTE_READONLY); e1 == nil {
+                                       if e = syscall.DeleteFile(p); e == nil {
+                                               return nil
+                                       }
+                               }
                        }
                }
        }

diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go
index 2f7d48d5bdf06c57112815a0a3fbed1195e65a80..05d7a8f34e9a9f4f721087ff57774d270ea461da 100644 (file)
--- a/src/os/os_windows_test.go
+++ b/src/os/os_windows_test.go
@@ -223,3 +223,25 @@ func TestOpenVolumeName(t *testing.T) {
                t.Fatalf("unexpected file list %q, want %q", have, want)
        }
 }
+
+func TestDeleteReadOnly(t *testing.T) {
+       tmpdir, err := ioutil.TempDir("", "TestDeleteReadOnly")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(tmpdir)
+       p := filepath.Join(tmpdir, "a")
+       // This sets FILE_ATTRIBUTE_READONLY.
+       f, err := os.OpenFile(p, os.O_CREATE, 0400)
+       if err != nil {
+               t.Fatal(err)
+       }
+       f.Close()
+
+       if err = os.Chmod(p, 0400); err != nil {
+               t.Fatal(err)
+       }
+       if err = os.Remove(p); err != nil {
+               t.Fatal(err)
+       }
+}