From 847e40ae6e8a726016fb4adbc15deb47047e0e33 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Tue, 24 Sep 2024 14:29:56 -0700 Subject: [PATCH 1/9] Requirements aren't guaranteed to resolve --- pkg/buildplan/hydrate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/buildplan/hydrate.go b/pkg/buildplan/hydrate.go index 216bcb7616..91e5a450f1 100644 --- a/pkg/buildplan/hydrate.go +++ b/pkg/buildplan/hydrate.go @@ -55,7 +55,7 @@ func (b *BuildPlan) hydrate() error { } ingredient, ok := ingredientLookup[source.IngredientID] if !ok { - return errs.New("missing ingredient for source ID: %s", req.Source) + continue } b.requirements = append(b.requirements, &Requirement{ Requirement: req.Requirement, From 99fd378a3bf8b03273146d8800f3fbf7889762c0 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Tue, 24 Sep 2024 14:42:27 -0700 Subject: [PATCH 2/9] Don't enforce artifact <-> ingredient relation as artifacts produced by wheels won't have an ingredient --- pkg/buildplan/hydrate.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/pkg/buildplan/hydrate.go b/pkg/buildplan/hydrate.go index 91e5a450f1..b61b5a8d5f 100644 --- a/pkg/buildplan/hydrate.go +++ b/pkg/buildplan/hydrate.go @@ -211,14 +211,6 @@ func (b *BuildPlan) hydrateWithIngredients(artifact *Artifact, platformID *strfm // If there are duplicates we're likely to see failures down the chain if live, though that's by no means guaranteed. // Surfacing it here will make it easier to reason about the failure. 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 raw.IsStateToolMimeType(a.MimeType) && len(a.Ingredients) == 0 { - return errs.New("artifact '%s (%s)' does not have an ingredient", a.ArtifactID, a.DisplayName) - } - } - // The remainder of sanity checks aren't checking for error conditions so much as they are checking for smoking guns // If these fail then it's likely the API has changed in a backward incompatible way, or we broke something. // In any case it does not necessarily mean runtime sourcing is broken. From 899fa42590dedd65638669ea75bb6a48f5772fa8 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 25 Sep 2024 10:11:14 -0700 Subject: [PATCH 3/9] Support identifying source behind python wheel --- internal/maputils/maputils.go | 7 ++++ pkg/buildplan/hydrate.go | 75 +++++++++++++++------------------ pkg/buildplan/hydrate_test.go | 19 +++++++-- pkg/buildplan/mock/mock.go | 77 ++++++++++++++++++++++++++++++++-- pkg/buildplan/raw/walk.go | 49 ++++++++++++---------- pkg/buildplan/raw/walk_test.go | 28 +++++++++---- 6 files changed, 180 insertions(+), 75 deletions(-) create mode 100644 internal/maputils/maputils.go diff --git a/internal/maputils/maputils.go b/internal/maputils/maputils.go new file mode 100644 index 0000000000..e35435bd6a --- /dev/null +++ b/internal/maputils/maputils.go @@ -0,0 +1,7 @@ +package maputils + +// Contains is a shortcut to `_, ok := map[key]`. This allows for evaluating +func Contains[T comparable, V any](m map[T]V, value T) bool { + _, ok := m[value] + return ok +} diff --git a/pkg/buildplan/hydrate.go b/pkg/buildplan/hydrate.go index b61b5a8d5f..11cc49b758 100644 --- a/pkg/buildplan/hydrate.go +++ b/pkg/buildplan/hydrate.go @@ -80,7 +80,7 @@ func (b *BuildPlan) hydrate() error { } func (b *BuildPlan) hydrateWithBuildClosure(nodeIDs []strfmt.UUID, platformID *strfmt.UUID, artifactLookup map[strfmt.UUID]*Artifact) error { - err := b.raw.WalkViaSteps(nodeIDs, raw.TagDependency, func(node interface{}, parent *raw.Artifact) error { + err := b.raw.WalkViaSteps(nodeIDs, raw.WalkViaDeps, func(node interface{}, parent *raw.Artifact) error { switch v := node.(type) { case *raw.Artifact: // logging.Debug("Walking build closure artifact '%s (%s)'", v.DisplayName, v.NodeID) @@ -151,51 +151,44 @@ func (b *BuildPlan) hydrateWithRuntimeClosure(nodeIDs []strfmt.UUID, platformID } func (b *BuildPlan) hydrateWithIngredients(artifact *Artifact, platformID *strfmt.UUID, ingredientLookup map[strfmt.UUID]*Ingredient) error { - err := b.raw.WalkViaSteps([]strfmt.UUID{artifact.ArtifactID}, raw.TagSource, + err := b.raw.WalkViaSteps([]strfmt.UUID{artifact.ArtifactID}, raw.WalkViaSingleSource, func(node interface{}, parent *raw.Artifact) error { - switch v := node.(type) { - case *raw.Source: - // logging.Debug("Walking source '%s (%s)'", v.Name, v.NodeID) - - // Ingredients aren't explicitly represented in buildplans. Technically all sources are ingredients - // but this may not always be true in the future. For our purposes we will initialize our own ingredients - // based on the source information, but we do not want to make the assumption in our logic that all - // sources are ingredients. - ingredient, ok := ingredientLookup[v.IngredientID] - if !ok { - ingredient = &Ingredient{ - IngredientSource: &v.IngredientSource, - platforms: []strfmt.UUID{}, - Artifacts: []*Artifact{}, - } - b.ingredients = append(b.ingredients, ingredient) - ingredientLookup[v.IngredientID] = ingredient - } + // logging.Debug("Walking source '%s (%s)'", v.Name, v.NodeID) + v, ok := node.(*raw.Source) + if !ok { + return nil // continue + } - // With multiple terminals it's possible we encounter the same combination multiple times. - // And an artifact usually only has one ingredient, so this is the cheapest lookup. - if !sliceutils.Contains(artifact.Ingredients, ingredient) { - artifact.Ingredients = append(artifact.Ingredients, ingredient) - ingredient.Artifacts = append(ingredient.Artifacts, artifact) - } - if platformID != nil { - ingredient.platforms = append(ingredient.platforms, *platformID) + // Ingredients aren't explicitly represented in buildplans. Technically all sources are ingredients + // but this may not always be true in the future. For our purposes we will initialize our own ingredients + // based on the source information, but we do not want to make the assumption in our logic that all + // sources are ingredients. + ingredient, ok := ingredientLookup[v.IngredientID] + if !ok { + ingredient = &Ingredient{ + IngredientSource: &v.IngredientSource, + platforms: []strfmt.UUID{}, + Artifacts: []*Artifact{}, } + b.ingredients = append(b.ingredients, ingredient) + ingredientLookup[v.IngredientID] = ingredient + } - if artifact.isBuildtimeDependency { - ingredient.IsBuildtimeDependency = true - } - if artifact.isRuntimeDependency { - ingredient.IsRuntimeDependency = true - } + // With multiple terminals it's possible we encounter the same combination multiple times. + // And an artifact usually only has one ingredient, so this is the cheapest lookup. + if !sliceutils.Contains(artifact.Ingredients, ingredient) { + artifact.Ingredients = append(artifact.Ingredients, ingredient) + ingredient.Artifacts = append(ingredient.Artifacts, artifact) + } + if platformID != nil { + ingredient.platforms = append(ingredient.platforms, *platformID) + } - return nil - default: - 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{} + if artifact.isBuildtimeDependency { + ingredient.IsBuildtimeDependency = true + } + if artifact.isRuntimeDependency { + ingredient.IsRuntimeDependency = true } return nil diff --git a/pkg/buildplan/hydrate_test.go b/pkg/buildplan/hydrate_test.go index da95c9826e..9ca9ba42bb 100644 --- a/pkg/buildplan/hydrate_test.go +++ b/pkg/buildplan/hydrate_test.go @@ -18,16 +18,22 @@ func TestBuildPlan_hydrateWithIngredients(t *testing.T) { }{ { "Ingredient solves for simple artifact > src hop", - &BuildPlan{raw: mock.BuildWithRuntimeDepsViaSrc}, + &BuildPlan{raw: mock.BuildWithInstallerDepsViaSrc}, &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}, + &BuildPlan{raw: mock.BuildWithInstallerDepsViaSrc}, &Artifact{ArtifactID: "00000000-0000-0000-0000-000000000002"}, "", }, + { + "State artifact should resolve to source even when hopping through a python wheel", + &BuildPlan{raw: mock.BuildWithStateArtifactThroughPyWheel}, + &Artifact{ArtifactID: "00000000-0000-0000-0000-000000000002"}, + "00000000-0000-0000-0000-000000000009", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -35,10 +41,17 @@ func TestBuildPlan_hydrateWithIngredients(t *testing.T) { if err := b.hydrateWithIngredients(tt.inputArtifact, nil, map[strfmt.UUID]*Ingredient{}); err != nil { t.Fatalf("hydrateWithIngredients() error = %v", errs.JoinMessage(err)) } + + // Use string slice so testify doesn't just dump a bunch of pointer addresses on failure -.- + ingredients := []string{} + for _, i := range tt.inputArtifact.Ingredients { + ingredients = append(ingredients, i.IngredientID.String()) + } if tt.wantIngredient == "" { - require.Empty(t, tt.inputArtifact.Ingredients) + require.Empty(t, ingredients) return } + if len(tt.inputArtifact.Ingredients) != 1 { t.Fatalf("expected 1 ingredient resolution, got %d", len(tt.inputArtifact.Ingredients)) } diff --git a/pkg/buildplan/mock/mock.go b/pkg/buildplan/mock/mock.go index aaf242ec7a..c715b775cc 100644 --- a/pkg/buildplan/mock/mock.go +++ b/pkg/buildplan/mock/mock.go @@ -153,7 +153,8 @@ var BuildWithRuntimeDeps = &raw.Build{ }, } -var BuildWithRuntimeDepsViaSrc = &raw.Build{ +// BuildWithInstallerDepsViaSrc is a build that includes an installer which has two artifacts as its dependencies. +var BuildWithInstallerDepsViaSrc = &raw.Build{ Terminals: []*raw.NamedTarget{ { Tag: "platform:00000000-0000-0000-0000-000000000001", @@ -165,7 +166,12 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{ StepID: "00000000-0000-0000-0000-000000000003", Outputs: []string{"00000000-0000-0000-0000-000000000002"}, Inputs: []*raw.NamedTarget{ - {Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000007"}}, + { + Tag: "src", NodeIDs: []strfmt.UUID{ + "00000000-0000-0000-0000-000000000007", + "00000000-0000-0000-0000-000000000010", + }, + }, }, }, { @@ -175,6 +181,13 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{ {Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000009"}}, }, }, + { + StepID: "00000000-0000-0000-0000-000000000011", + Outputs: []string{"00000000-0000-0000-0000-000000000010"}, + Inputs: []*raw.NamedTarget{ + {Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000012"}}, + }, + }, }, Artifacts: []*raw.Artifact{ { @@ -187,7 +200,65 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{ { NodeID: "00000000-0000-0000-0000-000000000007", DisplayName: "pkgOne", - MimeType: types.XActiveStateArtifactMimeType, + GeneratedBy: "00000000-0000-0000-0000-000000000008", + }, + { + NodeID: "00000000-0000-0000-0000-000000000010", + DisplayName: "pkgTwo", + GeneratedBy: "00000000-0000-0000-0000-000000000011", + }, + }, + Sources: []*raw.Source{ + { + "00000000-0000-0000-0000-000000000009", + raw.IngredientSource{ + IngredientID: "00000000-0000-0000-0000-000000000009", + }, + }, + { + "00000000-0000-0000-0000-000000000012", + raw.IngredientSource{ + IngredientID: "00000000-0000-0000-0000-000000000012", + }, + }, + }, +} + +// BuildWithStateArtifactThroughPyWheel is a build with a state tool artifact that has a python wheel as its dependency +var BuildWithStateArtifactThroughPyWheel = &raw.Build{ + Terminals: []*raw.NamedTarget{ + { + Tag: "platform:00000000-0000-0000-0000-000000000001", + NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000002"}, + }, + }, + Steps: []*raw.Step{ + { + StepID: "00000000-0000-0000-0000-000000000003", + Outputs: []string{"00000000-0000-0000-0000-000000000002"}, + Inputs: []*raw.NamedTarget{ + { + Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000007"}, + }, + }, + }, + { + StepID: "00000000-0000-0000-0000-000000000008", + Outputs: []string{"00000000-0000-0000-0000-000000000007"}, + Inputs: []*raw.NamedTarget{ + {Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000009"}}, + }, + }, + }, + Artifacts: []*raw.Artifact{ + { + NodeID: "00000000-0000-0000-0000-000000000002", + DisplayName: "pkgStateArtifact", + GeneratedBy: "00000000-0000-0000-0000-000000000003", + }, + { + NodeID: "00000000-0000-0000-0000-000000000007", + DisplayName: "pkgPyWheel", GeneratedBy: "00000000-0000-0000-0000-000000000008", }, }, diff --git a/pkg/buildplan/raw/walk.go b/pkg/buildplan/raw/walk.go index c9aa7058d6..89294a8cac 100644 --- a/pkg/buildplan/raw/walk.go +++ b/pkg/buildplan/raw/walk.go @@ -1,8 +1,6 @@ package raw import ( - "errors" - "github.com/go-openapi/strfmt" "github.com/ActiveState/cli/internal/errs" @@ -11,23 +9,30 @@ import ( type walkFunc func(node interface{}, parent *Artifact) error -type WalkNodeContext struct { - Node interface{} - ParentArtifact *Artifact - tag StepInputTag - lookup map[strfmt.UUID]interface{} +type WalkStrategy struct { + tag StepInputTag + stopAtMultiSource bool // If true, we will stop walking if the artifact relates to multiple sources (eg. installer, docker img) } -type WalkInterrupt struct{} - -func (w WalkInterrupt) Error() string { - return "interrupt walk" -} +var ( + WalkViaSingleSource = WalkStrategy{ + TagSource, + true, + } + WalkViaMultiSource = WalkStrategy{ + TagSource, + false, + } + WalkViaDeps = WalkStrategy{ + TagDependency, + false, + } +) // WalkViaSteps walks the graph and reports on nodes it encounters // Note that the same node can be encountered multiple times if it is referenced in the graph multiple times. // In this case the context around the node may be different, even if the node itself isn't. -func (b *Build) WalkViaSteps(nodeIDs []strfmt.UUID, inputTag StepInputTag, walk walkFunc) error { +func (b *Build) WalkViaSteps(nodeIDs []strfmt.UUID, strategy WalkStrategy, walk walkFunc) error { lookup := b.LookupMap() for _, nodeID := range nodeIDs { @@ -35,7 +40,7 @@ func (b *Build) WalkViaSteps(nodeIDs []strfmt.UUID, inputTag StepInputTag, walk if !ok { return errs.New("node ID '%s' does not exist in lookup table", nodeID) } - if err := b.walkNodeViaSteps(node, nil, inputTag, walk); err != nil { + if err := b.walkNodeViaSteps(node, nil, strategy, walk); err != nil { return errs.Wrap(err, "could not recurse over node IDs") } } @@ -43,13 +48,10 @@ func (b *Build) WalkViaSteps(nodeIDs []strfmt.UUID, inputTag StepInputTag, walk return nil } -func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, tag StepInputTag, walk walkFunc) error { +func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, strategy WalkStrategy, walk walkFunc) error { lookup := b.LookupMap() if err := walk(node, parent); err != nil { - if errors.As(err, &WalkInterrupt{}) { - return nil - } return errs.Wrap(err, "error walking over node") } @@ -74,24 +76,29 @@ func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, tag StepInp // tool, but it's technically possible to happen if someone requested a builder as part of their order. _, isSource = generatedByNode.(*Source) if isSource { - if err := b.walkNodeViaSteps(generatedByNode, ar, tag, walk); err != nil { + if err := b.walkNodeViaSteps(generatedByNode, ar, strategy, walk); err != nil { return errs.Wrap(err, "error walking source from generatedBy") } return nil // Sources are at the end of the recursion. } - nodeIDs, err := b.inputNodeIDsFromStep(ar, tag) + nodeIDs, err := b.inputNodeIDsFromStep(ar, strategy.tag) if err != nil { return errs.Wrap(err, "error walking over step inputs") } + // Stop if the next step has multiple input node ID's; this means we cannot determine a single source + if strategy.stopAtMultiSource && len(nodeIDs) > 1 { + return nil + } + for _, id := range nodeIDs { // Grab subnode that we want to iterate over next subNode, ok := lookup[id] if !ok { return errs.New("node ID '%s' does not exist in lookup table", id) } - if err := b.walkNodeViaSteps(subNode, ar, tag, walk); err != nil { + if err := b.walkNodeViaSteps(subNode, ar, strategy, walk); err != nil { return errs.Wrap(err, "error iterating over %s", id) } } diff --git a/pkg/buildplan/raw/walk_test.go b/pkg/buildplan/raw/walk_test.go index de002aef36..4e8f0518ae 100644 --- a/pkg/buildplan/raw/walk_test.go +++ b/pkg/buildplan/raw/walk_test.go @@ -24,7 +24,7 @@ func TestRawBuild_walkNodesViaSteps(t *testing.T) { tests := []struct { name string nodeIDs []strfmt.UUID - tag raw.StepInputTag + strategy raw.WalkStrategy build *raw.Build wantCalls []walkCall wantErr bool @@ -32,7 +32,7 @@ func TestRawBuild_walkNodesViaSteps(t *testing.T) { { "Ingredient from step", []strfmt.UUID{"00000000-0000-0000-0000-000000000002"}, - raw.TagSource, + raw.WalkViaSingleSource, mock.BuildWithSourceFromStep, []walkCall{ {"00000000-0000-0000-0000-000000000002", "Artifact", ""}, @@ -44,7 +44,7 @@ func TestRawBuild_walkNodesViaSteps(t *testing.T) { { "Ingredient from generatedBy, multiple artifacts to same ingredient", []strfmt.UUID{"00000000-0000-0000-0000-000000000002", "00000000-0000-0000-0000-000000000003"}, - raw.TagSource, + raw.WalkViaSingleSource, mock.BuildWithSourceFromGeneratedBy, []walkCall{ {"00000000-0000-0000-0000-000000000002", "Artifact", ""}, @@ -54,10 +54,24 @@ func TestRawBuild_walkNodesViaSteps(t *testing.T) { }, false, }, + { + "Multiple sources through installer artifact", + []strfmt.UUID{"00000000-0000-0000-0000-000000000002"}, + raw.WalkViaMultiSource, + mock.BuildWithInstallerDepsViaSrc, + []walkCall{ + {"00000000-0000-0000-0000-000000000002", "Artifact", ""}, + {"00000000-0000-0000-0000-000000000007", "Artifact", "00000000-0000-0000-0000-000000000002"}, + {"00000000-0000-0000-0000-000000000009", "Source", strfmt.UUID("00000000-0000-0000-0000-000000000007")}, + {"00000000-0000-0000-0000-000000000010", "Artifact", "00000000-0000-0000-0000-000000000002"}, + {"00000000-0000-0000-0000-000000000012", "Source", strfmt.UUID("00000000-0000-0000-0000-000000000010")}, + }, + false, + }, { "Build time deps", []strfmt.UUID{"00000000-0000-0000-0000-000000000002"}, - raw.TagDependency, + raw.WalkViaDeps, mock.BuildWithBuildDeps, []walkCall{ {"00000000-0000-0000-0000-000000000002", "Artifact", ""}, @@ -93,7 +107,7 @@ func TestRawBuild_walkNodesViaSteps(t *testing.T) { return nil } - if err := tt.build.WalkViaSteps(tt.nodeIDs, tt.tag, walk); (err != nil) != tt.wantErr { + if err := tt.build.WalkViaSteps(tt.nodeIDs, tt.strategy, walk); (err != nil) != tt.wantErr { t.Errorf("walkNodes() error = %v, wantErr %v", errs.JoinMessage(err), tt.wantErr) } @@ -140,8 +154,8 @@ func TestRawBuild_walkNodesViaRuntimeDeps(t *testing.T) { }, { "Runtime deps via src step", - mock.BuildWithRuntimeDepsViaSrc.Terminals[0].NodeIDs, - mock.BuildWithRuntimeDepsViaSrc, + mock.BuildWithInstallerDepsViaSrc.Terminals[0].NodeIDs, + mock.BuildWithInstallerDepsViaSrc, []walkCall{ {"00000000-0000-0000-0000-000000000007", "Artifact", "00000000-0000-0000-0000-000000000002"}, }, From 6a9f844d9cab8022dd5fcc7351469adfcf259393 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 25 Sep 2024 10:14:33 -0700 Subject: [PATCH 4/9] Drop unused code --- internal/maputils/maputils.go | 7 ------- 1 file changed, 7 deletions(-) delete mode 100644 internal/maputils/maputils.go diff --git a/internal/maputils/maputils.go b/internal/maputils/maputils.go deleted file mode 100644 index e35435bd6a..0000000000 --- a/internal/maputils/maputils.go +++ /dev/null @@ -1,7 +0,0 @@ -package maputils - -// Contains is a shortcut to `_, ok := map[key]`. This allows for evaluating -func Contains[T comparable, V any](m map[T]V, value T) bool { - _, ok := m[value] - return ok -} From 70dc6a4d6a448f608e7189de4f489a0146ff9077 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 25 Sep 2024 10:54:59 -0700 Subject: [PATCH 5/9] Don't enforce zero ingredient check --- pkg/buildplan/buildplan.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/buildplan/buildplan.go b/pkg/buildplan/buildplan.go index fe5bdf8c1f..a69a034291 100644 --- a/pkg/buildplan/buildplan.go +++ b/pkg/buildplan/buildplan.go @@ -37,9 +37,10 @@ func Unmarshal(data []byte) (*BuildPlan, error) { return nil, errs.Wrap(err, "error hydrating build plan") } - if len(b.artifacts) == 0 || len(b.ingredients) == 0 || len(b.platforms) == 0 { - return nil, errs.New("Buildplan unmarshalling failed as it got zero artifacts (%d), ingredients (%d) and or platforms (%d).", - len(b.artifacts), len(b.ingredients), len(b.platforms)) + if len(b.artifacts) == 0 || len(b.platforms) == 0 { + // Ingredients are not considered here because certain builds (eg. camel) won't be able to relate to a single ingredient + return nil, errs.New("Buildplan unmarshalling failed as it got zero artifacts (%d) and/or platforms (%d).", + len(b.artifacts), len(b.platforms)) } return b, nil From e69e7b9b9e03a809d0cdb1545a286a6be4c82a14 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 25 Sep 2024 13:49:05 -0700 Subject: [PATCH 6/9] Fix artifacts being filtered out just because they have no ingredient --- pkg/buildplan/filters.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/buildplan/filters.go b/pkg/buildplan/filters.go index 06a7f19b38..b172234b2b 100644 --- a/pkg/buildplan/filters.go +++ b/pkg/buildplan/filters.go @@ -47,7 +47,7 @@ func FilterStateArtifacts() FilterArtifact { internalIngredients := sliceutils.Filter(a.Ingredients, func(i *Ingredient) bool { return i.Namespace == NamespaceInternal }) - if len(a.Ingredients) == len(internalIngredients) { + if len(a.Ingredients) > 0 && len(a.Ingredients) == len(internalIngredients) { return false } if strings.Contains(a.URL, "as-builds/noop") { From 4999497ff7ed1c92d8066e85be198cb321237320 Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 25 Sep 2024 13:49:39 -0700 Subject: [PATCH 7/9] Fix panic on simultaneous map write --- pkg/runtime/depot.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/runtime/depot.go b/pkg/runtime/depot.go index c17b4aa9ea..610418e952 100644 --- a/pkg/runtime/depot.go +++ b/pkg/runtime/depot.go @@ -137,6 +137,9 @@ func (d *depot) Path(id strfmt.UUID) string { // necessary information. Writing externally is preferred because otherwise the depot would need a lot of specialized // logic that ultimately don't really need to be a concern of the depot. func (d *depot) Put(id strfmt.UUID) error { + d.fsMutex.Lock() + defer d.fsMutex.Unlock() + if !fileutils.TargetExists(d.Path(id)) { return errs.New("could not put %s, as dir does not exist: %s", id, d.Path(id)) } From 2bca991220d4061e923a710f8f74ef99a7c7044b Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 25 Sep 2024 14:35:20 -0700 Subject: [PATCH 8/9] Add comment --- pkg/buildplan/hydrate.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/buildplan/hydrate.go b/pkg/buildplan/hydrate.go index 11cc49b758..724c8ccc0b 100644 --- a/pkg/buildplan/hydrate.go +++ b/pkg/buildplan/hydrate.go @@ -55,6 +55,8 @@ func (b *BuildPlan) hydrate() error { } ingredient, ok := ingredientLookup[source.IngredientID] if !ok { + // It's possible that we haven't associated a source to an artifact if that source links to multiple artifacts. + // In this case we cannot determine which artifact relates to which source. continue } b.requirements = append(b.requirements, &Requirement{ From b7b933f35897a3378cd106f05c4062e9c3b47a6a Mon Sep 17 00:00:00 2001 From: Nathan Rijksen Date: Wed, 25 Sep 2024 14:44:17 -0700 Subject: [PATCH 9/9] Fix test --- pkg/buildplan/mock/mock.go | 2 ++ pkg/buildplan/raw/walk_test.go | 1 + 2 files changed, 3 insertions(+) diff --git a/pkg/buildplan/mock/mock.go b/pkg/buildplan/mock/mock.go index c715b775cc..ed575535ee 100644 --- a/pkg/buildplan/mock/mock.go +++ b/pkg/buildplan/mock/mock.go @@ -200,11 +200,13 @@ var BuildWithInstallerDepsViaSrc = &raw.Build{ { NodeID: "00000000-0000-0000-0000-000000000007", DisplayName: "pkgOne", + MimeType: types.XActiveStateArtifactMimeType, GeneratedBy: "00000000-0000-0000-0000-000000000008", }, { NodeID: "00000000-0000-0000-0000-000000000010", DisplayName: "pkgTwo", + MimeType: types.XActiveStateArtifactMimeType, GeneratedBy: "00000000-0000-0000-0000-000000000011", }, }, diff --git a/pkg/buildplan/raw/walk_test.go b/pkg/buildplan/raw/walk_test.go index 4e8f0518ae..12a887cf1b 100644 --- a/pkg/buildplan/raw/walk_test.go +++ b/pkg/buildplan/raw/walk_test.go @@ -158,6 +158,7 @@ func TestRawBuild_walkNodesViaRuntimeDeps(t *testing.T) { mock.BuildWithInstallerDepsViaSrc, []walkCall{ {"00000000-0000-0000-0000-000000000007", "Artifact", "00000000-0000-0000-0000-000000000002"}, + {"00000000-0000-0000-0000-000000000010", "Artifact", "00000000-0000-0000-0000-000000000002"}, }, false, },