From e28c7a30c779fe90e5977e707e2e90634695a641 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Mon, 18 Dec 2023 16:02:19 -0800 Subject: [PATCH 1/8] Don't set commit ID on build errors --- internal/runbits/refresh.go | 9 +++++++-- internal/runbits/requirements/requirements.go | 20 ++++++++++++++----- internal/runbits/runbits.go | 15 ++++++++++++++ pkg/platform/runtime/buildplan/buildplan.go | 8 +++++++- 4 files changed, 44 insertions(+), 8 deletions(-) diff --git a/internal/runbits/refresh.go b/internal/runbits/refresh.go index 0dd7d9341c..aba82b3e47 100644 --- a/internal/runbits/refresh.go +++ b/internal/runbits/refresh.go @@ -29,9 +29,14 @@ func RefreshRuntime( //if err != nil { // return locale.WrapError(err, "err_update_build_script") //} - target := target.NewProjectTarget(proj, nil, trigger) + var t *target.ProjectTarget + if proj != nil && *proj.Namespace().CommitID != commitID { + t = target.NewProjectTarget(proj, &commitID, trigger) + } else { + t = target.NewProjectTarget(proj, nil, trigger) + } isCached := true - rt, err := runtime.New(target, an, svcm, auth) + rt, err := runtime.New(t, an, svcm, auth) if err != nil { if runtime.IsNeedsUpdateError(err) { isCached = false diff --git a/internal/runbits/requirements/requirements.go b/internal/runbits/requirements/requirements.go index ea2c9fd6e9..fbd9102e8b 100644 --- a/internal/runbits/requirements/requirements.go +++ b/internal/runbits/requirements/requirements.go @@ -297,10 +297,6 @@ func (r *RequirementOperation) ExecuteRequirementOperation( // return errs.Wrap(err, "Could not get remote build expr") //} - if err := commitmediator.Set(r.Project, commitID.String()); err != nil { - return locale.WrapError(err, "err_package_update_commit_id") - } - // Note: a commit ID file needs to exist at this point. // Re-enable in DX-2307. //err = buildscript.Update(r.Project, expr, r.Auth) @@ -311,7 +307,11 @@ func (r *RequirementOperation) ExecuteRequirementOperation( // refresh or install runtime err = runbits.RefreshRuntime(r.Auth, r.Output, r.Analytics, r.Project, commitID, true, trigger, r.SvcModel) if err != nil { - return err + return handleRefreshError(err, r.Project, parentCommitID) + } + + if err := commitmediator.Set(r.Project, commitID.String()); err != nil { + return locale.WrapError(err, "err_package_update_commit_id") } if !hasParentCommit { @@ -348,6 +348,16 @@ func (r *RequirementOperation) ExecuteRequirementOperation( return nil } +func handleRefreshError(err error, project *project.Project, parentCommitID strfmt.UUID) error { + // If the error is a build error then return, if not update the commit ID then return + if !runbits.IsBuildError(err) { + if err := commitmediator.Set(project, parentCommitID.String()); err != nil { + return locale.WrapError(err, "err_package_update_commit_id") + } + } + return err +} + func supportedLanguageByName(supported []medmodel.SupportedLanguage, langName string) medmodel.SupportedLanguage { return funk.Find(supported, func(l medmodel.SupportedLanguage) bool { return l.Name == langName }).(medmodel.SupportedLanguage) } diff --git a/internal/runbits/runbits.go b/internal/runbits/runbits.go index 05a37c8e0d..3454875108 100644 --- a/internal/runbits/runbits.go +++ b/internal/runbits/runbits.go @@ -1,2 +1,17 @@ // Package runbits comprises logic that is shared between controllers, ie., code that prints package runbits + +import ( + "github.com/ActiveState/cli/internal/errs" + "github.com/ActiveState/cli/pkg/platform/api/buildplanner/model" + "github.com/ActiveState/cli/pkg/platform/runtime/buildplan" + "github.com/ActiveState/cli/pkg/platform/runtime/setup" + "github.com/ActiveState/cli/pkg/platform/runtime/setup/buildlog" +) + +func IsBuildError(err error) bool { + return errs.Matches(err, &setup.BuildError{}) || + errs.Matches(err, &buildlog.BuildError{}) || + errs.Matches(err, &model.BuildPlannerError{}) || + errs.Matches(err, &buildplan.ArtifactError{}) +} diff --git a/pkg/platform/runtime/buildplan/buildplan.go b/pkg/platform/runtime/buildplan/buildplan.go index d0937ed658..707a58aeb3 100644 --- a/pkg/platform/runtime/buildplan/buildplan.go +++ b/pkg/platform/runtime/buildplan/buildplan.go @@ -19,6 +19,10 @@ type ArtifactListing struct { artifactIDs []artifact.ArtifactID } +type ArtifactError struct { + *locale.LocalizedError +} + func NewArtifactListing(build *model.Build, buildtimeClosure bool) (*ArtifactListing, error) { al := &ArtifactListing{build: build} if buildtimeClosure { @@ -190,7 +194,9 @@ func buildTerminals(nodeID strfmt.UUID, lookup map[strfmt.UUID]interface{}, resu } if !model.IsSuccessArtifactStatus(targetArtifact.Status) { - return locale.NewError("err_artifact_failed", "Artifact '{{.V0}}' failed to build", trimDisplayName(targetArtifact.DisplayName)) + return &ArtifactError{ + locale.NewError("err_artifact_failed", "Artifact '{{.V0}}' failed to build", trimDisplayName(targetArtifact.DisplayName)), + } } if model.IsStateToolArtifact(targetArtifact.MimeType) { From 7b00beefdabece187f87f2010cfb564b5864ecd5 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 19 Dec 2023 08:42:09 -0800 Subject: [PATCH 2/8] Attempt to fix panic --- internal/runbits/refresh.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/runbits/refresh.go b/internal/runbits/refresh.go index aba82b3e47..e8e499f641 100644 --- a/internal/runbits/refresh.go +++ b/internal/runbits/refresh.go @@ -1,6 +1,8 @@ package runbits import ( + "strings" + "github.com/ActiveState/cli/internal/analytics" "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/output" @@ -30,7 +32,7 @@ func RefreshRuntime( // return locale.WrapError(err, "err_update_build_script") //} var t *target.ProjectTarget - if proj != nil && *proj.Namespace().CommitID != commitID { + if proj != nil && strings.EqualFold(proj.Namespace().CommitID.String(), commitID.String()) { t = target.NewProjectTarget(proj, &commitID, trigger) } else { t = target.NewProjectTarget(proj, nil, trigger) From 0af3d15eb87bf4a3bb462579a452a3970fc1331d Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 19 Dec 2023 09:16:31 -0800 Subject: [PATCH 3/8] Fix panic --- internal/runbits/refresh.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/internal/runbits/refresh.go b/internal/runbits/refresh.go index e8e499f641..30498eaf53 100644 --- a/internal/runbits/refresh.go +++ b/internal/runbits/refresh.go @@ -1,8 +1,6 @@ package runbits import ( - "strings" - "github.com/ActiveState/cli/internal/analytics" "github.com/ActiveState/cli/internal/locale" "github.com/ActiveState/cli/internal/output" @@ -32,7 +30,7 @@ func RefreshRuntime( // return locale.WrapError(err, "err_update_build_script") //} var t *target.ProjectTarget - if proj != nil && strings.EqualFold(proj.Namespace().CommitID.String(), commitID.String()) { + if commitIDFromProject(proj) != commitID { t = target.NewProjectTarget(proj, &commitID, trigger) } else { t = target.NewProjectTarget(proj, nil, trigger) @@ -71,3 +69,10 @@ func RefreshRuntime( return nil } + +func commitIDFromProject(proj *project.Project) strfmt.UUID { + if proj != nil && proj.Namespace() != nil && proj.Namespace().CommitID != nil { + return *proj.Namespace().CommitID + } + return "" +} From e01f977106aabfcc504e287d452638c6298938d8 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 19 Dec 2023 09:43:36 -0800 Subject: [PATCH 4/8] Debug failure that is not showing up in the logs --- pkg/platform/runtime/runtime.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/platform/runtime/runtime.go b/pkg/platform/runtime/runtime.go index e3441f5b72..0ec8491ee8 100644 --- a/pkg/platform/runtime/runtime.go +++ b/pkg/platform/runtime/runtime.go @@ -231,6 +231,7 @@ func (r *Runtime) recordCompletion(err error) { var action string if err != nil { action = anaConsts.ActRuntimeFailure + logging.Error("Runtime failed: %v", errs.JoinMessage(err)) } else { action = anaConsts.ActRuntimeSuccess r.recordUsage() From bbdb55abc88df6253ff01d28a28fb76a6867ecc2 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 19 Dec 2023 10:18:55 -0800 Subject: [PATCH 5/8] Move local orders code --- internal/runbits/requirements/requirements.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/runbits/requirements/requirements.go b/internal/runbits/requirements/requirements.go index fbd9102e8b..2269b10191 100644 --- a/internal/runbits/requirements/requirements.go +++ b/internal/runbits/requirements/requirements.go @@ -297,13 +297,6 @@ func (r *RequirementOperation) ExecuteRequirementOperation( // return errs.Wrap(err, "Could not get remote build expr") //} - // Note: a commit ID file needs to exist at this point. - // Re-enable in DX-2307. - //err = buildscript.Update(r.Project, expr, r.Auth) - //if err != nil { - // return locale.WrapError(err, "err_update_build_script") - //} - // refresh or install runtime err = runbits.RefreshRuntime(r.Auth, r.Output, r.Analytics, r.Project, commitID, true, trigger, r.SvcModel) if err != nil { @@ -314,6 +307,13 @@ func (r *RequirementOperation) ExecuteRequirementOperation( return locale.WrapError(err, "err_package_update_commit_id") } + // Note: a commit ID file needs to exist at this point. + // Re-enable in DX-2307. + //err = buildscript.Update(r.Project, expr, r.Auth) + //if err != nil { + // return locale.WrapError(err, "err_update_build_script") + //} + if !hasParentCommit { out.Notice(locale.Tr("install_initial_success", r.Project.Source().Path())) } From 1e99c7312ac37dd5b9b3771a35252a1c8ee8ae3b Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 19 Dec 2023 10:50:24 -0800 Subject: [PATCH 6/8] Remove debug line --- pkg/platform/runtime/runtime.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/platform/runtime/runtime.go b/pkg/platform/runtime/runtime.go index 0ec8491ee8..e3441f5b72 100644 --- a/pkg/platform/runtime/runtime.go +++ b/pkg/platform/runtime/runtime.go @@ -231,7 +231,6 @@ func (r *Runtime) recordCompletion(err error) { var action string if err != nil { action = anaConsts.ActRuntimeFailure - logging.Error("Runtime failed: %v", errs.JoinMessage(err)) } else { action = anaConsts.ActRuntimeSuccess r.recordUsage() From 5ce886bbdd6d9a06b5fb55000b9605e69bc8450e Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 19 Dec 2023 11:52:19 -0800 Subject: [PATCH 7/8] Simplify getting commit ID for target initialization --- internal/runbits/refresh.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/internal/runbits/refresh.go b/internal/runbits/refresh.go index 30498eaf53..7dd528e4e5 100644 --- a/internal/runbits/refresh.go +++ b/internal/runbits/refresh.go @@ -29,14 +29,9 @@ func RefreshRuntime( //if err != nil { // return locale.WrapError(err, "err_update_build_script") //} - var t *target.ProjectTarget - if commitIDFromProject(proj) != commitID { - t = target.NewProjectTarget(proj, &commitID, trigger) - } else { - t = target.NewProjectTarget(proj, nil, trigger) - } + target := target.NewProjectTarget(proj, resolveCommitID(proj, &commitID), trigger) isCached := true - rt, err := runtime.New(t, an, svcm, auth) + rt, err := runtime.New(target, an, svcm, auth) if err != nil { if runtime.IsNeedsUpdateError(err) { isCached = false @@ -70,9 +65,15 @@ func RefreshRuntime( return nil } -func commitIDFromProject(proj *project.Project) strfmt.UUID { +func resolveCommitID(proj *project.Project, customCommitID *strfmt.UUID) *strfmt.UUID { + var projectCommitID *strfmt.UUID if proj != nil && proj.Namespace() != nil && proj.Namespace().CommitID != nil { - return *proj.Namespace().CommitID + projectCommitID = proj.Namespace().CommitID + } + + if projectCommitID != customCommitID { + return customCommitID } - return "" + + return nil } From 4cde164304d99577a0cb8af6a803f8b256b1c287 Mon Sep 17 00:00:00 2001 From: mdrakos Date: Tue, 19 Dec 2023 11:52:32 -0800 Subject: [PATCH 8/8] Centralize commit updates --- internal/runbits/requirements/requirements.go | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/internal/runbits/requirements/requirements.go b/internal/runbits/requirements/requirements.go index 2269b10191..ccceef08e2 100644 --- a/internal/runbits/requirements/requirements.go +++ b/internal/runbits/requirements/requirements.go @@ -303,17 +303,10 @@ func (r *RequirementOperation) ExecuteRequirementOperation( return handleRefreshError(err, r.Project, parentCommitID) } - if err := commitmediator.Set(r.Project, commitID.String()); err != nil { + if err := updateCommitID(r.Project, commitID); err != nil { return locale.WrapError(err, "err_package_update_commit_id") } - // Note: a commit ID file needs to exist at this point. - // Re-enable in DX-2307. - //err = buildscript.Update(r.Project, expr, r.Auth) - //if err != nil { - // return locale.WrapError(err, "err_update_build_script") - //} - if !hasParentCommit { out.Notice(locale.Tr("install_initial_success", r.Project.Source().Path())) } @@ -351,13 +344,29 @@ func (r *RequirementOperation) ExecuteRequirementOperation( func handleRefreshError(err error, project *project.Project, parentCommitID strfmt.UUID) error { // If the error is a build error then return, if not update the commit ID then return if !runbits.IsBuildError(err) { - if err := commitmediator.Set(project, parentCommitID.String()); err != nil { + if err := updateCommitID(project, parentCommitID); err != nil { return locale.WrapError(err, "err_package_update_commit_id") } } return err } +func updateCommitID(project *project.Project, commitID strfmt.UUID) error { + if err := commitmediator.Set(project, commitID.String()); err != nil { + return locale.WrapError(err, "err_package_update_commit_id") + } + + // Note: a commit ID file needs to exist at this point. + // Re-enable in DX-2307. + // Will have to pass the buildscript as an argument to this function. + //err = buildscript.Update(r.Project, expr, r.Auth) + //if err != nil { + // return locale.WrapError(err, "err_update_build_script") + //} + + return nil +} + func supportedLanguageByName(supported []medmodel.SupportedLanguage, langName string) medmodel.SupportedLanguage { return funk.Find(supported, func(l medmodel.SupportedLanguage) bool { return l.Name == langName }).(medmodel.SupportedLanguage) }