[go/dev.cmdgo] cmd/go: maintain a go.work.sum file

371 views
Skip to first unread message

Michael Matloob (Gerrit)

unread,
Jul 28, 2021, 2:11:36 PM7/28/21
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Go Bot, Jay Conrod, Bryan C. Mills, golang-co...@googlegroups.com

Michael Matloob submitted this change.

View Change

Approvals: Jay Conrod: Looks good to me, approved Michael Matloob: Trusted; Run TryBots Go Bot: TryBots succeeded
[dev.cmdgo] cmd/go: maintain a go.work.sum file

This change causes the go command to maintain a separate go.work.sum
file when in workspace mode rather than using the go.sum files
from the individual modules. This isn't quite what the proposal
spec specifies, which is that the sums that don't exist in any
of the workspace modules are added to go.work.sum rather than
the necessary sums. That will be done in a future change.

Change-Id: I528b9b153a93a4cd67c5af471ad6d5bd3628578b
Reviewed-on: https://go-review.googlesource.com/c/go/+/334939
Trust: Michael Matloob <mat...@golang.org>
Run-TryBot: Michael Matloob <mat...@golang.org>
TryBot-Result: Go Bot <go...@golang.org>
Reviewed-by: Jay Conrod <jayc...@google.com>
---
M src/cmd/go/internal/modfetch/fetch.go
M src/cmd/go/internal/modload/buildlist.go
M src/cmd/go/internal/modload/import.go
M src/cmd/go/internal/modload/init.go
M src/cmd/go/internal/modload/load.go
M src/cmd/go/internal/modload/modfile.go
M src/cmd/go/testdata/script/work.txt
A src/cmd/go/testdata/script/work_sum.txt
8 files changed, 113 insertions(+), 19 deletions(-)

diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go
index e40593a..7b3525e 100644
--- a/src/cmd/go/internal/modfetch/fetch.go
+++ b/src/cmd/go/internal/modfetch/fetch.go
@@ -681,19 +681,21 @@
return true
}

+var ErrGoSumDirty = errors.New("updates to go.sum needed, disabled by -mod=readonly")
+
// WriteGoSum writes the go.sum file if it needs to be updated.
//
// keep is used to check whether a newly added sum should be saved in go.sum.
// It should have entries for both module content sums and go.mod sums
// (version ends with "/go.mod"). Existing sums will be preserved unless they
// have been marked for deletion with TrimGoSum.
-func WriteGoSum(keep map[module.Version]bool) {
+func WriteGoSum(keep map[module.Version]bool, readonly bool) error {
goSum.mu.Lock()
defer goSum.mu.Unlock()

// If we haven't read the go.sum file yet, don't bother writing it.
if !goSum.enabled {
- return
+ return nil
}

// Check whether we need to add sums for which keep[m] is true or remove
@@ -711,10 +713,10 @@
}
}
if !dirty {
- return
+ return nil
}
- if cfg.BuildMod == "readonly" {
- base.Fatalf("go: updates to go.sum needed, disabled by -mod=readonly")
+ if readonly {
+ return ErrGoSumDirty
}

// Make a best-effort attempt to acquire the side lock, only to exclude
@@ -759,11 +761,12 @@
})

if err != nil {
- base.Fatalf("go: updating go.sum: %v", err)
+ return fmt.Errorf("updating go.sum: %w", err)
}

goSum.status = make(map[modSum]modSumStatus)
goSum.overwrite = false
+ return nil
}

// TrimGoSum trims go.sum to contain only the modules needed for reproducible
diff --git a/src/cmd/go/internal/modload/buildlist.go b/src/cmd/go/internal/modload/buildlist.go
index 959ee25..d2957a3 100644
--- a/src/cmd/go/internal/modload/buildlist.go
+++ b/src/cmd/go/internal/modload/buildlist.go
@@ -807,7 +807,7 @@
// We've added or upgraded one or more roots, so load the full module
// graph so that we can update those roots to be consistent with other
// requirements.
- if cfg.BuildMod != "mod" {
+ if mustHaveCompleteRequirements() {
// Our changes to the roots may have moved dependencies into or out of
// the lazy-loading horizon, which could in turn change the selected
// versions of other modules. (Unlike for eager modules, for lazy
@@ -1007,7 +1007,7 @@
return rs, err
}

- if cfg.BuildMod != "mod" {
+ if mustHaveCompleteRequirements() {
// Instead of actually updating the requirements, just check that no updates
// are needed.
if rs == nil {
diff --git a/src/cmd/go/internal/modload/import.go b/src/cmd/go/internal/modload/import.go
index b6b9bf6..773d8b6 100644
--- a/src/cmd/go/internal/modload/import.go
+++ b/src/cmd/go/internal/modload/import.go
@@ -32,6 +32,8 @@
Module module.Version
QueryErr error

+ ImportingModule module.Version
+
// isStd indicates whether we would expect to find the package in the standard
// library. This is normally true for all dotless import paths, but replace
// directives can cause us to treat the replaced paths as also being in
@@ -673,7 +675,7 @@
mod = r
}

- if HasModRoot() && cfg.BuildMod == "readonly" && needSum && !modfetch.HaveSum(mod) {
+ if HasModRoot() && cfg.BuildMod == "readonly" && !inWorkspaceMode() && needSum && !modfetch.HaveSum(mod) {
return "", false, module.VersionError(mod, &sumMissingError{})
}

diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go
index 3758786..00dfc8b 100644
--- a/src/cmd/go/internal/modload/init.go
+++ b/src/cmd/go/internal/modload/init.go
@@ -367,6 +367,7 @@
if err != nil {
base.Fatalf("reading go.work: %v", err)
}
+ modfetch.GoSumFile = workFilePath + ".sum"
// TODO(matloob) should workRoot just be workFile?
} else if modRoots == nil {
// We're in module mode, but not inside a module.
@@ -1009,6 +1010,10 @@
cfg.BuildMod = "readonly"
}

+func mustHaveCompleteRequirements() bool {
+ return cfg.BuildMod != "mod" && !inWorkspaceMode()
+}
+
// convertLegacyConfig imports module requirements from a legacy vendoring
// configuration file, if one is present.
func convertLegacyConfig(modFile *modfile.File, modPath string) (from string, err error) {
@@ -1306,10 +1311,17 @@
return
}

+ if inWorkspaceMode() {
+ // go.mod files aren't updated in workspace mode, but we still want to
+ // update the go.work.sum file.
+ if err := modfetch.WriteGoSum(keepSums(ctx, loaded, rs, addBuildListZipSums), mustHaveCompleteRequirements()); err != nil {
+ base.Fatalf("go: %v", err)
+ }
+ return
+ }
+
if MainModules.Len() != 1 || MainModules.ModRoot(MainModules.Versions()[0]) == "" {
- _ = TODOWorkspaces("also check that workspace mode is off")
// We aren't in a module, so we don't have anywhere to write a go.mod file.
- _ = TODOWorkspaces("also check that workspace mode is off")
return
}
mainModule := MainModules.Versions()[0]
@@ -1346,7 +1358,9 @@
// Don't write go.mod, but write go.sum in case we added or trimmed sums.
// 'go mod init' shouldn't write go.sum, since it will be incomplete.
if cfg.CmdName != "mod init" {
- modfetch.WriteGoSum(keepSums(ctx, loaded, rs, addBuildListZipSums))
+ if err := modfetch.WriteGoSum(keepSums(ctx, loaded, rs, addBuildListZipSums), mustHaveCompleteRequirements()); err != nil {
+ base.Fatalf("go: %v", err)
+ }
}
return
}
@@ -1368,7 +1382,9 @@
// Update go.sum after releasing the side lock and refreshing the index.
// 'go mod init' shouldn't write go.sum, since it will be incomplete.
if cfg.CmdName != "mod init" {
- modfetch.WriteGoSum(keepSums(ctx, loaded, rs, addBuildListZipSums))
+ if err := modfetch.WriteGoSum(keepSums(ctx, loaded, rs, addBuildListZipSums), mustHaveCompleteRequirements()); err != nil {
+ base.Fatalf("go: %v", err)
+ }
}
}()

diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index 77d2dc4..e7b03b0 100644
--- a/src/cmd/go/internal/modload/load.go
+++ b/src/cmd/go/internal/modload/load.go
@@ -403,7 +403,9 @@
// loaded.requirements, but here we may have also loaded (and want to
// preserve checksums for) additional entities from compatRS, which are
// only needed for compatibility with ld.TidyCompatibleVersion.
- modfetch.WriteGoSum(keep)
+ if err := modfetch.WriteGoSum(keep, mustHaveCompleteRequirements()); err != nil {
+ base.Fatalf("go: %v", err)
+ }
}
}

diff --git a/src/cmd/go/internal/modload/modfile.go b/src/cmd/go/internal/modload/modfile.go
index f5332ef5..79126a4 100644
--- a/src/cmd/go/internal/modload/modfile.go
+++ b/src/cmd/go/internal/modload/modfile.go
@@ -532,7 +532,7 @@
}

actual := resolveReplacement(m)
- if HasModRoot() && cfg.BuildMod == "readonly" && actual.Version != "" {
+ if HasModRoot() && cfg.BuildMod == "readonly" && !inWorkspaceMode() && actual.Version != "" {
key := module.Version{Path: actual.Path, Version: actual.Version + "/go.mod"}
if !modfetch.HaveSum(key) {
suggestion := fmt.Sprintf("; to add it:\n\tgo mod download %s", m.Path)
diff --git a/src/cmd/go/testdata/script/work.txt b/src/cmd/go/testdata/script/work.txt
index c68ca89..0d820ff 100644
--- a/src/cmd/go/testdata/script/work.txt
+++ b/src/cmd/go/testdata/script/work.txt
@@ -1,13 +1,20 @@
go mod initwork ./a ./b
cmp go.work go.work.want

+! go run example.com/b
+stderr 'a(\\|/)a.go:4:8: no required module provides package rsc.io/quote; to add it:\n\tgo get rsc.io/quote'
+cd a
+go get rsc.io/quote
+go env GOMOD # go env GOMOD reports the module in a single module context
+stdout $GOPATH(\\|/)src(\\|/)a(\\|/)go.mod
+cd ..
go run example.com/b
-stdout 'Hello from module A'
+stdout 'Hello, world.'

# And try from a different directory
cd c
-go run example.com/b
-stdout 'Hello from module A'
+go run example.com/b
+stdout 'Hello, world.'
cd $GOPATH/src

go list all # all includes both modules
@@ -26,6 +33,9 @@
stderr 'reading go.work: path .* appears multiple times in workspace'
cp go.work.backup go.work

+cp go.work.d go.work
+go run example.com/d
+
-- go.work.dup --
go 1.17

@@ -41,6 +51,14 @@
./a
./b
)
+-- go.work.d --
+go 1.17
+
+directory (
+ a
+ b
+ d
+)
-- a/go.mod --

module example.com/a
@@ -49,9 +67,10 @@
package a

import "fmt"
+import "rsc.io/quote"

func HelloFromA() {
- fmt.Println("Hello from module A")
+ fmt.Println(quote.Hello())
}

-- b/go.mod --
@@ -66,8 +85,27 @@
func main() {
a.HelloFromA()
}
+-- b/lib/hello.go --
+package lib
+
+import "example.com/a"
+
+func Hello() {
+ a.HelloFromA()
+}

-- c/README --
Create this directory so we can cd to
it and make sure paths are interpreted
relative to the go.work, not the cwd.
+-- d/go.mod --
+module example.com/d
+
+-- d/main.go --
+package main
+
+import "example.com/b/lib"
+
+func main() {
+ lib.Hello()
+}
diff --git a/src/cmd/go/testdata/script/work_sum.txt b/src/cmd/go/testdata/script/work_sum.txt
new file mode 100644
index 0000000..99f66a4
--- /dev/null
+++ b/src/cmd/go/testdata/script/work_sum.txt
@@ -0,0 +1,33 @@
+# Test adding sums to go.work.sum when sum isn't in go.mod.
+
+go run .
+cmp go.work.sum want.sum
+
+-- want.sum --
+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=
+-- go.work --
+go 1.18
+
+directory .
+-- go.mod --
+go 1.18
+
+module example.com/hi
+
+require "rsc.io/quote" v1.5.2
+-- main.go --
+package main
+
+import (
+ "fmt"
+ "rsc.io/quote"
+)
+
+func main() {
+ fmt.Println(quote.Hello())
+}
\ No newline at end of file

18 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/internal/modfetch/fetch.go Insertions: 2, Deletions: 2. ``` @@ -683:684, +683:684 @@ - var ErrGoSumDirty = errors.New("go: updates to go.sum needed, disabled by -mod=readonly") + var ErrGoSumDirty = errors.New("updates to go.sum needed, disabled by -mod=readonly") @@ -763:764, +763:764 @@ - return err + return fmt.Errorf("updating go.sum: %w", err) ``` The name of the file: src/cmd/go/internal/modload/load.go Insertions: 1, Deletions: 5. ``` @@ -406:411, +406:407 @@ - if err == modfetch.ErrGoSumDirty { - base.Fatalf(err.Error()) - } else { - base.Fatalf("go: updating go.sum: %v", err) - } + base.Fatalf("go: %v", err) ``` The name of the file: src/cmd/go/internal/modload/init.go Insertions: 3, Deletions: 15. ``` @@ -1317:1322, +1317:1318 @@ - if err == modfetch.ErrGoSumDirty { - base.Fatalf(err.Error()) - } else { - base.Fatalf("go: updating go.sum: %v", err) - } + base.Fatalf("go: %v", err) @@ -1365:1370, +1361:1362 @@ - if err == modfetch.ErrGoSumDirty { - base.Fatalf(err.Error()) - } else { - base.Fatalf("go: updating go.sum: %v", err) - } + base.Fatalf("go: %v", err) @@ -1393:1398, +1385:1386 @@ - if err == modfetch.ErrGoSumDirty { - base.Fatalf(err.Error()) - } else { - base.Fatalf("go: updating go.sum: %v", err) - } + base.Fatalf("go: %v", err) ```

To view, visit change 334939. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go
Gerrit-Branch: dev.cmdgo
Gerrit-Change-Id: I528b9b153a93a4cd67c5af471ad6d5bd3628578b
Gerrit-Change-Number: 334939
Gerrit-PatchSet: 21
Gerrit-Owner: Michael Matloob <mat...@golang.org>
Gerrit-Reviewer: Bryan C. Mills <bcm...@google.com>
Gerrit-Reviewer: Go Bot <go...@golang.org>
Gerrit-Reviewer: Jay Conrod <jayc...@google.com>
Gerrit-Reviewer: Michael Matloob <mat...@golang.org>
Gerrit-MessageType: merged
Reply all
Reply to author
Forward
0 new messages