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 3b7841b3af.

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>

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")
 

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
+					}
+				}
 			}
 		}
 	}

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)
+	}
+}