]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/go: remove remaining uses of TODOWorkspaces
authorMichael Matloob <matloob@golang.org>
Fri, 29 Oct 2021 20:47:22 +0000 (16:47 -0400)
committerMichael Matloob <matloob@golang.org>
Sat, 13 Nov 2021 02:30:25 +0000 (02:30 +0000)
Most of them are fixed, but some of them have been rewritten to refer
to specific issues.

For #45713

Change-Id: Id24d9bd47afeac089835f7a26e7025332fb6119c
Reviewed-on: https://go-review.googlesource.com/c/go/+/359794
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
src/cmd/go/internal/modcmd/download.go
src/cmd/go/internal/modload/init.go
src/cmd/go/internal/workcmd/edit.go
src/cmd/go/internal/workcmd/init.go
src/cmd/go/internal/workcmd/sync.go
src/cmd/go/internal/workcmd/use.go
src/cmd/go/testdata/script/work_why_download_graph.txt [new file with mode: 0644]

index f252133762cdb7c560a87aeed1e60703c2ddfc1e..6b8a010fd9d71ca248b91a2ccd7375e3305f432a 100644 (file)
@@ -93,24 +93,27 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
        modload.ExplicitWriteGoMod = true
        haveExplicitArgs := len(args) > 0
 
-       if modload.HasModRoot() {
+       if modload.HasModRoot() || modload.WorkFilePath() != "" {
                modload.LoadModFile(ctx) // to fill MainModules
 
-               if len(modload.MainModules.Versions()) != 1 {
-                       panic(modload.TODOWorkspaces("Support workspace mode in go mod download"))
-               }
-               mainModule := modload.MainModules.Versions()[0]
-
                if haveExplicitArgs {
-                       targetAtUpgrade := mainModule.Path + "@upgrade"
-                       targetAtPatch := mainModule.Path + "@patch"
-                       for _, arg := range args {
-                               switch arg {
-                               case mainModule.Path, targetAtUpgrade, targetAtPatch:
-                                       os.Stderr.WriteString("go: skipping download of " + arg + " that resolves to the main module\n")
+                       for _, mainModule := range modload.MainModules.Versions() {
+                               targetAtUpgrade := mainModule.Path + "@upgrade"
+                               targetAtPatch := mainModule.Path + "@patch"
+                               for _, arg := range args {
+                                       switch arg {
+                                       case mainModule.Path, targetAtUpgrade, targetAtPatch:
+                                               os.Stderr.WriteString("go: skipping download of " + arg + " that resolves to the main module\n")
+                                       }
                                }
                        }
+               } else if modload.WorkFilePath() != "" {
+                       // TODO(#44435): Think about what the correct query is to download the
+                       // right set of modules. Also see code review comment at
+                       // https://go-review.googlesource.com/c/go/+/359794/comments/ce946a80_6cf53992.
+                       args = []string{"all"}
                } else {
+                       mainModule := modload.MainModules.Versions()[0]
                        modFile := modload.MainModules.ModFile(mainModule)
                        if modFile.Go == nil || semver.Compare("v"+modFile.Go.Version, modload.ExplicitIndirectVersionV) < 0 {
                                if len(modFile.Require) > 0 {
index fcf6ce2620902204f6a18415ae492d3128aa1d67..ab899fac1e1c2caa7247c050fd28ae861493cdb9 100644 (file)
@@ -12,6 +12,7 @@ import (
        "fmt"
        "go/build"
        "internal/lazyregexp"
+       "io/ioutil"
        "os"
        "path"
        "path/filepath"
@@ -56,10 +57,6 @@ var (
        ExplicitWriteGoMod bool
 )
 
-func TODOWorkspaces(s string) error {
-       return fmt.Errorf("need to support this for workspaces: %s", s)
-}
-
 // Variables set in Init.
 var (
        initialized bool
@@ -417,9 +414,6 @@ func Init() {
        // We're in module mode. Set any global variables that need to be set.
        cfg.ModulesEnabled = true
        setDefaultBuildMod()
-       _ = TODOWorkspaces("In workspace mode, mod will not be readonly for go mod download," +
-               "verify, graph, and why. Implement support for go mod download and add test cases" +
-               "to ensure verify, graph, and why work properly.")
        list := filepath.SplitList(cfg.BuildContext.GOPATH)
        if len(list) > 0 && list[0] != "" {
                gopath = list[0]
@@ -562,13 +556,8 @@ func (goModDirtyError) Error() string {
 var errGoModDirty error = goModDirtyError{}
 
 func loadWorkFile(path string) (goVersion string, modRoots []string, replaces []*modfile.Replace, err error) {
-       _ = TODOWorkspaces("Clean up and write back the go.work file: add module paths for workspace modules.")
        workDir := filepath.Dir(path)
-       workData, err := lockedfile.Read(path)
-       if err != nil {
-               return "", nil, nil, err
-       }
-       wf, err := modfile.ParseWork(path, workData, nil)
+       wf, err := ReadWorkFile(path)
        if err != nil {
                return "", nil, nil, err
        }
@@ -581,15 +570,60 @@ func loadWorkFile(path string) (goVersion string, modRoots []string, replaces []
                if !filepath.IsAbs(modRoot) {
                        modRoot = filepath.Join(workDir, modRoot)
                }
+
                if seen[modRoot] {
                        return "", nil, nil, fmt.Errorf("path %s appears multiple times in workspace", modRoot)
                }
                seen[modRoot] = true
                modRoots = append(modRoots, modRoot)
        }
+
        return goVersion, modRoots, wf.Replace, nil
 }
 
+// ReadWorkFile reads and parses the go.work file at the given path.
+func ReadWorkFile(path string) (*modfile.WorkFile, error) {
+       workData, err := ioutil.ReadFile(path)
+       if err != nil {
+               return nil, err
+       }
+       wf, err := modfile.ParseWork(path, workData, nil)
+
+       return wf, nil
+}
+
+// WriteWorkFile cleans and writes out the go.work file to the given path.
+func WriteWorkFile(path string, wf *modfile.WorkFile) error {
+       wf.SortBlocks()
+       wf.Cleanup()
+       out := modfile.Format(wf.Syntax)
+
+       return ioutil.WriteFile(path, out, 0666)
+}
+
+// UpdateWorkFile updates comments on directory directives in the go.work
+// file to include the associated module path.
+func UpdateWorkFile(wf *modfile.WorkFile) {
+       missingModulePaths := map[string]string{} // module directory listed in file -> abspath modroot
+
+       for _, d := range wf.Directory {
+               modRoot := d.Path
+               if d.ModulePath == "" {
+                       missingModulePaths[d.Path] = modRoot
+               }
+       }
+
+       // Clean up and annotate directories.
+       // TODO(matloob): update x/mod to actually add module paths.
+       for moddir, absmodroot := range missingModulePaths {
+               _, f, err := ReadModFile(filepath.Join(absmodroot, "go.mod"), nil)
+               if err != nil {
+                       continue // Error will be reported if modules are loaded.
+               }
+               wf.AddDirectory(moddir, f.Module.Mod.Path)
+       }
+}
+
 // LoadModFile sets Target and, if there is a main module, parses the initial
 // build list from its go.mod file.
 //
@@ -651,7 +685,9 @@ func LoadModFile(ctx context.Context) *Requirements {
                modfetch.GoSumFile = strings.TrimSuffix(modFilePath(modRoots[0]), ".mod") + ".sum"
        }
        if len(modRoots) == 0 {
-               _ = TODOWorkspaces("Instead of creating a fake module with an empty modroot, make MainModules.Len() == 0 mean that we're in module mode but not inside any module.")
+               // TODO(#49228): Instead of creating a fake module with an empty modroot,
+               // make MainModules.Len() == 0 mean that we're in module mode but not inside
+               // any module.
                mainModule := module.Version{Path: "command-line-arguments"}
                MainModules = makeMainModules([]module.Version{mainModule}, []string{""}, []*modfile.File{nil}, []*modFileIndex{nil}, "", nil)
                goVersion := LatestGoVersion()
@@ -854,8 +890,8 @@ func CreateWorkFile(ctx context.Context, workFile string, modDirs []string) {
                workF.AddDirectory(ToDirectoryPath(dir), f.Module.Mod.Path)
        }
 
-       data := modfile.Format(workF.Syntax)
-       lockedfile.Write(workFile, bytes.NewReader(data), 0666)
+       UpdateWorkFile(workF)
+       WriteWorkFile(workFile, workF)
 }
 
 // fixVersion returns a modfile.VersionFixer implemented using the Query function.
@@ -1233,9 +1269,10 @@ func findWorkspaceFile(dir string) (root string) {
                        break
                }
                if d == cfg.GOROOT {
-                       _ = TODOWorkspaces("If we end up checking in a go.work file to GOROOT/src," +
-                               "remove this case.")
-                       return "" // As a special case, don't cross GOROOT to find a go.work file.
+                       // As a special case, don't cross GOROOT to find a go.work file.
+                       // The standard library and commands built in go always use the vendored
+                       // dependencies, so avoid using a most likely irrelevant go.work file.
+                       return ""
                }
                dir = d
        }
index f4e630f43f996a19cef720f329022f4af72e3255..5158ac9b49ec68916bd5bfc915b905f23812a032 100644 (file)
@@ -7,13 +7,10 @@
 package workcmd
 
 import (
-       "bytes"
        "cmd/go/internal/base"
-       "cmd/go/internal/lockedfile"
        "cmd/go/internal/modload"
        "context"
        "encoding/json"
-       "errors"
        "fmt"
        "os"
        "path/filepath"
@@ -150,12 +147,7 @@ func runEditwork(ctx context.Context, cmd *base.Command, args []string) {
                }
        }
 
-       data, err := lockedfile.Read(gowork)
-       if err != nil {
-               base.Fatalf("go: %v", err)
-       }
-
-       workFile, err := modfile.ParseWork(gowork, data, nil)
+       workFile, err := modload.ReadWorkFile(gowork)
        if err != nil {
                base.Fatalf("go: errors parsing %s:\n%s", base.ShortPath(gowork), err)
        }
@@ -171,6 +163,9 @@ func runEditwork(ctx context.Context, cmd *base.Command, args []string) {
                        edit(workFile)
                }
        }
+
+       modload.UpdateWorkFile(workFile)
+
        workFile.SortBlocks()
        workFile.Cleanup() // clean file after edits
 
@@ -179,22 +174,12 @@ func runEditwork(ctx context.Context, cmd *base.Command, args []string) {
                return
        }
 
-       out := modfile.Format(workFile.Syntax)
-
        if *editPrint {
-               os.Stdout.Write(out)
+               os.Stdout.Write(modfile.Format(workFile.Syntax))
                return
        }
 
-       err = lockedfile.Transform(gowork, func(lockedData []byte) ([]byte, error) {
-               if !bytes.Equal(lockedData, data) {
-                       return nil, errors.New("go.work changed during editing; not overwriting")
-               }
-               return out, nil
-       })
-       if err != nil {
-               base.Fatalf("go: %v", err)
-       }
+       modload.WriteWorkFile(gowork, workFile)
 }
 
 // flagEditworkDirectory implements the -directory flag.
index 1342748023c792166e352732ce0babce7e34ac77..fde1483efbc9f4ee9b5895a79338c8a8ef8e11df 100644 (file)
@@ -13,9 +13,9 @@ import (
        "path/filepath"
 )
 
-var _ = modload.TODOWorkspaces("Add more documentation below. Though this is" +
-       "enough for those trying workspaces out, there should be more through" +
-       "documentation if the proposal is accepted and released.")
+// TODO(#49232) Add more documentation below. Though this is
+// enough for those trying workspaces out, there should be more through
+// documentation before Go 1.18 is released.
 
 var cmdInit = &base.Command{
        UsageLine: "go work init [moddirs]",
index 2723013bf89ddf4a877d23d42212ff84b0fe1d04..6f35dc4ff3a659e24679656e17bd1f7c660a2100 100644 (file)
@@ -15,9 +15,9 @@ import (
        "golang.org/x/mod/module"
 )
 
-var _ = modload.TODOWorkspaces("Add more documentation below. Though this is" +
-       "enough for those trying workspaces out, there should be more through" +
-       "documentation if the proposal is accepted and released.")
+// TODO(#49232) Add more documentation below. Though this is
+// enough for those trying workspaces out, there should be more thorough
+// documentation before Go 1.18 is released.
 
 var cmdSync = &base.Command{
        UsageLine: "go work sync [moddirs]",
@@ -71,6 +71,8 @@ func runSync(ctx context.Context, cmd *base.Command, args []string) {
                mustSelectFor[m] = mustSelect
        }
 
+       workFilePath := modload.WorkFilePath() // save go.work path because EnterModule clobbers it.
+
        for _, m := range mms.Versions() {
                // Use EnterModule to reset the global state in modload to be in
                // single-module mode using the modroot of m.
@@ -98,4 +100,13 @@ func runSync(ctx context.Context, cmd *base.Command, args []string) {
                }, "all")
                modload.WriteGoMod(ctx)
        }
+
+       wf, err := modload.ReadWorkFile(workFilePath)
+       if err != nil {
+               base.Fatalf("go: %v", err)
+       }
+       modload.UpdateWorkFile(wf)
+       if err := modload.WriteWorkFile(workFilePath, wf); err != nil {
+               base.Fatalf("go: %v", err)
+       }
 }
index 10c25da396ed6b92ff5299f4f87205eae1deefa8..b2218280e46aa1534a2e77ea90181ed2f45e0992 100644 (file)
@@ -9,20 +9,16 @@ package workcmd
 import (
        "cmd/go/internal/base"
        "cmd/go/internal/fsys"
-       "cmd/go/internal/lockedfile"
        "cmd/go/internal/modload"
        "context"
        "io/fs"
-       "io/ioutil"
        "os"
        "path/filepath"
-
-       "golang.org/x/mod/modfile"
 )
 
-var _ = modload.TODOWorkspaces("Add more documentation below. Though this is" +
-       "enough for those trying workspaces out, there should be more through" +
-       "documentation if the proposal is accepted and released.")
+// TODO(#49232) Add more documentation below. Though this is
+// enough for those trying workspaces out, there should be more thorough
+// documentation before Go 1.18 is released.
 
 var cmdUse = &base.Command{
        UsageLine: "go work use [-r] [moddirs]",
@@ -51,14 +47,9 @@ func runUse(ctx context.Context, cmd *base.Command, args []string) {
        modload.InitWorkfile()
        gowork = modload.WorkFilePath()
 
-       data, err := lockedfile.Read(gowork)
-       if err != nil {
-               base.Fatalf("goX: %v", err)
-       }
-
-       workFile, err := modfile.ParseWork(gowork, data, nil)
+       workFile, err := modload.ReadWorkFile(gowork)
        if err != nil {
-               base.Fatalf("go: errors parsing %s:\n%s", base.ShortPath(gowork), err)
+               base.Fatalf("go: %v", err)
        }
 
        haveDirs := make(map[string]bool)
@@ -119,11 +110,6 @@ func runUse(ctx context.Context, cmd *base.Command, args []string) {
        for dir := range addDirs {
                workFile.AddDirectory(filepath.ToSlash(dir), "")
        }
-       workFile.SortBlocks()
-       workFile.Cleanup() // clean file after edits
-       out := modfile.Format(workFile.Syntax)
-
-       if err := ioutil.WriteFile(gowork, out, 0666); err != nil {
-               base.Fatalf("go: %v", err)
-       }
+       modload.UpdateWorkFile(workFile)
+       modload.WriteWorkFile(gowork, workFile)
 }
diff --git a/src/cmd/go/testdata/script/work_why_download_graph.txt b/src/cmd/go/testdata/script/work_why_download_graph.txt
new file mode 100644 (file)
index 0000000..c03b4a7
--- /dev/null
@@ -0,0 +1,59 @@
+# Test go mod download, why, and graph work in workspace mode.
+# TODO(bcmills): clarify the interaction with #44435
+
+go mod download rsc.io/quote
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.info
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.mod
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.zip
+! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.0.info
+! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.0.mod
+
+go mod download
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.info
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.mod
+exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.2.zip
+! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.0.info
+! exists $GOPATH/pkg/mod/cache/download/rsc.io/quote/@v/v1.5.0.mod
+
+go mod why rsc.io/quote
+stdout '# rsc.io/quote\nexample.com/a\nrsc.io/quote'
+
+go mod graph
+stdout 'example.com/a rsc.io/quote@v1.5.2\nexample.com/b example.com/c@v1.0.0\nrsc.io/quote@v1.5.2 rsc.io/sampler@v1.3.0\nrsc.io/sampler@v1.3.0 golang.org/x/text@v0.0.0-20170915032832-14c0d48ead0c'
+
+-- go.work --
+go 1.18
+
+directory (
+    ./a
+    ./b
+)
+-- a/go.mod --
+go 1.18
+
+module example.com/a
+
+require "rsc.io/quote" v1.5.2
+-- a/main.go --
+package main
+
+import (
+       "fmt"
+       "rsc.io/quote"
+)
+
+func main() {
+       fmt.Println(quote.Hello())
+}
+-- b/go.mod --
+go 1.18
+
+module example.com/b
+
+require example.com/c v1.0.0
+replace example.com/c => ../c
+-- c/go.mod --
+go 1.18
+
+module example.com/c
+