cmd/go: add a flag to avoid creating unwritable directories in the module cache

This change adds the '-modcacherw' build flag, which leaves
newly-created directories (but not the files!) in the module cache
read-write instead of making them unwritable.

Fixes #31481

Change-Id: I7c21a53dd145676627c3b51096914ce797991d99
Reviewed-on: https://go-review.googlesource.com/c/go/+/202079
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

diff --git a/doc/go1.14.html b/doc/go1.14.html
index 363f57bb4dace89ce1fe6358b932ef04085b1afd..79efb2b0bae34df9023f441d04a1fd51511e42cd 100644 (file)
--- a/doc/go1.14.html
+++ b/doc/go1.14.html
@@ -112,6 +112,17 @@ TODO
   graphic characters and spaces.
 </p>
 
+<p><!-- golang.org/issue/31481 -->
+  The <code>go</code> command now accepts a new flag, <code>-modcacherw</code>,
+  which leaves newly-created directories in the module cache at their default
+  permissions rather than making them read-only.
+  The use of this flag makes it more likely that tests or other tools will
+  accidentally add files not included in the module's verified checksum.
+  However, it allows the use of <code>rm</code> <code>-rf</code>
+  (instead of <code>go</code> <code>clean</code> <code>-modcache</code>)
+  to remove the module cache.
+</p>
+
 <h2 id="runtime">Runtime</h2>
 
 <p>

diff --git a/src/cmd/go/alldocs.go b/src/cmd/go/alldocs.go
index 51b8fccb4552524de1346eea99329131b6e2e1dc..c5ceec800902cec878838542ccbb51a25c8794a2 100644 (file)
--- a/src/cmd/go/alldocs.go
+++ b/src/cmd/go/alldocs.go
@@ -150,6 +150,9 @@
 //     -mod mode
 //             module download mode to use: readonly or vendor.
 //             See 'go help modules' for more.
+//     -modcacherw
+//             leave newly-created directories in the module cache read-write
+//             instead of making them read-only.
 //     -pkgdir dir
 //             install and load all packages from dir instead of the usual locations.
 //             For example, when building with a non-standard configuration,

diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go
index a3277a6c3f0ac12019ba8139d0ce89f4e3994547..ea909b5b37d5e0899a574f489af1efe88044ffcb 100644 (file)
--- a/src/cmd/go/internal/cfg/cfg.go
+++ b/src/cmd/go/internal/cfg/cfg.go
@@ -33,6 +33,7 @@ var (
        BuildN                 bool               // -n flag
        BuildO                 string             // -o flag
        BuildP                 = runtime.NumCPU() // -p flag
+       BuildModcacheRW        bool               // -modcacherw flag
        BuildPkgdir            string             // -pkgdir flag
        BuildRace              bool               // -race flag
        BuildToolexec          []string           // -toolexec flag

diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go
index 8f792a77687a5adc01729ba91174bae6e3d3ced9..438c6212b5d692a0b66862aad238669ecc6e7f8c 100644 (file)
--- a/src/cmd/go/internal/modfetch/fetch.go
+++ b/src/cmd/go/internal/modfetch/fetch.go
@@ -125,9 +125,11 @@ func download(mod module.Version, dir string) (err error) {
                return err
        }
 
-       // Make dir read-only only *after* renaming it.
-       // os.Rename was observed to fail for read-only directories on macOS.
-       makeDirsReadOnly(dir)
+       if !cfg.BuildModcacheRW {
+               // Make dir read-only only *after* renaming it.
+               // os.Rename was observed to fail for read-only directories on macOS.
+               makeDirsReadOnly(dir)
+       }
        return nil
 }
 

diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go
index 54b049b68fe0db94cc9d955d65e78e020fad05ae..9b74963f43bb86bd896bc1d5f7fecd6dffbd67c2 100644 (file)
--- a/src/cmd/go/internal/work/build.go
+++ b/src/cmd/go/internal/work/build.go
@@ -102,6 +102,9 @@ and test commands:
        -mod mode
                module download mode to use: readonly or vendor.
                See 'go help modules' for more.
+       -modcacherw
+               leave newly-created directories in the module cache read-write
+               instead of making them read-only.
        -pkgdir dir
                install and load all packages from dir instead of the usual locations.
                For example, when building with a non-standard configuration,
@@ -243,6 +246,7 @@ func AddBuildFlags(cmd *base.Command, mask BuildFlagMask) {
        cmd.Flag.StringVar(&cfg.BuildContext.InstallSuffix, "installsuffix", "", "")
        cmd.Flag.Var(&load.BuildLdflags, "ldflags", "")
        cmd.Flag.BoolVar(&cfg.BuildLinkshared, "linkshared", false, "")
+       cmd.Flag.BoolVar(&cfg.BuildModcacheRW, "modcacherw", false, "")
        cmd.Flag.StringVar(&cfg.BuildPkgdir, "pkgdir", "", "")
        cmd.Flag.BoolVar(&cfg.BuildRace, "race", false, "")
        cmd.Flag.BoolVar(&cfg.BuildMSan, "msan", false, "")

diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go
index 5e50dd14c78be4d62aeadf027ae3545a516e4c6e..31e527fd4085c7e5dbd670c93b2b18419035dcdb 100644 (file)
--- a/src/cmd/go/script_test.go
+++ b/src/cmd/go/script_test.go
@@ -502,9 +502,6 @@ func (ts *testScript) doCmdCmp(args []string, env, quiet bool) {
 
 // cp copies files, maybe eventually directories.
 func (ts *testScript) cmdCp(neg bool, args []string) {
-       if neg {
-               ts.fatalf("unsupported: ! cp")
-       }
        if len(args) < 2 {
                ts.fatalf("usage: cp src... dst")
        }
@@ -543,7 +540,14 @@ func (ts *testScript) cmdCp(neg bool, args []string) {
                if dstDir {
                        targ = filepath.Join(dst, filepath.Base(src))
                }
-               ts.check(ioutil.WriteFile(targ, data, mode))
+               err := ioutil.WriteFile(targ, data, mode)
+               if neg {
+                       if err == nil {
+                               ts.fatalf("unexpected command success")
+                       }
+               } else {
+                       ts.check(err)
+               }
        }
 }
 

diff --git a/src/cmd/go/testdata/script/README b/src/cmd/go/testdata/script/README
index 46444d84d8f40bfc0a51ad3b1424448fde2ef4a0..ec886b18a15a7272a5ccc04bffec2eed65add7b2 100644 (file)
--- a/src/cmd/go/testdata/script/README
+++ b/src/cmd/go/testdata/script/README
@@ -107,7 +107,7 @@ The commands are:
   Like cmp, but environment variables are substituted in the file contents
   before the comparison. For example, $GOOS is replaced by the target GOOS.
 
-- cp src... dst
+- [!] cp src... dst
   Copy the listed files to the target file or existing directory.
   src can include "stdout" or "stderr" to use the standard output or standard error
   from the most recent exec or go command.

diff --git a/src/cmd/go/testdata/script/mod_cache_rw.txt b/src/cmd/go/testdata/script/mod_cache_rw.txt
new file mode 100644 (file)
index 0000000..ef91c7e
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_cache_rw.txt
@@ -0,0 +1,38 @@
+# Regression test for golang.org/issue/31481.
+
+env GO111MODULE=on
+
+# golang.org/issue/31481: an explicit flag should make directories in the module
+# cache writable in order to work around the historical inability of 'rm -rf' to
+# forcibly remove files in unwritable directories.
+go get -modcacherw -d rsc.io/quote@v1.5.2
+cp $WORK/extraneous.txt $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/extraneous_file.go
+
+# After adding an extraneous file, 'go mod verify' should fail.
+! go mod verify
+
+# However, files within those directories should still be read-only to avoid
+# accidental mutations.
+# TODO: Today, this does not seem to be effective on Windows.
+# (https://golang.org/issue/35033)
+[!windows] [!root] ! cp $WORK/extraneous.txt $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/go.mod
+
+# If all 'go' commands ran with the flag, the system's 'rm' binary
+# should be able to remove the module cache if the '-rf' flags are set.
+[!windows] [exec:rm] exec rm -rf $GOPATH/pkg/mod
+[!windows] [!exec:rm] go clean -modcache
+[windows] [exec:rmdir] exec rmdir /s /q $GOPATH\pkg\mod
+[windows] [!exec:rmdir] go clean -modcache
+! exists $GOPATH/pkg/mod
+
+# The directories in the module cache should by default be unwritable,
+# so that tests and tools will not accidentally add extraneous files to them.
+go get -d rsc.io/quote@latest
+[!windows] [!root] ! cp $WORK/extraneous.txt $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/go.mod
+[!windows] [!root] ! cp $WORK/extraneous.txt $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/extraneous_file.go
+! exists $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/extraneous_file.go
+
+-- $WORK/extraneous.txt --
+module oops
+-- go.mod --
+module golang.org/issue/31481