[go] cmd/go: forbid use of one module with two different paths
552 views
Skip to first unread message
Bryan C. Mills (Gerrit)
Jul 26, 2018, 4:47:55 PM7/26/18
to Ian Lance Taylor, goph...@pubsubhelper.golang.org, Bryan C. Mills, golang-co...@googlegroups.com
Bryan C. Mills has uploaded this change for review.
cmd/go: forbid use of one module with two different paths
If a single module is imported via two different paths (e.g., as itself and as a
replacement for something else), some users may be surprised if the two paths
do not share the same package-level state. Others may be surprised if the two
paths do share state.
Punt on the question for now by rejecting that condition explicitly.
Fixes #26607.
Change-Id: I15c3889f61f8dd4ba5e5c48ca33ad63aeecac04e
---
M src/cmd/go/internal/modload/load.go
M src/cmd/go/script_test.go
A src/cmd/go/testdata/script/mod_replace.txt
3 files changed, 109 insertions(+), 1 deletion(-)
diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index 02673a2..7014958 100644
--- a/src/cmd/go/internal/modload/load.go
+++ b/src/cmd/go/internal/modload/load.go
@@ -115,6 +115,25 @@
}
return roots
})
+
+ // A given module path may be used as itself or as a replacement for another
+ // module, but not both at the same time. Otherwise, the aliasing behavior is
+ // too subtle (see https://golang.org/issue/26607), and we don't want to
+ // commit to a specific behavior at this point.
+ firstPath := make(map[module.Version]string, len(buildList))
+ for _, mod := range buildList {
+ src := mod
+ if rep := Replacement(mod); rep.Path != "" {
+ src = rep
+ }
+ if prev, ok := firstPath[src]; !ok {
+ firstPath[src] = mod.Path
+ } else if prev != mod.Path {
+ base.Errorf("go: %s@%s used for two different module paths (%s and %s)", mod.Path, mod.Version, prev, mod.Path)
+ }
+ }
+ base.ExitIfErrors()
+
WriteGoMod()
// Process paths to produce final paths list.
diff --git a/src/cmd/go/script_test.go b/src/cmd/go/script_test.go
index d8bcd07..3858acb 100644
--- a/src/cmd/go/script_test.go
+++ b/src/cmd/go/script_test.go
@@ -702,7 +702,7 @@
// exec runs the given command line (an actual subprocess, not simulated)
// in ts.cd with environment ts.env and then returns collected standard output and standard error.
func (ts *testScript) exec(command string, args ...string) (stdout, stderr string, err error) {
- cmd := exec.Command(testGo, args...)
+ cmd := exec.Command(command, args...)
cmd.Dir = ts.cd
cmd.Env = append(ts.env, "PWD="+ts.cd)
var stdoutBuf, stderrBuf strings.Builder
diff --git a/src/cmd/go/testdata/script/mod_replace.txt b/src/cmd/go/testdata/script/mod_replace.txt
new file mode 100644
index 0000000..3e7f0bc
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_replace.txt
@@ -0,0 +1,89 @@
+env GO111MODULE=on
+
+go build -o a1.exe .
+exec ./a1.exe
+stdout 'Don''t communicate by sharing memory'
+
+# Modules can be replaced by local packages.
+go mod -replace=rsc.io/quote/v3=./local/rsc.io/quote/v3
+go build -o a2.exe .
+exec ./a2.exe
+stdout 'Concurrency is not parallelism.'
+
+# The module path of the replacement doesn't need to match.
+# (For example, it could be a long-running fork with its own import path.)
+go mod -replace=rsc.io/quote/v3=./local/not-rsc.io/quote/v3
+go build -o a3.exe .
+exec ./a3.exe
+stdout 'Clear is better than clever.'
+
+# However, the same module can't be used as two different paths.
+go mod -dropreplace=rsc.io/quote/v3
+go mod -replace=not-rsc.io/quote/v...@v3.0.0=rsc.io/quote/v...@v3.0.0
+go mod -require=not-rsc.io/quote/v...@v3.0.0
+! go build -o a4.exe .
+
+
+-- go.mod --
+module quoter
+
+require rsc.io/quote/v3 v3.0.0
+
+-- main.go --
+package main
+
+import (
+ "fmt"
+ "rsc.io/quote/v3"
+)
+
+func main() {
+ fmt.Println(quote.GoV3())
+}
+
+-- local/rsc.io/quote/v3/go.mod --
+module rsc.io/quote/v3
+
+require rsc.io/sampler v1.3.0
+
+-- local/rsc.io/quote/v3/quote.go --
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Package quote collects pithy sayings.
+package quote
+
+import "rsc.io/sampler"
+
+// Hello returns a greeting.
+func HelloV3() string {
+ return sampler.Hello()
+}
+
+// Glass returns a useful phrase for world travelers.
+func GlassV3() string {
+ // See http://www.oocities.org/nodotus/hbglass.html.
+ return "I can eat glass and it doesn't hurt me."
+}
+
+// Go returns a REPLACED Go proverb.
+func GoV3() string {
+ return "Concurrency is not parallelism."
+}
+
+// Opt returns a optimization truth.
+func OptV3() string {
+ // Wisdom from ken.
+ return "If a program is too slow, it must have a loop."
+}
+
+-- local/not-rsc.io/quote/v3/go.mod --
+module not-rsc.io/quote/v3
+
+-- local/not-rsc.io/quote/v3/quote.go --
+package quote
+
+func GoV3() string {
+ return "Clear is better than clever."
+}
To view, visit change 126156. To unsubscribe, or for help writing mail filters, visit settings.
Gobot Gobot (Gerrit)
Jul 26, 2018, 4:48:08 PM7/26/18
to Bryan C. Mills, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
TryBots beginning. Status page: https://farmer.golang.org/try?commit=6ca48710
Gobot Gobot (Gerrit)
Jul 26, 2018, 4:56:01 PM7/26/18
to Bryan C. Mills, goph...@pubsubhelper.golang.org, Russ Cox, golang-co...@googlegroups.com
TryBots are happy.
Patch set 1:TryBot-Result +1
Russ Cox (Gerrit)
Jul 30, 2018, 10:02:50 PM7/30/18
to Bryan C. Mills, goph...@pubsubhelper.golang.org, Russ Cox, Gobot Gobot, golang-co...@googlegroups.com
Patch set 1:Code-Review +2
1 comment:
File src/cmd/go/testdata/script/mod_replace.txt:
Patch Set #1, Line 3:
go build -o a1.exe .FWIW, I try to avoid compiling, linking, and running programs in tests, because that step is pretty slow. Some of these could be go list instead of go build (since go list does all the prep for compiling anyway). But I'll leave it for now.
To view, visit change 126156. To unsubscribe, or for help writing mail filters, visit settings.
Russ Cox (Gerrit)
Jul 30, 2018, 10:02:54 PM7/30/18
to Russ Cox, Bryan C. Mills, goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gobot Gobot, golang-co...@googlegroups.com
Russ Cox merged this change.
Approvals:
Russ Cox: Looks good to me, approved
Bryan C. Mills: Run TryBots
Gobot Gobot: TryBots succeeded
cmd/go: forbid use of one module with two different paths
If a single module is imported via two different paths (e.g., as itself and as a
replacement for something else), some users may be surprised if the two paths
do not share the same package-level state. Others may be surprised if the two
paths do share state.
Punt on the question for now by rejecting that condition explicitly.
Fixes #26607.
Change-Id: I15c3889f61f8dd4ba5e5c48ca33ad63aeecac04e
Reviewed-on: https://go-review.googlesource.com/126156
Run-TryBot: Bryan C. Mills <bcm...@google.com>
TryBot-Result: Gobot Gobot <go...@golang.org>
Reviewed-by: Russ Cox <r...@golang.org>
---
M src/cmd/go/internal/modload/load.go
A src/cmd/go/testdata/script/mod_replace.txt
2 files changed, 108 insertions(+), 0 deletions(-)
diff --git a/src/cmd/go/internal/modload/load.go b/src/cmd/go/internal/modload/load.go
index 36dc0de..e8c984b 100644
Reply all
Reply to author
Forward
0 new messages