Skip to content

Commit

Permalink
cmd/go: revert 3 CLs affecting par.Work, context propagation, tracing
Browse files Browse the repository at this point in the history
This reverts the following changes:

•	cmd/go: add tracing for querying and downloading from the proxy
	CL 242786, commit 1a35583

•	cmd/go: do context propagation for tracing downloads
	CL 248327, commit c0cf190

•	cmd/go/internal: remove some users of par.Work
	CL 248326, commit f30044a

Reason for revert: broke linux 386 and amd64 longtest builders.

The problem started with CL 248326, but CL 248327 and CL 242786
are reverted as well due to conflicts.

Updates #38714.
Fixes #40861.

Change-Id: I68496b4e5a27e47a42183553c3a645b288edac83
Reviewed-on: https://go-review.googlesource.com/c/go/+/249017
Run-TryBot: Dmitri Shuralyov <[email protected]>
Reviewed-by: Bryan C. Mills <[email protected]>
Reviewed-by: Carlos Amedee <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
dmitshur committed Aug 18, 2020
1 parent 6e876f1 commit c12d9ed
Show file tree
Hide file tree
Showing 28 changed files with 216 additions and 299 deletions.
4 changes: 2 additions & 2 deletions src/cmd/go/internal/get/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,9 +246,9 @@ func download(arg string, parent *load.Package, stk *load.ImportStack, mode int)
load1 := func(path string, mode int) *load.Package {
if parent == nil {
mode := 0 // don't do module or vendor resolution
return load.LoadImport(context.TODO(), path, base.Cwd, nil, stk, nil, mode)
return load.LoadImport(path, base.Cwd, nil, stk, nil, mode)
}
return load.LoadImport(context.TODO(), path, parent.Dir, parent, stk, nil, mode|load.ResolveModule)
return load.LoadImport(path, parent.Dir, parent, stk, nil, mode|load.ResolveModule)
}

p := load1(arg, mode)
Expand Down
5 changes: 2 additions & 3 deletions src/cmd/go/internal/list/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"cmd/go/internal/cache"
"cmd/go/internal/cfg"
"cmd/go/internal/load"
"cmd/go/internal/modinfo"
"cmd/go/internal/modload"
"cmd/go/internal/str"
"cmd/go/internal/work"
Expand Down Expand Up @@ -350,7 +349,7 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
fm := template.FuncMap{
"join": strings.Join,
"context": context,
"module": func(path string) *modinfo.ModulePublic { return modload.ModuleInfo(ctx, path) },
"module": modload.ModuleInfo,
}
tmpl, err := template.New("main").Funcs(fm).Parse(*listFmt)
if err != nil {
Expand Down Expand Up @@ -390,7 +389,7 @@ func runList(ctx context.Context, cmd *base.Command, args []string) {
base.Fatalf("go list -m: not using modules")
}

modload.InitMod(ctx) // Parses go.mod and sets cfg.BuildMod.
modload.InitMod() // Parses go.mod and sets cfg.BuildMod.
if cfg.BuildMod == "vendor" {
const actionDisabledFormat = "go list -m: can't %s using the vendor directory\n\t(Use -mod=mod or -mod=readonly to bypass.)"

Expand Down
40 changes: 20 additions & 20 deletions src/cmd/go/internal/load/pkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ var (
ModBinDir func() string // return effective bin directory
ModLookup func(parentPath string, parentIsStd bool, path string) (dir, realPath string, err error) // lookup effective meaning of import
ModPackageModuleInfo func(path string) *modinfo.ModulePublic // return module info for Package struct
ModImportPaths func(ctx context.Context, args []string) []*search.Match // expand import paths
ModImportPaths func(args []string) []*search.Match // expand import paths
ModPackageBuildInfo func(main string, deps []string) string // return module info to embed in binary
ModInfoProg func(info string, isgccgo bool) []byte // wrap module info in .go code for binary
ModImportFromFiles func(context.Context, []string) // update go.mod to add modules for imports in these files
ModImportFromFiles func([]string) // update go.mod to add modules for imports in these files
ModDirImportPath func(string) string // return effective import path for directory
)

Expand Down Expand Up @@ -553,7 +553,7 @@ func ReloadPackageNoFlags(arg string, stk *ImportStack) *Package {
})
packageDataCache.Delete(p.ImportPath)
}
return LoadImport(context.TODO(), arg, base.Cwd, nil, stk, nil, 0)
return LoadImport(arg, base.Cwd, nil, stk, nil, 0)
}

// dirToImportPath returns the pseudo-import path we use for a package
Expand Down Expand Up @@ -605,11 +605,11 @@ const (
// LoadImport does not set tool flags and should only be used by
// this package, as part of a bigger load operation, and by GOPATH-based "go get".
// TODO(rsc): When GOPATH-based "go get" is removed, unexport this function.
func LoadImport(ctx context.Context, path, srcDir string, parent *Package, stk *ImportStack, importPos []token.Position, mode int) *Package {
return loadImport(ctx, nil, path, srcDir, parent, stk, importPos, mode)
func LoadImport(path, srcDir string, parent *Package, stk *ImportStack, importPos []token.Position, mode int) *Package {
return loadImport(nil, path, srcDir, parent, stk, importPos, mode)
}

func loadImport(ctx context.Context, pre *preload, path, srcDir string, parent *Package, stk *ImportStack, importPos []token.Position, mode int) *Package {
func loadImport(pre *preload, path, srcDir string, parent *Package, stk *ImportStack, importPos []token.Position, mode int) *Package {
if path == "" {
panic("LoadImport called with empty package path")
}
Expand Down Expand Up @@ -657,7 +657,7 @@ func loadImport(ctx context.Context, pre *preload, path, srcDir string, parent *
// Load package.
// loadPackageData may return bp != nil even if an error occurs,
// in order to return partial information.
p.load(ctx, path, stk, importPos, bp, err)
p.load(path, stk, importPos, bp, err)

if !cfg.ModulesEnabled && path != cleanImport(path) {
p.Error = &PackageError{
Expand Down Expand Up @@ -1591,7 +1591,7 @@ func (p *Package) DefaultExecName() string {
// load populates p using information from bp, err, which should
// be the result of calling build.Context.Import.
// stk contains the import stack, not including path itself.
func (p *Package) load(ctx context.Context, path string, stk *ImportStack, importPos []token.Position, bp *build.Package, err error) {
func (p *Package) load(path string, stk *ImportStack, importPos []token.Position, bp *build.Package, err error) {
p.copyBuild(bp)

// The localPrefix is the path we interpret ./ imports relative to.
Expand Down Expand Up @@ -1800,7 +1800,7 @@ func (p *Package) load(ctx context.Context, path string, stk *ImportStack, impor
if path == "C" {
continue
}
p1 := LoadImport(ctx, path, p.Dir, p, stk, p.Internal.Build.ImportPos[path], ResolveImport)
p1 := LoadImport(path, p.Dir, p, stk, p.Internal.Build.ImportPos[path], ResolveImport)

path = p1.ImportPath
importPaths[i] = path
Expand Down Expand Up @@ -2073,7 +2073,7 @@ func PackageList(roots []*Package) []*Package {
// TestPackageList returns the list of packages in the dag rooted at roots
// as visited in a depth-first post-order traversal, including the test
// imports of the roots. This ignores errors in test packages.
func TestPackageList(ctx context.Context, roots []*Package) []*Package {
func TestPackageList(roots []*Package) []*Package {
seen := map[*Package]bool{}
all := []*Package{}
var walk func(*Package)
Expand All @@ -2089,7 +2089,7 @@ func TestPackageList(ctx context.Context, roots []*Package) []*Package {
}
walkTest := func(root *Package, path string) {
var stk ImportStack
p1 := LoadImport(ctx, path, root.Dir, root, &stk, root.Internal.Build.TestImportPos[path], ResolveImport)
p1 := LoadImport(path, root.Dir, root, &stk, root.Internal.Build.TestImportPos[path], ResolveImport)
if p1.Error == nil {
walk(p1)
}
Expand All @@ -2112,7 +2112,7 @@ func TestPackageList(ctx context.Context, roots []*Package) []*Package {
// TODO(jayconrod): delete this function and set flags automatically
// in LoadImport instead.
func LoadImportWithFlags(path, srcDir string, parent *Package, stk *ImportStack, importPos []token.Position, mode int) *Package {
p := LoadImport(context.TODO(), path, srcDir, parent, stk, importPos, mode)
p := LoadImport(path, srcDir, parent, stk, importPos, mode)
setToolFlags(p)
return p
}
Expand Down Expand Up @@ -2153,12 +2153,12 @@ func PackagesAndErrors(ctx context.Context, patterns []string) []*Package {
// We need to test whether the path is an actual Go file and not a
// package path or pattern ending in '.go' (see golang.org/issue/34653).
if fi, err := os.Stat(p); err == nil && !fi.IsDir() {
return []*Package{GoFilesPackage(ctx, patterns)}
return []*Package{GoFilesPackage(patterns)}
}
}
}

matches := ImportPaths(ctx, patterns)
matches := ImportPaths(patterns)
var (
pkgs []*Package
stk ImportStack
Expand All @@ -2174,7 +2174,7 @@ func PackagesAndErrors(ctx context.Context, patterns []string) []*Package {
if pkg == "" {
panic(fmt.Sprintf("ImportPaths returned empty package for pattern %s", m.Pattern()))
}
p := loadImport(ctx, pre, pkg, base.Cwd, nil, &stk, nil, 0)
p := loadImport(pre, pkg, base.Cwd, nil, &stk, nil, 0)
p.Match = append(p.Match, m.Pattern())
p.Internal.CmdlinePkg = true
if m.IsLiteral() {
Expand Down Expand Up @@ -2228,9 +2228,9 @@ func setToolFlags(pkgs ...*Package) {
}
}

func ImportPaths(ctx context.Context, args []string) []*search.Match {
func ImportPaths(args []string) []*search.Match {
if ModInit(); cfg.ModulesEnabled {
return ModImportPaths(ctx, args)
return ModImportPaths(args)
}
return search.ImportPaths(args)
}
Expand Down Expand Up @@ -2281,7 +2281,7 @@ func PackagesForBuild(ctx context.Context, args []string) []*Package {
// GoFilesPackage creates a package for building a collection of Go files
// (typically named on the command line). The target is named p.a for
// package p or named after the first Go file for package main.
func GoFilesPackage(ctx context.Context, gofiles []string) *Package {
func GoFilesPackage(gofiles []string) *Package {
ModInit()

for _, f := range gofiles {
Expand Down Expand Up @@ -2329,7 +2329,7 @@ func GoFilesPackage(ctx context.Context, gofiles []string) *Package {
ctxt.ReadDir = func(string) ([]os.FileInfo, error) { return dirent, nil }

if cfg.ModulesEnabled {
ModImportFromFiles(ctx, gofiles)
ModImportFromFiles(gofiles)
}

var err error
Expand All @@ -2345,7 +2345,7 @@ func GoFilesPackage(ctx context.Context, gofiles []string) *Package {
pkg := new(Package)
pkg.Internal.Local = true
pkg.Internal.CmdlineFiles = true
pkg.load(ctx, "command-line-arguments", &stk, nil, bp, err)
pkg.load("command-line-arguments", &stk, nil, bp, err)
pkg.Internal.LocalPrefix = dirToImportPath(dir)
pkg.ImportPath = "command-line-arguments"
pkg.Target = ""
Expand Down
6 changes: 3 additions & 3 deletions src/cmd/go/internal/load/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestPackagesAndErrors(ctx context.Context, p *Package, cover *TestCover) (p
stk.Push(p.ImportPath + " (test)")
rawTestImports := str.StringList(p.TestImports)
for i, path := range p.TestImports {
p1 := loadImport(ctx, pre, path, p.Dir, p, &stk, p.Internal.Build.TestImportPos[path], ResolveImport)
p1 := loadImport(pre, path, p.Dir, p, &stk, p.Internal.Build.TestImportPos[path], ResolveImport)
if str.Contains(p1.Deps, p.ImportPath) || p1.ImportPath == p.ImportPath {
// Same error that loadPackage returns (via reusePackage) in pkg.go.
// Can't change that code, because that code is only for loading the
Expand All @@ -127,7 +127,7 @@ func TestPackagesAndErrors(ctx context.Context, p *Package, cover *TestCover) (p
pxtestNeedsPtest := false
rawXTestImports := str.StringList(p.XTestImports)
for i, path := range p.XTestImports {
p1 := loadImport(ctx, pre, path, p.Dir, p, &stk, p.Internal.Build.XTestImportPos[path], ResolveImport)
p1 := loadImport(pre, path, p.Dir, p, &stk, p.Internal.Build.XTestImportPos[path], ResolveImport)
if p1.ImportPath == p.ImportPath {
pxtestNeedsPtest = true
} else {
Expand Down Expand Up @@ -244,7 +244,7 @@ func TestPackagesAndErrors(ctx context.Context, p *Package, cover *TestCover) (p
if dep == ptest.ImportPath {
pmain.Internal.Imports = append(pmain.Internal.Imports, ptest)
} else {
p1 := loadImport(ctx, pre, dep, "", nil, &stk, nil, 0)
p1 := loadImport(pre, dep, "", nil, &stk, nil, 0)
pmain.Internal.Imports = append(pmain.Internal.Imports, p1)
}
}
Expand Down
75 changes: 33 additions & 42 deletions src/cmd/go/internal/modcmd/download.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
package modcmd

import (
"cmd/go/internal/modfetch"
"context"
"encoding/json"
"os"
"runtime"

"cmd/go/internal/base"
"cmd/go/internal/cfg"
"cmd/go/internal/modfetch"
"cmd/go/internal/modload"
"cmd/go/internal/par"
"cmd/go/internal/work"

"golang.org/x/mod/module"
Expand Down Expand Up @@ -90,7 +90,7 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
if len(args) == 0 {
args = []string{"all"}
} else if modload.HasModRoot() {
modload.InitMod(ctx) // to fill Target
modload.InitMod() // to fill Target
targetAtLatest := modload.Target.Path + "@latest"
targetAtUpgrade := modload.Target.Path + "@upgrade"
targetAtPatch := modload.Target.Path + "@patch"
Expand All @@ -102,7 +102,33 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
}
}

downloadModule := func(m *moduleJSON) {
var mods []*moduleJSON
var work par.Work
listU := false
listVersions := false
for _, info := range modload.ListModules(ctx, args, listU, listVersions) {
if info.Replace != nil {
info = info.Replace
}
if info.Version == "" && info.Error == nil {
// main module or module replaced with file path.
// Nothing to download.
continue
}
m := &moduleJSON{
Path: info.Path,
Version: info.Version,
}
mods = append(mods, m)
if info.Error != nil {
m.Error = info.Error.Err
continue
}
work.Add(m)
}

work.Do(10, func(item interface{}) {
m := item.(*moduleJSON)
var err error
m.Info, err = modfetch.InfoFile(m.Path, m.Version)
if err != nil {
Expand All @@ -120,53 +146,18 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
return
}
mod := module.Version{Path: m.Path, Version: m.Version}
m.Zip, err = modfetch.DownloadZip(ctx, mod)
m.Zip, err = modfetch.DownloadZip(mod)
if err != nil {
m.Error = err.Error()
return
}
m.Sum = modfetch.Sum(mod)
m.Dir, err = modfetch.Download(ctx, mod)
m.Dir, err = modfetch.Download(mod)
if err != nil {
m.Error = err.Error()
return
}
}

var mods []*moduleJSON
listU := false
listVersions := false
type token struct{}
sem := make(chan token, runtime.GOMAXPROCS(0))
for _, info := range modload.ListModules(ctx, args, listU, listVersions) {
if info.Replace != nil {
info = info.Replace
}
if info.Version == "" && info.Error == nil {
// main module or module replaced with file path.
// Nothing to download.
continue
}
m := &moduleJSON{
Path: info.Path,
Version: info.Version,
}
mods = append(mods, m)
if info.Error != nil {
m.Error = info.Error.Err
continue
}
sem <- token{}
go func() {
downloadModule(m)
<-sem
}()
}

// Fill semaphore channel to wait for goroutines to finish.
for n := cap(sem); n > 0; n-- {
sem <- token{}
}
})

if *downloadJSON {
for _, m := range mods {
Expand Down
19 changes: 9 additions & 10 deletions src/cmd/go/internal/modcmd/graph.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"cmd/go/internal/base"
"cmd/go/internal/cfg"
"cmd/go/internal/modload"
"cmd/go/internal/par"
"cmd/go/internal/work"

"golang.org/x/mod/module"
Expand Down Expand Up @@ -58,25 +59,23 @@ func runGraph(ctx context.Context, cmd *base.Command, args []string) {
return m.Path + "@" + m.Version
}

// Note: using par.Work only to manage work queue.
// No parallelism here, so no locking.
var out []string
var deps int // index in out where deps start
seen := map[module.Version]bool{modload.Target: true}
queue := []module.Version{modload.Target}
for len(queue) > 0 {
var m module.Version
m, queue = queue[0], queue[1:]
var work par.Work
work.Add(modload.Target)
work.Do(1, func(item interface{}) {
m := item.(module.Version)
list, _ := reqs.Required(m)
for _, r := range list {
if !seen[r] {
queue = append(queue, r)
seen[r] = true
}
work.Add(r)
out = append(out, format(m)+" "+format(r)+"\n")
}
if m == modload.Target {
deps = len(out)
}
}
})

sort.Slice(out[deps:], func(i, j int) bool {
return out[deps+i][0] < out[deps+j][0]
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modcmd/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,6 @@ func runInit(ctx context.Context, cmd *base.Command, args []string) {
if strings.Contains(modload.CmdModModule, "@") {
base.Fatalf("go mod init: module path must not contain '@'")
}
modload.InitMod(ctx) // does all the hard work
modload.InitMod() // does all the hard work
modload.WriteGoMod()
}
2 changes: 1 addition & 1 deletion src/cmd/go/internal/modcmd/tidy.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func runTidy(ctx context.Context, cmd *base.Command, args []string) {
base.Fatalf("go mod tidy: no arguments allowed")
}

modload.LoadALL(ctx)
modload.LoadALL()
modload.TidyBuildList()
modload.TrimGoSum()
modload.WriteGoMod()
Expand Down
Loading

0 comments on commit c12d9ed

Please sign in to comment.