From 4363c5466290a9741d5cbada5b815c953cb439da Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 25 Sep 2023 14:57:29 -0700 Subject: [PATCH 01/32] Commit WIP --- pkg/platform/model/buildplanner.go | 7 +- pkg/platform/runtime/buildplan/buildplan.go | 215 ++++++++++++-------- pkg/platform/runtime/setup/setup.go | 17 +- 3 files changed, 154 insertions(+), 85 deletions(-) diff --git a/pkg/platform/model/buildplanner.go b/pkg/platform/model/buildplanner.go index a3673080de..28f0a1adb9 100644 --- a/pkg/platform/model/buildplanner.go +++ b/pkg/platform/model/buildplanner.go @@ -156,9 +156,10 @@ func (bp *BuildPlanner) FetchBuildResult(commitID strfmt.UUID, owner, project st } res := BuildResult{ - BuildEngine: buildEngine, - Build: build, - BuildReady: build.Status == bpModel.Completed, + BuildEngine: buildEngine, + Build: build, + // BuildReady: build.Status == bpModel.Completed, + BuildReady: false, CommitID: id, BuildExpression: expr, BuildStatus: build.Status, diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index f967923b48..d316b96b19 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -1,10 +1,13 @@ package buildplan import ( + "strings" + "github.com/ActiveState/cli/internal/errs" "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/logging" "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" + platformModel "github.com/ActiveState/cli/pkg/platform/model" "github.com/ActiveState/cli/pkg/platform/runtime/artifact" "github.com/go-openapi/strfmt" ) @@ -39,7 +42,7 @@ func NewMapFromBuildPlan(build *model.Build) (artifact.Map, error) { } for _, id := range terminalTargetIDs { - err := buildMap(id, lookup, res) + err := buildRuntimeClosureMap(id, lookup, res) if err != nil { return nil, errs.Wrap(err, "Could not build map for terminal %s", id) } @@ -82,7 +85,7 @@ func buildTerminals(nodeID strfmt.UUID, lookup map[strfmt.UUID]interface{}, resu } } -// buildMap recursively builds the artifact map from the lookup table. It expects an ID that +// buildRuntimeClosureMap recursively builds the artifact map from the lookup table. It expects an ID that // represents an artifact. With that ID it retrieves the artifact from the lookup table and // recursively calls itself with each of the artifacts dependencies. Finally, once all of the // dependencies have been processed, it adds the artifact to the result map. @@ -91,7 +94,7 @@ func buildTerminals(nodeID strfmt.UUID, lookup map[strfmt.UUID]interface{}, resu // iterate through the artifact's dependencies, we also have to build up the dependencies of // each of those dependencies. Once we have a complete list of dependencies for the artifact, // we can continue to build up the results map. -func buildMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { +func buildRuntimeClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { target := lookup[baseID] currentArtifact, ok := target.(*model.Artifact) if !ok { @@ -110,7 +113,7 @@ func buildMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result art deps[id] = struct{}{} } - err = buildMap(depID, lookup, result) + err = buildRuntimeClosureMap(depID, lookup, result) if err != nil { return errs.Wrap(err, "Could not build map for runtime dependency %s", currentArtifact.NodeID) } @@ -273,10 +276,31 @@ func NewNamedMapFromBuildPlan(build *model.Build) (artifact.NamedMap, error) { // runtime dependency calculation as it includes ALL of the input artifacts of the // step that generated each artifact. The includeBuilders argument determines whether // or not to include builder artifacts in the final result. -func BuildtimeArtifacts(build *model.Build, includeBuilders bool) (artifact.Map, error) { - result := make(artifact.Map) - lookup := make(map[strfmt.UUID]interface{}) +func BuildtimeArtifacts(build *model.Build) (artifact.Map, error) { + // Extract the available platforms from the build plan + var bpPlatforms []strfmt.UUID + for _, t := range build.Terminals { + if t.Tag == model.TagOrphan { + continue + } + bpPlatforms = append(bpPlatforms, strfmt.UUID(strings.TrimPrefix(t.Tag, "platform:"))) + } + // Get the platform ID for the current platform + platformID, err := platformModel.FilterCurrentPlatform(platformModel.HostPlatform, bpPlatforms) + if err != nil { + return nil, locale.WrapError(err, "err_filter_current_platform") + } + + // Filter the build terminals to only include the current platform + var filteredTerminals []*model.NamedTarget + for _, t := range build.Terminals { + if platformID.String() == strings.TrimPrefix(t.Tag, "platform:") { + filteredTerminals = append(filteredTerminals, t) + } + } + + lookup := make(map[strfmt.UUID]interface{}) for _, artifact := range build.Artifacts { lookup[artifact.NodeID] = artifact } @@ -287,80 +311,108 @@ func BuildtimeArtifacts(build *model.Build, includeBuilders bool) (artifact.Map, lookup[source.NodeID] = source } - for _, a := range build.Artifacts { - if !includeBuilders && a.MimeType == model.XActiveStateBuilderMimeType { - continue + var terminalTargetIDs []strfmt.UUID + for _, terminal := range filteredTerminals { + // If there is an artifact for this terminal and its mime type is not a state tool artifact + // then we need to recurse back through the DAG until we find nodeIDs that are state tool + // artifacts. These are the terminal targets. + for _, nodeID := range terminal.NodeIDs { + buildTerminals(nodeID, lookup, &terminalTargetIDs) } + } + logging.Debug("Terminal Targets: %v", terminalTargetIDs) - _, ok := result[strfmt.UUID(a.NodeID)] - // We are only interested in artifacts that have not already been added - // to the result and that have been submitted to be built. - if !ok && a.Status != model.ArtifactNotSubmitted { - // deps here refer to the dependencies of the artifact itself. - // This includes the direct dependencies, which we get through - // the RuntimeDependencies field, as well as the inputs of the - // step that generated the artifact. - deps := make(map[strfmt.UUID]struct{}) - for _, depID := range a.RuntimeDependencies { - artifact, ok := lookup[depID].(*model.Artifact) - if ok { - if !includeBuilders && artifact.MimeType == model.XActiveStateBuilderMimeType { - continue - } - } - - // Add our current dependency to the map of dependencies - // and then recursively add all of its dependencies. - deps[depID] = struct{}{} - recursiveDeps, err := generateBuildtimeDependencies(depID, includeBuilders, lookup, deps) - if err != nil { - return nil, errs.Wrap(err, "Could not resolve runtime dependencies for artifact: %s", depID) - } - - // This is a list of all of the dependencies of the current - // artifact, including the dependencies of its dependencies. - for id := range recursiveDeps { - deps[id] = struct{}{} - } - } + result := make(artifact.Map) + // TODO: How can we calculate the build time closure from the terminal targets? + // From the terminal targets we iterate through the runtime dependencies as we + // normall do but this time when we get to a step we also include it's dependencies. + // This will include the builder artifacts. + // We also need to inspect each dependencies to ensure that it was generated by a step + // and not a source. If the depenedency was generated by a source then it will not + // be built so we don't need to worry about events from it. + for _, id := range terminalTargetIDs { + err = buildBuildClosureMap(id, lookup, result) + if err != nil { + return nil, errs.Wrap(err, "Could not build map for terminal %s", id) + } + } - // We need to convert the map of dependencies to a list of - // dependencies. - var uniqueDeps []strfmt.UUID - for depID := range deps { - uniqueDeps = append(uniqueDeps, depID) - } + return result, nil +} - // We need to get the source information for the artifact. - // This is done by looking at the step that generated the - // artifact and then looking at the source that was used - // in that step. - info, err := getSourceInfo(a.GeneratedBy, lookup) - if err != nil { - return nil, errs.Wrap(err, "Could not resolve source information") - } +func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { + currentArtifact, ok := lookup[baseID].(*model.Artifact) + if !ok { + return errs.New("Incorrect target type for id %s, expected Artifact", baseID) + } - result[strfmt.UUID(a.NodeID)] = artifact.Artifact{ - ArtifactID: strfmt.UUID(a.NodeID), - Name: info.Name, - Namespace: info.Namespace, - Version: &info.Version, - RequestedByOrder: false, - GeneratedBy: a.GeneratedBy, - Dependencies: uniqueDeps, - } + _, ok = result[strfmt.UUID(currentArtifact.NodeID)] + // We are only interested in artifacts that have not already been added + // to the result and that have been submitted to be built. + if ok || currentArtifact.Status == model.ArtifactNotSubmitted { + return nil + } + + // deps here refer to the dependencies of the artifact itself. + // This includes the direct dependencies, which we get through + // the RuntimeDependencies field, as well as the inputs of the + // step that generated the artifact. + deps := make(map[strfmt.UUID]struct{}) + for _, depID := range currentArtifact.RuntimeDependencies { + // Add our current dependency to the map of dependencies + // and then recursively add all of its dependencies. + deps[depID] = struct{}{} + recursiveDeps, err := generateBuildtimeDependencies(depID, lookup, deps) + if err != nil { + return errs.Wrap(err, "Could not resolve runtime dependencies for artifact: %s", depID) + } + + // This is a list of all of the dependencies of the current + // artifact, including the dependencies of its dependencies. + for a := range recursiveDeps { + deps[a] = struct{}{} + } + + err = buildBuildClosureMap(depID, lookup, result) + if err != nil { + return errs.Wrap(err, "Could not build map for runtime dependency %s", currentArtifact.NodeID) } } - return result, nil + // We need to convert the map of dependencies to a list of + // dependencies. + var uniqueDeps []strfmt.UUID + for depID := range deps { + uniqueDeps = append(uniqueDeps, depID) + } + + // We need to get the source information for the artifact. + // This is done by looking at the step that generated the + // artifact and then looking at the source that was used + // in that step. + info, err := getSourceInfo(currentArtifact.GeneratedBy, lookup) + if err != nil { + return errs.Wrap(err, "Could not resolve source information") + } + + result[strfmt.UUID(currentArtifact.NodeID)] = artifact.Artifact{ + ArtifactID: strfmt.UUID(currentArtifact.NodeID), + Name: info.Name, + Namespace: info.Namespace, + Version: &info.Version, + RequestedByOrder: false, + GeneratedBy: currentArtifact.GeneratedBy, + Dependencies: uniqueDeps, + } + + return nil } // generateBuildtimeDependencies recursively iterates through an artifacts dependencies // looking to the step that generated the artifact and then to ALL of the artifacts that // are inputs to that step. This will lead to the result containing more than what is in -// the runtime closure. The includeBuilders argument is used to determine if we should -// include artifacts that are builders. -func generateBuildtimeDependencies(artifactID strfmt.UUID, includeBuilders bool, lookup map[strfmt.UUID]interface{}, result map[strfmt.UUID]struct{}) (map[strfmt.UUID]struct{}, error) { +// the runtime closure. +func generateBuildtimeDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result map[strfmt.UUID]struct{}) (map[strfmt.UUID]struct{}, error) { artifact, ok := lookup[artifactID].(*model.Artifact) if !ok { _, sourceOK := lookup[artifactID].(*model.Source) @@ -368,18 +420,23 @@ func generateBuildtimeDependencies(artifactID strfmt.UUID, includeBuilders bool, // Dependency is a source, skipping return nil, nil } + return nil, errs.New("Incorrect target type for id %s, expected Artifact or Source", artifactID) } - if !includeBuilders && artifact.MimeType == model.XActiveStateBuilderMimeType { + if artifact.MimeType == model.XActiveStateBuilderMimeType { + // Dependency is a builder, skipping return nil, nil } + // Once we have verified that the artifact has a step that generated it + // we add it to the result map. + result[artifactID] = struct{}{} + // We iterate through the direct dependencies of the artifact // and recursively add all of the dependencies of those artifacts map. for _, depID := range artifact.RuntimeDependencies { - result[depID] = struct{}{} - _, err := generateBuildtimeDependencies(depID, includeBuilders, lookup, result) + _, err := generateBuildtimeDependencies(depID, lookup, result) if err != nil { return nil, errs.Wrap(err, "Could not build map for runtime dependencies of artifact %s", artifact.NodeID) } @@ -387,26 +444,24 @@ func generateBuildtimeDependencies(artifactID strfmt.UUID, includeBuilders bool, step, ok := lookup[artifact.GeneratedBy].(*model.Step) if !ok { + // Artifact was not generated by a step or a source, meaning that + // the buildplan is likely malformed. _, ok := lookup[artifact.GeneratedBy].(*model.Source) if !ok { return nil, errs.New("Incorrect target type for id %s, expected Step or Source", artifact.GeneratedBy) } - // Artifact was not generated by a step, skipping + // Artifact was not generated by a step, skipping because these + // artifacts do not need to be built. return nil, nil } // We iterate through the inputs of the step that generated the - // artifact and recursively add all of the dependencies of those - // artifacts, skipping over the builder artifacts as those resolve - // directly to sources. This is because they are not built and therefore - // not generated by a step. + // artifact and recursively add all of the dependencies and builders + // of those artifacts. for _, input := range step.Inputs { - if input.Tag == model.TagBuilder { - continue - } for _, id := range input.NodeIDs { - _, err := generateBuildtimeDependencies(id, includeBuilders, lookup, result) + _, err := generateBuildtimeDependencies(id, lookup, result) if err != nil { return nil, errs.Wrap(err, "Could not build map for step dependencies of artifact %s", artifact.NodeID) } diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 5dd8a87023..0d5ebcd7aa 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -481,7 +481,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal ) artifactsToInstall := []artifact.ArtifactID{} - // buildtimeArtifacts := runtimeArtifacts + buildtimeArtifacts := runtimeArtifacts if buildResult.BuildReady { // If the build is already done we can just look at the downloadable artifacts as they will be a fully accurate // prediction of what we will be installing. @@ -498,6 +498,19 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal artifactsToInstall = append(artifactsToInstall, a.ArtifactID) } } + + // We also caclulate the artifacts to be built which includes more than the runtime artifacts. + // This is used to determine if we need to show the "build in progress" screen. + buildtimeArtifacts, err = buildplan.BuildtimeArtifacts(buildResult.Build) + if err != nil { + return nil, nil, errs.Wrap(err, "Could not get buildtime artifacts") + } + + buildList := []string{} + for _, a := range buildtimeArtifacts { + buildList = append(buildList, artifactNames[a.ArtifactID]) + } + logging.Debug("Buildtime artifacts: %v", buildList) } // The log file we want to use for builds @@ -514,7 +527,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal ArtifactNames: artifactNames, LogFilePath: logFilePath, ArtifactsToBuild: func() []artifact.ArtifactID { - return artifact.ArtifactIDsFromBuildPlanMap(runtimeArtifacts) // This does not account for cached builds + return artifact.ArtifactIDsFromBuildPlanMap(buildtimeArtifacts) // This does not account for cached builds }(), // Yes these have the same value; this is intentional. // Separating these out just allows us to be more explicit and intentional in our event handling logic. From c875c3e9922da64824f9485dd753e3f5520e6ea9 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 26 Sep 2023 09:52:41 -0700 Subject: [PATCH 02/32] We are not interested in step sources --- pkg/platform/runtime/buildplan/buildplan.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index d316b96b19..fce2d8c230 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -323,13 +323,6 @@ func BuildtimeArtifacts(build *model.Build) (artifact.Map, error) { logging.Debug("Terminal Targets: %v", terminalTargetIDs) result := make(artifact.Map) - // TODO: How can we calculate the build time closure from the terminal targets? - // From the terminal targets we iterate through the runtime dependencies as we - // normall do but this time when we get to a step we also include it's dependencies. - // This will include the builder artifacts. - // We also need to inspect each dependencies to ensure that it was generated by a step - // and not a source. If the depenedency was generated by a source then it will not - // be built so we don't need to worry about events from it. for _, id := range terminalTargetIDs { err = buildBuildClosureMap(id, lookup, result) if err != nil { @@ -355,8 +348,8 @@ func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{} // deps here refer to the dependencies of the artifact itself. // This includes the direct dependencies, which we get through - // the RuntimeDependencies field, as well as the inputs of the - // step that generated the artifact. + // the RuntimeDependencies field, as well as the inputs AND + // dependencies of the step that generated the artifact. deps := make(map[strfmt.UUID]struct{}) for _, depID := range currentArtifact.RuntimeDependencies { // Add our current dependency to the map of dependencies @@ -373,6 +366,8 @@ func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{} deps[a] = struct{}{} } + // For each runtime dependency we need to add its dependencies + // to the result map. err = buildBuildClosureMap(depID, lookup, result) if err != nil { return errs.Wrap(err, "Could not build map for runtime dependency %s", currentArtifact.NodeID) @@ -460,6 +455,10 @@ func generateBuildtimeDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI // artifact and recursively add all of the dependencies and builders // of those artifacts. for _, input := range step.Inputs { + if input.Tag != model.TagDependency || input.Tag != model.TagBuilder { + continue + } + for _, id := range input.NodeIDs { _, err := generateBuildtimeDependencies(id, lookup, result) if err != nil { From 95f2dd14eb4a4b3296868849922d66951d432e4b Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Tue, 26 Sep 2023 13:08:14 -0700 Subject: [PATCH 03/32] Get build time artifact calculation working --- internal/constants/constants.go | 3 ++ .../api/buildplanner/model/buildplan.go | 2 +- pkg/platform/model/buildplanner.go | 7 ++--- pkg/platform/runtime/buildplan/buildplan.go | 29 +++++++++---------- pkg/platform/runtime/setup/setup.go | 21 +++++++++++--- 5 files changed, 37 insertions(+), 25 deletions(-) diff --git a/internal/constants/constants.go b/internal/constants/constants.go index 335c57082e..81118222fa 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -62,6 +62,9 @@ const DisableUpdates = "ACTIVESTATE_CLI_DISABLE_UPDATES" // UpdateBranchEnvVarName is the env var that is used to override which branch to pull the update from const UpdateBranchEnvVarName = "ACTIVESTATE_CLI_UPDATE_BRANCH" +// InstallBuildDependencies is the env var that is used to override whether to install build dependencies +const InstallBuildDependencies = "ACTIVESTATE_CLI_INSTALL_BUILD_DEPENDENCIES" + // InternalConfigFileNameLegacy is effectively the same as InternalConfigName, but includes our preferred extension const InternalConfigFileNameLegacy = "config.yaml" diff --git a/pkg/platform/api/buildplanner/model/buildplan.go b/pkg/platform/api/buildplanner/model/buildplan.go index ef65076a09..ec26a5b6ac 100644 --- a/pkg/platform/api/buildplanner/model/buildplan.go +++ b/pkg/platform/api/buildplanner/model/buildplan.go @@ -36,7 +36,7 @@ const ( // Tag types TagSource = "src" - TagDependency = "dep" + TagDependency = "deps" TagBuilder = "builder" TagOrphan = "orphans" diff --git a/pkg/platform/model/buildplanner.go b/pkg/platform/model/buildplanner.go index 28f0a1adb9..a3673080de 100644 --- a/pkg/platform/model/buildplanner.go +++ b/pkg/platform/model/buildplanner.go @@ -156,10 +156,9 @@ func (bp *BuildPlanner) FetchBuildResult(commitID strfmt.UUID, owner, project st } res := BuildResult{ - BuildEngine: buildEngine, - Build: build, - // BuildReady: build.Status == bpModel.Completed, - BuildReady: false, + BuildEngine: buildEngine, + Build: build, + BuildReady: build.Status == bpModel.Completed, CommitID: id, BuildExpression: expr, BuildStatus: build.Status, diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index fce2d8c230..9df2090003 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -339,10 +339,8 @@ func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{} return errs.New("Incorrect target type for id %s, expected Artifact", baseID) } - _, ok = result[strfmt.UUID(currentArtifact.NodeID)] - // We are only interested in artifacts that have not already been added - // to the result and that have been submitted to be built. - if ok || currentArtifact.Status == model.ArtifactNotSubmitted { + if currentArtifact.MimeType == model.XActiveStateBuilderMimeType { + // Dependency is a builder, skipping return nil } @@ -365,13 +363,6 @@ func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{} for a := range recursiveDeps { deps[a] = struct{}{} } - - // For each runtime dependency we need to add its dependencies - // to the result map. - err = buildBuildClosureMap(depID, lookup, result) - if err != nil { - return errs.Wrap(err, "Could not build map for runtime dependency %s", currentArtifact.NodeID) - } } // We need to convert the map of dependencies to a list of @@ -379,6 +370,13 @@ func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{} var uniqueDeps []strfmt.UUID for depID := range deps { uniqueDeps = append(uniqueDeps, depID) + + // For each buildtime dependency we need to add it and its dependencies + // to the result map. + err := buildBuildClosureMap(depID, lookup, result) + if err != nil { + return errs.Wrap(err, "Could not build map for runtime dependency %s", currentArtifact.NodeID) + } } // We need to get the source information for the artifact. @@ -419,18 +417,17 @@ func generateBuildtimeDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI return nil, errs.New("Incorrect target type for id %s, expected Artifact or Source", artifactID) } + result[artifactID] = struct{}{} + if artifact.MimeType == model.XActiveStateBuilderMimeType { // Dependency is a builder, skipping return nil, nil } - // Once we have verified that the artifact has a step that generated it - // we add it to the result map. - result[artifactID] = struct{}{} - // We iterate through the direct dependencies of the artifact // and recursively add all of the dependencies of those artifacts map. for _, depID := range artifact.RuntimeDependencies { + result[artifactID] = struct{}{} _, err := generateBuildtimeDependencies(depID, lookup, result) if err != nil { return nil, errs.Wrap(err, "Could not build map for runtime dependencies of artifact %s", artifact.NodeID) @@ -455,7 +452,7 @@ func generateBuildtimeDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI // artifact and recursively add all of the dependencies and builders // of those artifacts. for _, input := range step.Inputs { - if input.Tag != model.TagDependency || input.Tag != model.TagBuilder { + if input.Tag != model.TagDependency && input.Tag != model.TagBuilder { continue } diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 0d5ebcd7aa..7f96bf7901 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -382,16 +382,21 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // Compute and handle the change summary // runtimeAndBuildtimeArtifacts records all artifacts that will need to be built in order to obtain the runtime. - // Disabled due to DX-2033. - // Please use this var when we come back to this in the future as we need to make a clear distinction between this - // and runtime-only artifacts. - // var runtimeAndBuildtimeArtifacts artifact.Map + var runtimeAndBuildtimeArtifacts artifact.Map var runtimeArtifacts artifact.Map // Artifacts required for the runtime to function if buildResult.Build != nil { runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } + + if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { + runtimeAndBuildtimeArtifacts, err = buildplan.BuildtimeArtifacts(buildResult.Build) + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") + } + runtimeArtifacts = runtimeAndBuildtimeArtifacts + } } setup, err := s.selectSetupImplementation(buildResult.BuildEngine, runtimeArtifacts) @@ -506,6 +511,14 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal return nil, nil, errs.Wrap(err, "Could not get buildtime artifacts") } + buildtimeArtifactIDs := []artifact.ArtifactID{} + for _, a := range buildtimeArtifacts { + buildtimeArtifactIDs = append(buildtimeArtifactIDs, a.ArtifactID) + } + + // Update artifactNames to ensure it now includes buildtime artifacts + artifactNames = artifact.ResolveArtifactNames(setup.ResolveArtifactName, buildtimeArtifactIDs) + buildList := []string{} for _, a := range buildtimeArtifacts { buildList = append(buildList, artifactNames[a.ArtifactID]) From 65829315ec7e2a3eb56f85d64a64f91657122e73 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Wed, 27 Sep 2023 14:23:22 -0700 Subject: [PATCH 04/32] Rework build closure calculation --- pkg/platform/runtime/buildplan/buildplan.go | 116 ++++++++++++++------ pkg/platform/runtime/setup/setup.go | 42 +------ 2 files changed, 86 insertions(+), 72 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 9df2090003..76a3726e78 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -320,7 +320,6 @@ func BuildtimeArtifacts(build *model.Build) (artifact.Map, error) { buildTerminals(nodeID, lookup, &terminalTargetIDs) } } - logging.Debug("Terminal Targets: %v", terminalTargetIDs) result := make(artifact.Map) for _, id := range terminalTargetIDs { @@ -334,58 +333,104 @@ func BuildtimeArtifacts(build *model.Build) (artifact.Map, error) { } func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { - currentArtifact, ok := lookup[baseID].(*model.Artifact) + if _, ok := result[baseID]; ok { + // We have already processed this artifact, skipping + return nil + } + + target := lookup[baseID] + currentArtifact, ok := target.(*model.Artifact) if !ok { return errs.New("Incorrect target type for id %s, expected Artifact", baseID) } - if currentArtifact.MimeType == model.XActiveStateBuilderMimeType { - // Dependency is a builder, skipping - return nil + deps := make(map[strfmt.UUID]struct{}) + buildTimeDeps, err := buildBuildClosureDependencies(baseID, lookup, deps, result) + if err != nil { + return errs.Wrap(err, "Could not build buildtime dependencies for artifact %s", baseID) + } + + var uniqueDeps []strfmt.UUID + for id := range buildTimeDeps { + if _, ok := deps[id]; !ok { + continue + } + uniqueDeps = append(uniqueDeps, id) + } + + info, err := getSourceInfo(currentArtifact.GeneratedBy, lookup) + if err != nil { + return errs.Wrap(err, "Could not resolve source information") + } + + result[strfmt.UUID(currentArtifact.NodeID)] = artifact.Artifact{ + ArtifactID: strfmt.UUID(currentArtifact.NodeID), + Name: info.Name, + Namespace: info.Namespace, + Version: &info.Version, + RequestedByOrder: true, + GeneratedBy: currentArtifact.GeneratedBy, + Dependencies: uniqueDeps, + URL: currentArtifact.URL, + } + + return nil +} + +func buildBuildClosureDependencies(depdendencyID strfmt.UUID, lookup map[strfmt.UUID]interface{}, deps map[strfmt.UUID]struct{}, result artifact.Map) (map[strfmt.UUID]struct{}, error) { + currentArtifact, ok := lookup[depdendencyID].(*model.Artifact) + if !ok { + return nil, errs.New("Incorrect target type for id %s, expected Artifact", depdendencyID) } - // deps here refer to the dependencies of the artifact itself. - // This includes the direct dependencies, which we get through - // the RuntimeDependencies field, as well as the inputs AND - // dependencies of the step that generated the artifact. - deps := make(map[strfmt.UUID]struct{}) for _, depID := range currentArtifact.RuntimeDependencies { - // Add our current dependency to the map of dependencies - // and then recursively add all of its dependencies. deps[depID] = struct{}{} - recursiveDeps, err := generateBuildtimeDependencies(depID, lookup, deps) + artifactDeps := make(map[strfmt.UUID]struct{}) + _, err := buildBuildClosureDependencies(depID, lookup, artifactDeps, result) if err != nil { - return errs.Wrap(err, "Could not resolve runtime dependencies for artifact: %s", depID) + return nil, errs.Wrap(err, "Could not build map for runtime dependencies of artifact %s", currentArtifact.NodeID) } + } - // This is a list of all of the dependencies of the current - // artifact, including the dependencies of its dependencies. - for a := range recursiveDeps { - deps[a] = struct{}{} + step, ok := lookup[currentArtifact.GeneratedBy].(*model.Step) + if !ok { + // Artifact was not generated by a step or a source, meaning that + // the buildplan is likely malformed. + _, ok := lookup[currentArtifact.GeneratedBy].(*model.Source) + if !ok { + return nil, errs.New("Incorrect target type for id %s, expected Step or Source", currentArtifact.GeneratedBy) } + + // Artifact was not generated by a step, skipping because these + // artifacts do not need to be built. + return nil, nil } - // We need to convert the map of dependencies to a list of - // dependencies. - var uniqueDeps []strfmt.UUID - for depID := range deps { - uniqueDeps = append(uniqueDeps, depID) + for _, input := range step.Inputs { + if input.Tag != model.TagDependency && input.Tag != model.TagBuilder { + continue + } - // For each buildtime dependency we need to add it and its dependencies - // to the result map. - err := buildBuildClosureMap(depID, lookup, result) - if err != nil { - return errs.Wrap(err, "Could not build map for runtime dependency %s", currentArtifact.NodeID) + for _, inputID := range input.NodeIDs { + deps[inputID] = struct{}{} + _, err := buildBuildClosureDependencies(inputID, lookup, deps, result) + if err != nil { + return nil, errs.Wrap(err, "Could not build map for step dependencies of artifact %s", currentArtifact.NodeID) + } } } - // We need to get the source information for the artifact. - // This is done by looking at the step that generated the - // artifact and then looking at the source that was used - // in that step. + var uniqueDeps []strfmt.UUID + for id := range deps { + if _, ok := deps[id]; !ok { + continue + } + uniqueDeps = append(uniqueDeps, id) + } + info, err := getSourceInfo(currentArtifact.GeneratedBy, lookup) if err != nil { - return errs.Wrap(err, "Could not resolve source information") + return nil, errs.Wrap(err, "Could not resolve source information") } result[strfmt.UUID(currentArtifact.NodeID)] = artifact.Artifact{ @@ -393,12 +438,13 @@ func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{} Name: info.Name, Namespace: info.Namespace, Version: &info.Version, - RequestedByOrder: false, + RequestedByOrder: true, GeneratedBy: currentArtifact.GeneratedBy, Dependencies: uniqueDeps, + URL: currentArtifact.URL, } - return nil + return deps, nil } // generateBuildtimeDependencies recursively iterates through an artifacts dependencies diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 7f96bf7901..efd6e8a6e8 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -390,7 +390,8 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } - if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { + if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") || !buildResult.BuildReady { + logging.Debug("Installing build dependencies") runtimeAndBuildtimeArtifacts, err = buildplan.BuildtimeArtifacts(buildResult.Build) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") @@ -487,43 +488,10 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal artifactsToInstall := []artifact.ArtifactID{} buildtimeArtifacts := runtimeArtifacts - if buildResult.BuildReady { - // If the build is already done we can just look at the downloadable artifacts as they will be a fully accurate - // prediction of what we will be installing. - for _, a := range downloadablePrebuiltResults { - if _, alreadyInstalled := alreadyInstalled[a.ArtifactID]; !alreadyInstalled { - artifactsToInstall = append(artifactsToInstall, a.ArtifactID) - } - } - } else { - // If the build is not yet complete then we have to speculate as to the artifacts that will be installed. - // The actual number of installable artifacts may be lower than what we have here, we can only do a best effort. - for _, a := range runtimeArtifacts { - if _, alreadyInstalled := alreadyInstalled[a.ArtifactID]; !alreadyInstalled { - artifactsToInstall = append(artifactsToInstall, a.ArtifactID) - } - } - - // We also caclulate the artifacts to be built which includes more than the runtime artifacts. - // This is used to determine if we need to show the "build in progress" screen. - buildtimeArtifacts, err = buildplan.BuildtimeArtifacts(buildResult.Build) - if err != nil { - return nil, nil, errs.Wrap(err, "Could not get buildtime artifacts") - } - - buildtimeArtifactIDs := []artifact.ArtifactID{} - for _, a := range buildtimeArtifacts { - buildtimeArtifactIDs = append(buildtimeArtifactIDs, a.ArtifactID) - } - - // Update artifactNames to ensure it now includes buildtime artifacts - artifactNames = artifact.ResolveArtifactNames(setup.ResolveArtifactName, buildtimeArtifactIDs) - - buildList := []string{} - for _, a := range buildtimeArtifacts { - buildList = append(buildList, artifactNames[a.ArtifactID]) + for _, a := range downloadablePrebuiltResults { + if _, alreadyInstalled := alreadyInstalled[a.ArtifactID]; !alreadyInstalled { + artifactsToInstall = append(artifactsToInstall, a.ArtifactID) } - logging.Debug("Buildtime artifacts: %v", buildList) } // The log file we want to use for builds From 4f7f3a69ec2913aae5cd1b58495b4038d5b32cc0 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 28 Sep 2023 10:02:13 -0700 Subject: [PATCH 05/32] Add integration test Adjust setup implementation to handle build in progress and env var being set --- pkg/platform/runtime/buildplan/buildplan.go | 4 +-- pkg/platform/runtime/setup/setup.go | 27 ++++++++++++++++----- test/integration/checkout_int_test.go | 20 +++++++++++++++ 3 files changed, 43 insertions(+), 8 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 76a3726e78..dd924d4a7c 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -271,12 +271,12 @@ func NewNamedMapFromBuildPlan(build *model.Build) (artifact.NamedMap, error) { return res, nil } -// BuildtimeArtifacts iterates through all artifacts in a given build and +// NewBuildtimeMapFromBuildPlan iterates through all artifacts in a given build and // adds the artifact's dependencies to a map. This is different from the // runtime dependency calculation as it includes ALL of the input artifacts of the // step that generated each artifact. The includeBuilders argument determines whether // or not to include builder artifacts in the final result. -func BuildtimeArtifacts(build *model.Build) (artifact.Map, error) { +func NewBuildtimeMapFromBuildPlan(build *model.Build) (artifact.Map, error) { // Extract the available platforms from the build plan var bpPlatforms []strfmt.UUID for _, t := range build.Terminals { diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index efd6e8a6e8..3071a5badf 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -391,12 +391,15 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") || !buildResult.BuildReady { - logging.Debug("Installing build dependencies") - runtimeAndBuildtimeArtifacts, err = buildplan.BuildtimeArtifacts(buildResult.Build) + runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMapFromBuildPlan(buildResult.Build) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } - runtimeArtifacts = runtimeAndBuildtimeArtifacts + + if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { + logging.Debug("Installing build dependencies") + runtimeArtifacts = runtimeAndBuildtimeArtifacts + } } } @@ -464,8 +467,14 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // Report resolved artifacts artifactIDs := []artifact.ArtifactID{} - for _, a := range runtimeArtifacts { - artifactIDs = append(artifactIDs, a.ArtifactID) + if runtimeAndBuildtimeArtifacts != nil { + for _, a := range runtimeAndBuildtimeArtifacts { + artifactIDs = append(artifactIDs, a.ArtifactID) + } + } else { + for _, a := range runtimeArtifacts { + artifactIDs = append(artifactIDs, a.ArtifactID) + } } artifactNames := artifact.ResolveArtifactNames(setup.ResolveArtifactName, artifactIDs) @@ -487,13 +496,19 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal ) artifactsToInstall := []artifact.ArtifactID{} - buildtimeArtifacts := runtimeArtifacts for _, a := range downloadablePrebuiltResults { if _, alreadyInstalled := alreadyInstalled[a.ArtifactID]; !alreadyInstalled { artifactsToInstall = append(artifactsToInstall, a.ArtifactID) } } + var buildtimeArtifacts artifact.Map + if runtimeAndBuildtimeArtifacts != nil { + buildtimeArtifacts = runtimeAndBuildtimeArtifacts + } else { + buildtimeArtifacts = runtimeArtifacts + } + // The log file we want to use for builds logFilePath := logging.FilePathFor(fmt.Sprintf("build-%s.log", s.target.CommitUUID().String()+"-"+time.Now().Format("20060102150405"))) diff --git a/test/integration/checkout_int_test.go b/test/integration/checkout_int_test.go index eaf23b1b6e..f02ecd7a5f 100644 --- a/test/integration/checkout_int_test.go +++ b/test/integration/checkout_int_test.go @@ -241,6 +241,26 @@ func (suite *CheckoutIntegrationTestSuite) TestCheckoutCaseInsensitive() { suite.Assert().NotContains(string(bytes), "ACTIVESTATE-CLI/SMALL-PYTHON", "kept incorrect namespace case") } +func (suite *CheckoutIntegrationTestSuite) TestCheckoutBuildtimeClosure() { + suite.OnlyRunForTags(tagsuite.Checkout) + + if runtime.GOOS != "linux" { + suite.T().Skip("Skipping buildtime closure test on non-linux platform") + } + + ts := e2e.New(suite.T(), false) + defer ts.Close() + + cp := ts.SpawnWithOpts( + e2e.WithArgs("checkout", "ActiveState-CLI/small-python"), + e2e.AppendEnv(constants.InstallBuildDependencies+"=true"), + e2e.AppendEnv(constants.DisableRuntime+"=false"), + ) + // Expect the number of build deps to be 27 which is more than the number of runtime deps. + cp.Expect("27") + cp.ExpectExitCode(0) +} + func TestCheckoutIntegrationTestSuite(t *testing.T) { suite.Run(t, new(CheckoutIntegrationTestSuite)) } From f4922cea89bd5bb68207d85a5f317cefbea61f46 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 28 Sep 2023 10:16:49 -0700 Subject: [PATCH 06/32] Fix test build failure --- test/integration/checkout_int_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/integration/checkout_int_test.go b/test/integration/checkout_int_test.go index 9ee9611fb6..517dc9e2f3 100644 --- a/test/integration/checkout_int_test.go +++ b/test/integration/checkout_int_test.go @@ -255,9 +255,9 @@ func (suite *CheckoutIntegrationTestSuite) TestCheckoutBuildtimeClosure() { defer ts.Close() cp := ts.SpawnWithOpts( - e2e.WithArgs("checkout", "ActiveState-CLI/small-python"), - e2e.AppendEnv(constants.InstallBuildDependencies+"=true"), - e2e.AppendEnv(constants.DisableRuntime+"=false"), + e2e.OptArgs("checkout", "ActiveState-CLI/small-python"), + e2e.OptAppendEnv(constants.InstallBuildDependencies+"=true"), + e2e.OptAppendEnv(constants.DisableRuntime+"=false"), ) // Expect the number of build deps to be 27 which is more than the number of runtime deps. cp.Expect("27") From f910770928ea90ed5f88c8c49759502dbb5ecbbf Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 28 Sep 2023 14:03:16 -0700 Subject: [PATCH 07/32] Skip artifacts that are already in the results --- pkg/platform/runtime/buildplan/buildplan.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index dd924d4a7c..9295da3e04 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -377,10 +377,15 @@ func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{} return nil } -func buildBuildClosureDependencies(depdendencyID strfmt.UUID, lookup map[strfmt.UUID]interface{}, deps map[strfmt.UUID]struct{}, result artifact.Map) (map[strfmt.UUID]struct{}, error) { - currentArtifact, ok := lookup[depdendencyID].(*model.Artifact) +func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUID]interface{}, deps map[strfmt.UUID]struct{}, result artifact.Map) (map[strfmt.UUID]struct{}, error) { + if _, ok := result[artifactID]; ok { + // We have already processed this artifact, skipping + return nil, nil + } + + currentArtifact, ok := lookup[artifactID].(*model.Artifact) if !ok { - return nil, errs.New("Incorrect target type for id %s, expected Artifact", depdendencyID) + return nil, errs.New("Incorrect target type for id %s, expected Artifact", artifactID) } for _, depID := range currentArtifact.RuntimeDependencies { From e41af5da37a15eb1da730b40ab3e8c3edb90b0cf Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 28 Sep 2023 14:53:07 -0700 Subject: [PATCH 08/32] Attempt to cleanup setup code --- pkg/platform/runtime/setup/setup.go | 42 ++++++++++++++++++----------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 3071a5badf..11482a82b2 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -384,26 +384,38 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // runtimeAndBuildtimeArtifacts records all artifacts that will need to be built in order to obtain the runtime. var runtimeAndBuildtimeArtifacts artifact.Map var runtimeArtifacts artifact.Map // Artifacts required for the runtime to function - if buildResult.Build != nil { - runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build) + + // If the build is not ready, we need to get the runtime and buildtime closure + if !buildResult.BuildReady { + runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMapFromBuildPlan(buildResult.Build) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } + } - if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") || !buildResult.BuildReady { + // If we are installing build dependencies, then buildtime dependences are also runtime dependencies + if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { + logging.Debug("Installing build dependencies") + if runtimeAndBuildtimeArtifacts == nil { runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMapFromBuildPlan(buildResult.Build) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } - - if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { - logging.Debug("Installing build dependencies") - runtimeArtifacts = runtimeAndBuildtimeArtifacts - } + } + runtimeArtifacts = runtimeAndBuildtimeArtifacts + } else { + runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build) + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } } - setup, err := s.selectSetupImplementation(buildResult.BuildEngine, runtimeArtifacts) + var setup Setuper + if !buildResult.BuildReady { + setup, err = s.selectSetupImplementation(buildResult.BuildEngine, runtimeAndBuildtimeArtifacts) + } else { + setup, err = s.selectSetupImplementation(buildResult.BuildEngine, runtimeArtifacts) + } if err != nil { return nil, nil, errs.Wrap(err, "Failed to select setup implementation") } @@ -467,7 +479,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // Report resolved artifacts artifactIDs := []artifact.ArtifactID{} - if runtimeAndBuildtimeArtifacts != nil { + if !buildResult.BuildReady { for _, a := range runtimeAndBuildtimeArtifacts { artifactIDs = append(artifactIDs, a.ArtifactID) } @@ -502,11 +514,11 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } } - var buildtimeArtifacts artifact.Map - if runtimeAndBuildtimeArtifacts != nil { - buildtimeArtifacts = runtimeAndBuildtimeArtifacts + var artifactsToBuild artifact.Map + if !buildResult.BuildReady { + artifactsToBuild = runtimeAndBuildtimeArtifacts } else { - buildtimeArtifacts = runtimeArtifacts + artifactsToBuild = runtimeArtifacts } // The log file we want to use for builds @@ -523,7 +535,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal ArtifactNames: artifactNames, LogFilePath: logFilePath, ArtifactsToBuild: func() []artifact.ArtifactID { - return artifact.ArtifactIDsFromBuildPlanMap(buildtimeArtifacts) // This does not account for cached builds + return artifact.ArtifactIDsFromBuildPlanMap(artifactsToBuild) // This does not account for cached builds }(), // Yes these have the same value; this is intentional. // Separating these out just allows us to be more explicit and intentional in our event handling logic. From ef23adff6a85c5b6db50a016d48e638008460f63 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 28 Sep 2023 15:37:31 -0700 Subject: [PATCH 09/32] Add comments and remove unused function --- pkg/platform/runtime/buildplan/buildplan.go | 75 +++------------------ 1 file changed, 8 insertions(+), 67 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 9295da3e04..7932255308 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -388,6 +388,9 @@ func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI return nil, errs.New("Incorrect target type for id %s, expected Artifact", artifactID) } + // We iterate through the direct dependencies of the artifact + // and recursively add all of the dependencies of those artifacts map. + // This is the same as the runtime closure calculation. for _, depID := range currentArtifact.RuntimeDependencies { deps[depID] = struct{}{} artifactDeps := make(map[strfmt.UUID]struct{}) @@ -397,6 +400,10 @@ func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI } } + // Here we iterate through the inputs of the step that generated the + // artifact, specifically the inputs that are tagged as dependencies. + // We recursively add all of the dependencies of the step intputs to + // the result map. This is the buildtime closure calculation. step, ok := lookup[currentArtifact.GeneratedBy].(*model.Step) if !ok { // Artifact was not generated by a step or a source, meaning that @@ -412,7 +419,7 @@ func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI } for _, input := range step.Inputs { - if input.Tag != model.TagDependency && input.Tag != model.TagBuilder { + if input.Tag != model.TagDependency { continue } @@ -451,69 +458,3 @@ func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI return deps, nil } - -// generateBuildtimeDependencies recursively iterates through an artifacts dependencies -// looking to the step that generated the artifact and then to ALL of the artifacts that -// are inputs to that step. This will lead to the result containing more than what is in -// the runtime closure. -func generateBuildtimeDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result map[strfmt.UUID]struct{}) (map[strfmt.UUID]struct{}, error) { - artifact, ok := lookup[artifactID].(*model.Artifact) - if !ok { - _, sourceOK := lookup[artifactID].(*model.Source) - if sourceOK { - // Dependency is a source, skipping - return nil, nil - } - - return nil, errs.New("Incorrect target type for id %s, expected Artifact or Source", artifactID) - } - - result[artifactID] = struct{}{} - - if artifact.MimeType == model.XActiveStateBuilderMimeType { - // Dependency is a builder, skipping - return nil, nil - } - - // We iterate through the direct dependencies of the artifact - // and recursively add all of the dependencies of those artifacts map. - for _, depID := range artifact.RuntimeDependencies { - result[artifactID] = struct{}{} - _, err := generateBuildtimeDependencies(depID, lookup, result) - if err != nil { - return nil, errs.Wrap(err, "Could not build map for runtime dependencies of artifact %s", artifact.NodeID) - } - } - - step, ok := lookup[artifact.GeneratedBy].(*model.Step) - if !ok { - // Artifact was not generated by a step or a source, meaning that - // the buildplan is likely malformed. - _, ok := lookup[artifact.GeneratedBy].(*model.Source) - if !ok { - return nil, errs.New("Incorrect target type for id %s, expected Step or Source", artifact.GeneratedBy) - } - - // Artifact was not generated by a step, skipping because these - // artifacts do not need to be built. - return nil, nil - } - - // We iterate through the inputs of the step that generated the - // artifact and recursively add all of the dependencies and builders - // of those artifacts. - for _, input := range step.Inputs { - if input.Tag != model.TagDependency && input.Tag != model.TagBuilder { - continue - } - - for _, id := range input.NodeIDs { - _, err := generateBuildtimeDependencies(id, lookup, result) - if err != nil { - return nil, errs.Wrap(err, "Could not build map for step dependencies of artifact %s", artifact.NodeID) - } - } - } - - return result, nil -} From e22b8993b9635650efa4da1c88f94acc250634e4 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Oct 2023 13:18:14 -0700 Subject: [PATCH 10/32] Rename functions --- pkg/platform/runtime/buildplan/buildplan.go | 8 ++++---- pkg/platform/runtime/setup/setup.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 7932255308..207246fc9d 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -271,12 +271,12 @@ func NewNamedMapFromBuildPlan(build *model.Build) (artifact.NamedMap, error) { return res, nil } -// NewBuildtimeMapFromBuildPlan iterates through all artifacts in a given build and +// NewBuildtimeMap iterates through all artifacts in a given build and // adds the artifact's dependencies to a map. This is different from the // runtime dependency calculation as it includes ALL of the input artifacts of the // step that generated each artifact. The includeBuilders argument determines whether // or not to include builder artifacts in the final result. -func NewBuildtimeMapFromBuildPlan(build *model.Build) (artifact.Map, error) { +func NewBuildtimeMap(build *model.Build) (artifact.Map, error) { // Extract the available platforms from the build plan var bpPlatforms []strfmt.UUID for _, t := range build.Terminals { @@ -323,7 +323,7 @@ func NewBuildtimeMapFromBuildPlan(build *model.Build) (artifact.Map, error) { result := make(artifact.Map) for _, id := range terminalTargetIDs { - err = buildBuildClosureMap(id, lookup, result) + err = newBuildClosureMap(id, lookup, result) if err != nil { return nil, errs.Wrap(err, "Could not build map for terminal %s", id) } @@ -332,7 +332,7 @@ func NewBuildtimeMapFromBuildPlan(build *model.Build) (artifact.Map, error) { return result, nil } -func buildBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { +func newBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { if _, ok := result[baseID]; ok { // We have already processed this artifact, skipping return nil diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 11482a82b2..a917e67834 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -387,7 +387,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // If the build is not ready, we need to get the runtime and buildtime closure if !buildResult.BuildReady { - runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMapFromBuildPlan(buildResult.Build) + runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMap(buildResult.Build) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } @@ -397,7 +397,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { logging.Debug("Installing build dependencies") if runtimeAndBuildtimeArtifacts == nil { - runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMapFromBuildPlan(buildResult.Build) + runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMap(buildResult.Build) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } From d029670773c5f5d63b6a411fce28f5da5b25a6c5 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Oct 2023 13:18:42 -0700 Subject: [PATCH 11/32] Only add platform terminals to list --- pkg/platform/runtime/buildplan/buildplan.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 207246fc9d..57c1b7d15e 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -280,7 +280,7 @@ func NewBuildtimeMap(build *model.Build) (artifact.Map, error) { // Extract the available platforms from the build plan var bpPlatforms []strfmt.UUID for _, t := range build.Terminals { - if t.Tag == model.TagOrphan { + if !strings.Contains(t.Tag, "platform:") { continue } bpPlatforms = append(bpPlatforms, strfmt.UUID(strings.TrimPrefix(t.Tag, "platform:"))) From 04443df8b300f91a61a1c91564c64aa4276e4b8e Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Oct 2023 13:50:59 -0700 Subject: [PATCH 12/32] Filter terminals when processing the build plan, not when fetching --- pkg/platform/model/buildplanner.go | 24 --------- pkg/platform/runtime/buildplan/buildplan.go | 56 +++++++++++++-------- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/pkg/platform/model/buildplanner.go b/pkg/platform/model/buildplanner.go index c1ef78b35f..a5112a9256 100644 --- a/pkg/platform/model/buildplanner.go +++ b/pkg/platform/model/buildplanner.go @@ -113,30 +113,6 @@ func (bp *BuildPlanner) FetchBuildResult(commitID strfmt.UUID, owner, project st // response with emtpy targets that we should remove removeEmptyTargets(build) - // Extract the available platforms from the build plan - var bpPlatforms []strfmt.UUID - for _, t := range build.Terminals { - if t.Tag == bpModel.TagOrphan { - continue - } - bpPlatforms = append(bpPlatforms, strfmt.UUID(strings.TrimPrefix(t.Tag, "platform:"))) - } - - // Get the platform ID for the current platform - platformID, err := FilterCurrentPlatform(HostPlatform, bpPlatforms) - if err != nil { - return nil, locale.WrapError(err, "err_filter_current_platform") - } - - // Filter the build terminals to only include the current platform - var filteredTerminals []*bpModel.NamedTarget - for _, t := range build.Terminals { - if platformID.String() == strings.TrimPrefix(t.Tag, "platform:") { - filteredTerminals = append(filteredTerminals, t) - } - } - build.Terminals = filteredTerminals - buildEngine := Alternative for _, s := range build.Sources { if s.Namespace == "builder" && s.Name == "camel" { diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 57c1b7d15e..8022344c19 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -31,8 +31,13 @@ func NewMapFromBuildPlan(build *model.Build) (artifact.Map, error) { lookup[source.NodeID] = source } + filtered, err := filterTerminals(build) + if err != nil { + return nil, errs.Wrap(err, "Could not filter terminals") + } + var terminalTargetIDs []strfmt.UUID - for _, terminal := range build.Terminals { + for _, terminal := range filtered { // If there is an artifact for this terminal and its mime type is not a state tool artifact // then we need to recurse back through the DAG until we find nodeIDs that are state tool // artifacts. These are the terminal targets. @@ -51,6 +56,33 @@ func NewMapFromBuildPlan(build *model.Build) (artifact.Map, error) { return res, nil } +func filterTerminals(build *model.Build) ([]*model.NamedTarget, error) { + // Extract the available platforms from the build plan + var bpPlatforms []strfmt.UUID + for _, t := range build.Terminals { + if !strings.Contains(t.Tag, "platform:") { + continue + } + bpPlatforms = append(bpPlatforms, strfmt.UUID(strings.TrimPrefix(t.Tag, "platform:"))) + } + + // Get the platform ID for the current platform + platformID, err := platformModel.FilterCurrentPlatform(platformModel.HostPlatform, bpPlatforms) + if err != nil { + return nil, locale.WrapError(err, "err_filter_current_platform") + } + + // Filter the build terminals to only include the current platform + var filteredTerminals []*model.NamedTarget + for _, t := range build.Terminals { + if platformID.String() == strings.TrimPrefix(t.Tag, "platform:") { + filteredTerminals = append(filteredTerminals, t) + } + } + + return filteredTerminals, nil +} + // buildTerminals recursively builds up a list of terminal targets. It expects an ID that // resolves to an artifact. If the artifact's mime type is that of a state tool artifact it // adds it to the terminal listing. Otherwise it looks up the step that generated the artifact @@ -277,27 +309,9 @@ func NewNamedMapFromBuildPlan(build *model.Build) (artifact.NamedMap, error) { // step that generated each artifact. The includeBuilders argument determines whether // or not to include builder artifacts in the final result. func NewBuildtimeMap(build *model.Build) (artifact.Map, error) { - // Extract the available platforms from the build plan - var bpPlatforms []strfmt.UUID - for _, t := range build.Terminals { - if !strings.Contains(t.Tag, "platform:") { - continue - } - bpPlatforms = append(bpPlatforms, strfmt.UUID(strings.TrimPrefix(t.Tag, "platform:"))) - } - - // Get the platform ID for the current platform - platformID, err := platformModel.FilterCurrentPlatform(platformModel.HostPlatform, bpPlatforms) + filteredTerminals, err := filterTerminals(build) if err != nil { - return nil, locale.WrapError(err, "err_filter_current_platform") - } - - // Filter the build terminals to only include the current platform - var filteredTerminals []*model.NamedTarget - for _, t := range build.Terminals { - if platformID.String() == strings.TrimPrefix(t.Tag, "platform:") { - filteredTerminals = append(filteredTerminals, t) - } + return nil, errs.Wrap(err, "Could not filter terminals") } lookup := make(map[strfmt.UUID]interface{}) From acc292d2d1f647ee433c7bc77215e7c76d617ae2 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Oct 2023 13:56:58 -0700 Subject: [PATCH 13/32] Avoid err type mismatches --- pkg/platform/runtime/buildplan/buildplan.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 8022344c19..afedb9b963 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -145,8 +145,7 @@ func buildRuntimeClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface deps[id] = struct{}{} } - err = buildRuntimeClosureMap(depID, lookup, result) - if err != nil { + if err := buildRuntimeClosureMap(depID, lookup, result); err != nil { return errs.Wrap(err, "Could not build map for runtime dependency %s", currentArtifact.NodeID) } } @@ -337,8 +336,7 @@ func NewBuildtimeMap(build *model.Build) (artifact.Map, error) { result := make(artifact.Map) for _, id := range terminalTargetIDs { - err = newBuildClosureMap(id, lookup, result) - if err != nil { + if err := newBuildClosureMap(id, lookup, result); err != nil { return nil, errs.Wrap(err, "Could not build map for terminal %s", id) } } From 516fd9a2715ad9aebd420ac1dcd3f88034903faa Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Oct 2023 14:18:26 -0700 Subject: [PATCH 14/32] Add comments --- pkg/platform/runtime/buildplan/buildplan.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index afedb9b963..9fe90b3112 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -344,6 +344,10 @@ func NewBuildtimeMap(build *model.Build) (artifact.Map, error) { return result, nil } +// newBuildClosureMap recursively builds the artifact map from the lookup table. +// If the current artifact is not already contained in the results map it first +// builds the artifacts build-time dependencies and then adds the artifact to the +// results map. func newBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { if _, ok := result[baseID]; ok { // We have already processed this artifact, skipping @@ -389,6 +393,12 @@ func newBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, return nil } +// buildBuildClosureDependencies is a recursive function that builds up a map +// of build-time dependencies for a given artifact if it is not already present +// in the results map. It first iterates through the runtime dependencies of the +// artifact recursively adding all of the dependencies to the results map. +// Then it iterates through the inputs of the step that generated the +// artifact and recursively adds all of those dependencies as well. func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUID]interface{}, deps map[strfmt.UUID]struct{}, result artifact.Map) (map[strfmt.UUID]struct{}, error) { if _, ok := result[artifactID]; ok { // We have already processed this artifact, skipping @@ -430,6 +440,9 @@ func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI return nil, nil } + // We iterate through the inputs of the step that generated the + // artifact, specifically the inputs that are tagged as dependencies and + // build a build-time closure for each. for _, input := range step.Inputs { if input.Tag != model.TagDependency { continue From 8e87b808b4bead2e0e0f08fe6d71fef3670089e5 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Oct 2023 15:09:24 -0700 Subject: [PATCH 15/32] Revert some setup changes Remove unnecessary integration test --- internal/constants/constants.go | 3 -- pkg/platform/runtime/buildplan/buildplan.go | 3 -- pkg/platform/runtime/setup/setup.go | 39 +++++++-------------- test/integration/checkout_int_test.go | 20 ----------- 4 files changed, 13 insertions(+), 52 deletions(-) diff --git a/internal/constants/constants.go b/internal/constants/constants.go index 81118222fa..335c57082e 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -62,9 +62,6 @@ const DisableUpdates = "ACTIVESTATE_CLI_DISABLE_UPDATES" // UpdateBranchEnvVarName is the env var that is used to override which branch to pull the update from const UpdateBranchEnvVarName = "ACTIVESTATE_CLI_UPDATE_BRANCH" -// InstallBuildDependencies is the env var that is used to override whether to install build dependencies -const InstallBuildDependencies = "ACTIVESTATE_CLI_INSTALL_BUILD_DEPENDENCIES" - // InternalConfigFileNameLegacy is effectively the same as InternalConfigName, but includes our preferred extension const InternalConfigFileNameLegacy = "config.yaml" diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 9fe90b3112..1dc21f1285 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -459,9 +459,6 @@ func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI var uniqueDeps []strfmt.UUID for id := range deps { - if _, ok := deps[id]; !ok { - continue - } uniqueDeps = append(uniqueDeps, id) } diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index a917e67834..fdbe529733 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -385,6 +385,11 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal var runtimeAndBuildtimeArtifacts artifact.Map var runtimeArtifacts artifact.Map // Artifacts required for the runtime to function + runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build) + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") + } + // If the build is not ready, we need to get the runtime and buildtime closure if !buildResult.BuildReady { runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMap(buildResult.Build) @@ -393,23 +398,6 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } } - // If we are installing build dependencies, then buildtime dependences are also runtime dependencies - if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { - logging.Debug("Installing build dependencies") - if runtimeAndBuildtimeArtifacts == nil { - runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMap(buildResult.Build) - if err != nil { - return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") - } - } - runtimeArtifacts = runtimeAndBuildtimeArtifacts - } else { - runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build) - if err != nil { - return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") - } - } - var setup Setuper if !buildResult.BuildReady { setup, err = s.selectSetupImplementation(buildResult.BuildEngine, runtimeAndBuildtimeArtifacts) @@ -508,17 +496,16 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal ) artifactsToInstall := []artifact.ArtifactID{} - for _, a := range downloadablePrebuiltResults { - if _, alreadyInstalled := alreadyInstalled[a.ArtifactID]; !alreadyInstalled { - artifactsToInstall = append(artifactsToInstall, a.ArtifactID) - } - } - var artifactsToBuild artifact.Map - if !buildResult.BuildReady { - artifactsToBuild = runtimeAndBuildtimeArtifacts - } else { + if buildResult.BuildReady { + for _, a := range downloadablePrebuiltResults { + if _, alreadyInstalled := alreadyInstalled[a.ArtifactID]; !alreadyInstalled { + artifactsToInstall = append(artifactsToInstall, a.ArtifactID) + } + } artifactsToBuild = runtimeArtifacts + } else { + artifactsToBuild = runtimeAndBuildtimeArtifacts } // The log file we want to use for builds diff --git a/test/integration/checkout_int_test.go b/test/integration/checkout_int_test.go index 517dc9e2f3..9c35f2b52f 100644 --- a/test/integration/checkout_int_test.go +++ b/test/integration/checkout_int_test.go @@ -244,26 +244,6 @@ func (suite *CheckoutIntegrationTestSuite) TestCheckoutCaseInsensitive() { suite.Assert().NotContains(string(bytes), "ACTIVESTATE-CLI/SMALL-PYTHON", "kept incorrect namespace case") } -func (suite *CheckoutIntegrationTestSuite) TestCheckoutBuildtimeClosure() { - suite.OnlyRunForTags(tagsuite.Checkout) - - if runtime.GOOS != "linux" { - suite.T().Skip("Skipping buildtime closure test on non-linux platform") - } - - ts := e2e.New(suite.T(), false) - defer ts.Close() - - cp := ts.SpawnWithOpts( - e2e.OptArgs("checkout", "ActiveState-CLI/small-python"), - e2e.OptAppendEnv(constants.InstallBuildDependencies+"=true"), - e2e.OptAppendEnv(constants.DisableRuntime+"=false"), - ) - // Expect the number of build deps to be 27 which is more than the number of runtime deps. - cp.Expect("27") - cp.ExpectExitCode(0) -} - func TestCheckoutIntegrationTestSuite(t *testing.T) { suite.Run(t, new(CheckoutIntegrationTestSuite)) } From d8c20304a0b4a26b5aaef2da62545ffdc96018a2 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 3 Oct 2023 16:08:17 -0700 Subject: [PATCH 16/32] Remove step check --- pkg/platform/runtime/buildplan/buildplan.go | 7 ------- 1 file changed, 7 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 1dc21f1285..6f4f0633ef 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -428,13 +428,6 @@ func buildBuildClosureDependencies(artifactID strfmt.UUID, lookup map[strfmt.UUI // the result map. This is the buildtime closure calculation. step, ok := lookup[currentArtifact.GeneratedBy].(*model.Step) if !ok { - // Artifact was not generated by a step or a source, meaning that - // the buildplan is likely malformed. - _, ok := lookup[currentArtifact.GeneratedBy].(*model.Source) - if !ok { - return nil, errs.New("Incorrect target type for id %s, expected Step or Source", currentArtifact.GeneratedBy) - } - // Artifact was not generated by a step, skipping because these // artifacts do not need to be built. return nil, nil From a2ad4b912d199a24f8eb89f3a45b7083bbc1357f Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 5 Oct 2023 14:49:54 -0700 Subject: [PATCH 17/32] Add back environment variable and ability to install build time closure --- internal/constants/constants.go | 3 +++ pkg/platform/runtime/setup/setup.go | 22 +++++++++++++++++----- test/integration/checkout_int_test.go | 20 ++++++++++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/internal/constants/constants.go b/internal/constants/constants.go index 335c57082e..81118222fa 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -62,6 +62,9 @@ const DisableUpdates = "ACTIVESTATE_CLI_DISABLE_UPDATES" // UpdateBranchEnvVarName is the env var that is used to override which branch to pull the update from const UpdateBranchEnvVarName = "ACTIVESTATE_CLI_UPDATE_BRANCH" +// InstallBuildDependencies is the env var that is used to override whether to install build dependencies +const InstallBuildDependencies = "ACTIVESTATE_CLI_INSTALL_BUILD_DEPENDENCIES" + // InternalConfigFileNameLegacy is effectively the same as InternalConfigName, but includes our preferred extension const InternalConfigFileNameLegacy = "config.yaml" diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index fdbe529733..0d20f4f561 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -385,11 +385,6 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal var runtimeAndBuildtimeArtifacts artifact.Map var runtimeArtifacts artifact.Map // Artifacts required for the runtime to function - runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build) - if err != nil { - return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") - } - // If the build is not ready, we need to get the runtime and buildtime closure if !buildResult.BuildReady { runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMap(buildResult.Build) @@ -398,6 +393,23 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } } + // If we are installing build dependencies, then buildtime dependencies are also runtime dependencies + if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { + logging.Debug("Installing build dependencies") + if runtimeAndBuildtimeArtifacts == nil { + runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMap(buildResult.Build) + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") + } + } + runtimeArtifacts = runtimeAndBuildtimeArtifacts + } else { + runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build) + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") + } + } + var setup Setuper if !buildResult.BuildReady { setup, err = s.selectSetupImplementation(buildResult.BuildEngine, runtimeAndBuildtimeArtifacts) diff --git a/test/integration/checkout_int_test.go b/test/integration/checkout_int_test.go index 9c35f2b52f..9f4fd36246 100644 --- a/test/integration/checkout_int_test.go +++ b/test/integration/checkout_int_test.go @@ -244,6 +244,26 @@ func (suite *CheckoutIntegrationTestSuite) TestCheckoutCaseInsensitive() { suite.Assert().NotContains(string(bytes), "ACTIVESTATE-CLI/SMALL-PYTHON", "kept incorrect namespace case") } +func (suite *CheckoutIntegrationTestSuite) TestCheckoutBuildtimeClosure() { + suite.OnlyRunForTags(tagsuite.Checkout) + + if runtime.GOOS != "linux" { + suite.T().Skip("Skipping buildtime closure test on non-linux platform") + } + + ts := e2e.New(suite.T(), false) + defer ts.Close() + + cp := ts.SpawnWithOpts( + e2e.OptArgs("checkout", "ActiveState-CLI/small-python"), + e2e.OptAppendEnv(constants.InstallBuildDependencies+"=true"), + e2e.OptAppendEnv(constants.DisableRuntime+"=false"), + ) + // Expect the number of build deps to be 27 which is more than the number of runtime deps. + cp.Expect("libxcrypt") + cp.ExpectExitCode(0) +} + func TestCheckoutIntegrationTestSuite(t *testing.T) { suite.Run(t, new(CheckoutIntegrationTestSuite)) } From 19dc8f1aff1193e5205a915869b68721620adc4b Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 12 Oct 2023 14:29:37 -0700 Subject: [PATCH 18/32] Move artifact name resolution --- .../implementations/alternative/resolver.go | 21 +++++++ .../implementations/alternative/runtime.go | 10 +--- .../setup/implementations/camel/resolver.go | 16 ++++++ pkg/platform/runtime/setup/setup.go | 55 +++++++++++++------ 4 files changed, 77 insertions(+), 25 deletions(-) create mode 100644 pkg/platform/runtime/setup/implementations/alternative/resolver.go create mode 100644 pkg/platform/runtime/setup/implementations/camel/resolver.go diff --git a/pkg/platform/runtime/setup/implementations/alternative/resolver.go b/pkg/platform/runtime/setup/implementations/alternative/resolver.go new file mode 100644 index 0000000000..3e358c6263 --- /dev/null +++ b/pkg/platform/runtime/setup/implementations/alternative/resolver.go @@ -0,0 +1,21 @@ +package alternative + +import ( + "github.com/ActiveState/cli/internal/locale" + "github.com/ActiveState/cli/pkg/platform/runtime/artifact" +) + +type Resolver struct { + artifactsForNameResolving artifact.Map +} + +func NewResolver(artifactsForNameResolving artifact.Map) *Resolver { + return &Resolver{artifactsForNameResolving: artifactsForNameResolving} +} + +func (r *Resolver) ResolveArtifactName(id artifact.ArtifactID) string { + if artf, ok := r.artifactsForNameResolving[id]; ok { + return artf.Name + } + return locale.Tl("alternative_unknown_pkg_name", "unknown") +} diff --git a/pkg/platform/runtime/setup/implementations/alternative/runtime.go b/pkg/platform/runtime/setup/implementations/alternative/runtime.go index 452c0fe80a..cac7b3c29a 100644 --- a/pkg/platform/runtime/setup/implementations/alternative/runtime.go +++ b/pkg/platform/runtime/setup/implementations/alternative/runtime.go @@ -18,12 +18,11 @@ import ( ) type Setup struct { - artifactsForNameResolving artifact.Map - store *store.Store + store *store.Store } -func NewSetup(store *store.Store, artifactsForNameResolving artifact.Map) *Setup { - return &Setup{store: store, artifactsForNameResolving: artifactsForNameResolving} +func NewSetup(store *store.Store) *Setup { + return &Setup{store: store} } func (s *Setup) DeleteOutdatedArtifacts(changeset artifact.ArtifactChangeset, storedArtifacted, alreadyInstalled store.StoredArtifactMap) error { @@ -140,9 +139,6 @@ func artifactsContainFile(file string, artifactCache map[artifact.ArtifactID]sto } func (s *Setup) ResolveArtifactName(a artifact.ArtifactID) string { - if artf, ok := s.artifactsForNameResolving[a]; ok { - return artf.Name - } return locale.Tl("alternative_unknown_pkg_name", "unknown") } diff --git a/pkg/platform/runtime/setup/implementations/camel/resolver.go b/pkg/platform/runtime/setup/implementations/camel/resolver.go new file mode 100644 index 0000000000..14e7526d83 --- /dev/null +++ b/pkg/platform/runtime/setup/implementations/camel/resolver.go @@ -0,0 +1,16 @@ +package camel + +import ( + "github.com/ActiveState/cli/internal/locale" + "github.com/ActiveState/cli/pkg/platform/runtime/artifact" +) + +type Resolver struct{} + +func NewResolver() *Resolver { + return &Resolver{} +} + +func (r *Resolver) ResolveArtifactName(_ artifact.ArtifactID) string { + return locale.Tl("camel_bundle_name", "bundle") +} diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 0d20f4f561..15959a0969 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -137,7 +137,6 @@ type Setup struct { type Setuper interface { // DeleteOutdatedArtifacts deletes outdated artifact as best as it can DeleteOutdatedArtifacts(artifact.ArtifactChangeset, store.StoredArtifactMap, store.StoredArtifactMap) error - ResolveArtifactName(artifact.ArtifactID) string DownloadsFromBuild(build bpModel.Build, artifacts map[strfmt.UUID]artifact.Artifact) (download []artifact.ArtifactDownload, err error) } @@ -148,6 +147,10 @@ type ArtifactSetuper interface { Unarchiver() unarchiver.Unarchiver } +type ArtifactResolver interface { + ResolveArtifactName(artifact.ArtifactID) string +} + type artifactInstaller func(artifact.ArtifactID, string, ArtifactSetuper) error type artifactUninstaller func() error @@ -410,12 +413,17 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } } - var setup Setuper + var resolver ArtifactResolver if !buildResult.BuildReady { - setup, err = s.selectSetupImplementation(buildResult.BuildEngine, runtimeAndBuildtimeArtifacts) + resolver, err = selectArtifactResolver(buildResult.BuildEngine, runtimeAndBuildtimeArtifacts) } else { - setup, err = s.selectSetupImplementation(buildResult.BuildEngine, runtimeArtifacts) + resolver, err = selectArtifactResolver(buildResult.BuildEngine, runtimeArtifacts) } + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to select artifact resolver") + } + + setup, err := s.selectSetupImplementation(buildResult.BuildEngine) if err != nil { return nil, nil, errs.Wrap(err, "Failed to select setup implementation") } @@ -489,7 +497,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } } - artifactNames := artifact.ResolveArtifactNames(setup.ResolveArtifactName, artifactIDs) + artifactNames := artifact.ResolveArtifactNames(resolver.ResolveArtifactName, artifactIDs) artifactNamesList := []string{} for _, n := range artifactNames { artifactNamesList = append(artifactNamesList, n) @@ -554,7 +562,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal s.analytics.Event(anaConsts.CatRuntimeDebug, anaConsts.ActRuntimeDownload, dimensions) } - err = s.installArtifactsFromBuild(buildResult, runtimeArtifacts, artifact.ArtifactIDsToMap(artifactsToInstall), downloadablePrebuiltResults, setup, installFunc, logFilePath) + err = s.installArtifactsFromBuild(buildResult, runtimeArtifacts, artifact.ArtifactIDsToMap(artifactsToInstall), downloadablePrebuiltResults, setup, resolver, installFunc, logFilePath) if err != nil { return nil, nil, err } @@ -599,7 +607,7 @@ func aggregateErrors() (chan<- error, <-chan error) { return bgErrs, aggErr } -func (s *Setup) installArtifactsFromBuild(buildResult *model.BuildResult, artifacts artifact.Map, artifactsToInstall map[artifact.ArtifactID]struct{}, downloads []artifact.ArtifactDownload, setup Setuper, installFunc artifactInstaller, logFilePath string) error { +func (s *Setup) installArtifactsFromBuild(buildResult *model.BuildResult, artifacts artifact.Map, artifactsToInstall map[artifact.ArtifactID]struct{}, downloads []artifact.ArtifactDownload, setup Setuper, resolver ArtifactResolver, installFunc artifactInstaller, logFilePath string) error { // Artifacts are installed in two stages // - The first stage runs concurrently in MaxConcurrency worker threads (download, unpacking, relocation) // - The second stage moves all files into its final destination is running in a single thread (using the mainthread library) to avoid file conflicts @@ -609,16 +617,16 @@ func (s *Setup) installArtifactsFromBuild(buildResult *model.BuildResult, artifa if err := s.handleEvent(events.BuildSkipped{}); err != nil { return errs.Wrap(err, "Could not handle BuildSkipped event") } - err = s.installFromBuildResult(buildResult, artifacts, artifactsToInstall, downloads, setup, installFunc) + err = s.installFromBuildResult(buildResult, artifacts, artifactsToInstall, downloads, setup, resolver, installFunc) } else { - err = s.installFromBuildLog(buildResult, artifacts, artifactsToInstall, setup, installFunc, logFilePath) + err = s.installFromBuildLog(buildResult, artifacts, artifactsToInstall, setup, resolver, installFunc, logFilePath) } return err } // setupArtifactSubmitFunction returns a function that sets up an artifact and can be submitted to a workerpool -func (s *Setup) setupArtifactSubmitFunction(a artifact.ArtifactDownload, ar *artifact.Artifact, expectedArtifactInstalls map[artifact.ArtifactID]struct{}, buildResult *model.BuildResult, setup Setuper, installFunc artifactInstaller, errors chan<- error) func() { +func (s *Setup) setupArtifactSubmitFunction(a artifact.ArtifactDownload, ar *artifact.Artifact, expectedArtifactInstalls map[artifact.ArtifactID]struct{}, buildResult *model.BuildResult, setup Setuper, resolver ArtifactResolver, installFunc artifactInstaller, errors chan<- error) func() { return func() { // If artifact has no valid download, just count it as completed and return if strings.Contains(ar.URL, "as-builds/noop") || @@ -645,21 +653,21 @@ func (s *Setup) setupArtifactSubmitFunction(a artifact.ArtifactDownload, ar *art unarchiver := as.Unarchiver() archivePath, err := s.obtainArtifact(a, unarchiver.Ext()) if err != nil { - name := setup.ResolveArtifactName(a.ArtifactID) + name := resolver.ResolveArtifactName(a.ArtifactID) errors <- locale.WrapError(err, "artifact_download_failed", "", name, a.ArtifactID.String()) return } err = installFunc(a.ArtifactID, archivePath, as) if err != nil { - name := setup.ResolveArtifactName(a.ArtifactID) + name := resolver.ResolveArtifactName(a.ArtifactID) errors <- locale.WrapError(err, "artifact_setup_failed", "", name, a.ArtifactID.String()) return } } } -func (s *Setup) installFromBuildResult(buildResult *model.BuildResult, artifacts artifact.Map, artifactsToInstall map[artifact.ArtifactID]struct{}, downloads []artifact.ArtifactDownload, setup Setuper, installFunc artifactInstaller) error { +func (s *Setup) installFromBuildResult(buildResult *model.BuildResult, artifacts artifact.Map, artifactsToInstall map[artifact.ArtifactID]struct{}, downloads []artifact.ArtifactDownload, setup Setuper, resolver ArtifactResolver, installFunc artifactInstaller) error { logging.Debug("Installing artifacts from build result") errs, aggregatedErr := aggregateErrors() mainthread.Run(func() { @@ -673,7 +681,7 @@ func (s *Setup) installFromBuildResult(buildResult *model.BuildResult, artifacts if arv, ok := artifacts[a.ArtifactID]; ok { ar = &arv } - wp.Submit(s.setupArtifactSubmitFunction(a, ar, map[artifact.ArtifactID]struct{}{}, buildResult, setup, installFunc, errs)) + wp.Submit(s.setupArtifactSubmitFunction(a, ar, map[artifact.ArtifactID]struct{}{}, buildResult, setup, resolver, installFunc, errs)) } wp.StopWait() @@ -682,7 +690,7 @@ func (s *Setup) installFromBuildResult(buildResult *model.BuildResult, artifacts return <-aggregatedErr } -func (s *Setup) installFromBuildLog(buildResult *model.BuildResult, artifacts artifact.Map, artifactsToInstall map[artifact.ArtifactID]struct{}, setup Setuper, installFunc artifactInstaller, logFilePath string) error { +func (s *Setup) installFromBuildLog(buildResult *model.BuildResult, artifacts artifact.Map, artifactsToInstall map[artifact.ArtifactID]struct{}, setup Setuper, resolver ArtifactResolver, installFunc artifactInstaller, logFilePath string) error { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -723,7 +731,7 @@ func (s *Setup) installFromBuildLog(buildResult *model.BuildResult, artifacts ar logging.Debug("Unmonitored artifact buildlog event discarded: %s", a.ArtifactID) continue } - wp.Submit(s.setupArtifactSubmitFunction(a, ar, artifactsToInstall, buildResult, setup, installFunc, errs)) + wp.Submit(s.setupArtifactSubmitFunction(a, ar, artifactsToInstall, buildResult, setup, resolver, installFunc, errs)) } }() @@ -872,10 +880,10 @@ func (s *Setup) unpackArtifact(ua unarchiver.Unarchiver, tarballPath string, tar return numUnpackedFiles, ua.Unarchive(proxy, i, targetDir) } -func (s *Setup) selectSetupImplementation(buildEngine model.BuildEngine, artifacts artifact.Map) (Setuper, error) { +func (s *Setup) selectSetupImplementation(buildEngine model.BuildEngine) (Setuper, error) { switch buildEngine { case model.Alternative: - return alternative.NewSetup(s.store, artifacts), nil + return alternative.NewSetup(s.store), nil case model.Camel: return camel.NewSetup(s.store), nil default: @@ -883,6 +891,17 @@ func (s *Setup) selectSetupImplementation(buildEngine model.BuildEngine, artifac } } +func selectArtifactResolver(buildEngine model.BuildEngine, artifacts artifact.Map) (ArtifactResolver, error) { + switch buildEngine { + case model.Alternative: + return alternative.NewResolver(artifacts), nil + case model.Camel: + return camel.NewResolver(), nil + default: + return nil, errs.New("Unknown build engine: %s", buildEngine) + } +} + func (s *Setup) selectArtifactSetupImplementation(buildEngine model.BuildEngine, a artifact.ArtifactID) (ArtifactSetuper, error) { switch buildEngine { case model.Alternative: From 3ba47f8806b9c6cfff7e4f29a7de1cc2826225c7 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 12 Oct 2023 15:31:11 -0700 Subject: [PATCH 19/32] Use one function to calculate both the buildtime and runtime artifact map --- pkg/platform/runtime/buildplan/buildplan.go | 68 ++++++--------------- pkg/platform/runtime/buildplan/changeset.go | 10 +-- pkg/platform/runtime/setup/setup.go | 12 ++-- 3 files changed, 31 insertions(+), 59 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 6f4f0633ef..28c16ef08e 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -16,7 +16,12 @@ import ( // lookup table and calls the recursive function buildMap to build up the // artifact map by traversing the build plan from the terminal targets through // all of the runtime dependencies for each of the artifacts in the DAG. -func NewMapFromBuildPlan(build *model.Build) (artifact.Map, error) { + +// Setting calculateBuildtimeClosure as true calculates the artifact map with the builtime +// dependencies. This is different from the runtime dependency calculation as it +// includes ALL of the input artifacts of the step that generated each artifact. +// The includeBuilders argument determines whether or not to include builder artifacts in the final result. +func NewMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (artifact.Map, error) { res := make(artifact.Map) lookup := make(map[strfmt.UUID]interface{}) @@ -46,10 +51,17 @@ func NewMapFromBuildPlan(build *model.Build) (artifact.Map, error) { } } - for _, id := range terminalTargetIDs { - err := buildRuntimeClosureMap(id, lookup, res) - if err != nil { - return nil, errs.Wrap(err, "Could not build map for terminal %s", id) + if calculateBuildtimeClosure { + for _, id := range terminalTargetIDs { + if err := newBuildClosureMap(id, lookup, res); err != nil { + return nil, errs.Wrap(err, "Could not build map for terminal %s", id) + } + } + } else { + for _, id := range terminalTargetIDs { + if err := buildRuntimeClosureMap(id, lookup, res); err != nil { + return nil, errs.Wrap(err, "Could not build map for terminal %s", id) + } } } @@ -288,8 +300,8 @@ func RecursiveDependenciesFor(a artifact.ArtifactID, artifacts artifact.Map) []a // NewMapFromBuildPlan creates an artifact map from a build plan // where the key is the artifact name rather than the artifact ID. -func NewNamedMapFromBuildPlan(build *model.Build) (artifact.NamedMap, error) { - am, err := NewMapFromBuildPlan(build) +func NewNamedMapFromBuildPlan(build *model.Build, buildtimeClosure bool) (artifact.NamedMap, error) { + am, err := NewMapFromBuildPlan(build, buildtimeClosure) if err != nil { return nil, errs.Wrap(err, "Could not create artifact map") } @@ -302,48 +314,6 @@ func NewNamedMapFromBuildPlan(build *model.Build) (artifact.NamedMap, error) { return res, nil } -// NewBuildtimeMap iterates through all artifacts in a given build and -// adds the artifact's dependencies to a map. This is different from the -// runtime dependency calculation as it includes ALL of the input artifacts of the -// step that generated each artifact. The includeBuilders argument determines whether -// or not to include builder artifacts in the final result. -func NewBuildtimeMap(build *model.Build) (artifact.Map, error) { - filteredTerminals, err := filterTerminals(build) - if err != nil { - return nil, errs.Wrap(err, "Could not filter terminals") - } - - lookup := make(map[strfmt.UUID]interface{}) - for _, artifact := range build.Artifacts { - lookup[artifact.NodeID] = artifact - } - for _, step := range build.Steps { - lookup[step.StepID] = step - } - for _, source := range build.Sources { - lookup[source.NodeID] = source - } - - var terminalTargetIDs []strfmt.UUID - for _, terminal := range filteredTerminals { - // If there is an artifact for this terminal and its mime type is not a state tool artifact - // then we need to recurse back through the DAG until we find nodeIDs that are state tool - // artifacts. These are the terminal targets. - for _, nodeID := range terminal.NodeIDs { - buildTerminals(nodeID, lookup, &terminalTargetIDs) - } - } - - result := make(artifact.Map) - for _, id := range terminalTargetIDs { - if err := newBuildClosureMap(id, lookup, result); err != nil { - return nil, errs.Wrap(err, "Could not build map for terminal %s", id) - } - } - - return result, nil -} - // newBuildClosureMap recursively builds the artifact map from the lookup table. // If the current artifact is not already contained in the results map it first // builds the artifacts build-time dependencies and then adds the artifact to the diff --git a/pkg/platform/runtime/buildplan/changeset.go b/pkg/platform/runtime/buildplan/changeset.go index 9a5614e856..48e8619554 100644 --- a/pkg/platform/runtime/buildplan/changeset.go +++ b/pkg/platform/runtime/buildplan/changeset.go @@ -6,13 +6,13 @@ import ( "github.com/ActiveState/cli/pkg/platform/runtime/artifact" ) -func NewArtifactChangesetByBuildPlan(oldBuildPlan *model.Build, build *model.Build, requestedOnly bool) (artifact.ArtifactChangeset, error) { - old, err := NewNamedMapFromBuildPlan(oldBuildPlan) +func NewArtifactChangesetByBuildPlan(oldBuildPlan *model.Build, build *model.Build, requestedOnly, buildtimeClosure bool) (artifact.ArtifactChangeset, error) { + old, err := NewNamedMapFromBuildPlan(oldBuildPlan, buildtimeClosure) if err != nil { return artifact.ArtifactChangeset{}, errs.Wrap(err, "failed to build map from old build plan") } - new, err := NewNamedMapFromBuildPlan(build) + new, err := NewNamedMapFromBuildPlan(build, buildtimeClosure) if err != nil { return artifact.ArtifactChangeset{}, errs.Wrap(err, "failed to build map from new build plan") } @@ -22,8 +22,8 @@ func NewArtifactChangesetByBuildPlan(oldBuildPlan *model.Build, build *model.Bui return cs, nil } -func NewBaseArtifactChangesetByBuildPlan(build *model.Build, requestedOnly bool) (artifact.ArtifactChangeset, error) { - new, err := NewNamedMapFromBuildPlan(build) +func NewBaseArtifactChangesetByBuildPlan(build *model.Build, requestedOnly, buildtimeClosure bool) (artifact.ArtifactChangeset, error) { + new, err := NewNamedMapFromBuildPlan(build, buildtimeClosure) if err != nil { return artifact.ArtifactChangeset{}, errs.Wrap(err, "failed to build map from new build plan") } diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 15959a0969..a4daede06a 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -390,24 +390,26 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // If the build is not ready, we need to get the runtime and buildtime closure if !buildResult.BuildReady { - runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMap(buildResult.Build) + runtimeAndBuildtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build, true) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } } + var includeBuildtimeClosure bool // If we are installing build dependencies, then buildtime dependencies are also runtime dependencies if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { logging.Debug("Installing build dependencies") + includeBuildtimeClosure = true if runtimeAndBuildtimeArtifacts == nil { - runtimeAndBuildtimeArtifacts, err = buildplan.NewBuildtimeMap(buildResult.Build) + runtimeAndBuildtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build, includeBuildtimeClosure) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } } runtimeArtifacts = runtimeAndBuildtimeArtifacts } else { - runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build) + runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build, false) if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } @@ -461,7 +463,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal s.analytics.Event(anaConsts.CatRuntimeDebug, anaConsts.ActRuntimeBuild, dimensions) } - changedArtifacts, err := buildplan.NewBaseArtifactChangesetByBuildPlan(buildResult.Build, false) + changedArtifacts, err := buildplan.NewBaseArtifactChangesetByBuildPlan(buildResult.Build, false, includeBuildtimeClosure) if err != nil { return nil, nil, errs.Wrap(err, "Could not compute base artifact changeset") } @@ -472,7 +474,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } if oldBuildPlan != nil { - changedArtifacts, err = buildplan.NewArtifactChangesetByBuildPlan(oldBuildPlan, buildResult.Build, false) + changedArtifacts, err = buildplan.NewArtifactChangesetByBuildPlan(oldBuildPlan, buildResult.Build, false, includeBuildtimeClosure) if err != nil { return nil, nil, errs.Wrap(err, "Could not compute artifact changeset") } From bc6f28c38a20ec1be3a2735678e8c6e30693772f Mon Sep 17 00:00:00 2001 From: mdrakos Date: Fri, 13 Oct 2023 13:52:07 -0700 Subject: [PATCH 20/32] Add comments and renaming --- pkg/platform/runtime/buildplan/buildplan.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 28c16ef08e..5c1a89fa62 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -36,7 +36,7 @@ func NewMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (ar lookup[source.NodeID] = source } - filtered, err := filterTerminals(build) + filtered, err := filterPlatformTerminals(build) if err != nil { return nil, errs.Wrap(err, "Could not filter terminals") } @@ -53,7 +53,7 @@ func NewMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (ar if calculateBuildtimeClosure { for _, id := range terminalTargetIDs { - if err := newBuildClosureMap(id, lookup, res); err != nil { + if err := buildBuildtimeClosureMap(id, lookup, res); err != nil { return nil, errs.Wrap(err, "Could not build map for terminal %s", id) } } @@ -68,8 +68,11 @@ func NewMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (ar return res, nil } -func filterTerminals(build *model.Build) ([]*model.NamedTarget, error) { +// filterPlatformTerminals filters the build terminal nodes to only include +// terminals that are for the current host platform. +func filterPlatformTerminals(build *model.Build) ([]*model.NamedTarget, error) { // Extract the available platforms from the build plan + // We are only interested in terminals with the platform tag var bpPlatforms []strfmt.UUID for _, t := range build.Terminals { if !strings.Contains(t.Tag, "platform:") { @@ -78,7 +81,7 @@ func filterTerminals(build *model.Build) ([]*model.NamedTarget, error) { bpPlatforms = append(bpPlatforms, strfmt.UUID(strings.TrimPrefix(t.Tag, "platform:"))) } - // Get the platform ID for the current platform + // Get the platform ID for the current host platform platformID, err := platformModel.FilterCurrentPlatform(platformModel.HostPlatform, bpPlatforms) if err != nil { return nil, locale.WrapError(err, "err_filter_current_platform") @@ -314,11 +317,11 @@ func NewNamedMapFromBuildPlan(build *model.Build, buildtimeClosure bool) (artifa return res, nil } -// newBuildClosureMap recursively builds the artifact map from the lookup table. +// buildBuildtimeClosureMap recursively builds the artifact map from the lookup table. // If the current artifact is not already contained in the results map it first // builds the artifacts build-time dependencies and then adds the artifact to the // results map. -func newBuildClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { +func buildBuildtimeClosureMap(baseID strfmt.UUID, lookup map[strfmt.UUID]interface{}, result artifact.Map) error { if _, ok := result[baseID]; ok { // We have already processed this artifact, skipping return nil From 2f01a4d0e404112aea8ebed1508fd5b076a49e90 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Wed, 18 Oct 2023 13:06:16 -0700 Subject: [PATCH 21/32] Update pkg/platform/runtime/setup/implementations/camel/resolver.go Co-authored-by: Nathan Rijksen --- pkg/platform/runtime/setup/implementations/camel/resolver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/platform/runtime/setup/implementations/camel/resolver.go b/pkg/platform/runtime/setup/implementations/camel/resolver.go index 14e7526d83..1075f1db3a 100644 --- a/pkg/platform/runtime/setup/implementations/camel/resolver.go +++ b/pkg/platform/runtime/setup/implementations/camel/resolver.go @@ -12,5 +12,5 @@ func NewResolver() *Resolver { } func (r *Resolver) ResolveArtifactName(_ artifact.ArtifactID) string { - return locale.Tl("camel_bundle_name", "bundle") + return locale.Tl("camel_bundle_name", "legacy bundle") } From 91bce30ea59e080e0326b20d230452d8bf7b61df Mon Sep 17 00:00:00 2001 From: mdrakos Date: Wed, 18 Oct 2023 14:58:41 -0700 Subject: [PATCH 22/32] Combine buildtime and runtime closures in a structure --- pkg/platform/runtime/buildplan/buildplan.go | 72 +++++++++++++++++++++ pkg/platform/runtime/setup/setup.go | 66 ++++++++----------- 2 files changed, 100 insertions(+), 38 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 5c1a89fa62..7f73d8910e 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -12,6 +12,78 @@ import ( "github.com/go-openapi/strfmt" ) +type ArtifactListing struct { + build *model.Build + runtimeClosure artifact.Map + buildtimeClosure artifact.Map + artifactIDs []artifact.ArtifactID +} + +func NewArtifactListing(build *model.Build) *ArtifactListing { + return &ArtifactListing{build: build} +} + +func (al *ArtifactListing) RuntimeClosure() (artifact.Map, error) { + if al.runtimeClosure != nil { + return al.runtimeClosure, nil + } + + runtimeClosure, err := NewMapFromBuildPlan(al.build, false) + if err != nil { + return nil, errs.Wrap(err, "Could not create runtime closure") + } + al.runtimeClosure = runtimeClosure + + return runtimeClosure, nil +} + +func (al *ArtifactListing) BuildtimeClosure() (artifact.Map, error) { + if al.buildtimeClosure != nil { + return al.buildtimeClosure, nil + } + + buildtimeClosure, err := NewMapFromBuildPlan(al.build, true) + if err != nil { + return nil, errs.Wrap(err, "Could not create buildtime closure") + } + al.buildtimeClosure = buildtimeClosure + + return buildtimeClosure, nil +} + +func (al *ArtifactListing) ArtifactIDs() ([]artifact.ArtifactID, error) { + if al.artifactIDs != nil { + return al.artifactIDs, nil + } + + if al.buildtimeClosure != nil { + for _, artifact := range al.buildtimeClosure { + al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + } + return al.artifactIDs, nil + } + + if al.runtimeClosure != nil { + for _, artifact := range al.runtimeClosure { + al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + } + return al.artifactIDs, nil + } + + // Favor the buildtime closure over the runtime closure as it will + // include more artifact IDs + buildTimeClosure, err := al.BuildtimeClosure() + if err != nil { + return nil, errs.Wrap(err, "Could not create buildtime closure") + } + + for _, artifact := range buildTimeClosure { + al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + } + + return al.artifactIDs, nil +} + // NewMapFromBuildPlan creates an artifact map from a build plan. It creates a // lookup table and calls the recursive function buildMap to build up the // artifact map by traversing the build plan from the terminal targets through diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index f6f1d39cb0..df6d8e00db 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -403,43 +403,25 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } // Compute and handle the change summary - // runtimeAndBuildtimeArtifacts records all artifacts that will need to be built in order to obtain the runtime. - var runtimeAndBuildtimeArtifacts artifact.Map var runtimeArtifacts artifact.Map // Artifacts required for the runtime to function - - // If the build is not ready, we need to get the runtime and buildtime closure - if !buildResult.BuildReady { - runtimeAndBuildtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build, true) - if err != nil { - return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") - } - } + artifactListing := buildplan.NewArtifactListing(buildResult.Build) var includeBuildtimeClosure bool // If we are installing build dependencies, then buildtime dependencies are also runtime dependencies if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { logging.Debug("Installing build dependencies") - includeBuildtimeClosure = true - if runtimeAndBuildtimeArtifacts == nil { - runtimeAndBuildtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build, includeBuildtimeClosure) - if err != nil { - return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") - } + runtimeArtifacts, err = artifactListing.BuildtimeClosure() + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to compute buildtime closure") } - runtimeArtifacts = runtimeAndBuildtimeArtifacts } else { - runtimeArtifacts, err = buildplan.NewMapFromBuildPlan(buildResult.Build, false) + runtimeArtifacts, err = artifactListing.RuntimeClosure() if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } } - var resolver ArtifactResolver - if !buildResult.BuildReady { - resolver, err = selectArtifactResolver(buildResult.BuildEngine, runtimeAndBuildtimeArtifacts) - } else { - resolver, err = selectArtifactResolver(buildResult.BuildEngine, runtimeArtifacts) - } + resolver, err := selectArtifactResolver(buildResult, artifactListing) if err != nil { return nil, nil, errs.Wrap(err, "Failed to select artifact resolver") } @@ -507,15 +489,9 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal alreadyInstalled := reusableArtifacts(buildResult.Build.Artifacts, storedArtifacts) // Report resolved artifacts - artifactIDs := []artifact.ArtifactID{} - if !buildResult.BuildReady { - for _, a := range runtimeAndBuildtimeArtifacts { - artifactIDs = append(artifactIDs, a.ArtifactID) - } - } else { - for _, a := range runtimeArtifacts { - artifactIDs = append(artifactIDs, a.ArtifactID) - } + artifactIDs, err := artifactListing.ArtifactIDs() + if err != nil { + return nil, nil, errs.Wrap(err, "Could not get artifact IDs from build plan") } artifactNames := artifact.ResolveArtifactNames(resolver.ResolveArtifactName, artifactIDs) @@ -544,9 +520,12 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal artifactsToInstall = append(artifactsToInstall, a.ArtifactID) } } - artifactsToBuild = runtimeArtifacts + artifactsToBuild, err = artifactListing.BuildtimeClosure() } else { - artifactsToBuild = runtimeAndBuildtimeArtifacts + artifactsToBuild, err = artifactListing.RuntimeClosure() + } + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to compute artifacts to build") } // The log file we want to use for builds @@ -912,14 +891,25 @@ func (s *Setup) selectSetupImplementation(buildEngine model.BuildEngine) (Setupe } } -func selectArtifactResolver(buildEngine model.BuildEngine, artifacts artifact.Map) (ArtifactResolver, error) { - switch buildEngine { +func selectArtifactResolver(buildResult *model.BuildResult, artifactListing *buildplan.ArtifactListing) (ArtifactResolver, error) { + var artifacts artifact.Map + var err error + if buildResult.BuildReady || strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { + artifacts, err = artifactListing.BuildtimeClosure() + } else { + artifacts, err = artifactListing.RuntimeClosure() + } + if err != nil { + return nil, errs.Wrap(err, "Failed to create artifact map from build plan") + } + + switch buildResult.BuildEngine { case model.Alternative: return alternative.NewResolver(artifacts), nil case model.Camel: return camel.NewResolver(), nil default: - return nil, errs.New("Unknown build engine: %s", buildEngine) + return nil, errs.New("Unknown build engine: %s", buildResult.BuildEngine) } } From 3dafc4e8cc4adfb1dfe446134daf07964925e353 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Thu, 19 Oct 2023 09:46:15 -0700 Subject: [PATCH 23/32] Update build time closure bool --- pkg/platform/runtime/buildplan/buildplan.go | 8 ++++---- pkg/platform/runtime/setup/setup.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 7f73d8910e..c53b917b92 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -28,7 +28,7 @@ func (al *ArtifactListing) RuntimeClosure() (artifact.Map, error) { return al.runtimeClosure, nil } - runtimeClosure, err := NewMapFromBuildPlan(al.build, false) + runtimeClosure, err := newMapFromBuildPlan(al.build, false) if err != nil { return nil, errs.Wrap(err, "Could not create runtime closure") } @@ -42,7 +42,7 @@ func (al *ArtifactListing) BuildtimeClosure() (artifact.Map, error) { return al.buildtimeClosure, nil } - buildtimeClosure, err := NewMapFromBuildPlan(al.build, true) + buildtimeClosure, err := newMapFromBuildPlan(al.build, true) if err != nil { return nil, errs.Wrap(err, "Could not create buildtime closure") } @@ -93,7 +93,7 @@ func (al *ArtifactListing) ArtifactIDs() ([]artifact.ArtifactID, error) { // dependencies. This is different from the runtime dependency calculation as it // includes ALL of the input artifacts of the step that generated each artifact. // The includeBuilders argument determines whether or not to include builder artifacts in the final result. -func NewMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (artifact.Map, error) { +func newMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (artifact.Map, error) { res := make(artifact.Map) lookup := make(map[strfmt.UUID]interface{}) @@ -376,7 +376,7 @@ func RecursiveDependenciesFor(a artifact.ArtifactID, artifacts artifact.Map) []a // NewMapFromBuildPlan creates an artifact map from a build plan // where the key is the artifact name rather than the artifact ID. func NewNamedMapFromBuildPlan(build *model.Build, buildtimeClosure bool) (artifact.NamedMap, error) { - am, err := NewMapFromBuildPlan(build, buildtimeClosure) + am, err := newMapFromBuildPlan(build, buildtimeClosure) if err != nil { return nil, errs.Wrap(err, "Could not create artifact map") } diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index df6d8e00db..7f7904bdeb 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -406,7 +406,6 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal var runtimeArtifacts artifact.Map // Artifacts required for the runtime to function artifactListing := buildplan.NewArtifactListing(buildResult.Build) - var includeBuildtimeClosure bool // If we are installing build dependencies, then buildtime dependencies are also runtime dependencies if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { logging.Debug("Installing build dependencies") @@ -464,6 +463,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal s.analytics.Event(anaConsts.CatRuntimeDebug, anaConsts.ActRuntimeBuild, dimensions) } + includeBuildtimeClosure := strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") || !buildResult.BuildReady changedArtifacts, err := buildplan.NewBaseArtifactChangesetByBuildPlan(buildResult.Build, false, includeBuildtimeClosure) if err != nil { return nil, nil, errs.Wrap(err, "Could not compute base artifact changeset") From d3a2d7b0cd584eb66034e5b4816653ef66c0a629 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Thu, 19 Oct 2023 09:51:46 -0700 Subject: [PATCH 24/32] Update test --- test/integration/checkout_int_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/checkout_int_test.go b/test/integration/checkout_int_test.go index fc923b7879..5815be0bf9 100644 --- a/test/integration/checkout_int_test.go +++ b/test/integration/checkout_int_test.go @@ -278,6 +278,8 @@ func (suite *CheckoutIntegrationTestSuite) TestCheckoutBuildtimeClosure() { e2e.OptAppendEnv(constants.DisableRuntime+"=false"), ) // Expect the number of build deps to be 27 which is more than the number of runtime deps. + // Also expect libxcrypt which should not be in the runtime closure. + cp.Expect("27") cp.Expect("libxcrypt") cp.ExpectExitCode(0) } From edc0b6abe1c85952d519b46c7810c22b6ce57615 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 23 Oct 2023 10:06:52 -0700 Subject: [PATCH 25/32] Rename and add comment --- pkg/platform/runtime/buildplan/buildplan.go | 39 ++++++++++----------- pkg/platform/runtime/setup/setup.go | 21 ++++++----- 2 files changed, 32 insertions(+), 28 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index c53b917b92..737163ac8a 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -51,34 +51,33 @@ func (al *ArtifactListing) BuildtimeClosure() (artifact.Map, error) { return buildtimeClosure, nil } -func (al *ArtifactListing) ArtifactIDs() ([]artifact.ArtifactID, error) { +func (al *ArtifactListing) ArtifactIDs(buildtimeClosure bool) ([]artifact.ArtifactID, error) { if al.artifactIDs != nil { return al.artifactIDs, nil } - if al.buildtimeClosure != nil { - for _, artifact := range al.buildtimeClosure { - al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + if buildtimeClosure { + if al.buildtimeClosure != nil { + for _, artifact := range al.buildtimeClosure { + al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + } + return al.artifactIDs, nil } - return al.artifactIDs, nil - } - if al.runtimeClosure != nil { - for _, artifact := range al.runtimeClosure { - al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + buildTimeClosure, err := al.BuildtimeClosure() + if err != nil { + return nil, errs.Wrap(err, "Could not create buildtime closure") } - return al.artifactIDs, nil - } - // Favor the buildtime closure over the runtime closure as it will - // include more artifact IDs - buildTimeClosure, err := al.BuildtimeClosure() - if err != nil { - return nil, errs.Wrap(err, "Could not create buildtime closure") - } - - for _, artifact := range buildTimeClosure { - al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + for _, artifact := range buildTimeClosure { + al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + } + } else { + if al.runtimeClosure != nil { + for _, artifact := range al.runtimeClosure { + al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + } + } } return al.artifactIDs, nil diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 7f7904bdeb..389af26017 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -403,18 +403,20 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal } // Compute and handle the change summary - var runtimeArtifacts artifact.Map // Artifacts required for the runtime to function + var requestedArtifacts artifact.Map // Artifacts required for the runtime to function artifactListing := buildplan.NewArtifactListing(buildResult.Build) - // If we are installing build dependencies, then buildtime dependencies are also runtime dependencies + // If we are installing build dependencies, then the requested artifacts + // will include the buildtime closure. Otherwise, we only need the runtime + // closure. if strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") { logging.Debug("Installing build dependencies") - runtimeArtifacts, err = artifactListing.BuildtimeClosure() + requestedArtifacts, err = artifactListing.BuildtimeClosure() if err != nil { return nil, nil, errs.Wrap(err, "Failed to compute buildtime closure") } } else { - runtimeArtifacts, err = artifactListing.RuntimeClosure() + requestedArtifacts, err = artifactListing.RuntimeClosure() if err != nil { return nil, nil, errs.Wrap(err, "Failed to create artifact map from build plan") } @@ -430,7 +432,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal return nil, nil, errs.Wrap(err, "Failed to select setup implementation") } - downloadablePrebuiltResults, err := setup.DownloadsFromBuild(*buildResult.Build, runtimeArtifacts) + downloadablePrebuiltResults, err := setup.DownloadsFromBuild(*buildResult.Build, requestedArtifacts) if err != nil { if errors.Is(err, artifact.CamelRuntimeBuilding) { localeID := "build_status_in_progress" @@ -446,7 +448,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal // buildResult doesn't have namespace info and will happily report internal only artifacts downloadablePrebuiltResults = funk.Filter(downloadablePrebuiltResults, func(ad artifact.ArtifactDownload) bool { - ar, ok := runtimeArtifacts[ad.ArtifactID] + ar, ok := requestedArtifacts[ad.ArtifactID] if !ok { return true } @@ -463,6 +465,9 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal s.analytics.Event(anaConsts.CatRuntimeDebug, anaConsts.ActRuntimeBuild, dimensions) } + // If the build is not ready or if we are installing the buildtime closure + // then we need to include the buildtime closure in the changed artifacts + // and the progress reporting. includeBuildtimeClosure := strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") || !buildResult.BuildReady changedArtifacts, err := buildplan.NewBaseArtifactChangesetByBuildPlan(buildResult.Build, false, includeBuildtimeClosure) if err != nil { @@ -489,7 +494,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal alreadyInstalled := reusableArtifacts(buildResult.Build.Artifacts, storedArtifacts) // Report resolved artifacts - artifactIDs, err := artifactListing.ArtifactIDs() + artifactIDs, err := artifactListing.ArtifactIDs(includeBuildtimeClosure) if err != nil { return nil, nil, errs.Wrap(err, "Could not get artifact IDs from build plan") } @@ -562,7 +567,7 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal s.analytics.Event(anaConsts.CatRuntimeDebug, anaConsts.ActRuntimeDownload, dimensions) } - err = s.installArtifactsFromBuild(buildResult, runtimeArtifacts, artifact.ArtifactIDsToMap(artifactsToInstall), downloadablePrebuiltResults, setup, resolver, installFunc, logFilePath) + err = s.installArtifactsFromBuild(buildResult, requestedArtifacts, artifact.ArtifactIDsToMap(artifactsToInstall), downloadablePrebuiltResults, setup, resolver, installFunc, logFilePath) if err != nil { return nil, nil, err } From 9c4ec885d3ca04831224410e62b0a0d99638380a Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 23 Oct 2023 11:19:50 -0700 Subject: [PATCH 26/32] Reorganize artifact listing logic --- pkg/platform/runtime/buildplan/buildplan.go | 49 +++++++++++++-------- pkg/platform/runtime/setup/setup.go | 14 +++--- 2 files changed, 40 insertions(+), 23 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 737163ac8a..ad646245ff 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -19,8 +19,23 @@ type ArtifactListing struct { artifactIDs []artifact.ArtifactID } -func NewArtifactListing(build *model.Build) *ArtifactListing { - return &ArtifactListing{build: build} +func NewArtifactListing(build *model.Build, buildtimeClosure bool) (*ArtifactListing, error) { + al := &ArtifactListing{build: build} + if buildtimeClosure { + buildtimeClosure, err := newMapFromBuildPlan(al.build, true) + if err != nil { + return nil, errs.Wrap(err, "Could not create buildtime closure") + } + al.buildtimeClosure = buildtimeClosure + } else { + runtimeClosure, err := newMapFromBuildPlan(al.build, false) + if err != nil { + return nil, errs.Wrap(err, "Could not create runtime closure") + } + al.runtimeClosure = runtimeClosure + } + + return al, nil } func (al *ArtifactListing) RuntimeClosure() (artifact.Map, error) { @@ -56,30 +71,28 @@ func (al *ArtifactListing) ArtifactIDs(buildtimeClosure bool) ([]artifact.Artifa return al.artifactIDs, nil } + var artifactMap artifact.Map + var err error if buildtimeClosure { - if al.buildtimeClosure != nil { - for _, artifact := range al.buildtimeClosure { - al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + if al.buildtimeClosure == nil { + artifactMap, err = al.BuildtimeClosure() + if err != nil { + return nil, errs.Wrap(err, "Could not calculate buildtime closure") } - return al.artifactIDs, nil - } - - buildTimeClosure, err := al.BuildtimeClosure() - if err != nil { - return nil, errs.Wrap(err, "Could not create buildtime closure") - } - - for _, artifact := range buildTimeClosure { - al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) } } else { - if al.runtimeClosure != nil { - for _, artifact := range al.runtimeClosure { - al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + if al.runtimeClosure == nil { + artifactMap, err = al.RuntimeClosure() + if err != nil { + return nil, errs.Wrap(err, "Could not calculate runtime closure") } } } + for _, artifact := range artifactMap { + al.artifactIDs = append(al.artifactIDs, artifact.ArtifactID) + } + return al.artifactIDs, nil } diff --git a/pkg/platform/runtime/setup/setup.go b/pkg/platform/runtime/setup/setup.go index 389af26017..b4f66c76cd 100644 --- a/pkg/platform/runtime/setup/setup.go +++ b/pkg/platform/runtime/setup/setup.go @@ -402,9 +402,17 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal return nil, nil, errs.Wrap(err, "Could not handle SolveSuccess event") } + // If the build is not ready or if we are installing the buildtime closure + // then we need to include the buildtime closure in the changed artifacts + // and the progress reporting. + includeBuildtimeClosure := strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") || !buildResult.BuildReady + // Compute and handle the change summary var requestedArtifacts artifact.Map // Artifacts required for the runtime to function - artifactListing := buildplan.NewArtifactListing(buildResult.Build) + artifactListing, err := buildplan.NewArtifactListing(buildResult.Build, includeBuildtimeClosure) + if err != nil { + return nil, nil, errs.Wrap(err, "Failed to create artifact listing") + } // If we are installing build dependencies, then the requested artifacts // will include the buildtime closure. Otherwise, we only need the runtime @@ -465,10 +473,6 @@ func (s *Setup) fetchAndInstallArtifactsFromBuildPlan(installFunc artifactInstal s.analytics.Event(anaConsts.CatRuntimeDebug, anaConsts.ActRuntimeBuild, dimensions) } - // If the build is not ready or if we are installing the buildtime closure - // then we need to include the buildtime closure in the changed artifacts - // and the progress reporting. - includeBuildtimeClosure := strings.EqualFold(os.Getenv(constants.InstallBuildDependencies), "true") || !buildResult.BuildReady changedArtifacts, err := buildplan.NewBaseArtifactChangesetByBuildPlan(buildResult.Build, false, includeBuildtimeClosure) if err != nil { return nil, nil, errs.Wrap(err, "Could not compute base artifact changeset") From 9c3233f978365e05786b06f9239b70de9b2c7725 Mon Sep 17 00:00:00 2001 From: Mike Drakos Date: Mon, 23 Oct 2023 13:40:13 -0700 Subject: [PATCH 27/32] Fix missing artifact names --- pkg/platform/runtime/buildplan/buildplan.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index ad646245ff..25f212ef65 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -74,18 +74,14 @@ func (al *ArtifactListing) ArtifactIDs(buildtimeClosure bool) ([]artifact.Artifa var artifactMap artifact.Map var err error if buildtimeClosure { - if al.buildtimeClosure == nil { - artifactMap, err = al.BuildtimeClosure() - if err != nil { - return nil, errs.Wrap(err, "Could not calculate buildtime closure") - } + artifactMap, err = al.BuildtimeClosure() + if err != nil { + return nil, errs.Wrap(err, "Could not calculate buildtime closure") } } else { - if al.runtimeClosure == nil { - artifactMap, err = al.RuntimeClosure() - if err != nil { - return nil, errs.Wrap(err, "Could not calculate runtime closure") - } + artifactMap, err = al.RuntimeClosure() + if err != nil { + return nil, errs.Wrap(err, "Could not calculate runtime closure") } } From 5cc4e0acde5c7feba4bc6cd225baa58e54437d6e Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 24 Oct 2023 10:08:17 -0700 Subject: [PATCH 28/32] Update comment --- pkg/platform/runtime/buildplan/buildplan.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 25f212ef65..93bbca8c5f 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -92,15 +92,14 @@ func (al *ArtifactListing) ArtifactIDs(buildtimeClosure bool) ([]artifact.Artifa return al.artifactIDs, nil } -// NewMapFromBuildPlan creates an artifact map from a build plan. It creates a +// newMapFromBuildPlan creates an artifact map from a build plan. It creates a // lookup table and calls the recursive function buildMap to build up the // artifact map by traversing the build plan from the terminal targets through // all of the runtime dependencies for each of the artifacts in the DAG. -// Setting calculateBuildtimeClosure as true calculates the artifact map with the builtime +// Setting calculateBuildtimeClosure as true calculates the artifact map with the buildtime // dependencies. This is different from the runtime dependency calculation as it // includes ALL of the input artifacts of the step that generated each artifact. -// The includeBuilders argument determines whether or not to include builder artifacts in the final result. func newMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (artifact.Map, error) { res := make(artifact.Map) From ecbc96a1f10715de12b0dcad86d6b3f3e59f8e8f Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 24 Oct 2023 10:11:47 -0700 Subject: [PATCH 29/32] Use function assignment --- pkg/platform/runtime/buildplan/buildplan.go | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 93bbca8c5f..22d0d6b5f4 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -130,17 +130,16 @@ func newMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (ar } } + var buildMap func(strfmt.UUID, map[strfmt.UUID]interface{}, artifact.Map) error if calculateBuildtimeClosure { - for _, id := range terminalTargetIDs { - if err := buildBuildtimeClosureMap(id, lookup, res); err != nil { - return nil, errs.Wrap(err, "Could not build map for terminal %s", id) - } - } + buildMap = buildBuildtimeClosureMap } else { - for _, id := range terminalTargetIDs { - if err := buildRuntimeClosureMap(id, lookup, res); err != nil { - return nil, errs.Wrap(err, "Could not build map for terminal %s", id) - } + buildMap = buildRuntimeClosureMap + } + + for _, id := range terminalTargetIDs { + if err := buildMap(id, lookup, res); err != nil { + return nil, errs.Wrap(err, "Could not build map for terminal %s", id) } } From 47321ff08983cae0cec967b64870583737c2170e Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 24 Oct 2023 10:13:01 -0700 Subject: [PATCH 30/32] Move locale entry --- internal/locale/locales/en-us.yaml | 2 ++ .../runtime/setup/implementations/alternative/resolver.go | 2 +- .../runtime/setup/implementations/alternative/runtime.go | 2 +- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/internal/locale/locales/en-us.yaml b/internal/locale/locales/en-us.yaml index 8813a80e7c..eca2eac53b 100644 --- a/internal/locale/locales/en-us.yaml +++ b/internal/locale/locales/en-us.yaml @@ -2082,3 +2082,5 @@ err_local_commit_file: Please avoid deleting or editing this file directly. err_searchingredient_toomany: other: Too many ingredients match the query '[ACTIONABLE]{{.V0}}[/RESET]', please try to be more specific. +alternative_unknown_pkg_name: + other: unknown \ No newline at end of file diff --git a/pkg/platform/runtime/setup/implementations/alternative/resolver.go b/pkg/platform/runtime/setup/implementations/alternative/resolver.go index 3e358c6263..28abf13492 100644 --- a/pkg/platform/runtime/setup/implementations/alternative/resolver.go +++ b/pkg/platform/runtime/setup/implementations/alternative/resolver.go @@ -17,5 +17,5 @@ func (r *Resolver) ResolveArtifactName(id artifact.ArtifactID) string { if artf, ok := r.artifactsForNameResolving[id]; ok { return artf.Name } - return locale.Tl("alternative_unknown_pkg_name", "unknown") + return locale.T("alternative_unknown_pkg_name") } diff --git a/pkg/platform/runtime/setup/implementations/alternative/runtime.go b/pkg/platform/runtime/setup/implementations/alternative/runtime.go index cac7b3c29a..f6a0930c9e 100644 --- a/pkg/platform/runtime/setup/implementations/alternative/runtime.go +++ b/pkg/platform/runtime/setup/implementations/alternative/runtime.go @@ -139,7 +139,7 @@ func artifactsContainFile(file string, artifactCache map[artifact.ArtifactID]sto } func (s *Setup) ResolveArtifactName(a artifact.ArtifactID) string { - return locale.Tl("alternative_unknown_pkg_name", "unknown") + return locale.T("alternative_unknown_pkg_name") } func (s *Setup) DownloadsFromBuild(build model.Build, artifacts map[strfmt.UUID]artifact.Artifact) (download []artifact.ArtifactDownload, err error) { From c49a3d157251ea9e78e2204e7d89e3af056cda04 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 24 Oct 2023 10:15:06 -0700 Subject: [PATCH 31/32] Update integrationt test to checkout commitID --- test/integration/checkout_int_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/checkout_int_test.go b/test/integration/checkout_int_test.go index 5815be0bf9..a1a5e56189 100644 --- a/test/integration/checkout_int_test.go +++ b/test/integration/checkout_int_test.go @@ -273,7 +273,7 @@ func (suite *CheckoutIntegrationTestSuite) TestCheckoutBuildtimeClosure() { defer ts.Close() cp := ts.SpawnWithOpts( - e2e.OptArgs("checkout", "ActiveState-CLI/small-python"), + e2e.OptArgs("checkout", "ActiveState-CLI/small-python#5a1e49e5-8ceb-4a09-b605-ed334474855b"), e2e.OptAppendEnv(constants.InstallBuildDependencies+"=true"), e2e.OptAppendEnv(constants.DisableRuntime+"=false"), ) From c0a07dba0bb3b5d0592f04ed20bcc7d8377afd72 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 24 Oct 2023 11:00:47 -0700 Subject: [PATCH 32/32] Simplify function selection --- pkg/platform/runtime/buildplan/buildplan.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index 22d0d6b5f4..6fe0be1084 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -130,11 +130,9 @@ func newMapFromBuildPlan(build *model.Build, calculateBuildtimeClosure bool) (ar } } - var buildMap func(strfmt.UUID, map[strfmt.UUID]interface{}, artifact.Map) error + buildMap := buildRuntimeClosureMap if calculateBuildtimeClosure { buildMap = buildBuildtimeClosureMap - } else { - buildMap = buildRuntimeClosureMap } for _, id := range terminalTargetIDs {