From 3505ce0a62f33516319b592d345adb5d039db5a1 Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Fri, 8 Mar 2024 17:03:14 +0000 Subject: [PATCH] internal/mod/modload: split Tidy from UpdateVersions As the code is reasonably involved, it seems clearer when the two pieces of functionality are in separate files, particularly as the complexity of the tidy piece is going to increase shortly. Signed-off-by: Roger Peppe Change-Id: I6ab8f943605000f35cbd21756cd04b7958fa8f61 Dispatch-Trailer: {"type":"trybot","CL":1178013,"patchset":1,"ref":"refs/changes/13/1178013/1","targetBranch":"master"} --- internal/mod/modload/{load.go => tidy.go} | 161 ---------------- .../modload/{load_test.go => tidy_test.go} | 33 ---- internal/mod/modload/update.go | 175 ++++++++++++++++++ internal/mod/modload/update_test.go | 49 +++++ 4 files changed, 224 insertions(+), 194 deletions(-) rename internal/mod/modload/{load.go => tidy.go} (79%) rename internal/mod/modload/{load_test.go => tidy_test.go} (77%) create mode 100644 internal/mod/modload/update.go create mode 100644 internal/mod/modload/update_test.go diff --git a/internal/mod/modload/load.go b/internal/mod/modload/tidy.go similarity index 79% rename from internal/mod/modload/load.go rename to internal/mod/modload/tidy.go index a66c4895a..755cda7ef 100644 --- a/internal/mod/modload/load.go +++ b/internal/mod/modload/tidy.go @@ -10,8 +10,6 @@ import ( "path" "runtime" "slices" - "strings" - "sync/atomic" "cuelang.org/go/internal/mod/modimports" "cuelang.org/go/internal/mod/modpkgload" @@ -82,165 +80,6 @@ func Tidy(ctx context.Context, fsys fs.FS, modRoot string, reg Registry, cueVers return modfileFromRequirements(mf, rs, cueVers), nil } -// UpdateVersions returns the main module's module file with the specified module versions -// updated if possible and added if not already present. It returns an error if asked -// to downgrade a module below a version already required by an external dependency. -// -// A module in the versions slice can be specified as one of the following: -// - $module@$fullVersion: a specific exact version -// - $module@$partialVersion: a non-canonical version -// specifies the latest version that has the same major/minor numbers. -// - $module@latest: the latest non-prerelease version, or latest prerelease version if -// there is no non-prerelease version -// - $module: equivalent to $module@latest if $module doesn't have a default major -// version or $module@$majorVersion if it does, where $majorVersion is the -// default major version for $module. -func UpdateVersions(ctx context.Context, fsys fs.FS, modRoot string, reg Registry, versions []string) (*modfile.File, error) { - mainModuleVersion, mf, err := readModuleFile(ctx, fsys, modRoot) - if err != nil { - return nil, err - } - rs := modrequirements.NewRequirements(mf.Module, reg, mf.DepVersions(), mf.DefaultMajorVersions()) - mversions, err := resolveUpdateVersions(ctx, reg, rs, mainModuleVersion, versions) - if err != nil { - return nil, err - } - // Now we know what versions we want to update to, make a new set of - // requirements with these versions in place. - - mversionsMap := make(map[string]module.Version) - for _, v := range mversions { - // Check existing membership of the map: if the same module has been specified - // twice, then choose t - if v1, ok := mversionsMap[v.Path()]; ok && v1.Version() != v.Version() { - // The same module has been specified twice with different requirements. - // Treat it as an error (an alternative approach might be to choose the greater - // version, but making it an error seems more appropriate to the "choose exact - // version" semantics of UpdateVersions. - return nil, fmt.Errorf("conflicting version update requirements %v vs %v", v1, v) - } - mversionsMap[v.Path()] = v - } - g, err := rs.Graph(ctx) - if err != nil { - return nil, fmt.Errorf("cannot determine module graph: %v", err) - } - var newVersions []module.Version - for _, v := range g.BuildList() { - if v.Path() == mainModuleVersion.Path() { - continue - } - if newv, ok := mversionsMap[v.Path()]; ok { - newVersions = append(newVersions, newv) - delete(mversionsMap, v.Path()) - } else { - newVersions = append(newVersions, v) - } - } - for _, v := range mversionsMap { - newVersions = append(newVersions, v) - } - rs = modrequirements.NewRequirements(mf.Module, reg, newVersions, mf.DefaultMajorVersions()) - g, err = rs.Graph(ctx) - if err != nil { - return nil, fmt.Errorf("cannot determine new module graph: %v", err) - } - // Now check that the resulting versions are the ones we wanted. - for _, v := range mversions { - actualVers := g.Selected(v.Path()) - if actualVers != v.Version() { - return nil, fmt.Errorf("other requirements prevent changing module %v to version %v (actual selected version: %v)", v.Path(), v.Version(), actualVers) - } - } - // Make a new requirements with the selected versions of the above as roots. - var finalVersions []module.Version - for _, v := range g.BuildList() { - if v.Path() != mainModuleVersion.Path() { - finalVersions = append(finalVersions, v) - } - } - rs = modrequirements.NewRequirements(mf.Module, reg, finalVersions, mf.DefaultMajorVersions()) - return modfileFromRequirements(mf, rs, ""), nil -} - -// resolveUpdateVersions resolves a set of version strings as accepted by [UpdateVersions] -// into the actual module versions they represent. -func resolveUpdateVersions(ctx context.Context, reg Registry, rs *modrequirements.Requirements, mainModuleVersion module.Version, versions []string) ([]module.Version, error) { - work := par.NewQueue(runtime.GOMAXPROCS(0)) - mversions := make([]module.Version, len(versions)) - var queryErr atomic.Pointer[error] - setError := func(err error) { - queryErr.CompareAndSwap(nil, &err) - } - for i, v := range versions { - i, v := i, v - if mv, err := module.ParseVersion(v); err == nil { - // It's already canonical: nothing more to do. - mversions[i] = mv - continue - } - mpath, vers, ok := strings.Cut(v, "@") - if !ok { - if major, status := rs.DefaultMajorVersion(mpath); status == modrequirements.ExplicitDefault { - // TODO allow a non-explicit default too? - vers = major - } else { - vers = "latest" - } - } - if err := module.CheckPathWithoutVersion(mpath); err != nil { - return nil, fmt.Errorf("invalid module path in %q", v) - } - versionPrefix := "" - if vers != "latest" { - if !semver.IsValid(vers) { - return nil, fmt.Errorf("%q does not specify a valid semantic version", v) - } - if semver.Build(vers) != "" { - return nil, fmt.Errorf("build version suffixes not supported (%v)", v) - } - // It's a valid version but has no build suffix and it's not canonical, - // which means it must be either a major-only or major-minor, so - // the conforming canonical versions must have it as a prefix, with - // a dot separating the last component and the next. - versionPrefix = vers + "." - } - work.Add(func() { - allVersions, err := reg.ModuleVersions(ctx, mpath) - if err != nil { - setError(err) - return - } - possibleVersions := make([]string, 0, len(allVersions)) - for _, v := range allVersions { - if strings.HasPrefix(v, versionPrefix) { - possibleVersions = append(possibleVersions, v) - } - } - chosen := latestVersion(possibleVersions) - mv, err := module.NewVersion(mpath, chosen) - if err != nil { - // Should never happen, because we've checked that - // mpath is valid and ModuleVersions - // should always return valid semver versions. - setError(err) - return - } - mversions[i] = mv - }) - } - <-work.Idle() - if errPtr := queryErr.Load(); errPtr != nil { - return nil, *errPtr - } - for _, v := range mversions { - if v.Path() == mainModuleVersion.Path() { - return nil, fmt.Errorf("cannot update version of main module") - } - } - return mversions, nil -} - func readModuleFile(ctx context.Context, fsys fs.FS, modRoot string) (module.Version, *modfile.File, error) { modFilePath := path.Join(modRoot, "cue.mod/module.cue") data, err := fs.ReadFile(fsys, modFilePath) diff --git a/internal/mod/modload/load_test.go b/internal/mod/modload/tidy_test.go similarity index 77% rename from internal/mod/modload/load_test.go rename to internal/mod/modload/tidy_test.go index 673869f26..8d4a317f1 100644 --- a/internal/mod/modload/load_test.go +++ b/internal/mod/modload/tidy_test.go @@ -55,39 +55,6 @@ func TestTidy(t *testing.T) { } } -func TestUpdateVersions(t *testing.T) { - files, err := filepath.Glob("testdata/updateversions/*.txtar") - qt.Assert(t, qt.IsNil(err)) - for _, f := range files { - t.Run(f, func(t *testing.T) { - ar, err := txtar.ParseFile(f) - qt.Assert(t, qt.IsNil(err)) - tfs := txtarfs.FS(ar) - reg := newRegistry(t, tfs, "_registry") - - want, err := fs.ReadFile(tfs, "want") - qt.Assert(t, qt.IsNil(err)) - - versionsData, _ := fs.ReadFile(tfs, "versions") - versions := strings.Fields(string(versionsData)) - - var out strings.Builder - mf, err := UpdateVersions(context.Background(), tfs, ".", reg, versions) - if err != nil { - fmt.Fprintf(&out, "error: %v\n", err) - } else { - data, err := mf.Format() - qt.Assert(t, qt.IsNil(err)) - out.Write(data) - } - if diff := cmp.Diff(string(want), out.String()); diff != "" { - t.Log("actual result:\n", out.String()) - t.Fatalf("unexpected results (-want +got):\n%s", diff) - } - }) - } -} - func newRegistry(t *testing.T, fsys fs.FS, root string) Registry { fsys, err := fs.Sub(fsys, "_registry") qt.Assert(t, qt.IsNil(err)) diff --git a/internal/mod/modload/update.go b/internal/mod/modload/update.go new file mode 100644 index 000000000..cede6439a --- /dev/null +++ b/internal/mod/modload/update.go @@ -0,0 +1,175 @@ +package modload + +import ( + "context" + "fmt" + "io/fs" + "runtime" + "strings" + "sync/atomic" + + "cuelang.org/go/internal/mod/modrequirements" + "cuelang.org/go/internal/mod/semver" + "cuelang.org/go/internal/par" + "cuelang.org/go/mod/modfile" + "cuelang.org/go/mod/module" +) + +// UpdateVersions returns the main module's module file with the specified module versions +// updated if possible and added if not already present. It returns an error if asked +// to downgrade a module below a version already required by an external dependency. +// +// A module in the versions slice can be specified as one of the following: +// - $module@$fullVersion: a specific exact version +// - $module@$partialVersion: a non-canonical version +// specifies the latest version that has the same major/minor numbers. +// - $module@latest: the latest non-prerelease version, or latest prerelease version if +// there is no non-prerelease version +// - $module: equivalent to $module@latest if $module doesn't have a default major +// version or $module@$majorVersion if it does, where $majorVersion is the +// default major version for $module. +func UpdateVersions(ctx context.Context, fsys fs.FS, modRoot string, reg Registry, versions []string) (*modfile.File, error) { + mainModuleVersion, mf, err := readModuleFile(ctx, fsys, modRoot) + if err != nil { + return nil, err + } + rs := modrequirements.NewRequirements(mf.Module, reg, mf.DepVersions(), mf.DefaultMajorVersions()) + mversions, err := resolveUpdateVersions(ctx, reg, rs, mainModuleVersion, versions) + if err != nil { + return nil, err + } + // Now we know what versions we want to update to, make a new set of + // requirements with these versions in place. + + mversionsMap := make(map[string]module.Version) + for _, v := range mversions { + // Check existing membership of the map: if the same module has been specified + // twice, then choose t + if v1, ok := mversionsMap[v.Path()]; ok && v1.Version() != v.Version() { + // The same module has been specified twice with different requirements. + // Treat it as an error (an alternative approach might be to choose the greater + // version, but making it an error seems more appropriate to the "choose exact + // version" semantics of UpdateVersions. + return nil, fmt.Errorf("conflicting version update requirements %v vs %v", v1, v) + } + mversionsMap[v.Path()] = v + } + g, err := rs.Graph(ctx) + if err != nil { + return nil, fmt.Errorf("cannot determine module graph: %v", err) + } + var newVersions []module.Version + for _, v := range g.BuildList() { + if v.Path() == mainModuleVersion.Path() { + continue + } + if newv, ok := mversionsMap[v.Path()]; ok { + newVersions = append(newVersions, newv) + delete(mversionsMap, v.Path()) + } else { + newVersions = append(newVersions, v) + } + } + for _, v := range mversionsMap { + newVersions = append(newVersions, v) + } + rs = modrequirements.NewRequirements(mf.Module, reg, newVersions, mf.DefaultMajorVersions()) + g, err = rs.Graph(ctx) + if err != nil { + return nil, fmt.Errorf("cannot determine new module graph: %v", err) + } + // Now check that the resulting versions are the ones we wanted. + for _, v := range mversions { + actualVers := g.Selected(v.Path()) + if actualVers != v.Version() { + return nil, fmt.Errorf("other requirements prevent changing module %v to version %v (actual selected version: %v)", v.Path(), v.Version(), actualVers) + } + } + // Make a new requirements with the selected versions of the above as roots. + var finalVersions []module.Version + for _, v := range g.BuildList() { + if v.Path() != mainModuleVersion.Path() { + finalVersions = append(finalVersions, v) + } + } + rs = modrequirements.NewRequirements(mf.Module, reg, finalVersions, mf.DefaultMajorVersions()) + return modfileFromRequirements(mf, rs, ""), nil +} + +// resolveUpdateVersions resolves a set of version strings as accepted by [UpdateVersions] +// into the actual module versions they represent. +func resolveUpdateVersions(ctx context.Context, reg Registry, rs *modrequirements.Requirements, mainModuleVersion module.Version, versions []string) ([]module.Version, error) { + work := par.NewQueue(runtime.GOMAXPROCS(0)) + mversions := make([]module.Version, len(versions)) + var queryErr atomic.Pointer[error] + setError := func(err error) { + queryErr.CompareAndSwap(nil, &err) + } + for i, v := range versions { + i, v := i, v + if mv, err := module.ParseVersion(v); err == nil { + // It's already canonical: nothing more to do. + mversions[i] = mv + continue + } + mpath, vers, ok := strings.Cut(v, "@") + if !ok { + if major, status := rs.DefaultMajorVersion(mpath); status == modrequirements.ExplicitDefault { + // TODO allow a non-explicit default too? + vers = major + } else { + vers = "latest" + } + } + if err := module.CheckPathWithoutVersion(mpath); err != nil { + return nil, fmt.Errorf("invalid module path in %q", v) + } + versionPrefix := "" + if vers != "latest" { + if !semver.IsValid(vers) { + return nil, fmt.Errorf("%q does not specify a valid semantic version", v) + } + if semver.Build(vers) != "" { + return nil, fmt.Errorf("build version suffixes not supported (%v)", v) + } + // It's a valid version but has no build suffix and it's not canonical, + // which means it must be either a major-only or major-minor, so + // the conforming canonical versions must have it as a prefix, with + // a dot separating the last component and the next. + versionPrefix = vers + "." + } + work.Add(func() { + allVersions, err := reg.ModuleVersions(ctx, mpath) + if err != nil { + setError(err) + return + } + possibleVersions := make([]string, 0, len(allVersions)) + for _, v := range allVersions { + if strings.HasPrefix(v, versionPrefix) { + possibleVersions = append(possibleVersions, v) + } + } + chosen := latestVersion(possibleVersions) + mv, err := module.NewVersion(mpath, chosen) + if err != nil { + // Should never happen, because we've checked that + // mpath is valid and ModuleVersions + // should always return valid semver versions. + setError(err) + return + } + mversions[i] = mv + }) + } + <-work.Idle() + if errPtr := queryErr.Load(); errPtr != nil { + return nil, *errPtr + } + for _, v := range mversions { + if v.Path() == mainModuleVersion.Path() { + return nil, fmt.Errorf("cannot update version of main module") + } + } + return mversions, nil +} diff --git a/internal/mod/modload/update_test.go b/internal/mod/modload/update_test.go new file mode 100644 index 000000000..ba8a3282a --- /dev/null +++ b/internal/mod/modload/update_test.go @@ -0,0 +1,49 @@ +package modload + +import ( + "context" + "fmt" + "io/fs" + "path/filepath" + "strings" + "testing" + + "github.com/go-quicktest/qt" + "github.com/google/go-cmp/cmp" + "golang.org/x/tools/txtar" + + "cuelang.org/go/internal/txtarfs" +) + +func TestUpdateVersions(t *testing.T) { + files, err := filepath.Glob("testdata/updateversions/*.txtar") + qt.Assert(t, qt.IsNil(err)) + for _, f := range files { + t.Run(f, func(t *testing.T) { + ar, err := txtar.ParseFile(f) + qt.Assert(t, qt.IsNil(err)) + tfs := txtarfs.FS(ar) + reg := newRegistry(t, tfs, "_registry") + + want, err := fs.ReadFile(tfs, "want") + qt.Assert(t, qt.IsNil(err)) + + versionsData, _ := fs.ReadFile(tfs, "versions") + versions := strings.Fields(string(versionsData)) + + var out strings.Builder + mf, err := UpdateVersions(context.Background(), tfs, ".", reg, versions) + if err != nil { + fmt.Fprintf(&out, "error: %v\n", err) + } else { + data, err := mf.Format() + qt.Assert(t, qt.IsNil(err)) + out.Write(data) + } + if diff := cmp.Diff(string(want), out.String()); diff != "" { + t.Log("actual result:\n", out.String()) + t.Fatalf("unexpected results (-want +got):\n%s", diff) + } + }) + } +}