Skip to content

Commit 899fa42

Browse files
NaatanNaatan
committed
Support identifying source behind python wheel
1 parent 99fd378 commit 899fa42

File tree

6 files changed

+180
-75
lines changed

6 files changed

+180
-75
lines changed

‎internal/maputils/maputils.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package maputils
2+
3+
// Contains is a shortcut to `_, ok := map[key]`. This allows for evaluating
4+
func Contains[T comparable, V any](m map[T]V, value T) bool {
5+
_, ok := m[value]
6+
return ok
7+
}

‎pkg/buildplan/hydrate.go‎

Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (b *BuildPlan) hydrate() error {
8080
}
8181

8282
func (b *BuildPlan) hydrateWithBuildClosure(nodeIDs []strfmt.UUID, platformID *strfmt.UUID, artifactLookup map[strfmt.UUID]*Artifact) error {
83-
err := b.raw.WalkViaSteps(nodeIDs, raw.TagDependency, func(node interface{}, parent *raw.Artifact) error {
83+
err := b.raw.WalkViaSteps(nodeIDs, raw.WalkViaDeps, func(node interface{}, parent *raw.Artifact) error {
8484
switch v := node.(type) {
8585
case *raw.Artifact:
8686
// logging.Debug("Walking build closure artifact '%s (%s)'", v.DisplayName, v.NodeID)
@@ -151,51 +151,44 @@ func (b *BuildPlan) hydrateWithRuntimeClosure(nodeIDs []strfmt.UUID, platformID
151151
}
152152

153153
func (b *BuildPlan) hydrateWithIngredients(artifact *Artifact, platformID *strfmt.UUID, ingredientLookup map[strfmt.UUID]*Ingredient) error {
154-
err := b.raw.WalkViaSteps([]strfmt.UUID{artifact.ArtifactID}, raw.TagSource,
154+
err := b.raw.WalkViaSteps([]strfmt.UUID{artifact.ArtifactID}, raw.WalkViaSingleSource,
155155
func(node interface{}, parent *raw.Artifact) error {
156-
switch v := node.(type) {
157-
case *raw.Source:
158-
// logging.Debug("Walking source '%s (%s)'", v.Name, v.NodeID)
159-
160-
// Ingredients aren't explicitly represented in buildplans. Technically all sources are ingredients
161-
// but this may not always be true in the future. For our purposes we will initialize our own ingredients
162-
// based on the source information, but we do not want to make the assumption in our logic that all
163-
// sources are ingredients.
164-
ingredient, ok := ingredientLookup[v.IngredientID]
165-
if !ok {
166-
ingredient = &Ingredient{
167-
IngredientSource: &v.IngredientSource,
168-
platforms: []strfmt.UUID{},
169-
Artifacts: []*Artifact{},
170-
}
171-
b.ingredients = append(b.ingredients, ingredient)
172-
ingredientLookup[v.IngredientID] = ingredient
173-
}
156+
// logging.Debug("Walking source '%s (%s)'", v.Name, v.NodeID)
157+
v, ok := node.(*raw.Source)
158+
if !ok {
159+
return nil // continue
160+
}
174161

175-
// With multiple terminals it's possible we encounter the same combination multiple times.
176-
// And an artifact usually only has one ingredient, so this is the cheapest lookup.
177-
if !sliceutils.Contains(artifact.Ingredients, ingredient) {
178-
artifact.Ingredients = append(artifact.Ingredients, ingredient)
179-
ingredient.Artifacts = append(ingredient.Artifacts, artifact)
180-
}
181-
if platformID != nil {
182-
ingredient.platforms = append(ingredient.platforms, *platformID)
162+
// Ingredients aren't explicitly represented in buildplans. Technically all sources are ingredients
163+
// but this may not always be true in the future. For our purposes we will initialize our own ingredients
164+
// based on the source information, but we do not want to make the assumption in our logic that all
165+
// sources are ingredients.
166+
ingredient, ok := ingredientLookup[v.IngredientID]
167+
if !ok {
168+
ingredient = &Ingredient{
169+
IngredientSource: &v.IngredientSource,
170+
platforms: []strfmt.UUID{},
171+
Artifacts: []*Artifact{},
183172
}
173+
b.ingredients = append(b.ingredients, ingredient)
174+
ingredientLookup[v.IngredientID] = ingredient
175+
}
184176

185-
if artifact.isBuildtimeDependency {
186-
ingredient.IsBuildtimeDependency = true
187-
}
188-
if artifact.isRuntimeDependency {
189-
ingredient.IsRuntimeDependency = true
190-
}
177+
// With multiple terminals it's possible we encounter the same combination multiple times.
178+
// And an artifact usually only has one ingredient, so this is the cheapest lookup.
179+
if !sliceutils.Contains(artifact.Ingredients, ingredient) {
180+
artifact.Ingredients = append(artifact.Ingredients, ingredient)
181+
ingredient.Artifacts = append(ingredient.Artifacts, artifact)
182+
}
183+
if platformID != nil {
184+
ingredient.platforms = append(ingredient.platforms, *platformID)
185+
}
191186

192-
return nil
193-
default:
194-
if a, ok := v.(*raw.Artifact); ok && a.NodeID == artifact.ArtifactID {
195-
return nil // continue
196-
}
197-
// Source ingredients are only relevant when they link DIRECTLY to the artifact
198-
return raw.WalkInterrupt{}
187+
if artifact.isBuildtimeDependency {
188+
ingredient.IsBuildtimeDependency = true
189+
}
190+
if artifact.isRuntimeDependency {
191+
ingredient.IsRuntimeDependency = true
199192
}
200193

201194
return nil

‎pkg/buildplan/hydrate_test.go‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,40 @@ func TestBuildPlan_hydrateWithIngredients(t *testing.T) {
1818
}{
1919
{
2020
"Ingredient solves for simple artifact > src hop",
21-
&BuildPlan{raw: mock.BuildWithRuntimeDepsViaSrc},
21+
&BuildPlan{raw: mock.BuildWithInstallerDepsViaSrc},
2222
&Artifact{ArtifactID: "00000000-0000-0000-0000-000000000007"},
2323
"00000000-0000-0000-0000-000000000009",
2424
},
2525
{
2626
"Installer should not resolve to an ingredient as it doesn't have a direct source",
27-
&BuildPlan{raw: mock.BuildWithRuntimeDepsViaSrc},
27+
&BuildPlan{raw: mock.BuildWithInstallerDepsViaSrc},
2828
&Artifact{ArtifactID: "00000000-0000-0000-0000-000000000002"},
2929
"",
3030
},
31+
{
32+
"State artifact should resolve to source even when hopping through a python wheel",
33+
&BuildPlan{raw: mock.BuildWithStateArtifactThroughPyWheel},
34+
&Artifact{ArtifactID: "00000000-0000-0000-0000-000000000002"},
35+
"00000000-0000-0000-0000-000000000009",
36+
},
3137
}
3238
for _, tt := range tests {
3339
t.Run(tt.name, func(t *testing.T) {
3440
b := tt.buildplan
3541
if err := b.hydrateWithIngredients(tt.inputArtifact, nil, map[strfmt.UUID]*Ingredient{}); err != nil {
3642
t.Fatalf("hydrateWithIngredients() error = %v", errs.JoinMessage(err))
3743
}
44+
45+
// Use string slice so testify doesn't just dump a bunch of pointer addresses on failure -.-
46+
ingredients := []string{}
47+
for _, i := range tt.inputArtifact.Ingredients {
48+
ingredients = append(ingredients, i.IngredientID.String())
49+
}
3850
if tt.wantIngredient == "" {
39-
require.Empty(t, tt.inputArtifact.Ingredients)
51+
require.Empty(t, ingredients)
4052
return
4153
}
54+
4255
if len(tt.inputArtifact.Ingredients) != 1 {
4356
t.Fatalf("expected 1 ingredient resolution, got %d", len(tt.inputArtifact.Ingredients))
4457
}

‎pkg/buildplan/mock/mock.go‎

Lines changed: 74 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,8 @@ var BuildWithRuntimeDeps = &raw.Build{
153153
},
154154
}
155155

156-
var BuildWithRuntimeDepsViaSrc = &raw.Build{
156+
// BuildWithInstallerDepsViaSrc is a build that includes an installer which has two artifacts as its dependencies.
157+
var BuildWithInstallerDepsViaSrc = &raw.Build{
157158
Terminals: []*raw.NamedTarget{
158159
{
159160
Tag: "platform:00000000-0000-0000-0000-000000000001",
@@ -165,7 +166,12 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{
165166
StepID: "00000000-0000-0000-0000-000000000003",
166167
Outputs: []string{"00000000-0000-0000-0000-000000000002"},
167168
Inputs: []*raw.NamedTarget{
168-
{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000007"}},
169+
{
170+
Tag: "src", NodeIDs: []strfmt.UUID{
171+
"00000000-0000-0000-0000-000000000007",
172+
"00000000-0000-0000-0000-000000000010",
173+
},
174+
},
169175
},
170176
},
171177
{
@@ -175,6 +181,13 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{
175181
{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000009"}},
176182
},
177183
},
184+
{
185+
StepID: "00000000-0000-0000-0000-000000000011",
186+
Outputs: []string{"00000000-0000-0000-0000-000000000010"},
187+
Inputs: []*raw.NamedTarget{
188+
{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000012"}},
189+
},
190+
},
178191
},
179192
Artifacts: []*raw.Artifact{
180193
{
@@ -187,7 +200,65 @@ var BuildWithRuntimeDepsViaSrc = &raw.Build{
187200
{
188201
NodeID: "00000000-0000-0000-0000-000000000007",
189202
DisplayName: "pkgOne",
190-
MimeType: types.XActiveStateArtifactMimeType,
203+
GeneratedBy: "00000000-0000-0000-0000-000000000008",
204+
},
205+
{
206+
NodeID: "00000000-0000-0000-0000-000000000010",
207+
DisplayName: "pkgTwo",
208+
GeneratedBy: "00000000-0000-0000-0000-000000000011",
209+
},
210+
},
211+
Sources: []*raw.Source{
212+
{
213+
"00000000-0000-0000-0000-000000000009",
214+
raw.IngredientSource{
215+
IngredientID: "00000000-0000-0000-0000-000000000009",
216+
},
217+
},
218+
{
219+
"00000000-0000-0000-0000-000000000012",
220+
raw.IngredientSource{
221+
IngredientID: "00000000-0000-0000-0000-000000000012",
222+
},
223+
},
224+
},
225+
}
226+
227+
// BuildWithStateArtifactThroughPyWheel is a build with a state tool artifact that has a python wheel as its dependency
228+
var BuildWithStateArtifactThroughPyWheel = &raw.Build{
229+
Terminals: []*raw.NamedTarget{
230+
{
231+
Tag: "platform:00000000-0000-0000-0000-000000000001",
232+
NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000002"},
233+
},
234+
},
235+
Steps: []*raw.Step{
236+
{
237+
StepID: "00000000-0000-0000-0000-000000000003",
238+
Outputs: []string{"00000000-0000-0000-0000-000000000002"},
239+
Inputs: []*raw.NamedTarget{
240+
{
241+
Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000007"},
242+
},
243+
},
244+
},
245+
{
246+
StepID: "00000000-0000-0000-0000-000000000008",
247+
Outputs: []string{"00000000-0000-0000-0000-000000000007"},
248+
Inputs: []*raw.NamedTarget{
249+
{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000009"}},
250+
},
251+
},
252+
},
253+
Artifacts: []*raw.Artifact{
254+
{
255+
NodeID: "00000000-0000-0000-0000-000000000002",
256+
DisplayName: "pkgStateArtifact",
257+
GeneratedBy: "00000000-0000-0000-0000-000000000003",
258+
},
259+
{
260+
NodeID: "00000000-0000-0000-0000-000000000007",
261+
DisplayName: "pkgPyWheel",
191262
GeneratedBy: "00000000-0000-0000-0000-000000000008",
192263
},
193264
},

‎pkg/buildplan/raw/walk.go‎

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
package raw
22

33
import (
4-
"errors"
5-
64
"github.com/go-openapi/strfmt"
75

86
"github.com/ActiveState/cli/internal/errs"
@@ -11,45 +9,49 @@ import (
119

1210
type walkFunc func(node interface{}, parent *Artifact) error
1311

14-
type WalkNodeContext struct {
15-
Node interface{}
16-
ParentArtifact *Artifact
17-
tag StepInputTag
18-
lookup map[strfmt.UUID]interface{}
12+
type WalkStrategy struct {
13+
tag StepInputTag
14+
stopAtMultiSource bool // If true, we will stop walking if the artifact relates to multiple sources (eg. installer, docker img)
1915
}
2016

21-
type WalkInterrupt struct{}
22-
23-
func (w WalkInterrupt) Error() string {
24-
return "interrupt walk"
25-
}
17+
var (
18+
WalkViaSingleSource = WalkStrategy{
19+
TagSource,
20+
true,
21+
}
22+
WalkViaMultiSource = WalkStrategy{
23+
TagSource,
24+
false,
25+
}
26+
WalkViaDeps = WalkStrategy{
27+
TagDependency,
28+
false,
29+
}
30+
)
2631

2732
// WalkViaSteps walks the graph and reports on nodes it encounters
2833
// Note that the same node can be encountered multiple times if it is referenced in the graph multiple times.
2934
// In this case the context around the node may be different, even if the node itself isn't.
30-
func (b *Build) WalkViaSteps(nodeIDs []strfmt.UUID, inputTag StepInputTag, walk walkFunc) error {
35+
func (b *Build) WalkViaSteps(nodeIDs []strfmt.UUID, strategy WalkStrategy, walk walkFunc) error {
3136
lookup := b.LookupMap()
3237

3338
for _, nodeID := range nodeIDs {
3439
node, ok := lookup[nodeID]
3540
if !ok {
3641
return errs.New("node ID '%s' does not exist in lookup table", nodeID)
3742
}
38-
if err := b.walkNodeViaSteps(node, nil, inputTag, walk); err != nil {
43+
if err := b.walkNodeViaSteps(node, nil, strategy, walk); err != nil {
3944
return errs.Wrap(err, "could not recurse over node IDs")
4045
}
4146
}
4247

4348
return nil
4449
}
4550

46-
func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, tag StepInputTag, walk walkFunc) error {
51+
func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, strategy WalkStrategy, walk walkFunc) error {
4752
lookup := b.LookupMap()
4853

4954
if err := walk(node, parent); err != nil {
50-
if errors.As(err, &WalkInterrupt{}) {
51-
return nil
52-
}
5355
return errs.Wrap(err, "error walking over node")
5456
}
5557

@@ -74,24 +76,29 @@ func (b *Build) walkNodeViaSteps(node interface{}, parent *Artifact, tag StepInp
7476
// tool, but it's technically possible to happen if someone requested a builder as part of their order.
7577
_, isSource = generatedByNode.(*Source)
7678
if isSource {
77-
if err := b.walkNodeViaSteps(generatedByNode, ar, tag, walk); err != nil {
79+
if err := b.walkNodeViaSteps(generatedByNode, ar, strategy, walk); err != nil {
7880
return errs.Wrap(err, "error walking source from generatedBy")
7981
}
8082
return nil // Sources are at the end of the recursion.
8183
}
8284

83-
nodeIDs, err := b.inputNodeIDsFromStep(ar, tag)
85+
nodeIDs, err := b.inputNodeIDsFromStep(ar, strategy.tag)
8486
if err != nil {
8587
return errs.Wrap(err, "error walking over step inputs")
8688
}
8789

90+
// Stop if the next step has multiple input node ID's; this means we cannot determine a single source
91+
if strategy.stopAtMultiSource && len(nodeIDs) > 1 {
92+
return nil
93+
}
94+
8895
for _, id := range nodeIDs {
8996
// Grab subnode that we want to iterate over next
9097
subNode, ok := lookup[id]
9198
if !ok {
9299
return errs.New("node ID '%s' does not exist in lookup table", id)
93100
}
94-
if err := b.walkNodeViaSteps(subNode, ar, tag, walk); err != nil {
101+
if err := b.walkNodeViaSteps(subNode, ar, strategy, walk); err != nil {
95102
return errs.Wrap(err, "error iterating over %s", id)
96103
}
97104
}

0 commit comments

Comments
 (0)