Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cycle support #3456

Merged
merged 5 commits into from
Aug 23, 2024
Merged

Add cycle support #3456

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eca219b
Add support for cycles
MDrakos Aug 20, 2024
07da8aa
Remove debug logs
MDrakos Aug 20, 2024
6a1a242
Add more unit tests
MDrakos Aug 22, 2024
afab365
Update comments
MDrakos Aug 22, 2024
cf76af6
Enable tests and remove debug prints
MDrakos Aug 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions pkg/buildplan/hydrate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ func (b *BuildPlan) hydrateWithBuildClosure(nodeIDs []strfmt.UUID, platformID *s
default:
return errs.New("unexpected node type '%T': %#v", v, v)
}
return nil
})
if err != nil {
return errs.Wrap(err, "error hydrating from build closure")
Expand Down Expand Up @@ -144,7 +143,6 @@ func (b *BuildPlan) hydrateWithRuntimeClosure(nodeIDs []strfmt.UUID, platformID
default:
return errs.New("unexpected node type '%T': %#v", v, v)
}
return nil
})
if err != nil {
return errs.Wrap(err, "error hydrating from runtime closure")
Expand Down
72 changes: 72 additions & 0 deletions pkg/buildplan/raw/mock_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,75 @@ var buildWithRuntimeDepsViaSrc = &Build{
},
},
}

var buildWithRuntimeDepsViaSrcCycle = &Build{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add a comment highlighting the cycle.

Terminals: []*NamedTarget{
{
Tag: "platform:00000000-0000-0000-0000-000000000001",
NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000002"},
},
},
Steps: []*Step{
{
StepID: "00000000-0000-0000-0000-000000000003",
Outputs: []string{"00000000-0000-0000-0000-000000000002"},
Inputs: []*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: []*NamedTarget{
{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000010"}},
},
},
{
StepID: "00000000-0000-0000-0000-000000000011",
Outputs: []string{
"00000000-0000-0000-0000-000000000010",
},
Inputs: []*NamedTarget{
{Tag: "src", NodeIDs: []strfmt.UUID{"00000000-0000-0000-0000-000000000013"}},
},
},
},
Artifacts: []*Artifact{
{
NodeID: "00000000-0000-0000-0000-000000000002",
DisplayName: "installer",
MimeType: "application/unrecognized",
GeneratedBy: "00000000-0000-0000-0000-000000000003",
},
{
NodeID: "00000000-0000-0000-0000-000000000007",
DisplayName: "pkgOne",
MimeType: "application/unrecognized",
GeneratedBy: "00000000-0000-0000-0000-000000000008",
},
{
NodeID: "00000000-0000-0000-0000-000000000010",
DisplayName: "pkgTwo",
MimeType: "application/unrecognized",
GeneratedBy: "00000000-0000-0000-0000-000000000011",
},
{
NodeID: "00000000-0000-0000-0000-000000000013",
DisplayName: "pkgThree",
MimeType: types.XActiveStateArtifactMimeType,
RuntimeDependencies: []strfmt.UUID{"00000000-0000-0000-0000-000000000010"},
GeneratedBy: "00000000-0000-0000-0000-000000000011",
},
},
Sources: []*Source{
{
NodeID: "00000000-0000-0000-0000-000000000006",
},
{
NodeID: "00000000-0000-0000-0000-000000000009",
},
{
NodeID: "00000000-0000-0000-0000-000000000012",
},
},
}
15 changes: 11 additions & 4 deletions pkg/buildplan/raw/walk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,22 +120,23 @@ func (b *Build) inputNodeIDsFromStep(ar *Artifact, tag StepInputTag) ([]strfmt.U

func (b *Build) WalkViaRuntimeDeps(nodeIDs []strfmt.UUID, walk walkFunc) error {
lookup := b.LookupMap()
visited := make(map[strfmt.UUID]bool)

for _, id := range nodeIDs {
node, ok := lookup[id]
if !ok {
return errs.New("node ID '%s' does not exist in lookup table", id)
}

if err := b.walkNodeViaRuntimeDeps(node, nil, walk); err != nil {
if err := b.walkNodeViaRuntimeDeps(node, nil, visited, walk); err != nil {
return errs.Wrap(err, "error walking over runtime dep %s", id)
}
}

return nil
}

func (b *Build) walkNodeViaRuntimeDeps(node interface{}, parent *Artifact, walk walkFunc) error {
func (b *Build) walkNodeViaRuntimeDeps(node interface{}, parent *Artifact, visited map[strfmt.UUID]bool, walk walkFunc) error {
lookup := b.LookupMap()

ar, ok := node.(*Artifact)
Expand All @@ -148,6 +149,12 @@ func (b *Build) walkNodeViaRuntimeDeps(node interface{}, parent *Artifact, walk
return nil
}

// If we detect a cycle we should stop
if visited[ar.NodeID] {
return nil
}
visited[ar.NodeID] = true

// Only state tool artifacts are considered to be a runtime dependency
if IsStateToolMimeType(ar.MimeType) {
if err := walk(ar, parent); err != nil {
Expand All @@ -168,7 +175,7 @@ func (b *Build) walkNodeViaRuntimeDeps(node interface{}, parent *Artifact, walk
if !ok {
return errs.New("step node ID '%s' does not exist in lookup table", id)
}
if err := b.walkNodeViaRuntimeDeps(subNode, ar, walk); err != nil {
if err := b.walkNodeViaRuntimeDeps(subNode, ar, visited, walk); err != nil {
return errs.Wrap(err, "error walking over runtime dep %s", id)
}
}
Expand All @@ -178,7 +185,7 @@ func (b *Build) walkNodeViaRuntimeDeps(node interface{}, parent *Artifact, walk
if !ok {
return errs.New("node ID '%s' does not exist in lookup table", id)
}
if err := b.walkNodeViaRuntimeDeps(subNode, ar, walk); err != nil {
if err := b.walkNodeViaRuntimeDeps(subNode, ar, visited, walk); err != nil {
return errs.Wrap(err, "error walking over runtime dep %s", id)
}
}
Expand Down
9 changes: 9 additions & 0 deletions pkg/buildplan/raw/walk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,15 @@ func TestRawBuild_walkNodesViaRuntimeDeps(t *testing.T) {
},
false,
},
{
"Runtime deps with cycle",
buildWithRuntimeDepsViaSrcCycle.Terminals[0].NodeIDs,
buildWithRuntimeDepsViaSrcCycle,
[]walkCall{
{"00000000-0000-0000-0000-000000000013", "Artifact", "00000000-0000-0000-0000-000000000010"},
},
false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
Loading