cmd/go: refactor -mod flag parsing
Keep track of whether the -mod flag was set explicitly. When
-mod=readonly is the default, we'll want to adjust our error messages
if it's set explicitly.
Also, register the -mod, -modcacherw, and -modfile flags in functions
in internal/base instead of internal/work. 'go mod' commands that
don't load packages shouldn't depend on internal/work.
For #40728
Change-Id: I272aea9e19908ba37e151baac4ea8630e90f241f
Reviewed-on: https://go-review.googlesource.com/c/go/+/253744
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
diff --git a/src/cmd/go/internal/base/flag.go b/src/cmd/go/internal/base/flag.go
index 6727196..c97c744 100644
--- a/src/cmd/go/internal/base/flag.go
+++ b/src/cmd/go/internal/base/flag.go
@@ -28,13 +28,42 @@
return "<StringsFlag>"
}
+// explicitStringFlag is like a regular string flag, but it also tracks whether
+// the string was set explicitly to a non-empty value.
+type explicitStringFlag struct {
+ value *string
+ explicit *bool
+}
+
+func (f explicitStringFlag) String() string {
+ if f.value == nil {
+ return ""
+ }
+ return *f.value
+}
+
+func (f explicitStringFlag) Set(v string) error {
+ *f.value = v
+ if v != "" {
+ *f.explicit = true
+ }
+ return nil
+}
+
// AddBuildFlagsNX adds the -n and -x build flags to the flag set.
func AddBuildFlagsNX(flags *flag.FlagSet) {
flags.BoolVar(&cfg.BuildN, "n", false, "")
flags.BoolVar(&cfg.BuildX, "x", false, "")
}
-// AddLoadFlags adds the -mod build flag to the flag set.
-func AddLoadFlags(flags *flag.FlagSet) {
- flags.StringVar(&cfg.BuildMod, "mod", "", "")
+// AddModFlag adds the -mod build flag to the flag set.
+func AddModFlag(flags *flag.FlagSet) {
+ flags.Var(explicitStringFlag{value: &cfg.BuildMod, explicit: &cfg.BuildModExplicit}, "mod", "")
+}
+
+// AddModCommonFlags adds the module-related flags common to build commands
+// and 'go mod' subcommands.
+func AddModCommonFlags(flags *flag.FlagSet) {
+ flags.BoolVar(&cfg.ModCacheRW, "modcacherw", false, "")
+ flags.StringVar(&cfg.ModFile, "modfile", "", "")
}
diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go
index f9bbcd9..f874b88 100644
--- a/src/cmd/go/internal/cfg/cfg.go
+++ b/src/cmd/go/internal/cfg/cfg.go
@@ -27,7 +27,8 @@
BuildBuildmode string // -buildmode flag
BuildContext = defaultContext()
BuildMod string // -mod flag
- BuildModReason string // reason -mod flag is set, if set by default
+ BuildModExplicit bool // whether -mod was set explicitly
+ BuildModReason string // reason -mod was set, if set by default
BuildI bool // -i flag
BuildLinkshared bool // -linkshared flag
BuildMSan bool // -msan flag
diff --git a/src/cmd/go/internal/fmtcmd/fmt.go b/src/cmd/go/internal/fmtcmd/fmt.go
index f96cff4..b0c1c59 100644
--- a/src/cmd/go/internal/fmtcmd/fmt.go
+++ b/src/cmd/go/internal/fmtcmd/fmt.go
@@ -23,7 +23,8 @@
func init() {
base.AddBuildFlagsNX(&CmdFmt.Flag)
- base.AddLoadFlags(&CmdFmt.Flag)
+ base.AddModFlag(&CmdFmt.Flag)
+ base.AddModCommonFlags(&CmdFmt.Flag)
}
var CmdFmt = &base.Command{
diff --git a/src/cmd/go/internal/modcmd/download.go b/src/cmd/go/internal/modcmd/download.go
index 41f294d..0ea5638 100644
--- a/src/cmd/go/internal/modcmd/download.go
+++ b/src/cmd/go/internal/modcmd/download.go
@@ -14,7 +14,6 @@
"cmd/go/internal/cfg"
"cmd/go/internal/modfetch"
"cmd/go/internal/modload"
- "cmd/go/internal/work"
"golang.org/x/mod/module"
)
@@ -64,7 +63,7 @@
// TODO(jayconrod): https://golang.org/issue/35849 Apply -x to other 'go mod' commands.
cmdDownload.Flag.BoolVar(&cfg.BuildX, "x", false, "")
- work.AddModCommonFlags(cmdDownload)
+ base.AddModCommonFlags(&cmdDownload.Flag)
}
type moduleJSON struct {
diff --git a/src/cmd/go/internal/modcmd/edit.go b/src/cmd/go/internal/modcmd/edit.go
index 18bdd34..03a774b 100644
--- a/src/cmd/go/internal/modcmd/edit.go
+++ b/src/cmd/go/internal/modcmd/edit.go
@@ -19,7 +19,6 @@
"cmd/go/internal/lockedfile"
"cmd/go/internal/modfetch"
"cmd/go/internal/modload"
- "cmd/go/internal/work"
"golang.org/x/mod/modfile"
"golang.org/x/mod/module"
@@ -154,7 +153,7 @@
cmdEdit.Flag.Var(flagFunc(flagRetract), "retract", "")
cmdEdit.Flag.Var(flagFunc(flagDropRetract), "dropretract", "")
- work.AddModCommonFlags(cmdEdit)
+ base.AddModCommonFlags(&cmdEdit.Flag)
base.AddBuildFlagsNX(&cmdEdit.Flag)
}
diff --git a/src/cmd/go/internal/modcmd/graph.go b/src/cmd/go/internal/modcmd/graph.go
index 513536a..a149b65 100644
--- a/src/cmd/go/internal/modcmd/graph.go
+++ b/src/cmd/go/internal/modcmd/graph.go
@@ -15,7 +15,6 @@
"cmd/go/internal/base"
"cmd/go/internal/cfg"
"cmd/go/internal/modload"
- "cmd/go/internal/work"
"golang.org/x/mod/module"
)
@@ -33,7 +32,7 @@
}
func init() {
- work.AddModCommonFlags(cmdGraph)
+ base.AddModCommonFlags(&cmdGraph.Flag)
}
func runGraph(ctx context.Context, cmd *base.Command, args []string) {
diff --git a/src/cmd/go/internal/modcmd/init.go b/src/cmd/go/internal/modcmd/init.go
index b6cffd3..21b2356 100644
--- a/src/cmd/go/internal/modcmd/init.go
+++ b/src/cmd/go/internal/modcmd/init.go
@@ -9,7 +9,6 @@
import (
"cmd/go/internal/base"
"cmd/go/internal/modload"
- "cmd/go/internal/work"
"context"
"os"
"strings"
@@ -30,7 +29,7 @@
}
func init() {
- work.AddModCommonFlags(cmdInit)
+ base.AddModCommonFlags(&cmdInit.Flag)
}
func runInit(ctx context.Context, cmd *base.Command, args []string) {
diff --git a/src/cmd/go/internal/modcmd/tidy.go b/src/cmd/go/internal/modcmd/tidy.go
index 4dcb62e..30df674 100644
--- a/src/cmd/go/internal/modcmd/tidy.go
+++ b/src/cmd/go/internal/modcmd/tidy.go
@@ -10,7 +10,6 @@
"cmd/go/internal/base"
"cmd/go/internal/cfg"
"cmd/go/internal/modload"
- "cmd/go/internal/work"
"context"
)
@@ -32,7 +31,7 @@
func init() {
cmdTidy.Run = runTidy // break init cycle
cmdTidy.Flag.BoolVar(&cfg.BuildV, "v", false, "")
- work.AddModCommonFlags(cmdTidy)
+ base.AddModCommonFlags(&cmdTidy.Flag)
}
func runTidy(ctx context.Context, cmd *base.Command, args []string) {
diff --git a/src/cmd/go/internal/modcmd/vendor.go b/src/cmd/go/internal/modcmd/vendor.go
index 30334f3..91d2509 100644
--- a/src/cmd/go/internal/modcmd/vendor.go
+++ b/src/cmd/go/internal/modcmd/vendor.go
@@ -19,7 +19,6 @@
"cmd/go/internal/cfg"
"cmd/go/internal/imports"
"cmd/go/internal/modload"
- "cmd/go/internal/work"
"golang.org/x/mod/module"
"golang.org/x/mod/semver"
@@ -41,7 +40,7 @@
func init() {
cmdVendor.Flag.BoolVar(&cfg.BuildV, "v", false, "")
- work.AddModCommonFlags(cmdVendor)
+ base.AddModCommonFlags(&cmdVendor.Flag)
}
func runVendor(ctx context.Context, cmd *base.Command, args []string) {
diff --git a/src/cmd/go/internal/modcmd/verify.go b/src/cmd/go/internal/modcmd/verify.go
index d542825..7700588 100644
--- a/src/cmd/go/internal/modcmd/verify.go
+++ b/src/cmd/go/internal/modcmd/verify.go
@@ -17,7 +17,6 @@
"cmd/go/internal/cfg"
"cmd/go/internal/modfetch"
"cmd/go/internal/modload"
- "cmd/go/internal/work"
"golang.org/x/mod/module"
"golang.org/x/mod/sumdb/dirhash"
@@ -38,7 +37,7 @@
}
func init() {
- work.AddModCommonFlags(cmdVerify)
+ base.AddModCommonFlags(&cmdVerify.Flag)
}
func runVerify(ctx context.Context, cmd *base.Command, args []string) {
diff --git a/src/cmd/go/internal/modcmd/why.go b/src/cmd/go/internal/modcmd/why.go
index 30b15fc..8454fdf 100644
--- a/src/cmd/go/internal/modcmd/why.go
+++ b/src/cmd/go/internal/modcmd/why.go
@@ -11,7 +11,6 @@
"cmd/go/internal/base"
"cmd/go/internal/modload"
- "cmd/go/internal/work"
"golang.org/x/mod/module"
)
@@ -58,7 +57,7 @@
func init() {
cmdWhy.Run = runWhy // break init cycle
- work.AddModCommonFlags(cmdWhy)
+ base.AddModCommonFlags(&cmdWhy.Flag)
}
func runWhy(ctx context.Context, cmd *base.Command, args []string) {
diff --git a/src/cmd/go/internal/modload/init.go b/src/cmd/go/internal/modload/init.go
index 8e8fb9e..1f50dcb 100644
--- a/src/cmd/go/internal/modload/init.go
+++ b/src/cmd/go/internal/modload/init.go
@@ -518,17 +518,20 @@
// setDefaultBuildMod sets a default value for cfg.BuildMod
// if it is currently empty.
func setDefaultBuildMod() {
- if cfg.BuildMod != "" {
+ if cfg.BuildModExplicit {
// Don't override an explicit '-mod=' argument.
return
}
- cfg.BuildMod = "mod"
+
if cfg.CmdName == "get" || strings.HasPrefix(cfg.CmdName, "mod ") {
- // Don't set -mod implicitly for commands whose purpose is to
- // manipulate the build list.
+ // 'get' and 'go mod' commands may update go.mod automatically.
+ // TODO(jayconrod): should this narrower? Should 'go mod download' or
+ // 'go mod graph' update go.mod by default?
+ cfg.BuildMod = "mod"
return
}
if modRoot == "" {
+ cfg.BuildMod = "mod"
return
}
@@ -546,18 +549,18 @@
}
}
- // Since a vendor directory exists, we have a non-trivial reason for
- // choosing -mod=mod, although it probably won't be used for anything.
- // Record the reason anyway for consistency.
- // It may be overridden if we switch to mod=readonly below.
- cfg.BuildModReason = fmt.Sprintf("Go version in go.mod is %s.", modGo)
+ // Since a vendor directory exists, we should record why we didn't use it.
+ // This message won't normally be shown, but it may appear with import errors.
+ cfg.BuildModReason = fmt.Sprintf("Go version in go.mod is %s, so vendor directory was not used.", modGo)
}
p := ModFilePath()
if fi, err := os.Stat(p); err == nil && !hasWritePerm(p, fi) {
cfg.BuildMod = "readonly"
cfg.BuildModReason = "go.mod file is read-only."
+ return
}
+ cfg.BuildMod = "mod"
}
func legacyModInit() {
@@ -857,7 +860,7 @@
// prefer to report a dirty go.mod over a dirty go.sum
if cfg.BuildModReason != "" {
base.Fatalf("go: updates to go.mod needed, disabled by -mod=readonly\n\t(%s)", cfg.BuildModReason)
- } else {
+ } else if cfg.BuildModExplicit {
base.Fatalf("go: updates to go.mod needed, disabled by -mod=readonly")
}
}
diff --git a/src/cmd/go/internal/work/build.go b/src/cmd/go/internal/work/build.go
index d020aa6..e99982e 100644
--- a/src/cmd/go/internal/work/build.go
+++ b/src/cmd/go/internal/work/build.go
@@ -240,13 +240,12 @@
// AddBuildFlags adds the flags common to the build, clean, get,
// install, list, run, and test commands.
func AddBuildFlags(cmd *base.Command, mask BuildFlagMask) {
+ base.AddBuildFlagsNX(&cmd.Flag)
cmd.Flag.BoolVar(&cfg.BuildA, "a", false, "")
- cmd.Flag.BoolVar(&cfg.BuildN, "n", false, "")
cmd.Flag.IntVar(&cfg.BuildP, "p", cfg.BuildP, "")
if mask&OmitVFlag == 0 {
cmd.Flag.BoolVar(&cfg.BuildV, "v", false, "")
}
- cmd.Flag.BoolVar(&cfg.BuildX, "x", false, "")
cmd.Flag.Var(&load.BuildAsmflags, "asmflags", "")
cmd.Flag.Var(buildCompiler{}, "compiler", "")
@@ -254,10 +253,10 @@
cmd.Flag.Var(&load.BuildGcflags, "gcflags", "")
cmd.Flag.Var(&load.BuildGccgoflags, "gccgoflags", "")
if mask&OmitModFlag == 0 {
- cmd.Flag.StringVar(&cfg.BuildMod, "mod", "", "")
+ base.AddModFlag(&cmd.Flag)
}
if mask&OmitModCommonFlags == 0 {
- AddModCommonFlags(cmd)
+ base.AddModCommonFlags(&cmd.Flag)
}
cmd.Flag.StringVar(&cfg.BuildContext.InstallSuffix, "installsuffix", "", "")
cmd.Flag.Var(&load.BuildLdflags, "ldflags", "")
@@ -275,13 +274,6 @@
cmd.Flag.StringVar(&cfg.DebugTrace, "debug-trace", "", "")
}
-// AddModCommonFlags adds the module-related flags common to build commands
-// and 'go mod' subcommands.
-func AddModCommonFlags(cmd *base.Command) {
- cmd.Flag.BoolVar(&cfg.ModCacheRW, "modcacherw", false, "")
- cmd.Flag.StringVar(&cfg.ModFile, "modfile", "", "")
-}
-
// tagsFlag is the implementation of the -tags flag.
type tagsFlag []string
diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go
index dad3b10..f780200 100644
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -252,7 +252,7 @@
switch cfg.BuildMod {
case "":
- // ok
+ // Behavior will be determined automatically, as if no flag were passed.
case "readonly", "vendor", "mod":
if !cfg.ModulesEnabled && !inGOFLAGS("-mod") {
base.Fatalf("build flag -mod=%s only valid when using modules", cfg.BuildMod)
