From e1443fb27537c940f220a83c52617150c7702493 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:10:45 -0700 Subject: [PATCH 01/16] Update changeset structure to allow iterating through all changes in one loop --- internal/runbits/cves/cves.go | 10 ++-- .../runbits/dependencies/changesummary.go | 14 +++-- pkg/buildplan/artifact.go | 55 +++++++++++++++---- pkg/buildplan/buildplan.go | 27 ++++----- 4 files changed, 72 insertions(+), 34 deletions(-) diff --git a/internal/runbits/cves/cves.go b/internal/runbits/cves/cves.go index 4d7bb0d787..d6970c5514 100644 --- a/internal/runbits/cves/cves.go +++ b/internal/runbits/cves/cves.go @@ -47,8 +47,8 @@ func (c *CveReport) Report(newBuildPlan *buildplan.BuildPlan, oldBuildPlan *buil } var ingredients []*request.Ingredient - for _, artifact := range changeset.Added { - for _, ing := range artifact.Ingredients { + for _, change := range changeset.Filter(buildplan.ArtifactAdded) { + for _, ing := range change.Artifact.Ingredients { ingredients = append(ingredients, &request.Ingredient{ Namespace: ing.Namespace, Name: ing.Name, @@ -57,12 +57,12 @@ func (c *CveReport) Report(newBuildPlan *buildplan.BuildPlan, oldBuildPlan *buil } } - for _, change := range changeset.Updated { + for _, change := range changeset.Filter(buildplan.ArtifactUpdated) { if !change.VersionsChanged() { continue // For CVE reporting we only care about ingredient changes } - for _, ing := range change.To.Ingredients { + for _, ing := range change.Artifact.Ingredients { ingredients = append(ingredients, &request.Ingredient{ Namespace: ing.Namespace, Name: ing.Name, @@ -118,7 +118,7 @@ func (c *CveReport) shouldSkipReporting(changeset buildplan.ArtifactChangeset) b return true } - return len(changeset.Added) == 0 && len(changeset.Updated) == 0 + return len(changeset.Filter(buildplan.ArtifactAdded, buildplan.ArtifactUpdated)) == 0 } func (c *CveReport) shouldPromptForSecurity(vulnerabilities model.VulnerableIngredientsByLevels) bool { diff --git a/internal/runbits/dependencies/changesummary.go b/internal/runbits/dependencies/changesummary.go index 4d4c74c755..1e6a3f8630 100644 --- a/internal/runbits/dependencies/changesummary.go +++ b/internal/runbits/dependencies/changesummary.go @@ -28,7 +28,8 @@ func OutputChangeSummary(out output.Outputer, newBuildPlan *buildplan.BuildPlan, dependencies := buildplan.Ingredients{} directDependencies := buildplan.Ingredients{} changeset := newBuildPlan.DiffArtifacts(oldBuildPlan, false) - for _, a := range changeset.Added { + for _, change := range changeset.Filter(buildplan.ArtifactAdded) { + a := change.Artifact if _, exists := requested[a.ArtifactID]; !exists { continue } @@ -43,16 +44,17 @@ func OutputChangeSummary(out output.Outputer, newBuildPlan *buildplan.BuildPlan, } // Check for any direct dependencies added by a requested package update. - for _, u := range changeset.Updated { - if _, exists := requested[u.To.ArtifactID]; !exists { + for _, change := range changeset.Filter(buildplan.ArtifactUpdated) { + if _, exists := requested[change.Artifact.ArtifactID]; !exists { continue } - for _, dep := range u.To.RuntimeDependencies(false) { - for _, a := range changeset.Added { + for _, dep := range change.Artifact.RuntimeDependencies(false) { + for _, subchange := range changeset.Filter(buildplan.ArtifactAdded) { + a := subchange.Artifact if a.ArtifactID != dep.ArtifactID { continue } - v := fmt.Sprintf("%s@%s", u.To.Name(), u.To.Version()) // updated/requested package, not added package + v := fmt.Sprintf("%s@%s", change.Artifact.Name(), change.Artifact.Version()) // updated/requested package, not added package addedString = append(addedLocale, v) addedLocale = append(addedLocale, fmt.Sprintf("[ACTIONABLE]%s[/RESET]", v)) diff --git a/pkg/buildplan/artifact.go b/pkg/buildplan/artifact.go index 212f9da773..01123ddebd 100644 --- a/pkg/buildplan/artifact.go +++ b/pkg/buildplan/artifact.go @@ -133,25 +133,60 @@ func (a Artifacts) ToNameMap() ArtifactNameMap { return result } -type ArtifactChangeset struct { - Added []*Artifact - Removed []*Artifact - Updated []ArtifactUpdate +type ChangeType int + +const ( + ArtifactAdded ChangeType = iota + ArtifactRemoved + ArtifactUpdated +) + +func (c ChangeType) String() string { + switch c { + case ArtifactAdded: + return "added" + case ArtifactRemoved: + return "removed" + case ArtifactUpdated: + return "updated" + } + + return "unknown" +} + +type ArtifactChange struct { + ChangeType ChangeType + Artifact *Artifact + Old *Artifact // Old is only set when ChangeType=ArtifactUpdated } -type ArtifactUpdate struct { - From *Artifact - To *Artifact +type ArtifactChangeset []ArtifactChange + +func (a ArtifactChangeset) Filter(t ...ChangeType) ArtifactChangeset { + lookup := make(map[ChangeType]struct{}, len(t)) + for _, t := range t { + lookup[t] = struct{}{} + } + result := ArtifactChangeset{} + for _, ac := range a { + if _, ok := lookup[ac.ChangeType]; ok { + result = append(result, ac) + } + } + return result } -func (a ArtifactUpdate) VersionsChanged() bool { +func (a ArtifactChange) VersionsChanged() bool { + if a.Old == nil { + return false + } fromVersions := []string{} - for _, ing := range a.From.Ingredients { + for _, ing := range a.Old.Ingredients { fromVersions = append(fromVersions, ing.Version) } sort.Strings(fromVersions) toVersions := []string{} - for _, ing := range a.To.Ingredients { + for _, ing := range a.Artifact.Ingredients { toVersions = append(toVersions, ing.Version) } sort.Strings(toVersions) diff --git a/pkg/buildplan/buildplan.go b/pkg/buildplan/buildplan.go index e985b47b1a..70480f9443 100644 --- a/pkg/buildplan/buildplan.go +++ b/pkg/buildplan/buildplan.go @@ -99,38 +99,39 @@ func (b *BuildPlan) DiffArtifacts(oldBp *BuildPlan, requestedOnly bool) Artifact } } - var updated []ArtifactUpdate - var added []*Artifact + changeset := ArtifactChangeset{} for name, artf := range new { if artfOld, notNew := old[name]; notNew { // The artifact name exists in both the old and new recipe, maybe it was updated though if artfOld.ArtifactID == artf.ArtifactID { continue } - updated = append(updated, ArtifactUpdate{ - From: artfOld, - To: artf, + changeset = append(changeset, ArtifactChange{ + ChangeType: ArtifactUpdated, + Artifact: artf, + Old: artfOld, }) } else { // If it's not an update it is a new artifact - added = append(added, artf) + changeset = append(changeset, ArtifactChange{ + ChangeType: ArtifactAdded, + Artifact: artf, + }) } } - var removed []*Artifact for name, artf := range old { if _, noDiff := new[name]; noDiff { continue } - removed = append(removed, artf) + changeset = append(changeset, ArtifactChange{ + ChangeType: ArtifactRemoved, + Artifact: artf, + }) } - return ArtifactChangeset{ - Added: added, - Removed: removed, - Updated: updated, - } + return changeset } func (b *BuildPlan) Engine() types.BuildEngine { From 18373263511eb9e48e381b45e5a7d7f4a716e99a Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:11:00 -0700 Subject: [PATCH 02/16] Expose org namespace prefix --- pkg/platform/model/vcs.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/platform/model/vcs.go b/pkg/platform/model/vcs.go index 8fd6626d0a..845b576a95 100644 --- a/pkg/platform/model/vcs.go +++ b/pkg/platform/model/vcs.go @@ -204,10 +204,12 @@ func NewNamespacePlatform() Namespace { return Namespace{NamespacePlatform, "platform"} } +const OrgNamespacePrefix = "private/" + func NewOrgNamespace(orgName string) Namespace { return Namespace{ nsType: NamespaceOrg, - value: fmt.Sprintf("private/%s", orgName), + value: OrgNamespacePrefix + orgName, } } From fa46b663f4b3424051d42f0fb9c142b6837416c5 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:11:12 -0700 Subject: [PATCH 03/16] Added buildscript.Clone() --- pkg/buildscript/buildscript.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/buildscript/buildscript.go b/pkg/buildscript/buildscript.go index 0ee5ca1037..159c15b1ff 100644 --- a/pkg/buildscript/buildscript.go +++ b/pkg/buildscript/buildscript.go @@ -37,3 +37,16 @@ func (b *BuildScript) Equals(other *BuildScript) (bool, error) { } return string(myBytes) == string(otherBytes), nil } + +func (b *BuildScript) Clone() (*BuildScript, error) { + m, err := b.Marshal() + if err != nil { + return nil, errs.Wrap(err, "unable to marshal this buildscript") + } + + u, err := Unmarshal(m) + if err != nil { + return nil, errs.Wrap(err, "unable to unmarshal buildscript") + } + return u, nil +} From e60d55e88456bed872ed80fef4869cafc91fbcb9 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:15:59 -0700 Subject: [PATCH 04/16] Added ability to ignore certain deps --- .../runbits/dependencies/changesummary.go | 2 +- pkg/buildplan/artifact.go | 37 +++++++++++-------- pkg/buildplan/artifact_test.go | 2 +- pkg/runtime/setup.go | 2 +- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/internal/runbits/dependencies/changesummary.go b/internal/runbits/dependencies/changesummary.go index 1e6a3f8630..a676b50c99 100644 --- a/internal/runbits/dependencies/changesummary.go +++ b/internal/runbits/dependencies/changesummary.go @@ -48,7 +48,7 @@ func OutputChangeSummary(out output.Outputer, newBuildPlan *buildplan.BuildPlan, if _, exists := requested[change.Artifact.ArtifactID]; !exists { continue } - for _, dep := range change.Artifact.RuntimeDependencies(false) { + for _, dep := range change.Artifact.RuntimeDependencies(false, nil) { for _, subchange := range changeset.Filter(buildplan.ArtifactAdded) { a := subchange.Artifact if a.ArtifactID != dep.ArtifactID { diff --git a/pkg/buildplan/artifact.go b/pkg/buildplan/artifact.go index 01123ddebd..fc4a0a6621 100644 --- a/pkg/buildplan/artifact.go +++ b/pkg/buildplan/artifact.go @@ -194,43 +194,46 @@ func (a ArtifactChange) VersionsChanged() bool { return !reflect.DeepEqual(fromVersions, toVersions) } -func (as Artifacts) RuntimeDependencies(recursive bool) Artifacts { - seen := make(map[strfmt.UUID]struct{}) +func (as Artifacts) RuntimeDependencies(recursive bool, ignore *map[strfmt.UUID]struct{}) Artifacts { dependencies := Artifacts{} for _, a := range as { - dependencies = append(dependencies, a.dependencies(recursive, seen, RuntimeRelation)...) + dependencies = append(dependencies, a.dependencies(recursive, ignore, RuntimeRelation)...) } return dependencies } -func (a *Artifact) RuntimeDependencies(recursive bool) Artifacts { - return a.dependencies(recursive, make(map[strfmt.UUID]struct{}), RuntimeRelation) +func (a *Artifact) RuntimeDependencies(recursive bool, ignore *map[strfmt.UUID]struct{}) Artifacts { + return a.dependencies(recursive, ignore, RuntimeRelation) } // Dependencies returns ALL dependencies that an artifact has, this covers runtime and build time dependencies. // It does not cover test dependencies as we have no use for them in the state tool. -func (as Artifacts) Dependencies(recursive bool) Artifacts { - seen := make(map[strfmt.UUID]struct{}) +func (as Artifacts) Dependencies(recursive bool, ignore *map[strfmt.UUID]struct{}) Artifacts { dependencies := Artifacts{} for _, a := range as { - dependencies = append(dependencies, a.dependencies(recursive, seen, RuntimeRelation, BuildtimeRelation)...) + dependencies = append(dependencies, a.dependencies(recursive, ignore, RuntimeRelation, BuildtimeRelation)...) } return dependencies } // Dependencies returns ALL dependencies that an artifact has, this covers runtime and build time dependencies. // It does not cover test dependencies as we have no use for them in the state tool. -func (a *Artifact) Dependencies(recursive bool) Artifacts { - return a.dependencies(recursive, make(map[strfmt.UUID]struct{}), RuntimeRelation, BuildtimeRelation) +func (a *Artifact) Dependencies(recursive bool, ignore *map[strfmt.UUID]struct{}) Artifacts { + return a.dependencies(recursive, ignore, RuntimeRelation, BuildtimeRelation) } -func (a *Artifact) dependencies(recursive bool, seen map[strfmt.UUID]struct{}, relations ...Relation) Artifacts { +func (a *Artifact) dependencies(recursive bool, maybeIgnore *map[strfmt.UUID]struct{}, relations ...Relation) Artifacts { + ignore := map[strfmt.UUID]struct{}{} + if maybeIgnore != nil { + ignore = *maybeIgnore + } + // Guard against recursion, this shouldn't really be possible but we don't know how the buildplan might evolve // so better safe than sorry. - if _, ok := seen[a.ArtifactID]; ok { + if _, ok := ignore[a.ArtifactID]; ok { return Artifacts{} } - seen[a.ArtifactID] = struct{}{} + ignore[a.ArtifactID] = struct{}{} dependencies := Artifacts{} for _, ac := range a.children { @@ -244,9 +247,11 @@ func (a *Artifact) dependencies(recursive bool, seen map[strfmt.UUID]struct{}, r continue } - dependencies = append(dependencies, ac.Artifact) - if recursive { - dependencies = append(dependencies, ac.Artifact.dependencies(recursive, seen, relations...)...) + if _, ok := ignore[ac.Artifact.ArtifactID]; !ok { + dependencies = append(dependencies, ac.Artifact) + if recursive { + dependencies = append(dependencies, ac.Artifact.dependencies(recursive, &ignore, relations...)...) + } } } return dependencies diff --git a/pkg/buildplan/artifact_test.go b/pkg/buildplan/artifact_test.go index a6493ee8da..fa26fff3e7 100644 --- a/pkg/buildplan/artifact_test.go +++ b/pkg/buildplan/artifact_test.go @@ -61,7 +61,7 @@ func TestArtifact_Dependencies(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { a := tt.artifact - deps := a.Dependencies(tt.recursive) + deps := a.Dependencies(tt.recursive, nil) got := make([]string, len(deps)) for i, dep := range deps { got[i] = dep.ArtifactID.String() diff --git a/pkg/runtime/setup.go b/pkg/runtime/setup.go index 33dbdca0a6..9ff5022e3c 100644 --- a/pkg/runtime/setup.go +++ b/pkg/runtime/setup.go @@ -143,7 +143,7 @@ func newSetup(path string, bp *buildplan.BuildPlan, env *envdef.Collection, depo // Now that we know which artifacts we'll need to download we can use this as our basis for calculating which artifacts // still need to be build. This encompasses the artifacts themselves, as well as any of their dependencies. And of // course we only want to filter artifacts that actually require a build, as the build may be cached server side. - artifactsToBuild := append(artifactsToDownload, artifactsToDownload.Dependencies(true)...).Filter(buildplan.FilterNotBuild()) + artifactsToBuild := append(artifactsToDownload, artifactsToDownload.Dependencies(true, nil)...).Filter(buildplan.FilterNotBuild()) artifactsToBuild = sliceutils.UniqueByProperty(artifactsToBuild, func(a *buildplan.Artifact) any { return a.ArtifactID }) // Check for cached build failures From d71eedc703a782a101b67d69ad779c2bac484a90 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:16:24 -0700 Subject: [PATCH 05/16] Added shortcuts for accessing revision and license info from artifact --- pkg/buildplan/artifact.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/pkg/buildplan/artifact.go b/pkg/buildplan/artifact.go index fc4a0a6621..b793c66f03 100644 --- a/pkg/buildplan/artifact.go +++ b/pkg/buildplan/artifact.go @@ -63,6 +63,26 @@ func (a *Artifact) Version() string { return "" } +// Revision returns the name of the ingredient for this artifact, if it only has exactly one ingredient associated. +// Otherwise it returns an empty version. +func (a *Artifact) Revision() int { + if len(a.Ingredients) == 1 { + return a.Ingredients[0].Revision + } + return -1 +} + +func (a *Artifact) Licenses() []string { + result := []string{} + if len(a.Ingredients) == 0 { + return result + } + for _, ing := range a.Ingredients { + result = append(result, ing.Licenses...) + } + return result +} + func (a *Artifact) NameAndVersion() string { version := a.Version() if version == "" { From fc2c65486b7de5ddbae6d8669ae94e04739003c4 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:16:47 -0700 Subject: [PATCH 06/16] Added slice comparison function that doesn't depend on ordering --- internal/sliceutils/sliceutils.go | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/internal/sliceutils/sliceutils.go b/internal/sliceutils/sliceutils.go index 511e5774a6..e31ff293d8 100644 --- a/internal/sliceutils/sliceutils.go +++ b/internal/sliceutils/sliceutils.go @@ -1,6 +1,8 @@ package sliceutils import ( + "cmp" + "github.com/ActiveState/cli/internal/errs" "github.com/go-openapi/strfmt" "golang.org/x/text/unicode/norm" @@ -112,3 +114,23 @@ func ToLookupMapByKey[T any, K string | int | strfmt.UUID](data []T, keyCb func( } return result } + +// EqualValues checks if two slices have equal values, regardless of ordering. This does not recurse into nested slices or structs. +func EqualValues[S ~[]E, E cmp.Ordered](a, b S) bool { + if len(a) != len(b) { + return false + } + + lookup := make(map[E]struct{}, len(a)) + for _, e := range a { + lookup[e] = struct{}{} + } + + for _, e := range b { + if _, ok := lookup[e]; !ok { + return false + } + } + + return true +} From 6d38a4d1f34b580c45ed9fa09223e73ab6138aa9 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:18:17 -0700 Subject: [PATCH 07/16] Added BOLD colorize tag --- internal/colorize/colorize.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/internal/colorize/colorize.go b/internal/colorize/colorize.go index 024d39ffca..9fdd9b06c5 100644 --- a/internal/colorize/colorize.go +++ b/internal/colorize/colorize.go @@ -18,7 +18,7 @@ func init() { defer profile.Measure("colorize:init", time.Now()) var err error colorRx, err = regexp.Compile( - `\[(HEADING|NOTICE|SUCCESS|ERROR|WARNING|DISABLED|ACTIONABLE|CYAN|GREEN|RED|ORANGE|YELLOW|MAGENTA|/RESET)!?\]`, + `\[(HEADING|BOLD|NOTICE|SUCCESS|ERROR|WARNING|DISABLED|ACTIONABLE|CYAN|GREEN|RED|ORANGE|YELLOW|MAGENTA|/RESET)!?\]`, ) if err != nil { panic(fmt.Sprintf("Could not compile regex: %v", err)) @@ -27,6 +27,7 @@ func init() { type ColorTheme interface { Heading(writer io.Writer) + Bold(writer io.Writer) Notice(writer io.Writer) Success(writer io.Writer) Error(writer io.Writer) @@ -51,6 +52,11 @@ func (dct defaultColorTheme) Heading(writer io.Writer) { c.SetStyle(colorstyle.Bold, false) } +func (dct defaultColorTheme) Bold(writer io.Writer) { + c := colorstyle.New(writer) + c.SetStyle(colorstyle.Bold, false) +} + // Notice switches to bright foreground func (dct defaultColorTheme) Notice(writer io.Writer) { colorstyle.New(writer).SetStyle(colorstyle.Default, true) From 1514bde1f4840f2a6c596c4705eb8cfa74e704ad Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:18:50 -0700 Subject: [PATCH 08/16] Added output.Structured() --- internal/output/presets.go | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/internal/output/presets.go b/internal/output/presets.go index 3fa98e7693..6b26c2bc46 100644 --- a/internal/output/presets.go +++ b/internal/output/presets.go @@ -32,19 +32,32 @@ func (h Emphasize) MarshalStructured(f Format) interface{} { return Suppress } -type preparedOutput struct { - plain interface{} +type plainOutput struct { + plain interface{} +} + +type structuredOutput struct { structured interface{} } -func (o *preparedOutput) MarshalOutput(_ Format) interface{} { +type preparedOutput struct { + *plainOutput + *structuredOutput +} + +func (o *plainOutput) MarshalOutput(_ Format) interface{} { return o.plain } -func (o *preparedOutput) MarshalStructured(_ Format) interface{} { +func (o *structuredOutput) MarshalStructured(_ Format) interface{} { return o.structured } func Prepare(plain interface{}, structured interface{}) *preparedOutput { - return &preparedOutput{plain, structured} + return &preparedOutput{&plainOutput{plain}, &structuredOutput{structured}} +} + +func Structured(structured interface{}) *structuredOutput { + return &structuredOutput{structured} } + From a6b056ee961d42b06447fa423aae4419c736ab19 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:24:44 -0700 Subject: [PATCH 09/16] Centralized tree symbols --- internal/output/presets.go | 3 +++ internal/runbits/dependencies/changesummary.go | 4 ++-- internal/runbits/dependencies/summary.go | 4 ++-- internal/runners/artifacts/artifacts.go | 8 ++++---- internal/runners/branch/tree.go | 7 ++++--- internal/runners/cve/cve.go | 4 ++-- internal/runners/manifest/requirements.go | 2 +- internal/runners/projects/projects.go | 6 +++--- internal/runners/show/show.go | 2 +- 9 files changed, 22 insertions(+), 18 deletions(-) diff --git a/internal/output/presets.go b/internal/output/presets.go index 6b26c2bc46..1033d44246 100644 --- a/internal/output/presets.go +++ b/internal/output/presets.go @@ -61,3 +61,6 @@ func Structured(structured interface{}) *structuredOutput { return &structuredOutput{structured} } +const TreeMid = "├─" +const TreeLink = "│" +const TreeEnd = "└─" diff --git a/internal/runbits/dependencies/changesummary.go b/internal/runbits/dependencies/changesummary.go index a676b50c99..1d7e7c3cd9 100644 --- a/internal/runbits/dependencies/changesummary.go +++ b/internal/runbits/dependencies/changesummary.go @@ -106,9 +106,9 @@ func OutputChangeSummary(out output.Outputer, newBuildPlan *buildplan.BuildPlan, // depending on whether or not it has subdependencies, and whether or not showUpdatedPackages is // `true`. for i, ingredient := range directDependencies { - prefix := "├─" + prefix := output.TreeMid if i == len(directDependencies)-1 { - prefix = "└─" + prefix = output.TreeEnd } // Retrieve runtime dependencies, and then filter out any dependencies that are common between all added ingredients. diff --git a/internal/runbits/dependencies/summary.go b/internal/runbits/dependencies/summary.go index 751f64dd28..61883d61f2 100644 --- a/internal/runbits/dependencies/summary.go +++ b/internal/runbits/dependencies/summary.go @@ -27,9 +27,9 @@ func OutputSummary(out output.Outputer, directDependencies buildplan.Artifacts) out.Notice(locale.Tl("setting_up_dependencies", " Setting up the following dependencies:")) for i, ingredient := range ingredients { - prefix := " ├─" + prefix := " " + output.TreeMid if i == len(ingredients)-1 { - prefix = " └─" + prefix = " " + output.TreeEnd } subdependencies := "" diff --git a/internal/runners/artifacts/artifacts.go b/internal/runners/artifacts/artifacts.go index 81ed37bbed..06fb06b513 100644 --- a/internal/runners/artifacts/artifacts.go +++ b/internal/runners/artifacts/artifacts.go @@ -203,8 +203,8 @@ func (b *Artifacts) outputPlain(out *StructuredOutput, fullID bool) error { switch { case len(artifact.Errors) > 0: b.out.Print(fmt.Sprintf(" • %s ([ERROR]%s[/RESET])", artifact.Name, locale.T("artifact_status_failed"))) - b.out.Print(fmt.Sprintf(" ├─ %s: [ERROR]%s[/RESET]", locale.T("artifact_status_failed_message"), strings.Join(artifact.Errors, ": "))) - b.out.Print(fmt.Sprintf(" └─ %s: [ACTIONABLE]%s[/RESET]", locale.T("artifact_status_failed_log"), artifact.LogURL)) + b.out.Print(fmt.Sprintf(" %s %s: [ERROR]%s[/RESET]", output.TreeMid, locale.T("artifact_status_failed_message"), strings.Join(artifact.Errors, ": "))) + b.out.Print(fmt.Sprintf(" %s %s: [ACTIONABLE]%s[/RESET]", output.TreeEnd, locale.T("artifact_status_failed_log"), artifact.LogURL)) continue case artifact.status == types.ArtifactSkipped: b.out.Print(fmt.Sprintf(" • %s ([NOTICE]%s[/RESET])", artifact.Name, locale.T("artifact_status_skipped"))) @@ -227,8 +227,8 @@ func (b *Artifacts) outputPlain(out *StructuredOutput, fullID bool) error { switch { case len(artifact.Errors) > 0: b.out.Print(fmt.Sprintf(" • %s ([ERROR]%s[/RESET])", artifact.Name, locale.T("artifact_status_failed"))) - b.out.Print(fmt.Sprintf(" ├─ %s: [ERROR]%s[/RESET]", locale.T("artifact_status_failed_message"), strings.Join(artifact.Errors, ": "))) - b.out.Print(fmt.Sprintf(" └─ %s: [ACTIONABLE]%s[/RESET]", locale.T("artifact_status_failed_log"), artifact.LogURL)) + b.out.Print(fmt.Sprintf(" %s %s: [ERROR]%s[/RESET]", output.TreeMid, locale.T("artifact_status_failed_message"), strings.Join(artifact.Errors, ": "))) + b.out.Print(fmt.Sprintf(" %s %s: [ACTIONABLE]%s[/RESET]", output.TreeEnd, locale.T("artifact_status_failed_log"), artifact.LogURL)) continue case artifact.status == types.ArtifactSkipped: b.out.Print(fmt.Sprintf(" • %s ([NOTICE]%s[/RESET])", artifact.Name, locale.T("artifact_status_skipped"))) diff --git a/internal/runners/branch/tree.go b/internal/runners/branch/tree.go index 7da0ab0c59..611401bccd 100644 --- a/internal/runners/branch/tree.go +++ b/internal/runners/branch/tree.go @@ -5,6 +5,7 @@ import ( "sort" "strings" + "github.com/ActiveState/cli/internal/output" "github.com/ActiveState/cli/pkg/platform/api/mono/mono_models" "github.com/ActiveState/cli/pkg/platform/model" ) @@ -17,9 +18,9 @@ type branchNode struct { type tree map[branchNode]tree const ( - prefixLink string = "│" - prefixMid string = "├─" - prefixEnd string = "└─" + prefixLink string = output.TreeLink + prefixMid string = output.TreeMid + prefixEnd string = output.TreeEnd branchFormatting string = "[NOTICE]%s[/RESET]" localBranchFormatting string = "[ACTIONABLE]%s[/RESET] [DISABLED](Current)[/RESET]" diff --git a/internal/runners/cve/cve.go b/internal/runners/cve/cve.go index b9c74d9179..20f6e51116 100644 --- a/internal/runners/cve/cve.go +++ b/internal/runners/cve/cve.go @@ -194,9 +194,9 @@ func (rd *cveOutput) MarshalOutput(format output.Format) interface{} { }) for i, d := range ap.Details { - bar := "├─" + bar := output.TreeMid if i == len(ap.Details)-1 { - bar = "└─" + bar = output.TreeEnd } severity := d.Severity if severity == "CRITICAL" { diff --git a/internal/runners/manifest/requirements.go b/internal/runners/manifest/requirements.go index 3329cb1a72..e93b719bdc 100644 --- a/internal/runners/manifest/requirements.go +++ b/internal/runners/manifest/requirements.go @@ -70,7 +70,7 @@ func (o requirements) Print(out output.Outputer) { } if req.Namespace != "" { - requirementOutput.Namespace = locale.Tl("manifest_namespace", " └─ [DISABLED]namespace:[/RESET] [CYAN]{{.V0}}[/RESET]", req.Namespace) + requirementOutput.Namespace = locale.Tr("namespace_row", output.TreeEnd, req.Namespace) } requirementsOutput = append(requirementsOutput, requirementOutput) diff --git a/internal/runners/projects/projects.go b/internal/runners/projects/projects.go index 5f0c7ac521..91357549c3 100644 --- a/internal/runners/projects/projects.go +++ b/internal/runners/projects/projects.go @@ -62,16 +62,16 @@ func (o *projectsOutput) MarshalOutput(f output.Format) interface{} { } execDir := v.Executables[i] if execDir != "" { - checkouts = append(checkouts, locale.Tl("projects_local_checkout_exec", " ├─ Local Checkout → {{.V0}}", checkout)) + checkouts = append(checkouts, locale.Tl("projects_local_checkout_exec", " {{.V0}} Local Checkout → {{.V1}}", output.TreeMid, checkout)) if f == output.PlainFormatName { // Show executable path below checkout path for plain text output. - checkouts = append(checkouts, locale.Tl("projects_executables", " └─ Executables → {{.V0}}", execDir)) + checkouts = append(checkouts, locale.Tl("projects_executables", " {{.V0}} Executables → {{.V1}}", output.TreeEnd, execDir)) } else { // Show executables in a separate table. executables = append(executables, execDir) } } else { - checkouts = append(checkouts, locale.Tl("projects_local_checkout", " └─ Local Checkout → {{.V0}}", checkout)) + checkouts = append(checkouts, locale.Tl("projects_local_checkout", " {{.V0}} Local Checkout → {{.V1}}", output.TreeEnd, checkout)) } } r = append(r, projectOutputPlain{v.Name, v.Organization, strings.Join(checkouts, "\n"), strings.Join(executables, "\n")}) diff --git a/internal/runners/show/show.go b/internal/runners/show/show.go index 604f30c74a..2c5387d561 100644 --- a/internal/runners/show/show.go +++ b/internal/runners/show/show.go @@ -78,7 +78,7 @@ func formatScripts(scripts map[string]string) string { for k, v := range scripts { res = append(res, fmt.Sprintf("• %s", k)) if v != "" { - res = append(res, fmt.Sprintf(" └─ %s", v)) + res = append(res, fmt.Sprintf(" %s %s", output.TreeEnd, v)) } } return strings.Join(res, "\n") From c755cc52b75317fd096b1fd25b7caf301f25e850 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 11:24:56 -0700 Subject: [PATCH 10/16] Added `state upgrade` --- cmd/state/internal/cmdtree/cmdtree.go | 1 + cmd/state/internal/cmdtree/upgrade.go | 38 +++ internal/locale/locale.go | 9 + internal/locale/locales/en-us.yaml | 30 +++ internal/runners/upgrade/upgrade.go | 343 ++++++++++++++++++++++++++ 5 files changed, 421 insertions(+) create mode 100644 cmd/state/internal/cmdtree/upgrade.go create mode 100644 internal/runners/upgrade/upgrade.go diff --git a/cmd/state/internal/cmdtree/cmdtree.go b/cmd/state/internal/cmdtree/cmdtree.go index cfc4dd6b71..ce3991b304 100644 --- a/cmd/state/internal/cmdtree/cmdtree.go +++ b/cmd/state/internal/cmdtree/cmdtree.go @@ -214,6 +214,7 @@ func New(prime *primer.Values, args ...string) *CmdTree { newEvalCommand(prime), newManifestCommmand(prime), artifactsCmd, + newUpgradeCommand(prime), ) return &CmdTree{ diff --git a/cmd/state/internal/cmdtree/upgrade.go b/cmd/state/internal/cmdtree/upgrade.go new file mode 100644 index 0000000000..05636d2cba --- /dev/null +++ b/cmd/state/internal/cmdtree/upgrade.go @@ -0,0 +1,38 @@ +package cmdtree + +import ( + "github.com/ActiveState/cli/internal/captain" + "github.com/ActiveState/cli/internal/locale" + "github.com/ActiveState/cli/internal/primer" + "github.com/ActiveState/cli/internal/runners/upgrade" +) + +func newUpgradeCommand(prime *primer.Values) *captain.Command { + runner := upgrade.New(prime) + + params := upgrade.NewParams() + + cmd := captain.NewCommand( + "upgrade", + locale.Tl("upgrade_cmd_title", "Upgrading Project"), + locale.Tl("upgrade_cmd_description", "Upgrade dependencies of a project"), + prime, + []*captain.Flag{ + { + Name: "expand", + Description: locale.T("flag_state_upgrade_expand_description"), + Value: ¶ms.Expand, + }, + }, + []*captain.Argument{}, + func(_ *captain.Command, _ []string) error { + return runner.Run(params) + }, + ) + + cmd.SetGroup(PackagesGroup) + cmd.SetSupportsStructuredOutput() + cmd.SetUnstable(true) + + return cmd +} diff --git a/internal/locale/locale.go b/internal/locale/locale.go index b6f4565741..17db70f89f 100644 --- a/internal/locale/locale.go +++ b/internal/locale/locale.go @@ -138,6 +138,15 @@ func T(translationID string, args ...interface{}) string { return translateFunction(translationID, args...) } +// Ts aliases to T, but accepts a list of translationIDs to be translated +func Ts(translationIDs ...string) []string { + result := []string{} + for _, id := range translationIDs { + result = append(result, T(id)) + } + return result +} + // Tr is like T but it accepts string params that will be used as numbered params, eg. V0, V1, V2 etc func Tr(translationID string, values ...string) string { return T(translationID, parseInput(values...)) diff --git a/internal/locale/locales/en-us.yaml b/internal/locale/locales/en-us.yaml index ac0f5deeab..2c9ec195bc 100644 --- a/internal/locale/locales/en-us.yaml +++ b/internal/locale/locales/en-us.yaml @@ -1549,3 +1549,33 @@ warn_additional_requirements: [WARNING]WARNING:[/RESET] This project has additional build criteria that cannot be managed through the State Tool. Please edit your [ACTIONABLE]buildscript.as[/RESET] file manually to modify these: migrate_project_error: other: Could not migrate your project files. Please address any errors and try again. For full detail view your log file with '[ACTIONABLE]state export log -i 0[/RESET]'. +upgrade_solving: + other: • Searching for available upgrades for your requirements +upgrade_confirm: + other: Would you like to install these upgrades? +upgrade_no_changes: + other: No upgrades were found for your project. Note that requirements with fixed version numbers will never be upgraded. +upgrade_aborted: + other: Upgrade aborted +upgrade_success: + other: Upgrade completed +upgrade_field_same: + other: "[CYAN]{{.V0}}[/RESET]" +upgrade_field_change: + other: "[CYAN]{{.V0}}[/RESET] > [BOLD][ACTIONABLE]{{.V1}}[/RESET]" +name: + other: Name +version: + other: Version +license: + other: License +vulnerabilities: + other: Vulnerabilities (CVEs) +dependency_row: + other: " [DISABLED]{{.V0}}[/RESET] [CYAN]{{.V1}}[/RESET] [DISABLED]transitive dependencies touched[/RESET]" +dependency_detail_row: + other: " [DISABLED]{{.V0}} {{.V1}}[/RESET] {{.V2}}" +namespace_row: + other: " [DISABLED]{{.V0}} namespace:[/RESET] [CYAN]{{.V1}}[/RESET]" +flag_state_upgrade_expand_description: + other: Show individual transitive dependency changes rather than a summary diff --git a/internal/runners/upgrade/upgrade.go b/internal/runners/upgrade/upgrade.go new file mode 100644 index 0000000000..55906ab017 --- /dev/null +++ b/internal/runners/upgrade/upgrade.go @@ -0,0 +1,343 @@ +package upgrade + +import ( + "errors" + "fmt" + "strconv" + "strings" + + "github.com/ActiveState/cli/internal/constants" + "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/internal/locale" + "github.com/ActiveState/cli/internal/output" + "github.com/ActiveState/cli/internal/primer" + "github.com/ActiveState/cli/internal/rtutils/ptr" + "github.com/ActiveState/cli/internal/runbits/rationalize" + "github.com/ActiveState/cli/internal/sliceutils" + "github.com/ActiveState/cli/internal/table" + "github.com/ActiveState/cli/pkg/buildplan" + "github.com/ActiveState/cli/pkg/localcommit" + "github.com/ActiveState/cli/pkg/platform/api/buildplanner/response" + "github.com/ActiveState/cli/pkg/platform/api/buildplanner/types" + "github.com/ActiveState/cli/pkg/platform/model" + bpModel "github.com/ActiveState/cli/pkg/platform/model/buildplanner" + "github.com/go-openapi/strfmt" +) + +type primeable interface { + primer.Outputer + primer.Auther + primer.Projecter + primer.Prompter +} + +type Params struct { + Expand bool +} + +func NewParams() *Params { + return &Params{} +} + +type Upgrade struct { + prime primeable +} + +func New(p primeable) *Upgrade { + return &Upgrade{ + prime: p, + } +} + +var ErrNoChanges = errors.New("no changes") +var ErrAbort = errors.New("aborted") + +func rationalizeError(err *error) { + switch { + case err == nil: + return + case errors.Is(*err, ErrNoChanges): + *err = errs.WrapUserFacing(*err, locale.T("upgrade_no_changes"), errs.SetInput()) + case errors.Is(*err, ErrAbort): + *err = errs.WrapUserFacing(*err, locale.T("upgrade_aborted"), errs.SetInput()) + } +} + +func (u *Upgrade) Run(params *Params) (rerr error) { + defer rationalizeError(&rerr) + + // Validate project + proj := u.prime.Project() + if proj == nil { + return rationalize.ErrNoProject + } + if proj.IsHeadless() { + return rationalize.ErrHeadless + } + + out := u.prime.Output() + out.Notice(locale.Tr("operating_message", proj.NamespaceString(), proj.Dir())) + + // Collect buildplans for before/after upgrade + pg := output.StartSpinner(out, locale.T("upgrade_solving"), constants.TerminalAnimationInterval) + defer func() { + if pg != nil { + pg.Stop(locale.T("progress_fail")) + } + }() + + // Collect "before" buildplan + localCommitID, err := localcommit.Get(proj.Dir()) + if err != nil { + return errs.Wrap(err, "Failed to get local commit") + } + + bpm := bpModel.NewBuildPlannerModel(u.prime.Auth()) + localCommit, err := bpm.FetchCommit(localCommitID, proj.Owner(), proj.Name(), nil) + if err != nil { + return errs.Wrap(err, "Failed to fetch build result") + } + + // Collect "after" buildplan + bumpedBS, err := localCommit.BuildScript().Clone() + if err != nil { + return errs.Wrap(err, "Failed to clone build script") + } + ts, err := model.FetchLatestTimeStamp(u.prime.Auth()) + if err != nil { + return errs.Wrap(err, "Failed to fetch latest timestamp") + } + bumpedBS.SetAtTime(ts) + + if bumpedBS.AtTime().String() == localCommit.BuildScript().AtTime().String() { + panic(fmt.Sprintf("bumped buildscript is same as local commit, old: %s, new: %s", bumpedBS.AtTime(), localCommit.BuildScript().AtTime())) + } + + // Since our platform is commit based we need to create a commit for the "after" buildplan, even though we may not + // end up using it it the user doesn't confirm the upgrade. + bumpedCommit, err := bpm.StageCommit(bpModel.StageCommitParams{ + Owner: proj.Owner(), + Project: proj.Name(), + ParentCommit: localCommitID.String(), + Script: bumpedBS, + }) + if err != nil { + // The buildplanner itself can assert that there are no new changes, in which case we don't want to handle + // this as an error + var commitErr *response.CommitError + if errors.As(err, &commitErr) { + if commitErr.Type == types.NoChangeSinceLastCommitErrorType { + pg.Stop(locale.T("progress_success")) + pg = nil + return ErrNoChanges + } + } + return errs.Wrap(err, "Failed to stage bumped commit") + } + bumpedBP := bumpedCommit.BuildPlan() + + // All done collecting buildplans + pg.Stop(locale.T("progress_success")) + pg = nil + + changeset := bumpedBP.DiffArtifacts(localCommit.BuildPlan(), false) + if len(changeset.Filter(buildplan.ArtifactUpdated)) == 0 { + // In most cases we would've already reached this error due to the commit failing. But it is possible for + // the commit to be created (ie. there were changes), but without those changes being relevant to any artifacts + // that we care about. + return ErrNoChanges + } + + changes := u.calculateChanges(changeset, bumpedCommit) + if out.Type().IsStructured() { + out.Print(output.Structured(changes)) + } else { + if err := u.renderUserFacing(changes, params.Expand); err != nil { + return errs.Wrap(err, "Failed to render user facing upgrade") + } + } + + if err := localcommit.Set(u.prime.Project().Dir(), bumpedCommit.CommitID.String()); err != nil { + return errs.Wrap(err, "Failed to set local commit") + } + + out.Notice(locale.Tr("upgrade_success")) + + return nil +} + +type structuredChange struct { + Type string `json:"type"` + Name string `json:"name"` + Namespace string `json:"namespace,omitempty"` + OldVersion string `json:"old_version,omitempty"` + NewVersion string `json:"new_version"` + OldRevision int `json:"old_revision"` + NewRevision int `json:"new_revision"` + OldLicenses []string `json:"old_licenses,omitempty"` + NewLicenses []string `json:"new_licenses"` + TransitiveDeps []structuredChange `json:"transitive_dependencies,omitempty"` +} + +func (u *Upgrade) calculateChanges(changedArtifacts buildplan.ArtifactChangeset, bumpedCommit *bpModel.Commit) []structuredChange { + requested := bumpedCommit.BuildPlan().RequestedArtifacts().ToIDMap() + + relevantChanges := changedArtifacts.Filter(buildplan.ArtifactUpdated) + relevantRequestedArtifacts := buildplan.Artifacts{} + + // Calculate relevant artifacts ahead of time, as we'll need them to calculate transitive dependencies + // (we want to avoid recursing into the same artifact multiple times) + for _, change := range relevantChanges { + if _, ok := requested[change.Artifact.ArtifactID]; !ok { + continue + } + relevantRequestedArtifacts = append(relevantRequestedArtifacts, change.Artifact) + } + + changes := []structuredChange{} + for _, artifactUpdate := range changedArtifacts.Filter(buildplan.ArtifactUpdated) { + if _, ok := requested[artifactUpdate.Artifact.ArtifactID]; !ok { + continue + } + + change := structuredChange{ + Type: artifactUpdate.ChangeType.String(), + Name: artifactUpdate.Artifact.Name(), + OldVersion: artifactUpdate.Old.Version(), + NewVersion: artifactUpdate.Artifact.Version(), + OldRevision: artifactUpdate.Old.Revision(), + NewRevision: artifactUpdate.Artifact.Revision(), + OldLicenses: artifactUpdate.Old.Licenses(), + NewLicenses: artifactUpdate.Artifact.Licenses(), + } + + if len(artifactUpdate.Artifact.Ingredients) == 1 { + change.Namespace = artifactUpdate.Artifact.Ingredients[0].Namespace + } + + changedDeps := calculateChangedDeps(artifactUpdate.Artifact, relevantRequestedArtifacts, changedArtifacts) + if len(changedDeps) > 0 { + change.TransitiveDeps = make([]structuredChange, len(changedDeps)) + for n, changedDep := range changedDeps { + change.TransitiveDeps[n] = structuredChange{ + Type: changedDep.ChangeType.String(), + Name: changedDep.Artifact.Name(), + NewVersion: changedDep.Artifact.Version(), + NewRevision: changedDep.Artifact.Revision(), + NewLicenses: changedDep.Artifact.Licenses(), + } + if changedDep.Old != nil { + change.TransitiveDeps[n].OldVersion = changedDep.Old.Version() + change.TransitiveDeps[n].OldRevision = changedDep.Old.Revision() + change.TransitiveDeps[n].OldLicenses = changedDep.Old.Licenses() + } + } + } + + changes = append(changes, change) + } + + return changes +} + +func (u *Upgrade) renderUserFacing(changes []structuredChange, expand bool) error { + out := u.prime.Output() + + out.Notice("") // Empty line + + tbl := table.New(locale.Ts("name", "version", "license")) + tbl.HideDash = true + for _, change := range changes { + tbl.AddRow([]string{ + change.Name, + renderVersionChange(change), + renderLicenseChange(change), + }) + + needsDepRow := len(change.TransitiveDeps) > 0 + needsNamespaceRow := strings.HasPrefix(change.Namespace, model.OrgNamespacePrefix) + + if needsNamespaceRow { + treeSymbol := output.TreeEnd + if needsDepRow { + treeSymbol = output.TreeMid + } + tbl.AddRow([]string{locale.Tr("namespace_row", treeSymbol, change.Namespace)}) + } + + if needsDepRow { + if expand { + for n, changedDep := range change.TransitiveDeps { + treeSymbol := output.TreeEnd + if n != len(change.TransitiveDeps)-1 { + treeSymbol = output.TreeMid + } + tbl.AddRow([]string{locale.Tr("dependency_detail_row", treeSymbol, changedDep.Name, renderVersionChange(changedDep))}) + } + } else { + tbl.AddRow([]string{locale.Tr("dependency_row", output.TreeEnd, strconv.Itoa(len(change.TransitiveDeps)))}) + } + } + } + + out.Print(tbl.Render()) + + out.Notice(" ") // Empty line (prompts use Notice) + confirm, err := u.prime.Prompt().Confirm("", locale.Tr("upgrade_confirm"), ptr.To(true)) + if err != nil { + return errs.Wrap(err, "confirmation failed") + } + if !confirm { + return ErrAbort + } + + return nil +} + +func calculateChangedDeps(artifact *buildplan.Artifact, dontCount buildplan.Artifacts, changeset buildplan.ArtifactChangeset) buildplan.ArtifactChangeset { + ignore := map[strfmt.UUID]struct{}{} + for _, skip := range dontCount { + if skip.ArtifactID == artifact.ArtifactID { + continue // Don't ignore the current artifact, or we won't get dependencies for it + } + ignore[skip.ArtifactID] = struct{}{} + } + + result := buildplan.ArtifactChangeset{} + + deps := artifact.Dependencies(true, &ignore) + for _, change := range changeset.Filter(buildplan.ArtifactAdded, buildplan.ArtifactUpdated) { + for _, dep := range deps { + if dep.ArtifactID == change.Artifact.ArtifactID { + result = append(result, change) + } + } + } + + return sliceutils.Unique(result) +} + +func renderVersionChange(change structuredChange) string { + if change.OldVersion == "" { + return locale.Tr("upgrade_field_same", change.NewVersion) + } + old := change.OldVersion + new := change.NewVersion + if change.OldVersion == change.NewVersion { + old = fmt.Sprintf("%s (%d)", old, change.OldRevision) + new = fmt.Sprintf("%s (%d)", new, change.NewRevision) + } + if old == new { + return locale.Tr("upgrade_field_same", change.NewVersion) + } + return locale.Tr("upgrade_field_change", change.OldVersion, change.NewVersion) +} + +func renderLicenseChange(change structuredChange) string { + from := change.OldLicenses + to := change.NewLicenses + if sliceutils.EqualValues(from, to) { + return locale.Tr("upgrade_field_same", strings.Join(from, ", ")) + } + return locale.Tr("upgrade_field_change", strings.Join(from, ", "), strings.Join(to, ", ")) +} From dfac51e96a8a521d39618a5d4d7aaca5c7060172 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 14:33:05 -0700 Subject: [PATCH 11/16] Make time expansion reusable between commands --- internal/runbits/commits_runbit/time.go | 65 +++++++++++++++++++++++++ internal/runners/packages/info.go | 5 +- internal/runners/packages/install.go | 5 +- internal/runners/packages/search.go | 7 +-- internal/runners/packages/time.go | 51 ------------------- internal/runners/packages/uninstall.go | 5 +- 6 files changed, 78 insertions(+), 60 deletions(-) create mode 100644 internal/runbits/commits_runbit/time.go delete mode 100644 internal/runners/packages/time.go diff --git a/internal/runbits/commits_runbit/time.go b/internal/runbits/commits_runbit/time.go new file mode 100644 index 0000000000..478cfc544d --- /dev/null +++ b/internal/runbits/commits_runbit/time.go @@ -0,0 +1,65 @@ +package commits_runbit + +import ( + "time" + + "github.com/ActiveState/cli/internal/captain" + "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/pkg/localcommit" + "github.com/ActiveState/cli/pkg/platform/authentication" + "github.com/ActiveState/cli/pkg/platform/model" + "github.com/ActiveState/cli/pkg/project" +) + +// ExpandTime returns a timestamp based on the given "--ts" value. +// If the timestamp was already defined, we just return it. +// If "now" was given, returns "now" according to the platform. +// Otherwise, returns the specified timestamp or nil (which falls back on the default Platform +// timestamp for a given operation) +func ExpandTime(ts *captain.TimeValue, auth *authentication.Auth) (time.Time, error) { + if ts.Time != nil { + return *ts.Time, nil + } + + if ts.Now() { + latest, err := model.FetchLatestRevisionTimeStamp(auth) + if err != nil { + return time.Time{}, errs.Wrap(err, "Unable to determine latest revision time") + } + return latest, nil + } + + latest, err := model.FetchLatestTimeStamp(auth) + if err != nil { + return time.Time{}, errs.Wrap(err, "Unable to fetch latest Platform timestamp") + } + + return latest, nil +} + +// ExpandTimeForProject is the same as ExpandTime except that it ensures the returned time is either the same or +// later than that of the most recent commit. +func ExpandTimeForProject(ts *captain.TimeValue, auth *authentication.Auth, proj *project.Project) (time.Time, error) { + timestamp, err := ExpandTime(ts, auth) + if err != nil { + return time.Time{}, errs.Wrap(err, "Unable to expand time") + } + + if proj != nil { + commitID, err := localcommit.Get(proj.Dir()) + if err != nil { + return time.Time{}, errs.Wrap(err, "Unable to get commit ID") + } + + atTime, err := model.FetchTimeStampForCommit(commitID, auth) + if err != nil { + return time.Time{}, errs.Wrap(err, "Unable to get commit time") + } + + if atTime.After(timestamp) { + return *atTime, nil + } + } + + return timestamp, nil +} diff --git a/internal/runners/packages/info.go b/internal/runners/packages/info.go index f81a4f63e1..fb559b8ecc 100644 --- a/internal/runners/packages/info.go +++ b/internal/runners/packages/info.go @@ -12,6 +12,7 @@ import ( "github.com/ActiveState/cli/internal/multilog" "github.com/ActiveState/cli/internal/output" "github.com/ActiveState/cli/internal/rtutils/ptr" + "github.com/ActiveState/cli/internal/runbits/commits_runbit" "github.com/ActiveState/cli/pkg/platform/api/inventory/inventory_models" "github.com/ActiveState/cli/pkg/platform/api/vulnerabilities/request" "github.com/ActiveState/cli/pkg/platform/authentication" @@ -70,12 +71,12 @@ func (i *Info) Run(params InfoRunParams, nstype model.NamespaceType) error { normalized = params.Package.Name } - ts, err := getTime(¶ms.Timestamp, i.auth, i.proj) + ts, err := commits_runbit.ExpandTimeForProject(¶ms.Timestamp, i.auth, i.proj) if err != nil { return errs.Wrap(err, "Unable to get timestamp from params") } - packages, err := model.SearchIngredientsStrict(ns.String(), normalized, false, false, ts, i.auth) // ideally case-sensitive would be true (PB-4371) + packages, err := model.SearchIngredientsStrict(ns.String(), normalized, false, false, &ts, i.auth) // ideally case-sensitive would be true (PB-4371) if err != nil { return locale.WrapError(err, "package_err_cannot_obtain_search_results") } diff --git a/internal/runners/packages/install.go b/internal/runners/packages/install.go index 16622378d3..30193f4cf8 100644 --- a/internal/runners/packages/install.go +++ b/internal/runners/packages/install.go @@ -5,6 +5,7 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/rtutils/ptr" + "github.com/ActiveState/cli/internal/runbits/commits_runbit" "github.com/ActiveState/cli/internal/runbits/runtime/requirements" "github.com/ActiveState/cli/pkg/platform/api/buildplanner/types" "github.com/ActiveState/cli/pkg/platform/model" @@ -51,10 +52,10 @@ func (a *Install) Run(params InstallRunParams, nsType model.NamespaceType) (rerr reqs = append(reqs, req) } - ts, err := getTime(¶ms.Timestamp, a.prime.Auth(), a.prime.Project()) + ts, err := commits_runbit.ExpandTimeForProject(¶ms.Timestamp, a.prime.Auth(), a.prime.Project()) if err != nil { return errs.Wrap(err, "Unable to get timestamp from params") } - return requirements.NewRequirementOperation(a.prime).ExecuteRequirementOperation(ts, reqs...) + return requirements.NewRequirementOperation(a.prime).ExecuteRequirementOperation(&ts, reqs...) } diff --git a/internal/runners/packages/search.go b/internal/runners/packages/search.go index ce077506a2..ed6ad2e4f0 100644 --- a/internal/runners/packages/search.go +++ b/internal/runners/packages/search.go @@ -8,6 +8,7 @@ import ( "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/output" + "github.com/ActiveState/cli/internal/runbits/commits_runbit" "github.com/ActiveState/cli/pkg/localcommit" "github.com/ActiveState/cli/pkg/platform/api/vulnerabilities/request" "github.com/ActiveState/cli/pkg/platform/authentication" @@ -58,16 +59,16 @@ func (s *Search) Run(params SearchRunParams, nstype model.NamespaceType) error { ns = model.NewRawNamespace(params.Ingredient.Namespace) } - ts, err := getTime(¶ms.Timestamp, s.auth, s.proj) + ts, err := commits_runbit.ExpandTimeForProject(¶ms.Timestamp, s.auth, s.proj) if err != nil { return errs.Wrap(err, "Unable to get timestamp from params") } var packages []*model.IngredientAndVersion if params.ExactTerm { - packages, err = model.SearchIngredientsLatestStrict(ns.String(), params.Ingredient.Name, true, true, ts, s.auth) + packages, err = model.SearchIngredientsLatestStrict(ns.String(), params.Ingredient.Name, true, true, &ts, s.auth) } else { - packages, err = model.SearchIngredientsLatest(ns.String(), params.Ingredient.Name, true, ts, s.auth) + packages, err = model.SearchIngredientsLatest(ns.String(), params.Ingredient.Name, true, &ts, s.auth) } if err != nil { return locale.WrapError(err, "package_err_cannot_obtain_search_results") diff --git a/internal/runners/packages/time.go b/internal/runners/packages/time.go deleted file mode 100644 index 65923fda24..0000000000 --- a/internal/runners/packages/time.go +++ /dev/null @@ -1,51 +0,0 @@ -package packages - -import ( - "time" - - "github.com/ActiveState/cli/internal/captain" - "github.com/ActiveState/cli/internal/errs" - "github.com/ActiveState/cli/pkg/localcommit" - "github.com/ActiveState/cli/pkg/platform/authentication" - "github.com/ActiveState/cli/pkg/platform/model" - "github.com/ActiveState/cli/pkg/project" -) - -// getTime returns a timestamp based on the given "--ts" value. -// If "now" was given, returns "now" according to the platform. -// If no timestamp was given but the current project's commit time is after the latest inventory -// timestamp, returns that commit time. -// Otherwise, returns the specified timestamp or nil (which falls back on the default Platform -// timestamp for a given operation) -func getTime(ts *captain.TimeValue, auth *authentication.Auth, proj *project.Project) (*time.Time, error) { - if ts.Now() { - latest, err := model.FetchLatestRevisionTimeStamp(auth) - if err != nil { - return nil, errs.Wrap(err, "Unable to determine latest revision time") - } - return &latest, nil - } - - if ts.Time == nil && proj != nil { - latest, err := model.FetchLatestTimeStamp(auth) - if err != nil { - return nil, errs.Wrap(err, "Unable to fetch latest Platform timestamp") - } - - commitID, err := localcommit.Get(proj.Dir()) - if err != nil { - return nil, errs.Wrap(err, "Unable to get commit ID") - } - - atTime, err := model.FetchTimeStampForCommit(commitID, auth) - if err != nil { - return nil, errs.Wrap(err, "Unable to get commit time") - } - - if atTime.After(latest) { - return atTime, nil - } - } - - return ts.Time, nil -} diff --git a/internal/runners/packages/uninstall.go b/internal/runners/packages/uninstall.go index 3156c01d97..a9008261dd 100644 --- a/internal/runners/packages/uninstall.go +++ b/internal/runners/packages/uninstall.go @@ -5,6 +5,7 @@ import ( "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/internal/rtutils/ptr" + "github.com/ActiveState/cli/internal/runbits/commits_runbit" "github.com/ActiveState/cli/internal/runbits/rationalize" "github.com/ActiveState/cli/internal/runbits/runtime/requirements" "github.com/ActiveState/cli/pkg/platform/api/buildplanner/types" @@ -50,10 +51,10 @@ func (u *Uninstall) Run(params UninstallRunParams, nsType model.NamespaceType) ( reqs = append(reqs, req) } - ts, err := getTime(&captain.TimeValue{}, u.prime.Auth(), u.prime.Project()) + ts, err := commits_runbit.ExpandTimeForProject(&captain.TimeValue{}, u.prime.Auth(), u.prime.Project()) if err != nil { return errs.Wrap(err, "Unable to get timestamp from params") } - return requirements.NewRequirementOperation(u.prime).ExecuteRequirementOperation(ts, reqs...) + return requirements.NewRequirementOperation(u.prime).ExecuteRequirementOperation(&ts, reqs...) } From 14216bb9adfa678e798c2770160601ecb40d4ea6 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 14:33:49 -0700 Subject: [PATCH 12/16] Add `--ts` flag to `state upgrade` --- cmd/state/internal/cmdtree/upgrade.go | 5 +++++ internal/locale/locales/en-us.yaml | 2 ++ internal/runners/upgrade/upgrade.go | 7 +++++-- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/cmd/state/internal/cmdtree/upgrade.go b/cmd/state/internal/cmdtree/upgrade.go index 05636d2cba..18019f5f3b 100644 --- a/cmd/state/internal/cmdtree/upgrade.go +++ b/cmd/state/internal/cmdtree/upgrade.go @@ -18,6 +18,11 @@ func newUpgradeCommand(prime *primer.Values) *captain.Command { locale.Tl("upgrade_cmd_description", "Upgrade dependencies of a project"), prime, []*captain.Flag{ + { + Name: "ts", + Description: locale.T("flag_state_upgrade_ts_description"), + Value: ¶ms.Timestamp, + }, { Name: "expand", Description: locale.T("flag_state_upgrade_expand_description"), diff --git a/internal/locale/locales/en-us.yaml b/internal/locale/locales/en-us.yaml index 2c9ec195bc..62aa856235 100644 --- a/internal/locale/locales/en-us.yaml +++ b/internal/locale/locales/en-us.yaml @@ -1579,3 +1579,5 @@ namespace_row: other: " [DISABLED]{{.V0}} namespace:[/RESET] [CYAN]{{.V1}}[/RESET]" flag_state_upgrade_expand_description: other: Show individual transitive dependency changes rather than a summary +flag_state_upgrade_ts_description: + other: Manually specify the timestamp to 'upgrade' to. Can be either 'now' or RFC3339 formatted timestamp. diff --git a/internal/runners/upgrade/upgrade.go b/internal/runners/upgrade/upgrade.go index 55906ab017..a716823480 100644 --- a/internal/runners/upgrade/upgrade.go +++ b/internal/runners/upgrade/upgrade.go @@ -6,12 +6,14 @@ import ( "strconv" "strings" + "github.com/ActiveState/cli/internal/captain" "github.com/ActiveState/cli/internal/constants" "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/output" "github.com/ActiveState/cli/internal/primer" "github.com/ActiveState/cli/internal/rtutils/ptr" + "github.com/ActiveState/cli/internal/runbits/commits_runbit" "github.com/ActiveState/cli/internal/runbits/rationalize" "github.com/ActiveState/cli/internal/sliceutils" "github.com/ActiveState/cli/internal/table" @@ -32,7 +34,8 @@ type primeable interface { } type Params struct { - Expand bool + Timestamp captain.TimeValue + Expand bool } func NewParams() *Params { @@ -103,7 +106,7 @@ func (u *Upgrade) Run(params *Params) (rerr error) { if err != nil { return errs.Wrap(err, "Failed to clone build script") } - ts, err := model.FetchLatestTimeStamp(u.prime.Auth()) + ts, err := commits_runbit.ExpandTime(¶ms.Timestamp, u.prime.Auth()) if err != nil { return errs.Wrap(err, "Failed to fetch latest timestamp") } From b9b71b3792aaf3435a2df71d4366b03cbae24e83 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 14:34:02 -0700 Subject: [PATCH 13/16] Added integration test for `state upgrade` --- internal/testhelpers/tagsuite/tagsuite.go | 1 + test/integration/upgrade_int_test.go | 70 +++++++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 test/integration/upgrade_int_test.go diff --git a/internal/testhelpers/tagsuite/tagsuite.go b/internal/testhelpers/tagsuite/tagsuite.go index cbaa75737a..ce3a5d6968 100644 --- a/internal/testhelpers/tagsuite/tagsuite.go +++ b/internal/testhelpers/tagsuite/tagsuite.go @@ -80,6 +80,7 @@ const ( Show = "show" Switch = "switch" Uninstall = "uninstall" + Upgrade = "upgrade" Update = "update" Use = "use" ) diff --git a/test/integration/upgrade_int_test.go b/test/integration/upgrade_int_test.go new file mode 100644 index 0000000000..27b0eb66be --- /dev/null +++ b/test/integration/upgrade_int_test.go @@ -0,0 +1,70 @@ +package integration + +import ( + "testing" + + "github.com/ActiveState/cli/internal/testhelpers/e2e" + "github.com/ActiveState/cli/internal/testhelpers/suite" + "github.com/ActiveState/cli/internal/testhelpers/tagsuite" + "github.com/ActiveState/termtest" +) + +type UpgradeIntegrationTestSuite struct { + tagsuite.Suite +} + +func (suite *UpgradeIntegrationTestSuite) TestUpgrade() { + suite.OnlyRunForTags(tagsuite.Upgrade) + + ts := e2e.New(suite.T(), false) + defer ts.Close() + + ts.PrepareProject("ActiveState-CLI-Testing/python-upgradeable", "d0e56f96-b956-4b0c-9c20-938d3e843ab9") + + commitBefore := ts.CommitID() + + // Normally you wouldn't specify the timestamp except for special use-cases, but tests work better if they're + // reproducible, not to mention faster if they can hit the cache. + time := "2024-08-23T18:35:55.818Z" + + cp := ts.Spawn("upgrade", "--ts", time) + cp.Expect("transitive dependencies touched") + cp.Expect("install these upgrades?") + cp.SendLine("y") + cp.Expect("Upgrade completed") + cp.ExpectExitCode(0) + + // The ordering of these is not guaranteed, so we get a bit creative here + snapshot := cp.Snapshot() + suite.Contains(snapshot, "pytest") + suite.Contains(snapshot, "requests") + suite.Contains(snapshot, "7.2.2 > 8.3.2") // old pytest version + suite.Contains(snapshot, "2.28.2 > 2.32.3") // old requests version + + suite.NotEqual(commitBefore, ts.CommitID()) +} + +func (suite *UpgradeIntegrationTestSuite) TestUpgradeJSON() { + suite.OnlyRunForTags(tagsuite.Upgrade) + + ts := e2e.New(suite.T(), false) + defer ts.Close() + + ts.PrepareProject("ActiveState-CLI-Testing/python-upgradeable", "d0e56f96-b956-4b0c-9c20-938d3e843ab9") + + commitBefore := ts.CommitID() + + cp := ts.SpawnWithOpts( + e2e.OptArgs("upgrade", "--output=json"), + e2e.OptTermTest(termtest.OptRows(500)), // Ensure json fits inside snapshot + ) + cp.ExpectExitCode(0) + + AssertValidJSON(suite.T(), cp) + + suite.NotEqual(commitBefore, ts.CommitID()) +} + +func TestUpgradeIntegrationTestSuite(t *testing.T) { + suite.Run(t, new(UpgradeIntegrationTestSuite)) +} From e70fae80ce01619091f8ad1d3778c869ecc065d4 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 28 Aug 2024 14:49:15 -0700 Subject: [PATCH 14/16] Fix assertion; this ID should never have shown up as a dependency --- pkg/buildplan/artifact_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/buildplan/artifact_test.go b/pkg/buildplan/artifact_test.go index fa26fff3e7..88ba3d707f 100644 --- a/pkg/buildplan/artifact_test.go +++ b/pkg/buildplan/artifact_test.go @@ -54,7 +54,6 @@ func TestArtifact_Dependencies(t *testing.T) { want: []string{ "00000000-0000-0000-0000-000000000002", "00000000-0000-0000-0000-000000000003", - "00000000-0000-0000-0000-000000000001", }, }, } From 66efe82484095a3ef9b0e8fd6a6a0067ef3b2a3b Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Thu, 29 Aug 2024 13:17:16 -0700 Subject: [PATCH 15/16] Remove debugging statement --- internal/runners/upgrade/upgrade.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/runners/upgrade/upgrade.go b/internal/runners/upgrade/upgrade.go index a716823480..9730b39361 100644 --- a/internal/runners/upgrade/upgrade.go +++ b/internal/runners/upgrade/upgrade.go @@ -112,10 +112,6 @@ func (u *Upgrade) Run(params *Params) (rerr error) { } bumpedBS.SetAtTime(ts) - if bumpedBS.AtTime().String() == localCommit.BuildScript().AtTime().String() { - panic(fmt.Sprintf("bumped buildscript is same as local commit, old: %s, new: %s", bumpedBS.AtTime(), localCommit.BuildScript().AtTime())) - } - // Since our platform is commit based we need to create a commit for the "after" buildplan, even though we may not // end up using it it the user doesn't confirm the upgrade. bumpedCommit, err := bpm.StageCommit(bpModel.StageCommitParams{ From b86f7a0a9a3e59c824e414eb704eea0ac5f33fb2 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Thu, 29 Aug 2024 13:26:16 -0700 Subject: [PATCH 16/16] Add unit test for sliceutils.EqualValues --- internal/sliceutils/sliceutils_test.go | 46 ++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/internal/sliceutils/sliceutils_test.go b/internal/sliceutils/sliceutils_test.go index 4989aea6f7..dbd6b5d69c 100644 --- a/internal/sliceutils/sliceutils_test.go +++ b/internal/sliceutils/sliceutils_test.go @@ -323,3 +323,49 @@ func TestToLookupMapByKey(t *testing.T) { "Key3": v3, }, lookupMap) } + +func TestEqualValues(t *testing.T) { + tests := []struct { + name string + comparison func() bool + expected bool + }{ + { + name: "Equal int slices", + comparison: func() bool { + return EqualValues([]int{1, 2, 3}, []int{3, 2, 1}) + }, + expected: true, + }, + { + name: "Unequal int slices", + comparison: func() bool { + return EqualValues([]int{1, 2, 3}, []int{4, 5, 6}) + }, + expected: false, + }, + { + name: "Equal string slices", + comparison: func() bool { + return EqualValues([]string{"a", "b", "c"}, []string{"c", "b", "a"}) + }, + expected: true, + }, + { + name: "Unequal string slices", + comparison: func() bool { + return EqualValues([]string{"a", "b", "c"}, []string{"a", "b", "d"}) + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := tt.comparison() + if result != tt.expected { + t.Errorf("%s = %v; want %v", tt.name, result, tt.expected) + } + }) + } +}