src/cmd/go/internal/modcmd/verify.go | 11 +++++------ src/cmd/go/internal/modfetch/cache.go | 40 +++++++++++++++++++++++++++++++++++++--- src/cmd/go/internal/modfetch/fetch.go | 62 +++++++++++++++++++++++++++++++++++------------------ src/cmd/go/internal/modload/build.go | 4 +--- src/cmd/go/internal/robustio/robustio_windows.go | 2 +- src/cmd/go/testdata/script/mod_download_partial.txt | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++++ diff --git a/src/cmd/go/internal/modcmd/verify.go b/src/cmd/go/internal/modcmd/verify.go index 81fc44dc97ae609cffc8e43b56ddaa1d9435f9e4..dd1665619b6241375cc7b71d21c90d5d1743f8aa 100644 --- a/src/cmd/go/internal/modcmd/verify.go +++ b/src/cmd/go/internal/modcmd/verify.go @@ -7,6 +7,7 @@ import ( "bytes" "cmd/go/internal/cfg" + "errors" "fmt" "io/ioutil" "os" @@ -61,12 +62,10 @@ if zipErr == nil { _, zipErr = os.Stat(zip) } dir, dirErr := modfetch.DownloadDir(mod) - if dirErr == nil { - _, dirErr = os.Stat(dir) - } data, err := ioutil.ReadFile(zip + "hash") if err != nil { - if zipErr != nil && os.IsNotExist(zipErr) && dirErr != nil && os.IsNotExist(dirErr) { + if zipErr != nil && errors.Is(zipErr, os.ErrNotExist) && + dirErr != nil && errors.Is(dirErr, os.ErrNotExist) { // Nothing downloaded yet. Nothing to verify. return true } @@ -75,7 +74,7 @@ return false } h := string(bytes.TrimSpace(data)) - if zipErr != nil && os.IsNotExist(zipErr) { + if zipErr != nil && errors.Is(zipErr, os.ErrNotExist) { // ok } else { hZ, err := dirhash.HashZip(zip, dirhash.DefaultHash) @@ -87,7 +86,7 @@ base.Errorf("%s %s: zip has been modified (%v)", mod.Path, mod.Version, zip) ok = false } } - if dirErr != nil && os.IsNotExist(dirErr) { + if dirErr != nil && errors.Is(dirErr, os.ErrNotExist) { // ok } else { hD, err := dirhash.HashDir(dir, mod.Path+"@"+mod.Version, dirhash.DefaultHash) diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go index c0062809d172de0008125120bd7ffbd05da8abf3..446c8fec8b53888e94135322fdeeb2777e4a4922 100644 --- a/src/cmd/go/internal/modfetch/cache.go +++ b/src/cmd/go/internal/modfetch/cache.go @@ -7,6 +7,7 @@ import ( "bytes" "encoding/json" + "errors" "fmt" "io" "io/ioutil" @@ -57,8 +58,11 @@ } return filepath.Join(dir, encVer+"."+suffix), nil } -// DownloadDir returns the directory to which m should be downloaded. -// Note that the directory may not yet exist. +// DownloadDir returns the directory to which m should have been downloaded. +// An error will be returned if the module path or version cannot be escaped. +// An error satisfying errors.Is(err, os.ErrNotExist) will be returned +// along with the directory if the directory does not exist or if the directory +// is not completely populated. func DownloadDir(m module.Version) (string, error) { if PkgMod == "" { return "", fmt.Errorf("internal error: modfetch.PkgMod not set") @@ -77,8 +81,38 @@ encVer, err := module.EncodeVersion(m.Version) if err != nil { return "", err } - return filepath.Join(PkgMod, enc+"@"+encVer), nil + + dir := filepath.Join(PkgMod, enc+"@"+encVer) + if fi, err := os.Stat(dir); os.IsNotExist(err) { + return dir, err + } else if err != nil { + return dir, &DownloadDirPartialError{dir, err} + } else if !fi.IsDir() { + return dir, &DownloadDirPartialError{dir, errors.New("not a directory")} + } + partialPath, err := CachePath(m, "partial") + if err != nil { + return dir, err + } + if _, err := os.Stat(partialPath); err == nil { + return dir, &DownloadDirPartialError{dir, errors.New("not completely extracted")} + } else if !os.IsNotExist(err) { + return dir, err + } + return dir, nil } + +// DownloadDirPartialError is returned by DownloadDir if a module directory +// exists but was not completely populated. +// +// DownloadDirPartialError is equivalent to os.ErrNotExist. +type DownloadDirPartialError struct { + Dir string + Err error +} + +func (e *DownloadDirPartialError) Error() string { return fmt.Sprintf("%s: %v", e.Dir, e.Err) } +func (e *DownloadDirPartialError) Is(err error) bool { return err == os.ErrNotExist } // lockVersion locks a file within the module cache that guards the downloading // and extraction of the zipfile for the given module version. diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go index 51a56028c4acbeab3d3c8b2130916b408a3e6921..d5516dd8d9b0ddd925f34f110b8fb09d5e90e7f5 100644 --- a/src/cmd/go/internal/modfetch/fetch.go +++ b/src/cmd/go/internal/modfetch/fetch.go @@ -7,6 +7,7 @@ import ( "archive/zip" "bytes" + "errors" "fmt" "io" "io/ioutil" @@ -22,6 +23,7 @@ "cmd/go/internal/dirhash" "cmd/go/internal/module" "cmd/go/internal/par" "cmd/go/internal/renameio" + "cmd/go/internal/robustio" ) var downloadCache par.Cache @@ -41,24 +43,27 @@ dir string err error } c := downloadCache.Do(mod, func() interface{} { - dir, err := DownloadDir(mod) + dir, err := download(mod) if err != nil { return cached{"", err} } - if err := download(mod, dir); err != nil { - return cached{"", err} - } checkMod(mod) return cached{dir, nil} }).(cached) 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 +func download(mod module.Version) (dir string, err error) { + // If the directory exists, and no .partial file exists, + // the module has already been completely extracted. + // .partial files may be created when future versions of cmd/go + // extract module zip directories in place instead of extracting + // to a random temporary directory and renaming. + dir, err = DownloadDir(mod) + if err == nil { + return dir, nil + } else if dir == "" || !errors.Is(err, os.ErrNotExist) { + return "", err } // To avoid cluttering the cache with extraneous files, @@ -66,7 +71,7 @@ // DownloadZip uses the same lockfile as Download. // Invoke DownloadZip before locking the file. zipfile, err := DownloadZip(mod) if err != nil { - return err + return "", err } if cfg.CmdName != "mod download" { @@ -75,17 +80,19 @@ } unlock, err := lockVersion(mod) if err != nil { - return err + 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 + _, dirErr := DownloadDir(mod) + if dirErr == nil { + return dir, nil } + _, dirExists := dirErr.(*DownloadDirPartialError) - // Clean up any remaining temporary directories from previous runs. + // Clean up any remaining temporary directories from previous runs, as well + // as partially extracted diectories created by future versions of cmd/go. // This is only safe to do because the lock file ensures that their writers // are no longer active. parentDir := filepath.Dir(dir) @@ -95,6 +102,19 @@ for _, path := range old { RemoveAll(path) // best effort } } + if dirExists { + if err := RemoveAll(dir); err != nil { + return "", err + } + } + + partialPath, err := CachePath(mod, "partial") + if err != nil { + return "", err + } + if err := os.Remove(partialPath); err != nil && !os.IsNotExist(err) { + return "", err + } // 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 @@ -102,11 +122,11 @@ // 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 + return "", err } tmpDir, err := ioutil.TempDir(parentDir, tmpPrefix) if err != nil { - return err + return "", err } defer func() { if err != nil { @@ -117,17 +137,17 @@ modpath := mod.Path + "@" + mod.Version if err := Unzip(tmpDir, zipfile, modpath, 0); err != nil { fmt.Fprintf(os.Stderr, "-> %s\n", err) - return err + return "", err } - if err := os.Rename(tmpDir, dir); err != nil { - return err + if err := robustio.Rename(tmpDir, dir); err != nil { + return "", err } // Make dir read-only only *after* renaming it. // os.Rename was observed to fail for read-only directories on macOS. makeDirsReadOnly(dir) - return nil + return dir, nil } var downloadZipCache par.Cache diff --git a/src/cmd/go/internal/modload/build.go b/src/cmd/go/internal/modload/build.go index ff42516c807ddce7ef94f2702849c8060d9afe5b..a0c6b0bf74f466b8a7a50a6121622703d6796341 100644 --- a/src/cmd/go/internal/modload/build.go +++ b/src/cmd/go/internal/modload/build.go @@ -143,9 +143,7 @@ } } dir, err := modfetch.DownloadDir(mod) if err == nil { - if info, err := os.Stat(dir); err == nil && info.IsDir() { - m.Dir = dir - } + m.Dir = dir } } } diff --git a/src/cmd/go/internal/robustio/robustio_windows.go b/src/cmd/go/internal/robustio/robustio_windows.go index a3d94e566fe420340f93aa1d4d3da5c544e1c051..1eb8cfbccfae605a80668e4bbe1060bab2b36edc 100644 --- a/src/cmd/go/internal/robustio/robustio_windows.go +++ b/src/cmd/go/internal/robustio/robustio_windows.go @@ -14,7 +14,7 @@ "syscall" "time" ) -const arbitraryTimeout = 500 * time.Millisecond +const arbitraryTimeout = 2000 * time.Millisecond // retry retries ephemeral errors from f up to an arbitrary timeout // to work around spurious filesystem errors on Windows diff --git a/src/cmd/go/testdata/script/mod_download_partial.txt b/src/cmd/go/testdata/script/mod_download_partial.txt new file mode 100644 index 0000000000000000000000000000000000000000..cda358b99f001233e883b5d202125af52b8e0aae --- /dev/null +++ b/src/cmd/go/testdata/script/mod_download_partial.txt @@ -0,0 +1,56 @@ +# Download a module +go mod download rsc.io/quote +exists $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/go.mod + +# 'go mod verify' should fail if we delete a file. +go mod verify +chmod 0755 $GOPATH/pkg/mod/rsc.io/quote@v1.5.2 +rm $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/go.mod +! go mod verify + +# Create a .partial file to simulate an failure extracting the zip file. +cp empty $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.partial + +# 'go mod verify' should not fail, since the module hasn't been completely +# ingested into the cache. +go mod verify + +# 'go list' should not load packages from the directory. +# NOTE: the message "directory $dir outside available modules" is reported +# for directories not in the main module, active modules in the module cache, +# or local replacements. In this case, the directory is in the right place, +# but it's incomplete, so 'go list' acts as if it's not an active module. +! go list $GOPATH/pkg/mod/rsc.io/quote@v1.5.2 +stderr 'outside available modules' + +# 'go list -m' should not print the directory. +go list -m -f '{{.Dir}}' rsc.io/quote +! stdout . + +# 'go mod download' should re-extract the module and remove the .partial file. +go mod download rsc.io/quote +! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.partial +exists $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/go.mod + +# 'go list' should succeed. +go list $GOPATH/pkg/mod/rsc.io/quote@v1.5.2 +stdout '^rsc.io/quote$' + +# 'go list -m' should print the directory. +go list -m -f '{{.Dir}}' rsc.io/quote +stdout 'pkg[/\\]mod[/\\]rsc.io[/\\]quote@v1.5.2' + +# go mod verify should fail if we delete a file. +go mod verify +chmod 0755 $GOPATH/pkg/mod/rsc.io/quote@v1.5.2 +rm $GOPATH/pkg/mod/rsc.io/quote@v1.5.2/go.mod +! go mod verify + +-- go.mod -- +module m + +go 1.14 + +require rsc.io/quote v1.5.2 + +-- empty --