From 04e12a5bfa51777c4ba46ed2e026f53578206754 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Mon, 15 Oct 2018 15:52:01 -0400 Subject: [PATCH] cmd/go/internal/modfetch: lock files and directories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We employ the following new locking mechanisms: • Zip files and list files within the module cache are written using atomic renames of temporary files, so that GOPROXY servers reading from the cache will never serve incomplete content. • A lock file for each module version guards downloading and extraction of (immutable) module contents. • A lock file alongside each version list (named 'list.lock') guards updates to the list. • A single lock file in the module cache guards updates to all go.sum files. The go.sum files themselves are written using an atomic rename to ensure that we never accidentally discard existing sums. Updates #26794 RELNOTE=yes Change-Id: I16ef8b06ee4bd7b94d0c0a6f5d17e1cecc379076 Reviewed-on: https://go-review.googlesource.com/c/146382 Run-TryBot: Bryan C. Mills Reviewed-by: Russ Cox --- src/cmd/go/internal/clean/clean.go | 16 +- src/cmd/go/internal/modfetch/cache.go | 75 +++-- src/cmd/go/internal/modfetch/fetch.go | 305 ++++++++++++++---- src/cmd/go/internal/modfetch/unzip.go | 22 +- src/cmd/go/testdata/script/mod_concurrent.txt | 31 ++ 5 files changed, 337 insertions(+), 112 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_concurrent.txt diff --git a/src/cmd/go/internal/clean/clean.go b/src/cmd/go/internal/clean/clean.go index 73e04960d2bbd..96fd653b745e3 100644 --- a/src/cmd/go/internal/clean/clean.go +++ b/src/cmd/go/internal/clean/clean.go @@ -176,27 +176,13 @@ func runClean(cmd *base.Command, args []string) { b.Showcmd("", "rm -rf %s", modfetch.PkgMod) } if !cfg.BuildN { - if err := removeAll(modfetch.PkgMod); err != nil { + if err := modfetch.RemoveAll(modfetch.PkgMod); err != nil { base.Errorf("go clean -modcache: %v", err) } } } } -func removeAll(dir string) error { - // Module cache has 0555 directories; make them writable in order to remove content. - filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { - if err != nil { - return nil // ignore errors walking in file system - } - if info.IsDir() { - os.Chmod(path, 0777) - } - return nil - }) - return os.RemoveAll(dir) -} - var cleaned = map[*load.Package]bool{} // TODO: These are dregs left by Makefile-based builds. diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go index f3f04a151d80a..80484d5b5e728 100644 --- a/src/cmd/go/internal/modfetch/cache.go +++ b/src/cmd/go/internal/modfetch/cache.go @@ -15,9 +15,11 @@ import ( "strings" "cmd/go/internal/base" + "cmd/go/internal/lockedfile" "cmd/go/internal/modfetch/codehost" "cmd/go/internal/module" "cmd/go/internal/par" + "cmd/go/internal/renameio" "cmd/go/internal/semver" ) @@ -75,6 +77,37 @@ func DownloadDir(m module.Version) (string, error) { return filepath.Join(PkgMod, enc+"@"+encVer), nil } +// lockVersion locks a file within the module cache that guards the downloading +// and extraction of the zipfile for the given module version. +func lockVersion(mod module.Version) (unlock func(), err error) { + path, err := CachePath(mod, "lock") + if err != nil { + return nil, err + } + if err := os.MkdirAll(filepath.Dir(path), 0777); err != nil { + return nil, err + } + return lockedfile.MutexAt(path).Lock() +} + +// SideLock locks a file within the module cache that that guards edits to files +// outside the cache, such as go.sum and go.mod files in the user's working +// directory. It returns a function that must be called to unlock the file. +func SideLock() (unlock func()) { + if PkgMod == "" { + base.Fatalf("go: internal error: modfetch.PkgMod not set") + } + path := filepath.Join(PkgMod, "cache", "lock") + if err := os.MkdirAll(filepath.Dir(path), 0777); err != nil { + base.Fatalf("go: failed to create cache directory %s: %v", filepath.Dir(path), err) + } + unlock, err := lockedfile.MutexAt(path).Lock() + if err != nil { + base.Fatalf("go: failed to lock file at %v", path) + } + return unlock +} + // A cachingRepo is a cache around an underlying Repo, // avoiding redundant calls to ModulePath, Versions, Stat, Latest, and GoMod (but not Zip). // It is also safe for simultaneous use by multiple goroutines @@ -386,7 +419,7 @@ func readDiskStatByHash(path, rev string) (file string, info *RevInfo, err error // and should ignore it. var oldVgoPrefix = []byte("//vgo 0.0.") -// readDiskGoMod reads a cached stat result from disk, +// readDiskGoMod reads a cached go.mod file from disk, // returning the name of the cache file and the result. // If the read fails, the caller can use // writeDiskGoMod(file, data) to write a new cache entry. @@ -452,22 +485,8 @@ func writeDiskCache(file string, data []byte) error { if err := os.MkdirAll(filepath.Dir(file), 0777); err != nil { return err } - // Write data to temp file next to target file. - f, err := ioutil.TempFile(filepath.Dir(file), filepath.Base(file)+".tmp-") - if err != nil { - return err - } - defer os.Remove(f.Name()) - defer f.Close() - if _, err := f.Write(data); err != nil { - return err - } - if err := f.Close(); err != nil { - return err - } - // Rename temp file onto cache file, - // so that the cache file is always a complete file. - if err := os.Rename(f.Name(), file); err != nil { + + if err := renameio.WriteFile(file, data); err != nil { return err } @@ -484,8 +503,18 @@ func rewriteVersionList(dir string) { base.Fatalf("go: internal error: misuse of rewriteVersionList") } - // TODO(rsc): We should do some kind of directory locking here, - // to avoid lost updates. + listFile := filepath.Join(dir, "list") + + // We use a separate lockfile here instead of locking listFile itself because + // we want to use Rename to write the file atomically. The list may be read by + // a GOPROXY HTTP server, and if we crash midway through a rewrite (or if the + // HTTP server ignores our locking and serves the file midway through a + // rewrite) it's better to serve a stale list than a truncated one. + unlock, err := lockedfile.MutexAt(listFile + ".lock").Lock() + if err != nil { + base.Fatalf("go: can't lock version list lockfile: %v", err) + } + defer unlock() infos, err := ioutil.ReadDir(dir) if err != nil { @@ -514,12 +543,12 @@ func rewriteVersionList(dir string) { buf.WriteString(v) buf.WriteString("\n") } - listFile := filepath.Join(dir, "list") old, _ := ioutil.ReadFile(listFile) if bytes.Equal(buf.Bytes(), old) { return } - // TODO: Use rename to install file, - // so that readers never see an incomplete file. - ioutil.WriteFile(listFile, buf.Bytes(), 0666) + + if err := renameio.WriteFile(listFile, buf.Bytes()); err != nil { + base.Fatalf("go: failed to write version list: %v", err) + } } diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go index e3bc7b51332d6..159bc569296b3 100644 --- a/src/cmd/go/internal/modfetch/fetch.go +++ b/src/cmd/go/internal/modfetch/fetch.go @@ -21,6 +21,7 @@ import ( "cmd/go/internal/dirhash" "cmd/go/internal/module" "cmd/go/internal/par" + "cmd/go/internal/renameio" ) var downloadCache par.Cache @@ -34,9 +35,7 @@ func Download(mod module.Version) (dir string, err error) { return "", fmt.Errorf("missing modfetch.PkgMod") } - // The par.Cache here avoids duplicate work but also - // avoids conflicts from simultaneous calls by multiple goroutines - // for the same version. + // The par.Cache here avoids duplicate work. type cached struct { dir string err error @@ -46,16 +45,8 @@ func Download(mod module.Version) (dir string, err error) { if err != nil { return cached{"", err} } - if files, _ := ioutil.ReadDir(dir); len(files) == 0 { - zipfile, err := DownloadZip(mod) - if err != nil { - return cached{"", err} - } - modpath := mod.Path + "@" + mod.Version - if err := Unzip(dir, zipfile, modpath, 0); err != nil { - fmt.Fprintf(os.Stderr, "-> %s\n", err) - return cached{"", err} - } + if err := download(mod, dir); err != nil { + return cached{"", err} } checkSum(mod) return cached{dir, nil} @@ -63,14 +54,81 @@ func Download(mod module.Version) (dir string, err error) { return c.dir, c.err } +func download(mod module.Version, dir string) (err error) { + // If the directory exists, the module has already been extracted. + fi, err := os.Stat(dir) + if err == nil && fi.IsDir() { + return nil + } + + // To avoid cluttering the cache with extraneous files, + // DownloadZip uses the same lockfile as Download. + // Invoke DownloadZip before locking the file. + zipfile, err := DownloadZip(mod) + if err != nil { + return err + } + + if cfg.CmdName != "mod download" { + fmt.Fprintf(os.Stderr, "go: extracting %s %s\n", mod.Path, mod.Version) + } + + unlock, err := lockVersion(mod) + if err != nil { + return err + } + defer unlock() + + // Check whether the directory was populated while we were waiting on the lock. + fi, err = os.Stat(dir) + if err == nil && fi.IsDir() { + return nil + } + + // Clean up any remaining temporary directories from previous runs. + // This is only safe to do because the lock file ensures that their writers + // are no longer active. + parentDir := filepath.Dir(dir) + tmpPrefix := filepath.Base(dir) + ".tmp-" + if old, err := filepath.Glob(filepath.Join(parentDir, tmpPrefix+"*")); err == nil { + for _, path := range old { + RemoveAll(path) // best effort + } + } + + // Extract the zip file to a temporary directory, then rename it to the + // final path. That way, we can use the existence of the source directory to + // signal that it has been extracted successfully, and if someone deletes + // the entire directory (e.g. as an attempt to prune out file corruption) + // the module cache will still be left in a recoverable state. + if err := os.MkdirAll(parentDir, 0777); err != nil { + return err + } + tmpDir, err := ioutil.TempDir(parentDir, tmpPrefix) + if err != nil { + return err + } + defer func() { + if err != nil { + RemoveAll(tmpDir) + } + }() + + modpath := mod.Path + "@" + mod.Version + if err := Unzip(tmpDir, zipfile, modpath, 0); err != nil { + fmt.Fprintf(os.Stderr, "-> %s\n", err) + return err + } + + return os.Rename(tmpDir, dir) +} + var downloadZipCache par.Cache // DownloadZip downloads the specific module version to the // local zip cache and returns the name of the zip file. func DownloadZip(mod module.Version) (zipfile string, err error) { - // The par.Cache here avoids duplicate work but also - // avoids conflicts from simultaneous calls by multiple goroutines - // for the same version. + // The par.Cache here avoids duplicate work. type cached struct { zipfile string err error @@ -80,52 +138,82 @@ func DownloadZip(mod module.Version) (zipfile string, err error) { if err != nil { return cached{"", err} } + + // Skip locking if the zipfile already exists. if _, err := os.Stat(zipfile); err == nil { - // Use it. - // This should only happen if the mod/cache directory is preinitialized - // or if pkg/mod/path was removed but not pkg/mod/cache/download. - if cfg.CmdName != "mod download" { - fmt.Fprintf(os.Stderr, "go: extracting %s %s\n", mod.Path, mod.Version) - } - } else { - if err := os.MkdirAll(filepath.Dir(zipfile), 0777); err != nil { - return cached{"", err} - } - if cfg.CmdName != "mod download" { - fmt.Fprintf(os.Stderr, "go: downloading %s %s\n", mod.Path, mod.Version) - } - if err := downloadZip(mod, zipfile); err != nil { - return cached{"", err} - } + return cached{zipfile, nil} + } + + // The zip file does not exist. Acquire the lock and create it. + if cfg.CmdName != "mod download" { + fmt.Fprintf(os.Stderr, "go: downloading %s %s\n", mod.Path, mod.Version) + } + unlock, err := lockVersion(mod) + if err != nil { + return cached{"", err} + } + defer unlock() + + // Double-check that the zipfile was not created while we were waiting for + // the lock. + if _, err := os.Stat(zipfile); err == nil { + return cached{zipfile, nil} + } + if err := os.MkdirAll(filepath.Dir(zipfile), 0777); err != nil { + return cached{"", err} + } + if err := downloadZip(mod, zipfile); err != nil { + return cached{"", err} } return cached{zipfile, nil} }).(cached) return c.zipfile, c.err } -func downloadZip(mod module.Version, target string) error { - repo, err := Lookup(mod.Path) - if err != nil { - return err +func downloadZip(mod module.Version, zipfile string) (err error) { + // Clean up any remaining tempfiles from previous runs. + // This is only safe to do because the lock file ensures that their + // writers are no longer active. + for _, base := range []string{zipfile, zipfile + "hash"} { + if old, err := filepath.Glob(renameio.Pattern(base)); err == nil { + for _, path := range old { + os.Remove(path) // best effort + } + } } - tmpfile, err := ioutil.TempFile("", "go-codezip-") + + // From here to the os.Rename call below is functionally almost equivalent to + // renameio.WriteToFile, with one key difference: we want to validate the + // contents of the file (by hashing it) before we commit it. Because the file + // is zip-compressed, we need an actual file — or at least an io.ReaderAt — to + // validate it: we can't just tee the stream as we write it. + f, err := ioutil.TempFile(filepath.Dir(zipfile), filepath.Base(renameio.Pattern(zipfile))) if err != nil { return err } defer func() { - tmpfile.Close() - os.Remove(tmpfile.Name()) + if err != nil { + f.Close() + os.Remove(f.Name()) + } }() - if err := repo.Zip(tmpfile, mod.Version); err != nil { + + repo, err := Lookup(mod.Path) + if err != nil { + return err + } + if err := repo.Zip(f, mod.Version); err != nil { return err } - // Double-check zip file looks OK. - fi, err := tmpfile.Stat() + // Double-check that the paths within the zip file are well-formed. + // + // TODO(bcmills): There is a similar check within the Unzip function. Can we eliminate one? + fi, err := f.Stat() if err != nil { return err } - z, err := zip.NewReader(tmpfile, fi.Size()) + z, err := zip.NewReader(f, fi.Size()) if err != nil { return err } @@ -136,33 +224,48 @@ func downloadZip(mod module.Version, target string) error { } } - hash, err := dirhash.HashZip(tmpfile.Name(), dirhash.DefaultHash) - if err != nil { + // Sync the file before renaming it: otherwise, after a crash the reader may + // observe a 0-length file instead of the actual contents. + // See https://golang.org/issue/22397#issuecomment-380831736. + if err := f.Sync(); err != nil { return err } - checkOneSum(mod, hash) // check before installing the zip file - if _, err := tmpfile.Seek(0, io.SeekStart); err != nil { + if err := f.Close(); err != nil { return err } - w, err := os.Create(target) + + // Hash the zip file and check the sum before renaming to the final location. + hash, err := dirhash.HashZip(f.Name(), dirhash.DefaultHash) if err != nil { return err } - if _, err := io.Copy(w, tmpfile); err != nil { - w.Close() - return fmt.Errorf("copying: %v", err) + checkOneSum(mod, hash) + + if err := renameio.WriteFile(zipfile+"hash", []byte(hash)); err != nil { + return err } - if err := w.Close(); err != nil { + if err := os.Rename(f.Name(), zipfile); err != nil { return err } - return ioutil.WriteFile(target+"hash", []byte(hash), 0666) + + // TODO(bcmills): Should we make the .zip and .ziphash files read-only to discourage tampering? + + return nil } var GoSumFile string // path to go.sum; set by package modload +type modSum struct { + mod module.Version + sum string +} + var goSum struct { mu sync.Mutex m map[module.Version][]string // content of go.sum file (+ go.modverify if present) + checked map[modSum]bool // sums actually checked during execution + dirty bool // whether we added any new sums to m + overwrite bool // if true, overwrite go.sum without incorporating its contents enabled bool // whether to use go.sum at all modverify string // path to go.modverify, to be deleted } @@ -179,18 +282,25 @@ func initGoSum() bool { } goSum.m = make(map[module.Version][]string) + goSum.checked = make(map[modSum]bool) data, err := ioutil.ReadFile(GoSumFile) if err != nil && !os.IsNotExist(err) { base.Fatalf("go: %v", err) } goSum.enabled = true - readGoSum(GoSumFile, data) + readGoSum(goSum.m, GoSumFile, data) // Add old go.modverify file. // We'll delete go.modverify in WriteGoSum. alt := strings.TrimSuffix(GoSumFile, ".sum") + ".modverify" if data, err := ioutil.ReadFile(alt); err == nil { - readGoSum(alt, data) + migrate := make(map[module.Version][]string) + readGoSum(migrate, alt, data) + for mod, sums := range migrate { + for _, sum := range sums { + checkOneSumLocked(mod, sum) + } + } goSum.modverify = alt } return true @@ -203,7 +313,7 @@ const emptyGoModHash = "h1:G7mAYYxgmS0lVkHyy2hEOLQCFB0DlQFTMLWggykrydY=" // readGoSum parses data, which is the content of file, // and adds it to goSum.m. The goSum lock must be held. -func readGoSum(file string, data []byte) { +func readGoSum(dst map[module.Version][]string, file string, data []byte) { lineno := 0 for len(data) > 0 { var line []byte @@ -227,7 +337,7 @@ func readGoSum(file string, data []byte) { continue } mod := module.Version{Path: f[0], Version: f[1]} - goSum.m[mod] = append(goSum.m[mod], f[2]) + dst[mod] = append(dst[mod], f[2]) } } @@ -241,7 +351,7 @@ func checkSum(mod module.Version) { // Do the file I/O before acquiring the go.sum lock. ziphash, err := CachePath(mod, "ziphash") if err != nil { - base.Fatalf("go: verifying %s@%s: %v", mod.Path, mod.Version, err) + base.Fatalf("verifying %s@%s: %v", mod.Path, mod.Version, err) } data, err := ioutil.ReadFile(ziphash) if err != nil { @@ -249,11 +359,11 @@ func checkSum(mod module.Version) { // This can happen if someone does rm -rf GOPATH/src/cache/download. So it goes. return } - base.Fatalf("go: verifying %s@%s: %v", mod.Path, mod.Version, err) + base.Fatalf("verifying %s@%s: %v", mod.Path, mod.Version, err) } h := strings.TrimSpace(string(data)) if !strings.HasPrefix(h, "h1:") { - base.Fatalf("go: verifying %s@%s: unexpected ziphash: %q", mod.Path, mod.Version, h) + base.Fatalf("verifying %s@%s: unexpected ziphash: %q", mod.Path, mod.Version, h) } checkOneSum(mod, h) @@ -271,7 +381,7 @@ func goModSum(data []byte) (string, error) { func checkGoMod(path, version string, data []byte) { h, err := goModSum(data) if err != nil { - base.Fatalf("go: verifying %s %s go.mod: %v", path, version, err) + base.Fatalf("verifying %s %s go.mod: %v", path, version, err) } checkOneSum(module.Version{Path: path, Version: version + "/go.mod"}, h) @@ -281,22 +391,27 @@ func checkGoMod(path, version string, data []byte) { func checkOneSum(mod module.Version, h string) { goSum.mu.Lock() defer goSum.mu.Unlock() - if !initGoSum() { - return + if initGoSum() { + checkOneSumLocked(mod, h) } +} + +func checkOneSumLocked(mod module.Version, h string) { + goSum.checked[modSum{mod, h}] = true for _, vh := range goSum.m[mod] { if h == vh { return } if strings.HasPrefix(vh, "h1:") { - base.Fatalf("go: verifying %s@%s: checksum mismatch\n\tdownloaded: %v\n\tgo.sum: %v", mod.Path, mod.Version, h, vh) + base.Fatalf("verifying %s@%s: checksum mismatch\n\tdownloaded: %v\n\tgo.sum: %v", mod.Path, mod.Version, h, vh) } } if len(goSum.m[mod]) > 0 { fmt.Fprintf(os.Stderr, "warning: verifying %s@%s: unknown hashes in go.sum: %v; adding %v", mod.Path, mod.Version, strings.Join(goSum.m[mod], ", "), h) } goSum.m[mod] = append(goSum.m[mod], h) + goSum.dirty = true } // Sum returns the checksum for the downloaded copy of the given module, @@ -322,10 +437,55 @@ func Sum(mod module.Version) string { func WriteGoSum() { goSum.mu.Lock() defer goSum.mu.Unlock() - if !initGoSum() { + + if !goSum.enabled { + // If we haven't read the go.sum file yet, don't bother writing it: at best, + // we could rename the go.modverify file if it isn't empty, but we haven't + // needed to touch it so far — how important could it be? + return + } + if !goSum.dirty { + // Don't bother opening the go.sum file if we don't have anything to add. return } + // We want to avoid races between creating the lockfile and deleting it, but + // we also don't want to leave a permanent lockfile in the user's repository. + // + // On top of that, if we crash while writing go.sum, we don't want to lose the + // sums that were already present in the file, so it's important that we write + // the file by renaming rather than truncating — which means that we can't + // lock the go.sum file itself. + // + // Instead, we'll lock a distinguished file in the cache directory: that will + // only race if the user runs `go clean -modcache` concurrently with a command + // that updates go.sum, and that's already racy to begin with. + // + // We'll end up slightly over-synchronizing go.sum writes if the user runs a + // bunch of go commands that update sums in separate modules simultaneously, + // but that's unlikely to matter in practice. + + unlock := SideLock() + defer unlock() + + if !goSum.overwrite { + // Re-read the go.sum file to incorporate any sums added by other processes + // in the meantime. + data, err := ioutil.ReadFile(GoSumFile) + if err != nil && !os.IsNotExist(err) { + base.Fatalf("go: re-reading go.sum: %v", err) + } + + // Add only the sums that we actually checked: the user may have edited or + // truncated the file to remove erroneous hashes, and we shouldn't restore + // them without good reason. + goSum.m = make(map[module.Version][]string, len(goSum.m)) + readGoSum(goSum.m, GoSumFile, data) + for ms := range goSum.checked { + checkOneSumLocked(ms.mod, ms.sum) + } + } + var mods []module.Version for m := range goSum.m { mods = append(mods, m) @@ -340,15 +500,16 @@ func WriteGoSum() { } } - data, _ := ioutil.ReadFile(GoSumFile) - if !bytes.Equal(data, buf.Bytes()) { - if err := ioutil.WriteFile(GoSumFile, buf.Bytes(), 0666); err != nil { - base.Fatalf("go: writing go.sum: %v", err) - } + if err := renameio.WriteFile(GoSumFile, buf.Bytes()); err != nil { + base.Fatalf("go: writing go.sum: %v", err) } + goSum.checked = make(map[modSum]bool) + goSum.dirty = false + goSum.overwrite = false + if goSum.modverify != "" { - os.Remove(goSum.modverify) + os.Remove(goSum.modverify) // best effort } } @@ -366,6 +527,8 @@ func TrimGoSum(keep map[module.Version]bool) { noGoMod := module.Version{Path: m.Path, Version: strings.TrimSuffix(m.Version, "/go.mod")} if !keep[m] && !keep[noGoMod] { delete(goSum.m, m) + goSum.dirty = true + goSum.overwrite = true } } } diff --git a/src/cmd/go/internal/modfetch/unzip.go b/src/cmd/go/internal/modfetch/unzip.go index a50431fd8629d..113d5b743bc98 100644 --- a/src/cmd/go/internal/modfetch/unzip.go +++ b/src/cmd/go/internal/modfetch/unzip.go @@ -21,12 +21,12 @@ import ( ) func Unzip(dir, zipfile, prefix string, maxSize int64) error { + // TODO(bcmills): The maxSize parameter is invariantly 0. Remove it. if maxSize == 0 { maxSize = codehost.MaxZipFile } // Directory can exist, but must be empty. - // except maybe files, _ := ioutil.ReadDir(dir) if len(files) > 0 { return fmt.Errorf("target directory %v exists and is not empty", dir) @@ -113,7 +113,7 @@ func Unzip(dir, zipfile, prefix string, maxSize int64) error { if err := os.MkdirAll(filepath.Dir(dst), 0777); err != nil { return err } - w, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0444) + w, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0444) if err != nil { return fmt.Errorf("unzip %v: %v", zipfile, err) } @@ -143,11 +143,27 @@ func Unzip(dir, zipfile, prefix string, maxSize int64) error { dirlist = append(dirlist, dir) } sort.Strings(dirlist) - // Run over list backward to chmod children before parents. for i := len(dirlist) - 1; i >= 0; i-- { + // TODO(bcmills): Does this end up stomping on the umask of the cache directory? os.Chmod(dirlist[i], 0555) } return nil } + +// RemoveAll removes a directory written by Download or Unzip, first applying +// any permission changes needed to do so. +func RemoveAll(dir string) error { + // Module cache has 0555 directories; make them writable in order to remove content. + filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return nil // ignore errors walking in file system + } + if info.IsDir() { + os.Chmod(path, 0777) + } + return nil + }) + return os.RemoveAll(dir) +} diff --git a/src/cmd/go/testdata/script/mod_concurrent.txt b/src/cmd/go/testdata/script/mod_concurrent.txt new file mode 100644 index 0000000000000..e03e5e5edbeda --- /dev/null +++ b/src/cmd/go/testdata/script/mod_concurrent.txt @@ -0,0 +1,31 @@ +env GO111MODULE=on + +# Concurrent builds should succeed, even if they need to download modules. +go build ./x & +go build ./y +wait + +# Concurrent builds should update go.sum to the union of the hashes for the +# modules they read. +cmp go.sum go.sum.want + +-- go.mod -- +module golang.org/issue/26794 + +require ( + golang.org/x/text v0.3.0 + rsc.io/sampler v1.0.0 +) +-- x/x.go -- +package x + +import _ "golang.org/x/text/language" +-- y/y.go -- +package y + +import _ "rsc.io/sampler" +-- go.sum.want -- +golang.org/x/text v0.3.0 h1:ivTorhoiROmZ1mcs15mO2czVF0uy0tnezXpBVNzgrmA= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +rsc.io/sampler v1.0.0 h1:SRJnjyQ07sAtq6G4RcfJEmz8JxqLyj3PoGXG2VhbDWo= +rsc.io/sampler v1.0.0/go.mod h1:cqxpM3ZVz9VtirqxZPmrWzkQ+UkiNiGtkrN+B+i8kx8=