[go] cmd/go/internal/modfetch: detect and recover from missing ziphash file
6 views
Skip to first unread message
Jay Conrod (Gerrit)
Mar 4, 2021, 5:41:07 PM3/4/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Jay Conrod, Michael Matloob.
Jay Conrod uploaded patch set #3 to this change.
cmd/go/internal/modfetch: detect and recover from missing ziphash file
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash and re-extract the zip file.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes #44749
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
---
M src/cmd/go/internal/modfetch/cache.go
M src/cmd/go/internal/modfetch/fetch.go
A src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
M src/cmd/go/testdata/script/mod_verify.txt
4 files changed, 123 insertions(+), 31 deletions(-)
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 4, 2021, 5:49:58 PM3/4/21
to goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, Michael Matloob, golang-co...@googlegroups.com
Attention is currently required from: Michael Matloob.
Patch set 3:Run-TryBot +1Trust +1
3 comments:
Patchset:
TRY=linux-amd64-longtest,windows-amd64-longtest
File src/cmd/go/internal/modfetch/cache.go:
Patch Set #2, Line 113:
return dir, &DownloadDirPartialError{dir, errors.New("ziphash file is missing")}(not sure) […]
Done. This should also mean we won't re-extract the directory if it's present.
File src/cmd/go/testdata/script/mod_verify.txt:
Patch Set #2, Line 53:
! go mod verifyMaybe add a stderr assertion here, too? I'm not entirely confident that we're adding the module and […]
Done
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 4, 2021, 6:21:52 PM3/4/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Jay Conrod, Michael Matloob.
Jay Conrod uploaded patch set #4 to this change.
cmd/go/internal/modfetch: detect and recover from missing ziphash file
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes #44749
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
---
M src/cmd/go/internal/modfetch/cache.go
M src/cmd/go/internal/modfetch/fetch.go
A src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
M src/cmd/go/testdata/script/mod_verify.txt
4 files changed, 123 insertions(+), 31 deletions(-)
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 4, 2021, 6:22:04 PM3/4/21
to goph...@pubsubhelper.golang.org, Go Bot, Bryan C. Mills, Michael Matloob, golang-co...@googlegroups.com
Attention is currently required from: Michael Matloob.
Patch set 4:Run-TryBot +1Trust +1
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Mar 4, 2021, 10:36:52 PM3/4/21
to Jay Conrod, goph...@pubsubhelper.golang.org, Go Bot, Bryan C. Mills, Michael Matloob, golang-co...@googlegroups.com
Attention is currently required from: Jay Conrod, Michael Matloob.
Patch set 4:Code-Review +2
1 comment:
File src/cmd/go/testdata/script/mod_get_missing_ziphash.txt:
Can we set GOPROXY=off at this point to ensure that the zip is really not deleted and redownloaded?
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 5, 2021, 9:29:38 AM3/5/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Jay Conrod, Michael Matloob.
Jay Conrod uploaded patch set #5 to this change.
cmd/go/internal/modfetch: detect and recover from missing ziphash file
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes #44749
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
---
M src/cmd/go/internal/modfetch/cache.go
M src/cmd/go/internal/modfetch/fetch.go
A src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
M src/cmd/go/testdata/script/mod_verify.txt
4 files changed, 126 insertions(+), 31 deletions(-)
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 5, 2021, 9:32:53 AM3/5/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Jay Conrod, Michael Matloob.
Jay Conrod uploaded patch set #6 to this change.
cmd/go/internal/modfetch: detect and recover from missing ziphash file
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes #44749
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
---
M src/cmd/go/internal/modfetch/cache.go
M src/cmd/go/internal/modfetch/fetch.go
A src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
M src/cmd/go/testdata/script/mod_verify.txt
4 files changed, 125 insertions(+), 31 deletions(-)
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 5, 2021, 9:34:07 AM3/5/21
to goph...@pubsubhelper.golang.org, Go Bot, Bryan C. Mills, Michael Matloob, golang-co...@googlegroups.com
Attention is currently required from: Michael Matloob.
Patch set 6:Run-TryBot +1Trust +1
1 comment:
File src/cmd/go/testdata/script/mod_get_missing_ziphash.txt:
Can we set GOPROXY=off at this point to ensure that the zip is really not deleted and redownloaded?
Done. Apparently we need to disable GOSUMDB as well because we access the sumdb through the test proxy. I'm a little surprised we're doing a lookup after this point instead of just hitting the sumdb cache. Is that WAI?
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 5, 2021, 10:27:56 AM3/5/21
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go Bot, Bryan C. Mills, Michael Matloob, golang-co...@googlegroups.com
Jay Conrod submitted this change.
Approvals:
Bryan C. Mills: Looks good to me, approved
Jay Conrod: Trusted; Run TryBots
Go Bot: TryBots succeeded
cmd/go/internal/modfetch: detect and recover from missing ziphash file
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes #44749
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
Reviewed-on: https://go-review.googlesource.com/c/go/+/298352
Trust: Jay Conrod <jayc...@google.com>
Run-TryBot: Jay Conrod <jayc...@google.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Bryan C. Mills <bcm...@google.com>
---
M src/cmd/go/internal/modfetch/cache.go
M src/cmd/go/internal/modfetch/fetch.go
A src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
M src/cmd/go/testdata/script/mod_verify.txt
4 files changed, 125 insertions(+), 31 deletions(-)
diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go
index 9e75193..10f7745 100644
--- a/src/cmd/go/internal/modfetch/cache.go
+++ b/src/cmd/go/internal/modfetch/cache.go
@@ -80,6 +80,7 @@
return "", err
}
+ // Check whether the directory itself exists.
dir := filepath.Join(cfg.GOMODCACHE, enc+"@"+encVer)
if fi, err := os.Stat(dir); os.IsNotExist(err) {
return dir, err
@@ -88,6 +89,9 @@
} else if !fi.IsDir() {
return dir, &DownloadDirPartialError{dir, errors.New("not a directory")}
}
+
+ // Check if a .partial file exists. This is created at the beginning of
+ // a download and removed after the zip is extracted.
partialPath, err := CachePath(m, "partial")
if err != nil {
return dir, err
@@ -97,6 +101,19 @@
} else if !os.IsNotExist(err) {
return dir, err
}
+
+ // Check if a .ziphash file exists. It should be created before the
+ // zip is extracted, but if it was deleted (by another program?), we need
+ // to re-calculate it.
+ ziphashPath, err := CachePath(m, "ziphash")
+ if err != nil {
+ return dir, err
+ }
+ if _, err := os.Stat(ziphashPath); os.IsNotExist(err) {
+ return dir, &DownloadDirPartialError{dir, errors.New("ziphash file is missing")}
+ } else if err != nil {
+ return dir, err
+ }
return dir, nil
}
diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go
index d5ad277..7b4ce21 100644
--- a/src/cmd/go/internal/modfetch/fetch.go
+++ b/src/cmd/go/internal/modfetch/fetch.go
@@ -168,13 +168,16 @@
if err != nil {
return cached{"", err}
}
+ ziphashfile := zipfile + "hash"
- // Skip locking if the zipfile already exists.
+ // Return without locking if the zip and ziphash files exist.
if _, err := os.Stat(zipfile); err == nil {
- return cached{zipfile, nil}
+ if _, err := os.Stat(ziphashfile); err == nil {
+ return cached{zipfile, nil}
+ }
}
- // The zip file does not exist. Acquire the lock and create it.
+ // The zip or ziphash file does not exist. Acquire the lock and create them.
if cfg.CmdName != "mod download" {
fmt.Fprintf(os.Stderr, "go: downloading %s %s\n", mod.Path, mod.Version)
}
@@ -184,14 +187,6 @@
}
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(ctx, mod, zipfile); err != nil {
return cached{"", err}
}
@@ -204,6 +199,25 @@
ctx, span := trace.StartSpan(ctx, "modfetch.downloadZip "+zipfile)
defer span.Done()
+ // Double-check that the zipfile was not created while we were waiting for
+ // the lock in DownloadZip.
+ ziphashfile := zipfile + "hash"
+ var zipExists, ziphashExists bool
+ if _, err := os.Stat(zipfile); err == nil {
+ zipExists = true
+ }
+ if _, err := os.Stat(ziphashfile); err == nil {
+ ziphashExists = true
+ }
+ if zipExists && ziphashExists {
+ return nil
+ }
+
+ // Create parent directories.
+ if err := os.MkdirAll(filepath.Dir(zipfile), 0777); err != nil {
+ return err
+ }
+
// 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.
@@ -215,6 +229,12 @@
}
}
+ // If the zip file exists, the ziphash file must have been deleted
+ // or lost after a file system crash. Re-hash the zip without downloading.
+ if zipExists {
+ return hashZip(mod, zipfile, ziphashfile)
+ }
+
// 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
@@ -287,15 +307,7 @@
}
// 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 := checkModSum(mod, hash); err != nil {
- return err
- }
-
- if err := renameio.WriteFile(zipfile+"hash", []byte(hash), 0666); err != nil {
+ if err := hashZip(mod, f.Name(), ziphashfile); err != nil {
return err
}
if err := os.Rename(f.Name(), zipfile); err != nil {
@@ -307,6 +319,22 @@
return nil
}
+// hashZip reads the zip file opened in f, then writes the hash to ziphashfile,
+// overwriting that file if it exists.
+//
+// If the hash does not match go.sum (or the sumdb if enabled), hashZip returns
+// an error and does not write ziphashfile.
+func hashZip(mod module.Version, zipfile, ziphashfile string) error {
+ hash, err := dirhash.HashZip(zipfile, dirhash.DefaultHash)
+ if err != nil {
+ return err
+ }
+ if err := checkModSum(mod, hash); err != nil {
+ return err
+ }
+ return renameio.WriteFile(ziphashfile, []byte(hash), 0666)
+}
+
// makeDirsReadOnly makes a best-effort attempt to remove write permissions for dir
// and its transitive contents.
func makeDirsReadOnly(dir string) {
@@ -450,11 +478,6 @@
// checkMod checks the given module's checksum.
func checkMod(mod module.Version) {
- if cfg.GOMODCACHE == "" {
- // Do not use current directory.
- return
- }
-
// Do the file I/O before acquiring the go.sum lock.
ziphash, err := CachePath(mod, "ziphash")
if err != nil {
@@ -462,10 +485,6 @@
}
data, err := renameio.ReadFile(ziphash)
if err != nil {
- if errors.Is(err, fs.ErrNotExist) {
- // This can happen if someone does rm -rf GOPATH/src/cache/download. So it goes.
- return
- }
base.Fatalf("verifying %v", module.VersionError(mod, err))
}
h := strings.TrimSpace(string(data))
diff --git a/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt b/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
new file mode 100644
index 0000000..8f6793e
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
@@ -0,0 +1,55 @@
+# Test that if the module cache contains an extracted source directory but not
+# a ziphash, 'go build' complains about a missing sum, and 'go get' adds
+# the sum. Verifies #44749.
+
+# With a tidy go.sum, go build succeeds. This also populates the module cache.
+cp go.sum.tidy go.sum
+go build -n use
+env GOPROXY=off
+env GOSUMDB=off
+
+# Control case: if we delete the hash for rsc.io/quote v1.5.2,
+# 'go build' reports an error. 'go get' adds the sum.
+cp go.sum.bug go.sum
+! go build -n use
+stderr '^use.go:3:8: missing go.sum entry for module providing package rsc.io/quote \(imported by use\); to add:\n\tgo get use$'
+go get -d use
+cmp go.sum go.sum.tidy
+go build -n use
+
+# If we delete the hash *and* the ziphash file, we should see the same behavior.
+cp go.sum.bug go.sum
+rm $WORK/gopath/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.ziphash
+! go build -n use
+stderr '^use.go:3:8: missing go.sum entry for module providing package rsc.io/quote \(imported by use\); to add:\n\tgo get use$'
+go get -d use
+cmp go.sum go.sum.tidy
+go build -n use
+
+-- go.mod --
+module use
+
+go 1.17
+
+require rsc.io/quote v1.5.2
+-- go.sum.tidy --
+golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:pvCbr/wm8HzDD3fVywevekufpn6tCGPY3spdHeZJEsw=
+golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+rsc.io/quote v1.5.2 h1:3fEykkD9k7lYzXqCYrwGAf7iNhbk4yCjHmKBN9td4L0=
+rsc.io/quote v1.5.2/go.mod h1:LzX7hefJvL54yjefDEDHNONDjII0t9xZLPXsUe+TKr0=
+rsc.io/sampler v1.3.0 h1:HLGR/BgEtI3r0uymSP/nl2uPLsUnNJX8toRyhfpBTII=
+rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
+rsc.io/testonly v1.0.0 h1:K/VWHdO+Jv7woUXG0GzVNx1czBXUt3Ib1deaMn+xk64=
+rsc.io/testonly v1.0.0/go.mod h1:OqmGbIFOcF+XrFReLOGZ6BhMM7uMBiQwZsyNmh74SzY=
+-- go.sum.bug --
+golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c h1:pvCbr/wm8HzDD3fVywevekufpn6tCGPY3spdHeZJEsw=
+golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
+rsc.io/quote v1.5.2/go.mod h1:LzX7hefJvL54yjefDEDHNONDjII0t9xZLPXsUe+TKr0=
+rsc.io/sampler v1.3.0 h1:HLGR/BgEtI3r0uymSP/nl2uPLsUnNJX8toRyhfpBTII=
+rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
+rsc.io/testonly v1.0.0 h1:K/VWHdO+Jv7woUXG0GzVNx1czBXUt3Ib1deaMn+xk64=
+rsc.io/testonly v1.0.0/go.mod h1:OqmGbIFOcF+XrFReLOGZ6BhMM7uMBiQwZsyNmh74SzY=
+-- use.go --
+package use
+
+import _ "rsc.io/quote"
diff --git a/src/cmd/go/testdata/script/mod_verify.txt b/src/cmd/go/testdata/script/mod_verify.txt
index 43812d0..b510665 100644
--- a/src/cmd/go/testdata/script/mod_verify.txt
+++ b/src/cmd/go/testdata/script/mod_verify.txt
@@ -48,10 +48,13 @@
grep '^rsc.io/quote v1.1.0/go.mod ' go.sum
grep '^rsc.io/quote v1.1.0 ' go.sum
-# sync should ignore missing ziphash; verify should not
+# verify should fail on a missing ziphash. tidy should restore it.
rm $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.ziphash
-go mod tidy
! go mod verify
+stderr '^rsc.io/quote v1.1.0: missing ziphash: open '$GOPATH'[/\\]pkg[/\\]mod[/\\]cache[/\\]download[/\\]rsc.io[/\\]quote[/\\]@v[/\\]v1.1.0.ziphash'
+go mod tidy
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.ziphash
+go mod verify
# Packages below module root should not be mentioned in go.sum.
rm go.sum
4 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
The name of the file: src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
Insertions: 2, Deletions: 0.
```
@@ +7:9 @@
+ env GOPROXY=off
+ env GOSUMDB=off
```
To view, visit change 298352. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 5, 2021, 10:30:42 AM3/5/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Jay Conrod has uploaded this change for review.
[release-branch.go1.16] cmd/go/internal/modfetch: detect and recover from missing ziphash file
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes #44812
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
Reviewed-on: https://go-review.googlesource.com/c/go/+/298352
Trust: Jay Conrod <jayc...@google.com>
Run-TryBot: Jay Conrod <jayc...@google.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Bryan C. Mills <bcm...@google.com>
(cherry picked from commit 302a400316319501748c0f034464fa70e7815272)
---
M src/cmd/go/internal/modfetch/cache.go
M src/cmd/go/internal/modfetch/fetch.go
A src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
M src/cmd/go/testdata/script/mod_verify.txt
4 files changed, 125 insertions(+), 31 deletions(-)
diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go
index 3a2ff63..07e046c 100644
--- a/src/cmd/go/internal/modfetch/cache.go
+++ b/src/cmd/go/internal/modfetch/cache.go
@@ -84,6 +84,7 @@
return "", err
}
+ // Check whether the directory itself exists.
dir := filepath.Join(cfg.GOMODCACHE, enc+"@"+encVer)
if fi, err := os.Stat(dir); os.IsNotExist(err) {
return dir, err
@@ -92,6 +93,9 @@
} else if !fi.IsDir() {
return dir, &DownloadDirPartialError{dir, errors.New("not a directory")}
}
+
+ // Check if a .partial file exists. This is created at the beginning of
+ // a download and removed after the zip is extracted.
partialPath, err := CachePath(m, "partial")
if err != nil {
return dir, err
@@ -101,6 +105,19 @@
} else if !os.IsNotExist(err) {
return dir, err
}
+
+ // Check if a .ziphash file exists. It should be created before the
+ // zip is extracted, but if it was deleted (by another program?), we need
+ // to re-calculate it.
+ ziphashPath, err := CachePath(m, "ziphash")
+ if err != nil {
+ return dir, err
+ }
+ if _, err := os.Stat(ziphashPath); os.IsNotExist(err) {
+ return dir, &DownloadDirPartialError{dir, errors.New("ziphash file is missing")}
+ } else if err != nil {
+ return dir, err
+ }
return dir, nil
}
diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go
index c55c3cf..eb7d30e 100644
--- a/src/cmd/go/internal/modfetch/fetch.go
+++ b/src/cmd/go/internal/modfetch/fetch.go
@@ -170,13 +170,16 @@
if err != nil {
return cached{"", err}
}
+ ziphashfile := zipfile + "hash"
- // Skip locking if the zipfile already exists.
+ // Return without locking if the zip and ziphash files exist.
if _, err := os.Stat(zipfile); err == nil {
- return cached{zipfile, nil}
+ if _, err := os.Stat(ziphashfile); err == nil {
+ return cached{zipfile, nil}
+ }
}
- // The zip file does not exist. Acquire the lock and create it.
+ // The zip or ziphash file does not exist. Acquire the lock and create them.
if cfg.CmdName != "mod download" {
fmt.Fprintf(os.Stderr, "go: downloading %s %s\n", mod.Path, mod.Version)
}
@@ -186,14 +189,6 @@
}
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(ctx, mod, zipfile); err != nil {
return cached{"", err}
}
@@ -206,6 +201,25 @@
ctx, span := trace.StartSpan(ctx, "modfetch.downloadZip "+zipfile)
defer span.Done()
+ // Double-check that the zipfile was not created while we were waiting for
+ // the lock in DownloadZip.
+ ziphashfile := zipfile + "hash"
+ var zipExists, ziphashExists bool
+ if _, err := os.Stat(zipfile); err == nil {
+ zipExists = true
+ }
+ if _, err := os.Stat(ziphashfile); err == nil {
+ ziphashExists = true
+ }
+ if zipExists && ziphashExists {
+ return nil
+ }
+
+ // Create parent directories.
+ if err := os.MkdirAll(filepath.Dir(zipfile), 0777); err != nil {
+ return err
+ }
+
// 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.
@@ -217,6 +231,12 @@
}
}
+ // If the zip file exists, the ziphash file must have been deleted
+ // or lost after a file system crash. Re-hash the zip without downloading.
+ if zipExists {
+ return hashZip(mod, zipfile, ziphashfile)
+ }
+
// 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
@@ -289,15 +309,7 @@
}
// 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 := checkModSum(mod, hash); err != nil {
- return err
- }
-
- if err := renameio.WriteFile(zipfile+"hash", []byte(hash), 0666); err != nil {
+ if err := hashZip(mod, f.Name(), ziphashfile); err != nil {
return err
}
if err := os.Rename(f.Name(), zipfile); err != nil {
@@ -309,6 +321,22 @@
return nil
}
+// hashZip reads the zip file opened in f, then writes the hash to ziphashfile,
+// overwriting that file if it exists.
+//
+// If the hash does not match go.sum (or the sumdb if enabled), hashZip returns
+// an error and does not write ziphashfile.
+func hashZip(mod module.Version, zipfile, ziphashfile string) error {
+ hash, err := dirhash.HashZip(zipfile, dirhash.DefaultHash)
+ if err != nil {
+ return err
+ }
+ if err := checkModSum(mod, hash); err != nil {
+ return err
+ }
+ return renameio.WriteFile(ziphashfile, []byte(hash), 0666)
+}
+
// makeDirsReadOnly makes a best-effort attempt to remove write permissions for dir
// and its transitive contents.
func makeDirsReadOnly(dir string) {
@@ -452,11 +480,6 @@
// checkMod checks the given module's checksum.
func checkMod(mod module.Version) {
- if cfg.GOMODCACHE == "" {
- // Do not use current directory.
- return
- }
-
// Do the file I/O before acquiring the go.sum lock.
ziphash, err := CachePath(mod, "ziphash")
if err != nil {
@@ -464,10 +487,6 @@To view, visit change 298851. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 5, 2021, 10:31:17 AM3/5/21
to goph...@pubsubhelper.golang.org, Bryan C. Mills, golang-co...@googlegroups.com
Attention is currently required from: Bryan C. Mills.
Patch set 1:Run-TryBot +1Trust +1
1 comment:
Patchset:
TRY=linux-amd64-longtest,windows-amd64-longtest
To view, visit change 298851. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Mar 5, 2021, 10:59:24 AM3/5/21
to Jay Conrod, goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, golang-co...@googlegroups.com
Attention is currently required from: Jay Conrod.
Patch set 1:Code-Review +2
Jay Conrod (Gerrit)
Mar 8, 2021, 4:49:20 PM3/8/21
to goph...@pubsubhelper.golang.org, Bryan C. Mills, golang-co...@googlegroups.com
Attention is currently required from: Bryan C. Mills.
Patch set 2:Run-TryBot +1Trust +1
1 comment:
Patchset:
TRY=linux-amd64-longtest
To view, visit change 299830. To unsubscribe, or for help writing mail filters, visit settings.
Bryan C. Mills (Gerrit)
Mar 8, 2021, 5:15:13 PM3/8/21
to Jay Conrod, goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, golang-co...@googlegroups.com
Attention is currently required from: Jay Conrod.
Patch set 2:Code-Review +2
1 comment:
Patchset:
Patch Set 2: TryBot-Result-1
1 of 22 SlowBots failed:
Failed on windows-amd64-longtest: https://storage.googleapis.com/go-build-log/d1887908/windows-amd64-longtest_deed7f7f.logConsult https://build.golang.org/ to see whether they are new failures. Keep in mind that TryBots currently test *exactly* your git commit, without rebasing. If your commit's git parent is old, the failure might've already been fixed.
SlowBot builds that ran:
- linux-amd64-longtest
That looks like maybe https://golang.org/issue/42942?
To view, visit change 299830. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 9, 2021, 10:18:03 AM3/9/21
to goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, golang-co...@googlegroups.com
Jay Conrod removed a vote from this change.
Removed TryBot-Result-1 by Go Bot <go...@golang.org>
To view, visit change 299830. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 9, 2021, 10:18:03 AM3/9/21
to goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, golang-co...@googlegroups.com
1 comment:
Patchset:
> Patch Set 2: TryBot-Result-1 […]
Possibly... definitely not related to this CL. Starting TryAgainBots...
To view, visit change 299830. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 9, 2021, 10:18:16 AM3/9/21
to goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, golang-co...@googlegroups.com
Jay Conrod removed a vote from this change.
Removed Run-TryBot+1 by Jay Conrod <jayc...@google.com>
To view, visit change 299830. To unsubscribe, or for help writing mail filters, visit settings.
Jay Conrod (Gerrit)
Mar 9, 2021, 10:18:25 AM3/9/21
to goph...@pubsubhelper.golang.org, Bryan C. Mills, Go Bot, golang-co...@googlegroups.com
Patch set 2:Run-TryBot +1
Alexander Rakoczy (Gerrit)
Mar 25, 2021, 2:28:38 PM3/25/21
to Jay Conrod, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Bryan C. Mills, Go Bot, golang-co...@googlegroups.com
Alexander Rakoczy submitted this change.
Approvals:
Bryan C. Mills: Looks good to me, approved
Jay Conrod: Trusted; Run TryBots
Go Bot: TryBots succeeded
[release-branch.go1.15] cmd/go/internal/modfetch: detect and recover from missing ziphash file
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes #44872
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
Reviewed-on: https://go-review.googlesource.com/c/go/+/298352
Trust: Jay Conrod <jayc...@google.com>
Run-TryBot: Jay Conrod <jayc...@google.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Bryan C. Mills <bcm...@google.com>
(cherry picked from commit 302a400316319501748c0f034464fa70e7815272)
Reviewed-on: https://go-review.googlesource.com/c/go/+/299830
---
M src/cmd/go/internal/modfetch/cache.go
M src/cmd/go/internal/modfetch/fetch.go
A src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
M src/cmd/go/testdata/script/mod_verify.txt
4 files changed, 125 insertions(+), 31 deletions(-)
diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go
index e3074b7..0b64a69 100644
--- a/src/cmd/go/internal/modfetch/cache.go
+++ b/src/cmd/go/internal/modfetch/cache.go
@@ -83,6 +83,7 @@
return "", err
}
+ // Check whether the directory itself exists.
dir := filepath.Join(cfg.GOMODCACHE, enc+"@"+encVer)
if fi, err := os.Stat(dir); os.IsNotExist(err) {
return dir, err
@@ -91,6 +92,9 @@
} else if !fi.IsDir() {
return dir, &DownloadDirPartialError{dir, errors.New("not a directory")}
}
+
+ // Check if a .partial file exists. This is created at the beginning of
+ // a download and removed after the zip is extracted.
partialPath, err := CachePath(m, "partial")
if err != nil {
return dir, err
@@ -100,6 +104,19 @@
} else if !os.IsNotExist(err) {
return dir, err
}
+
+ // Check if a .ziphash file exists. It should be created before the
+ // zip is extracted, but if it was deleted (by another program?), we need
+ // to re-calculate it.
+ ziphashPath, err := CachePath(m, "ziphash")
+ if err != nil {
+ return dir, err
+ }
+ if _, err := os.Stat(ziphashPath); os.IsNotExist(err) {
+ return dir, &DownloadDirPartialError{dir, errors.New("ziphash file is missing")}
+ } else if err != nil {
+ return dir, err
+ }
return dir, nil
}
diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go
index fd7a5ce..6a6c972 100644
--- a/src/cmd/go/internal/modfetch/fetch.go
+++ b/src/cmd/go/internal/modfetch/fetch.go
@@ -206,13 +206,16 @@
if err != nil {
return cached{"", err}
}
+ ziphashfile := zipfile + "hash"
- // Skip locking if the zipfile already exists.
+ // Return without locking if the zip and ziphash files exist.
if _, err := os.Stat(zipfile); err == nil {
- return cached{zipfile, nil}
+ if _, err := os.Stat(ziphashfile); err == nil {
+ return cached{zipfile, nil}
+ }
}
- // The zip file does not exist. Acquire the lock and create it.
+ // The zip or ziphash file does not exist. Acquire the lock and create them.
if cfg.CmdName != "mod download" {
fmt.Fprintf(os.Stderr, "go: downloading %s %s\n", mod.Path, mod.Version)
}
@@ -222,14 +225,6 @@
}
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}
}
@@ -239,6 +234,25 @@
}
func downloadZip(mod module.Version, zipfile string) (err error) {
+ // Double-check that the zipfile was not created while we were waiting for
+ // the lock in DownloadZip.
+ ziphashfile := zipfile + "hash"
+ var zipExists, ziphashExists bool
+ if _, err := os.Stat(zipfile); err == nil {
+ zipExists = true
+ }
+ if _, err := os.Stat(ziphashfile); err == nil {
+ ziphashExists = true
+ }
+ if zipExists && ziphashExists {
+ return nil
+ }
+
+ // Create parent directories.
+ if err := os.MkdirAll(filepath.Dir(zipfile), 0777); err != nil {
+ return err
+ }
+
// 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.
@@ -250,6 +264,12 @@
}
}
+ // If the zip file exists, the ziphash file must have been deleted
+ // or lost after a file system crash. Re-hash the zip without downloading.
+ if zipExists {
+ return hashZip(mod, zipfile, ziphashfile)
+ }
+
// 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
@@ -306,15 +326,7 @@
}
// 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 := checkModSum(mod, hash); err != nil {
- return err
- }
-
- if err := renameio.WriteFile(zipfile+"hash", []byte(hash), 0666); err != nil {
+ if err := hashZip(mod, f.Name(), ziphashfile); err != nil {
return err
}
if err := os.Rename(f.Name(), zipfile); err != nil {
@@ -326,6 +338,22 @@
return nil
}
+// hashZip reads the zip file opened in f, then writes the hash to ziphashfile,
+// overwriting that file if it exists.
+//
+// If the hash does not match go.sum (or the sumdb if enabled), hashZip returns
+// an error and does not write ziphashfile.
+func hashZip(mod module.Version, zipfile, ziphashfile string) error {
+ hash, err := dirhash.HashZip(zipfile, dirhash.DefaultHash)
+ if err != nil {
+ return err
+ }
+ if err := checkModSum(mod, hash); err != nil {
+ return err
+ }
+ return renameio.WriteFile(ziphashfile, []byte(hash), 0666)
+}
+
// makeDirsReadOnly makes a best-effort attempt to remove write permissions for dir
// and its transitive contents.
func makeDirsReadOnly(dir string) {
@@ -457,11 +485,6 @@
// checkMod checks the given module's checksum.
func checkMod(mod module.Version) {
- if cfg.GOMODCACHE == "" {
- // Do not use current directory.
- return
- }
-
// Do the file I/O before acquiring the go.sum lock.
ziphash, err := CachePath(mod, "ziphash")
if err != nil {
@@ -469,10 +492,6 @@
}
data, err := renameio.ReadFile(ziphash)
if err != nil {
- if errors.Is(err, os.ErrNotExist) {
- // This can happen if someone does rm -rf GOPATH/src/cache/download. So it goes.
- return
- }
base.Fatalf("verifying %v", module.VersionError(mod, err))
}
h := strings.TrimSpace(string(data))
diff --git a/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt b/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
new file mode 100644
index 0000000..afb5d2e
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
@@ -0,0 +1,55 @@
+# Test that if the module cache contains an extracted source directory but not
+# a ziphash, 'go build' complains about a missing sum, and 'go get' adds
+# the sum. Verifies #44749.
+
+# With a tidy go.sum, go build succeeds. This also populates the module cache.
+cp go.sum.tidy go.sum
+go build -n use
+env GOPROXY=off
+env GOSUMDB=off
+
+# Control case: if we delete the hash for rsc.io/quote v1.5.2,
+# 'go build' reports an error. 'go get' adds the sum.
+cp go.sum.bug go.sum
+! go build -n -mod=readonly use
+stderr '^go: updates to go.sum needed, disabled by -mod=readonly$'
+go get -d use
+cmp go.sum go.sum.tidy
+go build -n use
+
+# If we delete the hash *and* the ziphash file, we should see the same behavior.
+cp go.sum.bug go.sum
+rm $WORK/gopath/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.ziphash
+! go build -n -mod=readonly use
+stderr '^go: updates to go.sum needed, disabled by -mod=readonly$'
index 646bc62..7163d1a 100644
--- a/src/cmd/go/testdata/script/mod_verify.txt
+++ b/src/cmd/go/testdata/script/mod_verify.txt
@@ -50,10 +50,13 @@
grep '^rsc.io/quote v1.1.0/go.mod ' go.sum
grep '^rsc.io/quote v1.1.0 ' go.sum
-# sync should ignore missing ziphash; verify should not
+# verify should fail on a missing ziphash. tidy should restore it.
rm $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.ziphash
-go mod tidy
! go mod verify
+stderr '^rsc.io/quote v1.1.0: missing ziphash: open '$GOPATH'[/\\]pkg[/\\]mod[/\\]cache[/\\]download[/\\]rsc.io[/\\]quote[/\\]@v[/\\]v1.1.0.ziphash'
+go mod tidy
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.ziphash
+go mod verify
# Packages below module root should not be mentioned in go.sum.
rm go.sum
To view, visit change 299830. To unsubscribe, or for help writing mail filters, visit settings.
Alexander Rakoczy (Gerrit)
Mar 25, 2021, 2:28:42 PM3/25/21
to Jay Conrod, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Bryan C. Mills, Go Bot, golang-co...@googlegroups.com
Alexander Rakoczy submitted this change.
Approvals:
Bryan C. Mills: Looks good to me, approved
Jay Conrod: Trusted; Run TryBots
Go Bot: TryBots succeeded
[release-branch.go1.16] cmd/go/internal/modfetch: detect and recover from missing ziphash file
Previously, if an extracted module directory existed in the module
cache, but the corresponding ziphash file did not, if the sum was
missing from go.sum, we would not verify the sum. This caused 'go get'
not to write missing sums. 'go build' in readonly mode (now the
default) checks for missing sums and doesn't attempt to fetch modules
that can't be verified against go.sum.
With this change, when requesting the module directory with
modfetch.DownloadDir, if the ziphash file is missing, the go command
will re-hash the zip without downloading or re-extracting it again.
Note that the go command creates the ziphash file before the module
directory, but another program could remove it separately, and it
might not be present after a crash.
Fixes #44812
Change-Id: I64551e048a3ba17d069de1ec123d5b8b2757543c
Reviewed-on: https://go-review.googlesource.com/c/go/+/298352
Trust: Jay Conrod <jayc...@google.com>
Run-TryBot: Jay Conrod <jayc...@google.com>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Bryan C. Mills <bcm...@google.com>
(cherry picked from commit 302a400316319501748c0f034464fa70e7815272)
Reviewed-on: https://go-review.googlesource.com/c/go/+/298851
---
M src/cmd/go/internal/modfetch/cache.go
M src/cmd/go/internal/modfetch/fetch.go
A src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
M src/cmd/go/testdata/script/mod_verify.txt
4 files changed, 125 insertions(+), 31 deletions(-)
diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go
index 3a2ff63..07e046c 100644
--- a/src/cmd/go/internal/modfetch/cache.go
+++ b/src/cmd/go/internal/modfetch/cache.go
@@ -84,6 +84,7 @@
return "", err
}
+ // Check whether the directory itself exists.
dir := filepath.Join(cfg.GOMODCACHE, enc+"@"+encVer)
if fi, err := os.Stat(dir); os.IsNotExist(err) {
return dir, err
@@ -92,6 +93,9 @@
} else if !fi.IsDir() {
return dir, &DownloadDirPartialError{dir, errors.New("not a directory")}
}
+
+ // Check if a .partial file exists. This is created at the beginning of
+ // a download and removed after the zip is extracted.
partialPath, err := CachePath(m, "partial")
if err != nil {
return dir, err
@@ -101,6 +105,19 @@
} else if !os.IsNotExist(err) {
return dir, err
}
+
+ // Check if a .ziphash file exists. It should be created before the
+ // zip is extracted, but if it was deleted (by another program?), we need
+ // to re-calculate it.
+ ziphashPath, err := CachePath(m, "ziphash")
+ if err != nil {
+ return dir, err
+ }
+ if _, err := os.Stat(ziphashPath); os.IsNotExist(err) {
+ return dir, &DownloadDirPartialError{dir, errors.New("ziphash file is missing")}
+ } else if err != nil {
+ return dir, err
+ }
return dir, nil
}
diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go
index c55c3cf..eb7d30e 100644
--- a/src/cmd/go/internal/modfetch/fetch.go
+++ b/src/cmd/go/internal/modfetch/fetch.go
@@ -170,13 +170,16 @@
if err != nil {
return cached{"", err}
}
+ ziphashfile := zipfile + "hash"
- // Skip locking if the zipfile already exists.
+ // Return without locking if the zip and ziphash files exist.
if _, err := os.Stat(zipfile); err == nil {
- return cached{zipfile, nil}
+ if _, err := os.Stat(ziphashfile); err == nil {
+ return cached{zipfile, nil}
+ }
}
- // The zip file does not exist. Acquire the lock and create it.
+ // The zip or ziphash file does not exist. Acquire the lock and create them.
if cfg.CmdName != "mod download" {
fmt.Fprintf(os.Stderr, "go: downloading %s %s\n", mod.Path, mod.Version)
}
@@ -186,14 +189,6 @@
}
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(ctx, mod, zipfile); err != nil {
return cached{"", err}
}
@@ -206,6 +201,25 @@
ctx, span := trace.StartSpan(ctx, "modfetch.downloadZip "+zipfile)
defer span.Done()
+ // Double-check that the zipfile was not created while we were waiting for
+ // the lock in DownloadZip.
+ ziphashfile := zipfile + "hash"
+ var zipExists, ziphashExists bool
+ if _, err := os.Stat(zipfile); err == nil {
+ zipExists = true
+ }
+ if _, err := os.Stat(ziphashfile); err == nil {
+ ziphashExists = true
+ }
+ if zipExists && ziphashExists {
+ return nil
+ }
+
+ // Create parent directories.
+ if err := os.MkdirAll(filepath.Dir(zipfile), 0777); err != nil {
+ return err
+ }
+
// 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.
@@ -217,6 +231,12 @@
}
}
+ // If the zip file exists, the ziphash file must have been deleted
+ // or lost after a file system crash. Re-hash the zip without downloading.
+ if zipExists {
+ return hashZip(mod, zipfile, ziphashfile)
+ }
+
// 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
@@ -289,15 +309,7 @@
}
// 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 := checkModSum(mod, hash); err != nil {
- return err
- }
-
- if err := renameio.WriteFile(zipfile+"hash", []byte(hash), 0666); err != nil {
+ if err := hashZip(mod, f.Name(), ziphashfile); err != nil {
return err
}
if err := os.Rename(f.Name(), zipfile); err != nil {
@@ -309,6 +321,22 @@
return nil
}
+// hashZip reads the zip file opened in f, then writes the hash to ziphashfile,
+// overwriting that file if it exists.
+//
+// If the hash does not match go.sum (or the sumdb if enabled), hashZip returns
+// an error and does not write ziphashfile.
+func hashZip(mod module.Version, zipfile, ziphashfile string) error {
+ hash, err := dirhash.HashZip(zipfile, dirhash.DefaultHash)
+ if err != nil {
+ return err
+ }
+ if err := checkModSum(mod, hash); err != nil {
+ return err
+ }
+ return renameio.WriteFile(ziphashfile, []byte(hash), 0666)
+}
+
// makeDirsReadOnly makes a best-effort attempt to remove write permissions for dir
// and its transitive contents.
func makeDirsReadOnly(dir string) {
@@ -452,11 +480,6 @@
// checkMod checks the given module's checksum.
func checkMod(mod module.Version) {
- if cfg.GOMODCACHE == "" {
- // Do not use current directory.
- return
- }
-
// Do the file I/O before acquiring the go.sum lock.
ziphash, err := CachePath(mod, "ziphash")
if err != nil {
@@ -464,10 +487,6 @@
}
data, err := renameio.ReadFile(ziphash)
if err != nil {
- if errors.Is(err, fs.ErrNotExist) {
- // This can happen if someone does rm -rf GOPATH/src/cache/download. So it goes.
- return
- }
base.Fatalf("verifying %v", module.VersionError(mod, err))
}
h := strings.TrimSpace(string(data))
diff --git a/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt b/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
new file mode 100644
index 0000000..8f6793e
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_missing_ziphash.txt
@@ -0,0 +1,55 @@
+# Test that if the module cache contains an extracted source directory but not
+# a ziphash, 'go build' complains about a missing sum, and 'go get' adds
+# the sum. Verifies #44749.
+
+# With a tidy go.sum, go build succeeds. This also populates the module cache.
+cp go.sum.tidy go.sum
+go build -n use
+env GOPROXY=off
+env GOSUMDB=off
+
+# Control case: if we delete the hash for rsc.io/quote v1.5.2,
+# 'go build' reports an error. 'go get' adds the sum.
+cp go.sum.bug go.sum
+! go build -n use
+stderr '^use.go:3:8: missing go.sum entry for module providing package rsc.io/quote \(imported by use\); to add:\n\tgo get use$'
+go get -d use
+cmp go.sum go.sum.tidy
+go build -n use
+
+# If we delete the hash *and* the ziphash file, we should see the same behavior.
+cp go.sum.bug go.sum
+rm $WORK/gopath/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.ziphash
+! go build -n use
+stderr '^use.go:3:8: missing go.sum entry for module providing package rsc.io/quote \(imported by use\); to add:\n\tgo get use$'
index 43812d0..b510665 100644
--- a/src/cmd/go/testdata/script/mod_verify.txt
+++ b/src/cmd/go/testdata/script/mod_verify.txt
@@ -48,10 +48,13 @@
grep '^rsc.io/quote v1.1.0/go.mod ' go.sum
grep '^rsc.io/quote v1.1.0 ' go.sum
-# sync should ignore missing ziphash; verify should not
+# verify should fail on a missing ziphash. tidy should restore it.
rm $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.ziphash
-go mod tidy
! go mod verify
+stderr '^rsc.io/quote v1.1.0: missing ziphash: open '$GOPATH'[/\\]pkg[/\\]mod[/\\]cache[/\\]download[/\\]rsc.io[/\\]quote[/\\]@v[/\\]v1.1.0.ziphash'
+go mod tidy
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.1.0.ziphash
+go mod verify
# Packages below module root should not be mentioned in go.sum.
rm go.sum
To view, visit change 298851. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages