Merge pull request ActiveState#3496 from ActiveState/nathan/DX-3039

Fix checkout/init change summaries

Author: Naatan

internal/runbits/dependencies/summary.go

@@ -18,6 +18,7 @@ func OutputSummary(out output.Outputer, directDependencies buildplan.Artifacts)
 	}
 
 	ingredients := directDependencies.Filter(buildplan.FilterStateArtifacts()).Ingredients()
+	commonDependencies := ingredients.CommonRuntimeDependencies().ToIDMap()
 
 	sort.SliceStable(ingredients, func(i, j int) bool {
 		return ingredients[i].Name < ingredients[j].Name
@@ -32,12 +33,18 @@ func OutputSummary(out output.Outputer, directDependencies buildplan.Artifacts)
 			prefix = "  └─"
 		}
 
-		subdependencies := ""
-		if numSubs := len(ingredient.RuntimeDependencies(true)); numSubs > 0 {
-			subdependencies = locale.Tl("summary_subdeps", "([ACTIONABLE]{{.V0}}[/RESET] sub-dependencies)", strconv.Itoa(numSubs))
+		subDependencies := ingredient.RuntimeDependencies(true)
+		if _, isCommon := commonDependencies[ingredient.IngredientID]; !isCommon {
+			// If the ingredient is itself not a common sub-dependency; filter out any common sub dependencies so we don't
+			// report counts multiple times.
+			subDependencies = subDependencies.Filter(buildplan.FilterOutIngredients{commonDependencies}.Filter)
+		}
+		subdepLocale := ""
+		if numSubs := len(subDependencies); numSubs > 0 {
+			subdepLocale = locale.Tl("summary_subdeps", "([ACTIONABLE]{{.V0}}[/RESET] sub-dependencies)", strconv.Itoa(numSubs))
 		}
 
-		item := fmt.Sprintf("[ACTIONABLE]%s@%s[/RESET] %s", ingredient.Name, ingredient.Version, subdependencies)
+		item := fmt.Sprintf("[ACTIONABLE]%s@%s[/RESET] %s", ingredient.Name, ingredient.Version, subdepLocale)
 
 		out.Notice(fmt.Sprintf("[DISABLED]%s[/RESET] %s", prefix, item))
 	}

pkg/buildplan/artifact.go

@@ -186,7 +186,9 @@ func (as Artifacts) Dependencies(recursive bool) Artifacts {
 // 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)
+	as := a.dependencies(recursive, make(map[strfmt.UUID]struct{}), RuntimeRelation, BuildtimeRelation)
+	as = sliceutils.UniqueByProperty(as, func(a *Artifact) any { return a.ArtifactID })
+	return as
 }
 
 func (a *Artifact) dependencies(recursive bool, seen map[strfmt.UUID]struct{}, relations ...Relation) Artifacts {

pkg/buildplan/buildplan.go

@@ -2,6 +2,7 @@ package buildplan
 
 import (
 	"encoding/json"
+	"sort"
 
 	"github.com/go-openapi/strfmt"
 
@@ -30,7 +31,7 @@ func Unmarshal(data []byte) (*BuildPlan, error) {
 
 	b.raw = &rawBuild
 
-	b.cleanup()
+	b.sanitize()
 
 	if err := b.hydrate(); err != nil {
 		return nil, errs.Wrap(err, "error hydrating build plan")
@@ -45,12 +46,13 @@ func Unmarshal(data []byte) (*BuildPlan, error) {
 }
 
 func (b *BuildPlan) Marshal() ([]byte, error) {
-	return json.Marshal(b.raw)
+	return json.MarshalIndent(b.raw, "", "  ")
 }
 
-// cleanup empty targets
-// The type aliasing in the query populates the response with emtpy targets that we should remove
-func (b *BuildPlan) cleanup() {
+// sanitize will remove empty targets and sort slices to ensure consistent interpretation of the same buildplan
+// Empty targets: The type aliasing in the query populates the response with emtpy targets that we should remove
+// Sorting: The API does not do any slice ordering, meaning the same buildplan retrieved twice can use different ordering
+func (b *BuildPlan) sanitize() {
 	b.raw.Steps = sliceutils.Filter(b.raw.Steps, func(s *raw.Step) bool {
 		return s.StepID != ""
 	})
@@ -62,6 +64,27 @@ func (b *BuildPlan) cleanup() {
 	b.raw.Artifacts = sliceutils.Filter(b.raw.Artifacts, func(a *raw.Artifact) bool {
 		return a.NodeID != ""
 	})
+
+	sort.Slice(b.raw.Sources, func(i, j int) bool { return b.raw.Sources[i].NodeID < b.raw.Sources[j].NodeID })
+	sort.Slice(b.raw.Steps, func(i, j int) bool { return b.raw.Steps[i].StepID < b.raw.Steps[j].StepID })
+	sort.Slice(b.raw.Artifacts, func(i, j int) bool { return b.raw.Artifacts[i].NodeID < b.raw.Artifacts[j].NodeID })
+	sort.Slice(b.raw.Terminals, func(i, j int) bool { return b.raw.Terminals[i].Tag < b.raw.Terminals[j].Tag })
+	sort.Slice(b.raw.ResolvedRequirements, func(i, j int) bool {
+		return b.raw.ResolvedRequirements[i].Source < b.raw.ResolvedRequirements[j].Source
+	})
+	for _, t := range b.raw.Terminals {
+		sort.Slice(t.NodeIDs, func(i, j int) bool { return t.NodeIDs[i] < t.NodeIDs[j] })
+	}
+	for _, a := range b.raw.Artifacts {
+		sort.Slice(a.RuntimeDependencies, func(i, j int) bool { return a.RuntimeDependencies[i] < a.RuntimeDependencies[j] })
+	}
+	for _, step := range b.raw.Steps {
+		sort.Slice(step.Inputs, func(i, j int) bool { return step.Inputs[i].Tag < step.Inputs[j].Tag })
+		sort.Slice(step.Outputs, func(i, j int) bool { return step.Outputs[i] < step.Outputs[j] })
+		for _, input := range step.Inputs {
+			sort.Slice(input.NodeIDs, func(i, j int) bool { return input.NodeIDs[i] < input.NodeIDs[j] })
+		}
+	}
 }
 
 func (b *BuildPlan) Platforms() []strfmt.UUID {

pkg/buildplan/filters.go

@@ -76,3 +76,12 @@ func FilterNotBuild() FilterArtifact {
 		return a.Status != types.ArtifactSucceeded
 	}
 }
+
+type FilterOutIngredients struct {
+	Ingredients IngredientIDMap
+}
+
+func (f FilterOutIngredients) Filter(i *Ingredient) bool {
+	_, blacklist := f.Ingredients[i.IngredientID]
+	return !blacklist
+}

pkg/buildplan/hydrate.go

@@ -154,8 +154,6 @@ func (b *BuildPlan) hydrateWithIngredients(artifact *Artifact, platformID *strfm
 	err := b.raw.WalkViaSteps([]strfmt.UUID{artifact.ArtifactID}, raw.TagSource,
 		func(node interface{}, parent *raw.Artifact) error {
 			switch v := node.(type) {
-			case *raw.Artifact:
-				return nil // We've already got our artifacts
 			case *raw.Source:
 				// logging.Debug("Walking source '%s (%s)'", v.Name, v.NodeID)
 
@@ -193,7 +191,11 @@ func (b *BuildPlan) hydrateWithIngredients(artifact *Artifact, platformID *strfm
 
 				return nil
 			default:
-				return errs.New("unexpected node type '%T': %#v", v, v)
+				if a, ok := v.(*raw.Artifact); ok && a.NodeID == artifact.ArtifactID {
+					return nil // continue
+				}
+				// Source ingredients are only relevant when they link DIRECTLY to the artifact
+				return raw.WalkInterrupt{}
 			}
 
 			return nil
@@ -212,7 +214,7 @@ func (b *BuildPlan) sanityCheck() error {
 	// Ensure all artifacts have an associated ingredient
 	// If this fails either the API is bugged or the hydrate logic is bugged
 	for _, a := range b.Artifacts() {
-		if len(a.Ingredients) == 0 {
+		if raw.IsStateToolMimeType(a.MimeType) && len(a.Ingredients) == 0 {
 			return errs.New("artifact '%s (%s)' does not have an ingredient", a.ArtifactID, a.DisplayName)
 		}
 	}

pkg/buildplan/hydrate_test.go

@@ -0,0 +1,50 @@
+package buildplan
+
+import (
+	"testing"
+
+	"github.com/ActiveState/cli/internal/errs"
+	"github.com/ActiveState/cli/pkg/buildplan/mock"
+	"github.com/go-openapi/strfmt"
+	"github.com/stretchr/testify/require"
+)
+
+func TestBuildPlan_hydrateWithIngredients(t *testing.T) {
+	tests := []struct {
+		name           string
+		buildplan      *BuildPlan
+		inputArtifact  *Artifact
+		wantIngredient string
+	}{
+		{
+			"Ingredient solves for simple artifact > src hop",
+			&BuildPlan{raw: mock.BuildWithRuntimeDepsViaSrc},
+			&Artifact{ArtifactID: "00000000-0000-0000-0000-000000000007"},
+			"00000000-0000-0000-0000-000000000009",
+		},
+		{
+			"Installer should not resolve to an ingredient as it doesn't have a direct source",
+			&BuildPlan{raw: mock.BuildWithRuntimeDepsViaSrc},
+			&Artifact{ArtifactID: "00000000-0000-0000-0000-000000000002"},
+			"",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			b := tt.buildplan
+			if err := b.hydrateWithIngredients(tt.inputArtifact, nil, map[strfmt.UUID]*Ingredient{}); err != nil {
+				t.Fatalf("hydrateWithIngredients() error = %v", errs.JoinMessage(err))
+			}
+			if tt.wantIngredient == "" {
+				require.Empty(t, tt.inputArtifact.Ingredients)
+				return
+			}
+			if len(tt.inputArtifact.Ingredients) != 1 {
+				t.Fatalf("expected 1 ingredient resolution, got %d", len(tt.inputArtifact.Ingredients))
+			}
+			if string(tt.inputArtifact.Ingredients[0].IngredientID) != tt.wantIngredient {
+				t.Errorf("expected ingredient ID %s, got %s", tt.wantIngredient, tt.inputArtifact.Ingredients[0].IngredientID)
+			}
+		})
+	}
+}

pkg/buildplan/ingredient.go

@@ -11,7 +11,7 @@ type Ingredient struct {
 
 	IsBuildtimeDependency bool
 	IsRuntimeDependency   bool
-	Artifacts             []*Artifact
+	Artifacts             Artifacts
 
 	platforms []strfmt.UUID
 }
@@ -61,7 +61,20 @@ func (i Ingredients) ToNameMap() IngredientNameMap {
 // CommonRuntimeDependencies returns the set of runtime dependencies that are common between all ingredients.
 // For example, given a set of python ingredients this will return at the very least the python language ingredient.
 func (i Ingredients) CommonRuntimeDependencies() Ingredients {
+	var is []ingredientsWithRuntimeDeps
+	for _, ig := range i {
+		is = append(is, ig)
+	}
+	return commonRuntimeDependencies(is)
+}
+
+type ingredientsWithRuntimeDeps interface {
+	RuntimeDependencies(recursive bool) Ingredients
+}
+
+func commonRuntimeDependencies(i []ingredientsWithRuntimeDeps) Ingredients {
 	counts := map[strfmt.UUID]int{}
+	common := Ingredients{}
 
 	for _, ig := range i {
 		runtimeDeps := ig.RuntimeDependencies(true)
@@ -70,13 +83,9 @@ func (i Ingredients) CommonRuntimeDependencies() Ingredients {
 				counts[rd.IngredientID] = 0
 			}
 			counts[rd.IngredientID]++
-		}
-	}
-
-	common := Ingredients{}
-	for _, ig := range i {
-		if counts[ig.IngredientID] == len(i) {
-			common = append(common, ig)
+			if counts[rd.IngredientID] == 2 { // only append on 2; we don't want dupes
+				common = append(common, rd)
+			}
 		}
 	}
 

pkg/buildplan/ingredient_test.go

@@ -2,7 +2,10 @@ package buildplan
 
 import (
 	"reflect"
+	"sort"
 	"testing"
+
+	"github.com/ActiveState/cli/pkg/buildplan/raw"
 )
 
 func TestIngredient_RuntimeDependencies(t *testing.T) {
@@ -55,3 +58,54 @@ func TestIngredient_RuntimeDependencies(t *testing.T) {
 		})
 	}
 }
+
+type mockIngredient struct {
+	deps Ingredients
+}
+
+func (m mockIngredient) RuntimeDependencies(recursive bool) Ingredients {
+	return m.deps
+}
+
+func TestIngredients_CommonRuntimeDependencies(t *testing.T) {
+	tests := []struct {
+		name string
+		i    []ingredientsWithRuntimeDeps
+		want []string
+	}{
+		{
+			"Simple",
+			[]ingredientsWithRuntimeDeps{
+				mockIngredient{
+					deps: Ingredients{
+						{
+							IngredientSource: &raw.IngredientSource{IngredientID: "sub-ingredient-1"},
+						},
+					},
+				},
+				mockIngredient{
+					deps: Ingredients{
+						{
+							IngredientSource: &raw.IngredientSource{IngredientID: "sub-ingredient-1"},
+						},
+					},
+				},
+			},
+			[]string{"sub-ingredient-1"},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := commonRuntimeDependencies(tt.i)
+			gotIDs := []string{}
+			for _, i := range got {
+				gotIDs = append(gotIDs, string(i.IngredientID))
+			}
+			sort.Strings(gotIDs)
+			sort.Strings(tt.want)
+			if !reflect.DeepEqual(gotIDs, tt.want) {
+				t.Errorf("Ingredients.CommonRuntimeDependencies() = %v, want %v", gotIDs, tt.want)
+			}
+		})
+	}
+}