]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: refactor -mod flag parsing
authorJay Conrod <jayconrod@google.com>
Wed, 9 Sep 2020 20:35:56 +0000 (16:35 -0400)
committerJay Conrod <jayconrod@google.com>
Fri, 11 Sep 2020 14:22:17 +0000 (14:22 +0000)
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>
14 files changed:
src/cmd/go/internal/base/flag.go
src/cmd/go/internal/cfg/cfg.go
src/cmd/go/internal/fmtcmd/fmt.go
src/cmd/go/internal/modcmd/download.go
src/cmd/go/internal/modcmd/edit.go
src/cmd/go/internal/modcmd/graph.go
src/cmd/go/internal/modcmd/init.go
src/cmd/go/internal/modcmd/tidy.go
src/cmd/go/internal/modcmd/vendor.go
src/cmd/go/internal/modcmd/verify.go
src/cmd/go/internal/modcmd/why.go
src/cmd/go/internal/modload/init.go
src/cmd/go/internal/work/build.go
src/cmd/go/internal/work/init.go

index 6727196816b56c620d8cee65aff4eec60d438658..c97c7445200e55620c813aef710cf5ce978d6772 100644 (file)
@@ -28,13 +28,42 @@ func (v *StringsFlag) String() string {
        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", "", "")
 }
index f9bbcd9180a21946310c42ac5d075065b94d6f4a..f874b880a6daee9ee6c43b63f4e48b5e50c523db 100644 (file)
@@ -27,7 +27,8 @@ var (
        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
index f96cff429cbba326646aacbd2229f7c7f8f9befb..b0c1c59b40c58b4818f0317b6683ba10422eec6a 100644 (file)
@@ -23,7 +23,8 @@ import (
 
 func init() {
        base.AddBuildFlagsNX(&CmdFmt.Flag)
-       base.AddLoadFlags(&CmdFmt.Flag)
+       base.AddModFlag(&CmdFmt.Flag)
+       base.AddModCommonFlags(&CmdFmt.Flag)
 }
 
 var CmdFmt = &base.Command{
index 41f294d47552ce70e8387fd1349668d4cd2042f2..0ea5638e703c377252abd4d8536120b953c1e173 100644 (file)
@@ -14,7 +14,6 @@ import (
        "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 @@ func init() {
 
        // 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 {
index 18bdd34cd07372428dd9445ba8fa27bc6926b8bf..03a774b824896b90b64eb3d0ba2a197d5d8488a9 100644 (file)
@@ -19,7 +19,6 @@ import (
        "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 @@ func init() {
        cmdEdit.Flag.Var(flagFunc(flagRetract), "retract", "")
        cmdEdit.Flag.Var(flagFunc(flagDropRetract), "dropretract", "")
 
-       work.AddModCommonFlags(cmdEdit)
+       base.AddModCommonFlags(&cmdEdit.Flag)
        base.AddBuildFlagsNX(&cmdEdit.Flag)
 }
 
index 513536a010c1cbf8dbbe5358ed75649dd264185d..a149b656053698930975489649995a096a14f1d1 100644 (file)
@@ -15,7 +15,6 @@ import (
        "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 @@ path@version, except for the main module, which has no @version suffix.
 }
 
 func init() {
-       work.AddModCommonFlags(cmdGraph)
+       base.AddModCommonFlags(&cmdGraph.Flag)
 }
 
 func runGraph(ctx context.Context, cmd *base.Command, args []string) {
index b6cffd332df0d9f8767eb1936555a5141c240c43..21b235653ea3d16af63a40a0ba108446d8d348fa 100644 (file)
@@ -9,7 +9,6 @@ package modcmd
 import (
        "cmd/go/internal/base"
        "cmd/go/internal/modload"
-       "cmd/go/internal/work"
        "context"
        "os"
        "strings"
@@ -30,7 +29,7 @@ To override this guess, supply the module path as an argument.
 }
 
 func init() {
-       work.AddModCommonFlags(cmdInit)
+       base.AddModCommonFlags(&cmdInit.Flag)
 }
 
 func runInit(ctx context.Context, cmd *base.Command, args []string) {
index 4dcb62e02f6695214081d257422e4dbb474cd3cb..30df674ef628f35c31e78e6b918c09e05a4d2b26 100644 (file)
@@ -10,7 +10,6 @@ import (
        "cmd/go/internal/base"
        "cmd/go/internal/cfg"
        "cmd/go/internal/modload"
-       "cmd/go/internal/work"
        "context"
 )
 
@@ -32,7 +31,7 @@ to standard error.
 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) {
index 30334f3a4299f650c2a056e3034c88f6ec37b691..91d2509452cbc8fa3048edb205ae87d6e4495a9b 100644 (file)
@@ -19,7 +19,6 @@ import (
        "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 @@ modules and packages to standard error.
 
 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) {
index d5428258234a475806df16b794a4d4643a0e22c7..7700588bde50466d332054bd6400771811360d1b 100644 (file)
@@ -17,7 +17,6 @@ import (
        "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 @@ non-zero status.
 }
 
 func init() {
-       work.AddModCommonFlags(cmdVerify)
+       base.AddModCommonFlags(&cmdVerify.Flag)
 }
 
 func runVerify(ctx context.Context, cmd *base.Command, args []string) {
index 30b15fc153217d3f94cb4643d6b7e444e461226d..8454fdfec6b63402f5146fb0967284e4d46b6a89 100644 (file)
@@ -11,7 +11,6 @@ import (
 
        "cmd/go/internal/base"
        "cmd/go/internal/modload"
-       "cmd/go/internal/work"
 
        "golang.org/x/mod/module"
 )
@@ -58,7 +57,7 @@ var (
 
 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) {
index 8e8fb9e6a1ff6a4d193b094de6eb6afec877d291..1f50dcb11c1e0a96ba0ad2e02d417e2d4c884270 100644 (file)
@@ -518,17 +518,20 @@ func modFileToBuildList() {
 // 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 @@ func setDefaultBuildMod() {
                        }
                }
 
-               // 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 @@ func WriteGoMod() {
                // 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")
                }
        }
index d020aa6e9ffc985e0b2a70ffec18a7ed104882d7..e99982ed36e6d91ee3a8732587bb270b11bd8e61 100644 (file)
@@ -240,13 +240,12 @@ const (
 // 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 @@ func AddBuildFlags(cmd *base.Command, mask BuildFlagMask) {
        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 @@ func AddBuildFlags(cmd *base.Command, mask BuildFlagMask) {
        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
 
index dad3b10111d0d1d177799849561a0bfd68e94990..f78020032cd74c67ee86c3032c7635ad3c655826 100644 (file)
@@ -252,7 +252,7 @@ func buildModeInit() {
 
        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)