]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/go: refactor {Allow,Disallow}WriteGoMod to ExplicitWriteGoMod
authorJay Conrod <jayconrod@google.com>
Mon, 13 Sep 2021 23:48:53 +0000 (16:48 -0700)
committerJay Conrod <jayconrod@google.com>
Fri, 24 Sep 2021 17:53:04 +0000 (17:53 +0000)
Subcommands may now set the global flag modload.ExplicitWriteGoMod
instead of calling {Allow,Disallow}WriteGoMod.

When ExplicitWriteGoMod is false (default), modload.LoadPackages and
ListModules will either update go.mod and go.sum or report an error if
they need to be updated, depending on cfg.BuildMod.

When ExplicitWriteGoMod is true, commands must explicitly call
modload.WriteGoMod to update go.mod and go.sum or report an
error. Commands that perform some operation after loading the build
list (like downloading zips or building packages) and commands that
load packages multiple times should set this. For now, only 'go get'
and 'go mod download' set this.

This CL is a pure refactor: no change in behavior is expected.
There are some other minor changes in here, too: commitRequirements no
longer sets the global requirements: that should be done separately first.

Change-Id: I69942a808bb177faf7904a53aaf2d4ac68500e82
Reviewed-on: https://go-review.googlesource.com/c/go/+/349600
Trust: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
src/cmd/go/internal/modcmd/download.go
src/cmd/go/internal/modcmd/why.go
src/cmd/go/internal/modget/get.go
src/cmd/go/internal/modload/buildlist.go
src/cmd/go/internal/modload/init.go
src/cmd/go/internal/modload/list.go
src/cmd/go/internal/modload/load.go

index 0f6478565660830cc3af5b06b9efdf0e086090a8..5ea56c34bd88924682474add04e99c3da39b5b10 100644 (file)
@@ -86,6 +86,7 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
 
        // Check whether modules are enabled and whether we're in a module.
        modload.ForceUseModules = true
+       modload.ExplicitWriteGoMod = true
        if !modload.HasModRoot() && len(args) == 0 {
                base.Fatalf("go: no modules specified (see 'go help mod download')")
        }
@@ -149,13 +150,16 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
        if !haveExplicitArgs {
                // 'go mod download' is sometimes run without arguments to pre-populate the
                // module cache. It may fetch modules that aren't needed to build packages
-               // in the main mdoule. This is usually not intended, so don't save sums for
-               // downloaded modules (golang.org/issue/45332).
-               // TODO(golang.org/issue/45551): For now, in ListModules, save sums needed
-               // to load the build list (same as 1.15 behavior). In the future, report an
-               // error if go.mod or go.sum need to be updated after loading the build
-               // list.
-               modload.DisallowWriteGoMod()
+               // in the main module. This is usually not intended, so don't save sums for
+               // downloaded modules (golang.org/issue/45332). We do still fix
+               // inconsistencies in go.mod though.
+               //
+               // TODO(#45551): In the future, report an error if go.mod or go.sum need to
+               // be updated after loading the build list. This may require setting
+               // the mode to "mod" or "readonly" depending on haveExplicitArgs.
+               if err := modload.WriteGoMod(ctx); err != nil {
+                       base.Fatalf("go: %v", err)
+               }
        }
 
        for _, info := range infos {
@@ -215,7 +219,9 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
        //
        // Don't save sums for 'go mod download' without arguments; see comment above.
        if haveExplicitArgs {
-               modload.WriteGoMod(ctx)
+               if err := modload.WriteGoMod(ctx); err != nil {
+                       base.Errorf("go: %v", err)
+               }
        }
 
        // If there was an error matching some of the requested packages, emit it now
index ed5e9d7f1aaa1737511e6ec8be48dcf3cd1c140f..9647784b67a4acb8a88d6210cf2158c13eae74f2 100644 (file)
@@ -68,6 +68,7 @@ func runWhy(ctx context.Context, cmd *base.Command, args []string) {
        modload.InitWorkfile()
        modload.ForceUseModules = true
        modload.RootMode = modload.NeedRoot
+       modload.ExplicitWriteGoMod = true // don't write go.mod in ListModules
 
        loadOpts := modload.PackageOpts{
                Tags:                     imports.AnyTags(),
index db07624e38659fcbef34248aa6a4614a908cb005..20734bdd2369c1883a1ecf72facf361178d94a7a 100644 (file)
@@ -281,7 +281,7 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
        // Do not allow any updating of go.mod until we've applied
        // all the requested changes and checked that the result matches
        // what was requested.
-       modload.DisallowWriteGoMod()
+       modload.ExplicitWriteGoMod = true
 
        // Allow looking up modules for import paths when outside of a module.
        // 'go get' is expected to do this, unlike other commands.
@@ -423,9 +423,9 @@ func runGet(ctx context.Context, cmd *base.Command, args []string) {
        // Everything succeeded. Update go.mod.
        oldReqs := reqsFromGoMod(modload.ModFile())
 
-       modload.AllowWriteGoMod()
-       modload.WriteGoMod(ctx)
-       modload.DisallowWriteGoMod()
+       if err := modload.WriteGoMod(ctx); err != nil {
+               base.Fatalf("go: %v", err)
+       }
 
        newReqs := reqsFromGoMod(modload.ModFile())
        r.reportChanges(oldReqs, newReqs)
index da9e6406b14c39c638fa6982e6bc1582d74ada2c..4634ad009d2f3981db40f713d05af09cb505043c 100644 (file)
@@ -469,7 +469,8 @@ func LoadModGraph(ctx context.Context, goVersion string) *ModuleGraph {
                base.Fatalf("go: %v", err)
        }
 
-       commitRequirements(ctx, rs)
+       requirements = rs
+
        return mg
 }
 
@@ -534,7 +535,7 @@ func EditBuildList(ctx context.Context, add, mustSelect []module.Version) (chang
        if err != nil {
                return false, err
        }
-       commitRequirements(ctx, rs)
+       requirements = rs
        return changed, err
 }
 
index 83414feb3c5134176fd7fe7fd6105e02d1e9ce14..621099beb317413080da3f60269dadcb01af2c08 100644 (file)
@@ -44,6 +44,16 @@ var (
        ForceUseModules bool
 
        allowMissingModuleImports bool
+
+       // ExplicitWriteGoMod prevents LoadPackages, ListModules, and other functions
+       // from updating go.mod and go.sum or reporting errors when updates are
+       // needed. A package should set this if it would cause go.mod to be written
+       // multiple times (for example, 'go get' calls LoadPackages multiple times) or
+       // if it needs some other operation to be successful before go.mod and go.sum
+       // can be written (for example, 'go mod download' must download modules before
+       // adding sums to go.sum). Packages that set this are responsible for calling
+       // WriteGoMod explicitly.
+       ExplicitWriteGoMod bool
 )
 
 func TODOWorkspaces(s string) error {
@@ -591,8 +601,9 @@ func loadWorkFile(path string) (goVersion string, modRoots []string, err error)
 // build list from its go.mod file.
 //
 // LoadModFile may make changes in memory, like adding a go directive and
-// ensuring requirements are consistent, and will write those changes back to
-// disk unless DisallowWriteGoMod is in effect.
+// ensuring requirements are consistent. The caller is responsible for ensuring
+// those changes are written to disk by calling LoadPackages or ListModules
+// (unless ExplicitWriteGoMod is set) or by calling WriteGoMod directly.
 //
 // As a side-effect, LoadModFile may change cfg.BuildMod to "vendor" if
 // -mod wasn't set explicitly and automatic vendoring should be enabled.
@@ -605,22 +616,8 @@ func loadWorkFile(path string) (goVersion string, modRoots []string, err error)
 // it for global consistency. Most callers outside of the modload package should
 // use LoadModGraph instead.
 func LoadModFile(ctx context.Context) *Requirements {
-       rs, needCommit := loadModFile(ctx)
-       if needCommit {
-               commitRequirements(ctx, rs)
-       }
-       return rs
-}
-
-// loadModFile is like LoadModFile, but does not implicitly commit the
-// requirements back to disk after fixing inconsistencies.
-//
-// If needCommit is true, after the caller makes any other needed changes to the
-// returned requirements they should invoke commitRequirements to fix any
-// inconsistencies that may be present in the on-disk go.mod file.
-func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) {
        if requirements != nil {
-               return requirements, false
+               return requirements
        }
 
        Init()
@@ -631,7 +628,7 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) {
                goVersion := LatestGoVersion()
                rawGoVersion.Store(mainModule, goVersion)
                requirements = newRequirements(pruningForGoVersion(goVersion), nil, nil)
-               return requirements, false
+               return requirements
        }
 
        var modFiles []*modfile.File
@@ -660,13 +657,13 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) {
 
        MainModules = makeMainModules(mainModules, modRoots, modFiles, indices, workFileGoVersion)
        setDefaultBuildMod() // possibly enable automatic vendoring
-       rs = requirementsFromModFiles(ctx, modFiles)
+       rs := requirementsFromModFiles(ctx, modFiles)
 
        if inWorkspaceMode() {
                // We don't need to do anything for vendor or update the mod file so
                // return early.
-
-               return rs, true
+               requirements = rs
+               return rs
        }
 
        mainModule := MainModules.mustGetSingleMainModule()
@@ -712,7 +709,7 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) {
        }
 
        requirements = rs
-       return requirements, true
+       return requirements
 }
 
 // CreateModFile initializes a new module by creating a go.mod file.
@@ -777,7 +774,10 @@ func CreateModFile(ctx context.Context, modPath string) {
        if err != nil {
                base.Fatalf("go: %v", err)
        }
-       commitRequirements(ctx, rs)
+       requirements = rs
+       if err := commitRequirements(ctx); err != nil {
+               base.Fatalf("go: %v", err)
+       }
 
        // Suggest running 'go mod tidy' unless the project is empty. Even if we
        // imported all the correct requirements above, we're probably missing
@@ -1003,7 +1003,6 @@ func setDefaultBuildMod() {
                // go.mod is inconsistent. They're useful for debugging, and they need
                // to work in buggy situations.
                cfg.BuildMod = "mod"
-               allowWriteGoMod = false
                return
        case "mod vendor":
                cfg.BuildMod = "readonly"
@@ -1309,62 +1308,44 @@ func findImportComment(file string) string {
        return path
 }
 
-var allowWriteGoMod = true
-
-// DisallowWriteGoMod causes future calls to WriteGoMod to do nothing at all.
-func DisallowWriteGoMod() {
-       allowWriteGoMod = false
-}
-
-// AllowWriteGoMod undoes the effect of DisallowWriteGoMod:
-// future calls to WriteGoMod will update go.mod if needed.
-// Note that any past calls have been discarded, so typically
-// a call to AlowWriteGoMod should be followed by a call to WriteGoMod.
-func AllowWriteGoMod() {
-       allowWriteGoMod = true
-}
-
 // WriteGoMod writes the current build list back to go.mod.
-func WriteGoMod(ctx context.Context) {
-       if !allowWriteGoMod {
-               panic("WriteGoMod called while disallowed")
-       }
-       commitRequirements(ctx, LoadModFile(ctx))
+func WriteGoMod(ctx context.Context) error {
+       requirements = LoadModFile(ctx)
+       return commitRequirements(ctx)
 }
 
-// commitRequirements writes sets the global requirements variable to rs and
-// writes its contents back to the go.mod file on disk.
-// goVersion, if non-empty, is used to set the version on the go.mod file.
-func commitRequirements(ctx context.Context, rs *Requirements) {
-       requirements = rs
-
-       if !allowWriteGoMod {
-               // Some package outside of modload promised to update the go.mod file later.
-               return
-       }
-
+// commitRequirements ensures go.mod and go.sum are up to date with the current
+// requirements.
+//
+// In "mod" mode, commitRequirements writes changes to go.mod and go.sum.
+//
+// In "readonly" and "vendor" modes, commitRequirements returns an error if
+// go.mod or go.sum are out of date in a semantically significant way.
+//
+// In workspace mode, commitRequirements only writes changes to go.work.sum.
+func commitRequirements(ctx context.Context) (err error) {
        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
+               return modfetch.WriteGoSum(keepSums(ctx, loaded, requirements, addBuildListZipSums), mustHaveCompleteRequirements())
        }
-
        if MainModules.Len() != 1 || MainModules.ModRoot(MainModules.Versions()[0]) == "" {
                // We aren't in a module, so we don't have anywhere to write a go.mod file.
-               return
+               return nil
        }
        mainModule := MainModules.mustGetSingleMainModule()
-       modFilePath := modFilePath(MainModules.ModRoot(mainModule))
        modFile := MainModules.ModFile(mainModule)
+       if modFile == nil {
+               // command-line-arguments has no .mod file to write.
+               return nil
+       }
+       modFilePath := modFilePath(MainModules.ModRoot(mainModule))
 
        var list []*modfile.Require
-       for _, m := range rs.rootModules {
+       for _, m := range requirements.rootModules {
                list = append(list, &modfile.Require{
                        Mod:      m,
-                       Indirect: !rs.direct[m.Path],
+                       Indirect: !requirements.direct[m.Path],
                })
        }
        if modFile.Go == nil || modFile.Go.Version == "" {
@@ -1382,7 +1363,7 @@ func commitRequirements(ctx context.Context, rs *Requirements) {
        if dirty && cfg.BuildMod != "mod" {
                // If we're about to fail due to -mod=readonly,
                // prefer to report a dirty go.mod over a dirty go.sum
-               base.Fatalf("go: %v", errGoModDirty)
+               return errGoModDirty
        }
 
        if !dirty && cfg.CmdName != "mod tidy" {
@@ -1391,22 +1372,22 @@ func commitRequirements(ctx context.Context, rs *Requirements) {
                // 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" {
-                       if err := modfetch.WriteGoSum(keepSums(ctx, loaded, rs, addBuildListZipSums), mustHaveCompleteRequirements()); err != nil {
-                               base.Fatalf("go: %v", err)
+                       if err := modfetch.WriteGoSum(keepSums(ctx, loaded, requirements, addBuildListZipSums), mustHaveCompleteRequirements()); err != nil {
+                               return err
                        }
                }
-               return
+               return nil
        }
        if _, ok := fsys.OverlayPath(modFilePath); ok {
                if dirty {
-                       base.Fatalf("go: updates to go.mod needed, but go.mod is part of the overlay specified with -overlay")
+                       return errors.New("updates to go.mod needed, but go.mod is part of the overlay specified with -overlay")
                }
-               return
+               return nil
        }
 
        new, err := modFile.Format()
        if err != nil {
-               base.Fatalf("go: %v", err)
+               return err
        }
        defer func() {
                // At this point we have determined to make the go.mod file on disk equal to new.
@@ -1415,8 +1396,8 @@ func commitRequirements(ctx context.Context, rs *Requirements) {
                // 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" {
-                       if err := modfetch.WriteGoSum(keepSums(ctx, loaded, rs, addBuildListZipSums), mustHaveCompleteRequirements()); err != nil {
-                               base.Fatalf("go: %v", err)
+                       if err == nil {
+                               err = modfetch.WriteGoSum(keepSums(ctx, loaded, requirements, addBuildListZipSums), mustHaveCompleteRequirements())
                        }
                }
        }()
@@ -1450,8 +1431,9 @@ func commitRequirements(ctx context.Context, rs *Requirements) {
        })
 
        if err != nil && err != errNoChange {
-               base.Fatalf("go: updating go.mod: %v", err)
+               return fmt.Errorf("updating go.mod: %w", err)
        }
+       return nil
 }
 
 // keepSums returns the set of modules (and go.mod file entries) for which
index ac10a42c5a23c64c9f382ce9201027ef5c9ac520..f782cd93db3d25d2258303fc22239c09156ff1c8 100644 (file)
@@ -72,7 +72,10 @@ func ListModules(ctx context.Context, args []string, mode ListMode) ([]*modinfo.
        }
 
        if err == nil {
-               commitRequirements(ctx, rs)
+               requirements = rs
+               if !ExplicitWriteGoMod {
+                       err = commitRequirements(ctx)
+               }
        }
        return mods, err
 }
index 48f268ce5f82ecfba494088f51002aad035f5188..3498c662f3cb28d125581b5db1258e068cb5eea9 100644 (file)
@@ -327,7 +327,7 @@ func LoadPackages(ctx context.Context, opts PackageOpts, patterns ...string) (ma
                }
        }
 
-       initialRS, _ := loadModFile(ctx) // Ignore needCommit — we're going to commit at the end regardless.
+       initialRS := LoadModFile(ctx)
 
        ld := loadFromRoots(ctx, loaderParams{
                PackageOpts:  opts,
@@ -396,7 +396,7 @@ func LoadPackages(ctx context.Context, opts PackageOpts, patterns ...string) (ma
                        }
                }
 
-               if allowWriteGoMod {
+               if !ExplicitWriteGoMod {
                        modfetch.TrimGoSum(keep)
 
                        // commitRequirements below will also call WriteGoSum, but the "keep" map
@@ -417,8 +417,11 @@ func LoadPackages(ctx context.Context, opts PackageOpts, patterns ...string) (ma
        }
 
        // Success! Update go.mod and go.sum (if needed) and return the results.
+       // We'll skip updating if ExplicitWriteGoMod is true (the caller has opted
+       // to call WriteGoMod itself) or if ResolveMissingImports is false (the
+       // command wants to examine the package graph as-is).
        loaded = ld
-       commitRequirements(ctx, loaded.requirements)
+       requirements = loaded.requirements
 
        for _, pkg := range ld.pkgs {
                if !pkg.isTest() {
@@ -426,6 +429,13 @@ func LoadPackages(ctx context.Context, opts PackageOpts, patterns ...string) (ma
                }
        }
        sort.Strings(loadedPackages)
+
+       if !ExplicitWriteGoMod && opts.ResolveMissingImports {
+               if err := commitRequirements(ctx); err != nil {
+                       base.Fatalf("go: %v", err)
+               }
+       }
+
        return matches, loadedPackages
 }
 
@@ -685,7 +695,7 @@ func ImportFromFiles(ctx context.Context, gofiles []string) {
                        return roots
                },
        })
-       commitRequirements(ctx, loaded.requirements)
+       requirements = loaded.requirements
 }
 
 // DirImportPath returns the effective import path for dir,