Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add separate checkout info block to buildscripts. #3483

Open
wants to merge 18 commits into
base: version/0-47-0-RC1
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
2081db6
Participle should unquote strings.
mitchell-as Sep 6, 2024
5033503
Initial support for commit info in buildscript.
mitchell-as Sep 6, 2024
72cb762
Fetching buildscripts should also populate the Project commit info.
mitchell-as Sep 9, 2024
ec39083
localcommit package should update buildscript's Project commit info.
mitchell-as Sep 9, 2024
5b403b6
Rename "commit info" to "checkout info" in recent changes.
mitchell-as Sep 9, 2024
41ee7a1
Rename localcommit package to checkoutinfo.
mitchell-as Sep 9, 2024
e504147
Fixed failing unit tests.
mitchell-as Sep 9, 2024
a2e043d
Add warning about using an outdated buildscript.
mitchell-as Sep 9, 2024
e036299
Sort artifact changesets by name.
mitchell-as Sep 10, 2024
1bbf6c2
Revert "Sort artifact changesets by name."
mitchell-as Sep 10, 2024
008161f
Pass project details to the buildscript runbit, not project objects.
mitchell-as Sep 10, 2024
4a91dda
Include branch in buildscript commit info.
mitchell-as Sep 11, 2024
ccd9554
Use YAML parser when unmarshaling buildscript checkout info.
mitchell-as Sep 11, 2024
b1bb954
Update activestate.yaml with any buildscript checkout info changes.
mitchell-as Sep 11, 2024
9bcce03
Make build scripts first-class citizens.
mitchell-as Sep 12, 2024
a54541d
Fix some failing tests.
mitchell-as Sep 13, 2024
6b174f8
Make checkoutinfo a primer property and use it to set commit IDs, and…
mitchell-as Sep 14, 2024
4a150c0
Moved `buildscript_runbit.ScriptFromProject()` into `checkoutinfo.Bui…
mitchell-as Sep 16, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions internal/captain/rationalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ import (
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/runbits/rationalize"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/pkg/buildscript"
"github.com/ActiveState/cli/pkg/checkoutinfo"
)

func rationalizeError(err *error) {
var errInvalidCommitID *localcommit.ErrInvalidCommitID
var errInvalidCommitID *checkoutinfo.ErrInvalidCommitID

switch {
case err == nil:
Expand All @@ -31,5 +32,11 @@ func rationalizeError(err *error) {
*err = errs.WrapUserFacing(*err,
locale.Tr("err_commit_id_invalid", errInvalidCommitID.CommitID),
errs.SetInput())

// Outdated build script.
case errors.Is(*err, buildscript.ErrOutdatedAtTime):
*err = errs.WrapUserFacing(*err,
locale.T("err_outdated_buildscript"),
errs.SetInput())
}
}
2 changes: 1 addition & 1 deletion internal/constraints/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ func NewPrimeConditional(auth *authentication.Auth, pj projectable, subshellName
pjName = pj.Name()
pjNamespace = pj.NamespaceString()
pjURL = pj.URL()
pjCommit = pj.LegacyCommitID() // Not using localcommit due to import cycle. See anti-pattern comment in localcommit pkg.
pjCommit = pj.LegacyCommitID() // Not using checkoutinfo due to import cycle. See anti-pattern comment in checkoutinfo pkg.
pjBranch = pj.BranchName()
pjDir = pj.Dir()
}
Expand Down
2 changes: 2 additions & 0 deletions internal/locale/locales/en-us.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1431,6 +1431,8 @@ err_commit_id_invalid_given:
other: "Invalid commit ID: '{{.V0}}'."
err_commit_id_not_in_history:
other: "The project '[ACTIONABLE]{{.V0}}[/RESET]' does not contain the provided commit: '[ACTIONABLE]{{.V1}}[/RESET]'."
err_outdated_buildscript:
other: "[WARNING]Warning:[/RESET] You are using an outdated version of the buildscript. Please run '[ACTIONABLE]state reset LOCAL[/RESET]' in order to reinitialize your buildscript file."
err_shortcutdir_writable:
other: Could not continue as we don't have permission to create [ACTIONABLE]{{.V0}}[/RESET].
move_prompt:
Expand Down
9 changes: 3 additions & 6 deletions internal/migrator/migrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"path/filepath"

"github.com/ActiveState/cli/internal/config"
"github.com/ActiveState/cli/internal/constants"
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
Expand All @@ -26,11 +25,9 @@ func NewMigrator(auth *authentication.Auth, cfg *config.Instance) projectfile.Mi
// WARNING: When we return a version along with an error we need to ensure that all updates UP TO THAT VERSION
// have completed. Ensure you roll back any partial updates in the case of an error as they will need to be attempted again.
case 0:
if cfg.GetBool(constants.OptinBuildscriptsConfig) {
logging.Debug("Creating buildscript")
if err := buildscript_runbit.Initialize(filepath.Dir(project.Path()), auth); err != nil {
return v, errs.Wrap(err, "Failed to initialize buildscript")
}
logging.Debug("Attempting to create buildscript")
if err := buildscript_runbit.Initialize(filepath.Dir(project.Path()), project.Owner(), project.Name(), project.BranchName(), project.LegacyCommitID(), auth, cfg); err != nil {
return v, errs.Wrap(err, "Failed to initialize buildscript")
}
}
}
Expand Down
37 changes: 25 additions & 12 deletions internal/runbits/buildplanner/buildplanner.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package buildplanner

import (
"github.com/ActiveState/cli/internal/config"
"github.com/ActiveState/cli/internal/constants"
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/rtutils/ptr"
buildscript_runbit "github.com/ActiveState/cli/internal/runbits/buildscript"
"github.com/ActiveState/cli/internal/runbits/rationalize"
"github.com/ActiveState/cli/pkg/buildplan"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/pkg/platform/api/buildplanner/request"
"github.com/ActiveState/cli/pkg/platform/authentication"
"github.com/ActiveState/cli/pkg/platform/model"
Expand Down Expand Up @@ -43,7 +44,8 @@ func GetCommit(
commitID string,
target string,
auth *authentication.Auth,
out output.Outputer) (commit *bpModel.Commit, rerr error) {
out output.Outputer,
cfg *config.Instance) (commit *bpModel.Commit, rerr error) {
if pj == nil && !namespace.IsValid() {
return nil, rationalize.ErrNoProject
}
Expand Down Expand Up @@ -76,21 +78,21 @@ func GetCommit(
switch {
// Return the buildplan from this runtime.
case !namespaceProvided && !commitIdProvided:
localCommitID, err := localcommit.Get(pj.Path())
localCommitID, err := buildscript_runbit.CommitID(pj.Path(), cfg)
if err != nil {
return nil, errs.Wrap(err, "Could not get local commit")
return nil, errs.Wrap(err, "Could not get commit ID")
}

bp := bpModel.NewBuildPlannerModel(auth)
commit, err = bp.FetchCommit(localCommitID, pj.Owner(), pj.Name(), targetPtr)
commit, err = bp.FetchCommit(localCommitID, pj.Owner(), pj.Name(), pj.BranchName(), targetPtr)
if err != nil {
return nil, errs.Wrap(err, "Failed to fetch commit")
}

// Return buildplan from the given commitID for the current project.
case !namespaceProvided && commitIdProvided:
bp := bpModel.NewBuildPlannerModel(auth)
commit, err = bp.FetchCommit(commitUUID, pj.Owner(), pj.Name(), targetPtr)
commit, err = bp.FetchCommit(commitUUID, pj.Owner(), pj.Name(), pj.BranchName(), targetPtr)
if err != nil {
return nil, errs.Wrap(err, "Failed to fetch commit")
}
Expand All @@ -114,15 +116,25 @@ func GetCommit(
commitUUID = *branchCommitUUID

bp := bpModel.NewBuildPlannerModel(auth)
commit, err = bp.FetchCommit(commitUUID, namespace.Owner, namespace.Project, targetPtr)
commit, err = bp.FetchCommit(commitUUID, namespace.Owner, namespace.Project, branch.Label, targetPtr)
if err != nil {
return nil, errs.Wrap(err, "Failed to fetch commit")
}

// Return the buildplan for the given commitID of the given project.
case namespaceProvided && commitIdProvided:
pj, err := model.FetchProjectByName(namespace.Owner, namespace.Project, auth)
if err != nil {
return nil, locale.WrapExternalError(err, "err_fetch_project", "", namespace.String())
}

branch, err := model.DefaultBranchForProject(pj)
if err != nil {
return nil, errs.Wrap(err, "Could not grab branch for project")
}

bp := bpModel.NewBuildPlannerModel(auth)
commit, err = bp.FetchCommit(commitUUID, namespace.Owner, namespace.Project, targetPtr)
commit, err = bp.FetchCommit(commitUUID, namespace.Owner, namespace.Project, branch.Label, targetPtr)
if err != nil {
return nil, errs.Wrap(err, "Failed to fetch commit")
}
Expand All @@ -143,9 +155,9 @@ func GetCommit(
owner = pj.Owner()
name = pj.Name()
nsString = pj.NamespaceString()
commitID, err := localcommit.Get(pj.Path())
commitID, err := buildscript_runbit.CommitID(pj.Path(), cfg)
if err != nil {
return nil, errs.Wrap(err, "Could not get local commit")
return nil, errs.Wrap(err, "Could not get commit ID")
}
localCommitID = &commitID
}
Expand All @@ -170,8 +182,9 @@ func GetBuildPlan(
commitID string,
target string,
auth *authentication.Auth,
out output.Outputer) (bp *buildplan.BuildPlan, rerr error) {
commit, err := GetCommit(pj, namespace, commitID, target, auth, out)
out output.Outputer,
cfg *config.Instance) (bp *buildplan.BuildPlan, rerr error) {
commit, err := GetCommit(pj, namespace, commitID, target, auth, out, cfg)
if err != nil {
return nil, errs.Wrap(err, "Could not get commit")
}
Expand Down
29 changes: 22 additions & 7 deletions internal/runbits/buildscript/buildscript_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,19 @@ import (
"github.com/stretchr/testify/require"
)

const testProject = "https://platform.activestate.com/org/project?branch=main&commitID=00000000-0000-0000-0000-000000000000"
const testTime = "2000-01-01T00:00:00.000Z"

func checkoutInfo(project, time string) string {
return "```\n" +
"Project: " + project + "\n" +
"Time: " + time + "\n" +
"```\n"
}

func TestDiff(t *testing.T) {
script, err := buildscript.Unmarshal([]byte(
`at_time = "2000-01-01T00:00:00.000Z"
checkoutInfo(testProject, testTime) + `
runtime = solve(
at_time = at_time,
platforms = [
Expand All @@ -38,7 +48,7 @@ main = runtime`))
// Generate the difference between the modified script and the original expression.
result, err := generateDiff(modifiedScript, script)
require.NoError(t, err)
assert.Equal(t, `at_time = "2000-01-01T00:00:00.000Z"
assert.Equal(t, checkoutInfo(testProject, testTime)+`
runtime = solve(
at_time = at_time,
platforms = [
Expand Down Expand Up @@ -71,11 +81,16 @@ func TestRealWorld(t *testing.T) {
require.NoError(t, err)
result, err := generateDiff(script1, script2)
require.NoError(t, err)
assert.Equal(t, `<<<<<<< local
at_time = "2023-10-16T22:20:29.000Z"
=======
at_time = "2023-08-01T16:20:11.985Z"
>>>>>>> remote
assert.Equal(t,
"```\n"+
"<<<<<<< local\n"+
"Project: https://platform.activestate.com/ActiveState-CLI/Merge?branch=main&commitID=d908a758-6a81-40d4-b0eb-87069cd7f07d\n"+
"Time: 2024-05-10T00:00:13.138Z\n"+
"=======\n"+
"Project: https://platform.activestate.com/ActiveState-CLI/Merge?branch=main&commitID=f3263ee4-ac4c-41ee-b778-2585333f49f7\n"+
"Time: 2023-08-01T16:20:11.985Z\n"+
">>>>>>> remote\n"+
"```\n"+`
runtime = state_tool_artifacts_v1(
build_flags = [
],
Expand Down
36 changes: 36 additions & 0 deletions internal/runbits/buildscript/commitid.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package buildscript_runbit

import (
"errors"

"github.com/go-openapi/strfmt"

"github.com/ActiveState/cli/internal/constants"
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/pkg/buildscript"
"github.com/ActiveState/cli/pkg/checkoutinfo"
)

func CommitID(path string, cfg configurer) (strfmt.UUID, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to checkoutinfo? I get where you're coming from, but runbits are about sharing controller logic. This isn't controller logic. And this type of muliplexing is exactly what the checkoutinfo package is intended for.

script, err := buildscript.New()
if err != nil {
return "", errs.Wrap(err, "Could not create build script")
}

project, err := checkoutinfo.GetProject(path)
if err != nil {
return "", errs.Wrap(err, "Could not get project")
}
script.SetProjectURL(project) // script.CommitID() is extracted from project URL

if cfg.GetBool(constants.OptinBuildscriptsConfig) {
if script2, err := ScriptFromProject(path); err == nil {
script = script2
} else if !errors.Is(err, ErrBuildscriptNotExist) {
return "", errs.Wrap(err, "Could not get build script")
}
// ErrBuildscriptNotExist will fall back on activestate.yaml
}

return script.CommitID()
}
Loading
Loading