qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-09-29 16:24:28 -06:00
Code Releases Activity

[dev.cmdgo] cmd/go: clean up TODOWorkspaces instances

Browse Source 
Address some of the easier todos to address and remove the todos that
have already been done and redundant todos.

For #45713

Change-Id: I3fe4393168b10c6e005325258d9701713c92e9e4
Reviewed-on: https://go-review.googlesource.com/c/go/+/344491
Trust: Michael Matloob <matloob@golang.org>
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
This commit is contained in:
Michael Matloob 2021-08-23 14:51:39 -04:00
parent 109c13b64f
commit 3b523caf41
9 changed files with 71 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/cmd/go/internal/modcmd/download.go
View File

@ -97,7 +97,7 @@ func runDownload(ctx context.Context, cmd *base.Command, args []string) {
modload.LoadModFile(ctx) // to fill MainModules
if len(modload.MainModules.Versions()) != 1 {
panic(modload.TODOWorkspaces("TODO: multiple main modules not supported in Download"))
panic(modload.TODOWorkspaces("Support workspace mode in go mod download"))
}
mainModule := modload.MainModules.Versions()[0]

4
src/cmd/go/internal/modcmd/initwork.go
View File

@ -13,9 +13,9 @@ import (
"path/filepath"
)
var _ = modload.TODOWorkspaces("Add more documentation below.T hough this is" +
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.")
"documentation if the proposal is accepted and released.")
var cmdInitwork = &base.Command{
UsageLine: "go mod initwork [moddirs]",

20
src/cmd/go/internal/modget/get.go
View File

@ -690,7 +690,7 @@ func (r *resolver) queryNone(ctx context.Context, q *query) {
// However, neither of those behaviors would be consistent with the
// plain meaning of the query. To try to reduce confusion, reject the
// query explicitly.
return errSet(&modload.QueryMatchesMainModuleError{MainModule: v, Pattern: q.pattern, Query: q.version})
return errSet(&modload.QueryMatchesMainModulesError{MainModules: []module.Version{v}, Pattern: q.pattern, Query: q.version})
}
return pathSet{mod: module.Version{Path: q.pattern, Version: "none"}}
@ -703,7 +703,7 @@ func (r *resolver) queryNone(ctx context.Context, q *query) {
}
q.pathOnce(curM.Path, func() pathSet {
if modload.HasModRoot() && curM.Version == "" && modload.MainModules.Contains(curM.Path) {
return errSet(&modload.QueryMatchesMainModuleError{MainModule: curM, Pattern: q.pattern, Query: q.version})
return errSet(&modload.QueryMatchesMainModulesError{MainModules: []module.Version{curM}, Pattern: q.pattern, Query: q.version})
}
return pathSet{mod: module.Version{Path: curM.Path, Version: "none"}}
})
@ -805,10 +805,10 @@ func (r *resolver) queryWildcard(ctx context.Context, q *query) {
if modload.MainModules.Contains(curM.Path) && !versionOkForMainModule(q.version) {
if q.matchesPath(curM.Path) {
return errSet(&modload.QueryMatchesMainModuleError{
MainModule: curM,
Pattern: q.pattern,
Query: q.version,
return errSet(&modload.QueryMatchesMainModulesError{
MainModules: []module.Version{curM},
Pattern: q.pattern,
Query: q.version,
})
}
@ -1760,10 +1760,10 @@ func (r *resolver) resolve(q *query, m module.Version) {
}
if modload.MainModules.Contains(m.Path) && m.Version != "" {
reportError(q, &modload.QueryMatchesMainModuleError{
MainModule: module.Version{Path: m.Path},
Pattern: q.pattern,
Query: q.version,
reportError(q, &modload.QueryMatchesMainModulesError{
MainModules: []module.Version{{Path: m.Path}},
Pattern: q.pattern,
Query: q.version,
})
return
}

1
src/cmd/go/internal/modload/build.go
View File

@ -218,7 +218,6 @@ func moduleInfo(ctx context.Context, rs *Requirements, m module.Version, mode Li
Version: m.Version,
Main: true,
}
_ = TODOWorkspaces("handle rawGoVersion here")
if v, ok := rawGoVersion.Load(m); ok {
info.GoVersion = v.(string)
} else {

8
src/cmd/go/internal/modload/buildlist.go
View File

@ -139,7 +139,7 @@ func (rs *Requirements) initVendor(vendorList []module.Version) {
}
if MainModules.Len() != 1 {
panic("There should be exactly one main moudle in Vendor mode.")
panic("There should be exactly one main module in Vendor mode.")
}
mainModule := MainModules.Versions()[0]
@ -284,8 +284,10 @@ func readModGraph(ctx context.Context, depth modDepth, roots []module.Version) (
)
for _, m := range MainModules.Versions() {
// Require all roots from all main modules.
_ = TODOWorkspaces("This isn't the correct behavior. " +
"Fix this when the requirements struct is updated to reflect the struct of the module graph.")
_ = TODOWorkspaces("This flattens a level of the module graph, adding the dependencies " +
"of all main modules to a single requirements struct, and losing the information of which " +
"main module required which requirement. Rework the requirements struct and change this" +
"to reflect the structure of the main modules.")
mg.g.Require(m, roots)
}

48
src/cmd/go/internal/modload/init.go
View File

@ -123,8 +123,6 @@ func (mms *MainModuleSet) Contains(path string) bool {
}
func (mms *MainModuleSet) ModRoot(m module.Version) string {
_ = TODOWorkspaces(" Do we need the Init? The original modRoot calls it. Audit callers.")
Init()
if mms == nil {
return ""
}
@ -143,8 +141,11 @@ func (mms *MainModuleSet) mustGetSingleMainModule() module.Version {
panic("internal error: mustGetSingleMainModule called in context with no main modules")
}
if len(mms.versions) != 1 {
_ = TODOWorkspaces("Check if we're in workspace mode before returning the below error.")
panic("internal error: mustGetSingleMainModule called in workspace mode")
if inWorkspaceMode() {
panic("internal error: mustGetSingleMainModule called in workspace mode")
} else {
panic("internal error: multiple main modules present outside of workspace mode")
}
}
return mms.versions[0]
}
@ -156,11 +157,7 @@ func (mms *MainModuleSet) GetSingleIndexOrNil() *modFileIndex {
if len(mms.versions) == 0 {
return nil
}
if len(mms.versions) != 1 {
_ = TODOWorkspaces("Check if we're in workspace mode before returning the below error.")
panic("internal error: mustGetSingleMainModule called in workspace mode")
}
return mms.indices[mms.versions[0]]
return mms.indices[mms.mustGetSingleMainModule()]
}
func (mms *MainModuleSet) Index(m module.Version) *modFileIndex {
@ -363,7 +360,9 @@ func Init() {
// We're in module mode. Set any global variables that need to be set.
cfg.ModulesEnabled = true
setDefaultBuildMod()
_ = TODOWorkspaces("ensure that buildmod is readonly")
_ = 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] == "" {
base.Fatalf("missing $GOPATH")
@ -374,15 +373,14 @@ func Init() {
}
if inWorkspaceMode() {
_ = TODOWorkspaces("go.work.sum, and also allow modfetch to fall back to individual go.sums")
_ = TODOWorkspaces("replaces")
var err error
modRoots, err = loadWorkFile(workFilePath)
if err != nil {
base.Fatalf("reading go.work: %v", err)
}
_ = TODOWorkspaces("Support falling back to individual module go.sum " +
"files for sums not in the workspace sum file.")
modfetch.GoSumFile = workFilePath + ".sum"
// TODO(matloob) should workRoot just be workFile?
} else if modRoots == nil {
// We're in module mode, but not inside a module.
//
@ -539,6 +537,7 @@ func (goModDirtyError) Error() string {
var errGoModDirty error = goModDirtyError{}
func loadWorkFile(path string) (modRoots []string, 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 {
@ -661,8 +660,7 @@ func loadModFile(ctx context.Context) (rs *Requirements, needCommit bool) {
// We don't need to do anything for vendor or update the mod file so
// return early.
_ = TODOWorkspaces("don't worry about commits for now, but eventually will want to update go.work files")
return rs, false
return rs, true
}
mainModule := MainModules.mustGetSingleMainModule()
@ -761,7 +759,7 @@ func CreateModFile(ctx context.Context, modPath string) {
MainModules = makeMainModules([]module.Version{modFile.Module.Mod}, []string{modRoot}, []*modfile.File{modFile}, []*modFileIndex{nil})
addGoStmt(modFile, modFile.Module.Mod, LatestGoVersion()) // Add the go directive before converted module requirements.
convertedFrom, err := convertLegacyConfig(modFile, modPath)
convertedFrom, err := convertLegacyConfig(modFile, modRoot)
if convertedFrom != "" {
fmt.Fprintf(os.Stderr, "go: copying requirements from %s\n", base.ShortPath(convertedFrom))
}
@ -1037,7 +1035,7 @@ func mustHaveCompleteRequirements() bool {
// convertLegacyConfig imports module requirements from a legacy vendoring
// configuration file, if one is present.
func convertLegacyConfig(modFile *modfile.File, modPath string) (from string, err error) {
func convertLegacyConfig(modFile *modfile.File, modRoot string) (from string, err error) {
noneSelected := func(path string) (version string) { return "none" }
queryPackage := func(path, rev string) (module.Version, error) {
pkgMods, modOnly, err := QueryPattern(context.Background(), path, rev, noneSelected, nil)
@ -1050,10 +1048,7 @@ func convertLegacyConfig(modFile *modfile.File, modPath string) (from string, er
return modOnly.Mod, nil
}
for _, name := range altConfigs {
if len(modRoots) != 1 {
panic(TODOWorkspaces("what do do here?"))
}
cfg := filepath.Join(modRoots[0], name)
cfg := filepath.Join(modRoot, name)
data, err := os.ReadFile(cfg)
if err == nil {
convert := modconv.Converters[name]
@ -1166,7 +1161,8 @@ func findWorkspaceFile(dir string) (root string) {
break
}
if d == cfg.GOROOT {
_ = TODOWorkspaces("Address how go.work files interact with 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.
}
dir = d
@ -1345,7 +1341,7 @@ func commitRequirements(ctx context.Context, goVersion string, rs *Requirements)
// We aren't in a module, so we don't have anywhere to write a go.mod file.
return
}
mainModule := MainModules.Versions()[0]
mainModule := MainModules.mustGetSingleMainModule()
modFilePath := modFilePath(MainModules.ModRoot(mainModule))
modFile := MainModules.ModFile(mainModule)
@ -1398,12 +1394,6 @@ func commitRequirements(ctx context.Context, goVersion string, rs *Requirements)
base.Fatalf("go: %v", err)
}
defer func() {
if MainModules.Len() != 1 {
panic(TODOWorkspaces("There should be exactly one main module when committing reqs"))
}
mainModule := MainModules.Versions()[0]
// At this point we have determined to make the go.mod file on disk equal to new.
MainModules.SetIndex(mainModule, indexModFile(new, modFile, mainModule, false))

3
src/cmd/go/internal/modload/load.go
View File

@ -552,8 +552,7 @@ func resolveLocalPackage(ctx context.Context, dir string, rs *Requirements) (str
// return an error.
if len(mainModulePrefix) > len(pkgNotFoundLongestPrefix) {
pkgNotFoundLongestPrefix = mainModulePrefix
pkgNotFoundErr = &PackageNotInModuleError{Mod: mainModule, Pattern: pkg}
pkgNotFoundErr = &PackageNotInModuleError{MainModules: []module.Version{mainModule}, Pattern: pkg}
}
continue
}

2
src/cmd/go/internal/modload/modfile.go
View File

@ -322,7 +322,7 @@ func replacement(mod module.Version, index *modFileIndex) (fromVersion string, t
// If there is no replacement for mod, Replacement returns
// a module.Version with Path == "".
func Replacement(mod module.Version) (module.Version, string) {
_ = TODOWorkspaces("support replaces in the go.work file")
_ = TODOWorkspaces("Support replaces in the go.work file.")
foundFrom, found, foundModRoot := "", module.Version{}, ""
for _, v := range MainModules.Versions() {
if index := MainModules.Index(v); index != nil {

53
src/cmd/go/internal/modload/query.go
View File

@ -632,18 +632,16 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
if modOnly != nil {
return nil, modOnly, nil
} else if len(mainModuleMatches) != 0 {
_ = TODOWorkspaces("add multiple main modules to the error?")
return nil, nil, &QueryMatchesMainModuleError{
MainModule: mainModuleMatches[0],
Pattern: pattern,
Query: query,
return nil, nil, &QueryMatchesMainModulesError{
MainModules: mainModuleMatches,
Pattern: pattern,
Query: query,
}
} else {
_ = TODOWorkspaces("This should maybe be PackageNotInModule*s* error with the main modules that are prefixes of base")
return nil, nil, &PackageNotInModuleError{
Mod: MainModules.Versions()[0],
Query: query,
Pattern: pattern,
MainModules: mainModuleMatches,
Query: query,
Pattern: pattern,
}
}
}
@ -695,7 +693,7 @@ func QueryPattern(ctx context.Context, pattern, query string, current func(strin
})
if len(mainModuleMatches) > 0 && len(results) == 0 && modOnly == nil && errors.Is(err, fs.ErrNotExist) {
return nil, nil, &QueryMatchesMainModuleError{
return nil, nil, &QueryMatchesMainModulesError{
Pattern: pattern,
Query: query,
}
@ -893,6 +891,7 @@ func (e *WildcardInFirstElementError) Error() string {
// code for the versions it knows about, and thus did not have the opportunity
// to return a non-400 status code to suppress fallback.
type PackageNotInModuleError struct {
MainModules []module.Version
Mod module.Version
Replacement module.Version
Query string
@ -900,11 +899,15 @@ type PackageNotInModuleError struct {
}
func (e *PackageNotInModuleError) Error() string {
if MainModules.Contains(e.Mod.Path) {
if strings.Contains(e.Pattern, "...") {
return fmt.Sprintf("main module (%s) does not contain packages matching %s", e.Mod.Path, e.Pattern)
if len(e.MainModules) > 0 {
prefix := "workspace modules do"
if len(e.MainModules) == 1 {
prefix = fmt.Sprintf("main module (%s) does", e.MainModules[0])
}
return fmt.Sprintf("main module (%s) does not contain package %s", e.Mod.Path, e.Pattern)
if strings.Contains(e.Pattern, "...") {
return fmt.Sprintf("%s not contain packages matching %s", prefix, e.Pattern)
}
return fmt.Sprintf("%s not contain package %s", prefix, e.Pattern)
}
found := ""
@ -1153,21 +1156,29 @@ func (rr *replacementRepo) replacementStat(v string) (*modfetch.RevInfo, error)
return rev, nil
}
// A QueryMatchesMainModuleError indicates that a query requests
// A QueryMatchesMainModulesError indicates that a query requests
// a version of the main module that cannot be satisfied.
// (The main module's version cannot be changed.)
type QueryMatchesMainModuleError struct {
MainModule module.Version
Pattern string
Query string
type QueryMatchesMainModulesError struct {
MainModules []module.Version
Pattern string
Query string
}
func (e *QueryMatchesMainModuleError) Error() string {
func (e *QueryMatchesMainModulesError) Error() string {
if MainModules.Contains(e.Pattern) {
return fmt.Sprintf("can't request version %q of the main module (%s)", e.Query, e.Pattern)
}
return fmt.Sprintf("can't request version %q of pattern %q that includes the main module (%s)", e.Query, e.Pattern, e.MainModule.Path)
plural := ""
mainModulePaths := make([]string, len(e.MainModules))
for i := range e.MainModules {
mainModulePaths[i] = e.MainModules[i].Path
}
if len(e.MainModules) > 1 {
plural = "s"
}
return fmt.Sprintf("can't request version %q of pattern %q that includes the main module%s (%s)", e.Query, e.Pattern, plural, strings.Join(mainModulePaths, ", "))
}
// A QueryUpgradesAllError indicates that a query requests