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. #3545

Merged
merged 6 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions internal/captain/rationalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ 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/buildscript"
"github.com/ActiveState/cli/pkg/localcommit"
)

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: 2 additions & 0 deletions internal/locale/locales/en-us.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -1371,6 +1371,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
4 changes: 1 addition & 3 deletions internal/migrator/migrator.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package migrator

import (
"path/filepath"

"github.com/ActiveState/cli/internal/config"
"github.com/ActiveState/cli/internal/constants"
"github.com/ActiveState/cli/internal/errs"
Expand All @@ -29,7 +27,7 @@ func NewMigrator(auth *authentication.Auth, cfg *config.Instance, svcm *model.Sv
case 0:
if cfg.GetBool(constants.OptinBuildscriptsConfig) {
logging.Debug("Creating buildscript")
if err := buildscript_runbit.Initialize(filepath.Dir(project.Path()), auth, svcm); err != nil {
if err := buildscript_runbit.Initialize(project, auth, svcm); err != nil {
return v, errs.Wrap(err, "Failed to initialize buildscript")
}
}
Expand Down
35 changes: 25 additions & 10 deletions internal/runbits/buildscript/buildscript_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,21 @@ 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,
at_time = TIME,
platforms = [
"12345",
"67890"
Expand All @@ -38,9 +48,9 @@ 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,
at_time = TIME,
platforms = [
<<<<<<< local
"77777",
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 All @@ -84,7 +99,7 @@ runtime = state_tool_artifacts_v1(
src = sources
)
sources = solve(
at_time = at_time,
at_time = TIME,
platforms = [
"78977bc8-0f32-519d-80f3-9043f059398c",
"7c998ec2-7491-4e75-be4d-8885800ef5f2",
Expand Down
67 changes: 41 additions & 26 deletions internal/runbits/buildscript/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package buildscript_runbit

import (
"errors"
"net/url"
"os"
"path/filepath"

Expand All @@ -19,15 +20,14 @@ import (

// projecter is a union between project.Project and setup.Targeter
type projecter interface {
ProjectDir() string
Owner() string
Name() string
Dir() string
URL() string
}

var ErrBuildscriptNotExist = errors.New("Build script does not exist")

func ScriptFromProject(proj projecter) (*buildscript.BuildScript, error) {
path := filepath.Join(proj.ProjectDir(), constants.BuildScriptFileName)
path := filepath.Join(proj.Dir(), constants.BuildScriptFileName)
return ScriptFromFile(path)
}

Expand All @@ -47,33 +47,34 @@ type primeable interface {
primer.SvcModeler
}

func Initialize(path string, auth *authentication.Auth, svcm *model.SvcModel) error {
scriptPath := filepath.Join(path, constants.BuildScriptFileName)
script, err := ScriptFromFile(scriptPath)
if err == nil {
return nil // nothing to do, buildscript already exists
}
if !errors.Is(err, os.ErrNotExist) {
return errs.Wrap(err, "Could not read build script from file")
}

logging.Debug("Build script does not exist. Creating one.")
commitId, err := localcommit.Get(path)
// Initialize creates a new build script for the local project. It will overwrite an existing one so
// commands like `state reset` will work.
func Initialize(proj projecter, auth *authentication.Auth, svcm *model.SvcModel) error {
logging.Debug("Initializing build script")
commitId, err := localcommit.Get(proj.Dir())
if err != nil {
return errs.Wrap(err, "Unable to get the local commit ID")
}

buildplanner := buildplanner.NewBuildPlannerModel(auth, svcm)
script, err = buildplanner.GetBuildScript(commitId.String())
script, err := buildplanner.GetBuildScript(commitId.String())
if err != nil {
return errs.Wrap(err, "Unable to get the remote build expression and time")
}

if url, err := projectURL(proj, commitId.String()); err == nil {
script.SetProject(url)
} else {
return errs.Wrap(err, "Unable to set project")
}
Comment on lines +65 to +69
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to use project.URL() here.

Copy link
Contributor Author

@mitchell-as mitchell-as Oct 18, 2024

Choose a reason for hiding this comment

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

No, I cannot because the projecter has the old URL (with the old commit ID) from before a mutation. localcommit.Set() updates the URL on disk, but the primed project was not reloaded/reset/whatever, so its URL remains the same as it was originally.

I don't really see the point in changing this because when we replace project files with build scripts, we'll have to do something like constructing this URL again. I thought the point of this runbit was to know as little about projects as possible.

Copy link
Member

Choose a reason for hiding this comment

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

Good point. I don't like how opinionated the current solution is wrt the formatting of the URL though. I guess what this is touching on is that we probably need some central mechanic for project URLs. Still, to avoid further churn I think for this story we can maybe have a middle ground; like use project.URL() and only splice in the commitID. So you'd avoid having any opinion on the URL beyond the commitID parameter.

// Note: script.SetAtTime() was done in GetBuildScript().

scriptBytes, err := script.Marshal()
if err != nil {
return errs.Wrap(err, "Unable to marshal build script")
}

scriptPath := filepath.Join(proj.Dir(), constants.BuildScriptFileName)
logging.Debug("Initializing build script at %s", scriptPath)
err = fileutils.WriteFile(scriptPath, scriptBytes)
if err != nil {
Expand All @@ -83,27 +84,41 @@ func Initialize(path string, auth *authentication.Auth, svcm *model.SvcModel) er
return nil
}

func Update(proj projecter, newScript *buildscript.BuildScript) error {
script, err := ScriptFromProject(proj)
// projectURL returns proj.URL(), but with the given, updated commitID.
func projectURL(proj projecter, commitID string) (string, error) {
u, err := url.Parse(proj.URL())
if err != nil {
return errs.Wrap(err, "Could not read build script")
return "", errs.Wrap(err, "Unable to parse URL")
}
q := u.Query()
q.Set("commitID", commitID)
u.RawQuery = q.Encode()
return u.String(), nil
}

equals, err := script.Equals(newScript)
func Update(proj projecter, newScript *buildscript.BuildScript) error {
// Update the new script's project field to match the current one, except for a new commit ID.
commitID, err := localcommit.Get(proj.Dir())
if err != nil {
return errs.Wrap(err, "Unable to get the local commit ID")
}
url, err := projectURL(proj, commitID.String())
if err != nil {
return errs.Wrap(err, "Could not compare build script")
return errs.Wrap(err, "Could not construct project URL")
}
if script != nil && equals {
return nil // no changes to write
newScript2, err := newScript.Clone()
if err != nil {
return errs.Wrap(err, "Could not clone buildscript")
}
newScript2.SetProject(url)

sb, err := newScript.Marshal()
sb, err := newScript2.Marshal()
if err != nil {
return errs.Wrap(err, "Could not marshal build script")
}

logging.Debug("Writing build script")
if err := fileutils.WriteFile(filepath.Join(proj.ProjectDir(), constants.BuildScriptFileName), sb); err != nil {
if err := fileutils.WriteFile(filepath.Join(proj.Dir(), constants.BuildScriptFileName), sb); err != nil {
return errs.Wrap(err, "Could not write build script to file")
}
return nil
Expand Down
10 changes: 7 additions & 3 deletions internal/runbits/buildscript/testdata/buildscript1.as
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
at_time = "2023-10-16T22:20:29.000000Z"
```
Project: https://platform.activestate.com/ActiveState-CLI/Merge?branch=main&commitID=d908a758-6a81-40d4-b0eb-87069cd7f07d
Time: 2024-05-10T00:00:13.138Z
```

runtime = state_tool_artifacts_v1(
build_flags = [
],
Expand All @@ -7,7 +11,7 @@ runtime = state_tool_artifacts_v1(
src = sources
)
sources = solve(
at_time = at_time,
at_time = TIME,
platforms = [
"78977bc8-0f32-519d-80f3-9043f059398c",
"7c998ec2-7491-4e75-be4d-8885800ef5f2",
Expand All @@ -20,4 +24,4 @@ sources = solve(
solver_version = null
)

main = runtime
main = runtime
10 changes: 7 additions & 3 deletions internal/runbits/buildscript/testdata/buildscript2.as
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
at_time = "2023-08-01T16:20:11.985000Z"
```
Project: https://platform.activestate.com/ActiveState-CLI/Merge?branch=main&commitID=f3263ee4-ac4c-41ee-b778-2585333f49f7
Time: 2023-08-01T16:20:11.985000Z
```

runtime = state_tool_artifacts_v1(
build_flags = [
],
Expand All @@ -7,7 +11,7 @@ runtime = state_tool_artifacts_v1(
src = sources
)
sources = solve(
at_time = at_time,
at_time = TIME,
platforms = [
"78977bc8-0f32-519d-80f3-9043f059398c",
"7c998ec2-7491-4e75-be4d-8885800ef5f2",
Expand All @@ -20,4 +24,4 @@ sources = solve(
solver_version = null
)

main = runtime
main = runtime
7 changes: 6 additions & 1 deletion internal/runbits/checkout/checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,12 @@ func (r *Checkout) Run(ns *project.Namespaced, branchName, cachePath, targetPath
}

if r.prime.Config().GetBool(constants.OptinBuildscriptsConfig) {
if err := buildscript_runbit.Initialize(path, r.prime.Auth(), r.prime.SvcModel()); err != nil {
pjf, err := projectfile.FromPath(path)
if err != nil {
return "", errs.Wrap(err, "Unable to load project file")
}

if err := buildscript_runbit.Initialize(pjf, r.prime.Auth(), r.prime.SvcModel()); err != nil {
return "", errs.Wrap(err, "Unable to initialize buildscript")
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/runbits/commits_runbit/time.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func ExpandTimeForBuildScript(ts *captain.TimeValue, auth *authentication.Auth,
}

atTime := script.AtTime()
if atTime.After(timestamp) {
if atTime != nil && atTime.After(timestamp) {
return *atTime, nil
}

Expand Down
2 changes: 1 addition & 1 deletion internal/runners/initialize/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ func (r *Initialize) Run(params *RunParams) (rerr error) {
}

if r.config.GetBool(constants.OptinBuildscriptsConfig) {
if err := buildscript_runbit.Initialize(proj.Dir(), r.auth, r.svcModel); err != nil {
if err := buildscript_runbit.Initialize(proj, r.auth, r.svcModel); err != nil {
return errs.Wrap(err, "Unable to initialize buildscript")
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/runners/reset/reset.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (r *Reset) Run(params *Params) error {
// Ensure the buildscript exists. Normally we should never do this, but reset is used for resetting from a corrupted
// state, so it is appropriate.
if r.cfg.GetBool(constants.OptinBuildscriptsConfig) {
if err := buildscript_runbit.Initialize(r.project.Dir(), r.auth, r.svcModel); err != nil {
if err := buildscript_runbit.Initialize(r.project, r.auth, r.svcModel); err != nil {
return errs.Wrap(err, "Unable to initialize buildscript")
}
}
Expand Down
Loading
Loading