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

Support old project format with commitID in activestate.yaml. #2829

Merged
merged 12 commits into from
Oct 20, 2023
Merged
Show file tree
Hide file tree
Changes from 8 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
8 changes: 8 additions & 0 deletions cmd/state/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/ActiveState/cli/internal/prompt"
_ "github.com/ActiveState/cli/internal/prompt" // Sets up survey defaults
"github.com/ActiveState/cli/internal/rollbar"
"github.com/ActiveState/cli/internal/runbits/legacy/projectmigration"
"github.com/ActiveState/cli/internal/runbits/panics"
"github.com/ActiveState/cli/internal/subshell"
"github.com/ActiveState/cli/internal/svcctl"
Expand Down Expand Up @@ -211,6 +212,13 @@ func run(args []string, isInteractive bool, cfg *config.Instance, out output.Out
// Set up prompter
prompter := prompt.New(isInteractive, an)

// This is an anti-pattern. DO NOT DO THIS! Normally we should be passing prompt and out as
// arguments everywhere it is needed. However, we need to support legacy projects with commitId in
// activestate.yaml, and whenever that commitId is needed, we need to prompt the user to migrate
// their project. This would result in a lot of boilerplate for a legacy feature, so we're
// working around it with package "globals".
projectmigration.Register(prompter, out)

// Set up conditional, which accesses a lot of primer data
sshell := subshell.New(cfg)

Expand Down
7 changes: 4 additions & 3 deletions internal/constraints/constraints.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/multilog"
"github.com/ActiveState/cli/internal/rtutils/ptr"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/platform/authentication"
"github.com/ActiveState/cli/pkg/projectfile"
"github.com/ActiveState/cli/pkg/sysinfo"
Expand Down Expand Up @@ -81,6 +81,7 @@ type projectable interface {
Path() string
Dir() string
URL() string
LegacyCommitID() string // for commitmediator.Get
}

func NewPrimeConditional(auth *authentication.Auth, pj projectable, subshellName string) *Conditional {
Expand All @@ -98,8 +99,8 @@ func NewPrimeConditional(auth *authentication.Auth, pj projectable, subshellName
pjName = pj.Name()
pjNamespace = pj.NamespaceString()
pjURL = pj.URL()
commitID, err := localcommit.Get(pj.Dir())
if err != nil && !localcommit.IsFileDoesNotExistError(err) {
commitID, err := commitmediator.Get(pj)
if err != nil {
multilog.Error("Unable to get local commit: %v", errs.JoinMessage(err))
}
pjCommit = commitID.String()
Expand Down
5 changes: 5 additions & 0 deletions internal/locale/locales/en-us.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2080,5 +2080,10 @@ err_local_commit_file:
To view your projects commits run '[ACTIONABLE]state history[/RESET]'.

Please avoid deleting or editing this file directly.
projectmigration_confirm:
other: |
Your project is currently read-only until you migrate to the new project format.
After you migrate, older versions of the State Tool will not be able to use your project until they update.
Would you like to perform the migration now?
Copy link
Member

Choose a reason for hiding this comment

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

@rawktron please review

Copy link
Member

Choose a reason for hiding this comment

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

What's the full message? We have to give them some context for this. Is there another part of this message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rawktron That is the full message. Whenever they run a state command that needs to read a commitId from activestate.yaml, this message is presented to them. The ticket did not specify what the prompt should look like, so I came up with this for the time being. Please advise what message you'd like to give.

Copy link
Member

Choose a reason for hiding this comment

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

It should be something like "State Tool has introduced a new project format to provide enhanced functionality. Your project is currently using the old format and will remain read-only until you migrate to the new project format.

WARNING: After you migrate, older versions of the State Tool will not be able to use your project.

Would you like to perform the migration now?"

err_searchingredient_toomany:
other: Too many ingredients match the query '[ACTIONABLE]{{.V0}}[/RESET]', please try to be more specific.
33 changes: 33 additions & 0 deletions internal/runbits/commitmediator/commitmediator.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package commitmediator

import (
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/runbits/legacy/projectmigration"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/go-openapi/strfmt"
)

type projecter interface {
Dir() string
URL() string
Path() string
LegacyCommitID() string
}

// Get returns the given project's commit ID in either the new format (commit file), or the old
// format (activestate.yaml).
// If you require the commit file, use localcommit.Get().
func Get(proj projecter) (strfmt.UUID, error) {
if commitID, err := localcommit.Get(proj.Dir()); err == nil {
return commitID, nil
} else if localcommit.IsFileDoesNotExistError(err) {
if migrated, err := projectmigration.PromptAndMigrate(proj); err == nil && migrated {
return localcommit.Get(proj.Dir())
} else if err != nil {
return "", errs.Wrap(err, "Could not prompt and/or migrate project")
}
return strfmt.UUID(proj.LegacyCommitID()), nil
} else {
return "", errs.Wrap(err, "Could not get local commit")
}
}
79 changes: 79 additions & 0 deletions internal/runbits/legacy/projectmigration/projectmigration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package projectmigration

import (
"path/filepath"

"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/fileutils"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/multilog"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/prompt"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/pkg/projectfile"
)

type projecter interface {
Dir() string
URL() string
Path() string
LegacyCommitID() string
}

var prompter prompt.Prompter
var out output.Outputer
var declined bool

// Register exists to avoid boilerplate in passing prompt and out to every caller of
// commitmediator.Get() for retrieving legacy commitId from activestate.yaml.
// This is an anti-pattern and is only used to make this legacy feature palatable.
func Register(prompter_ prompt.Prompter, out_ output.Outputer) {
prompter = prompter_
out = out_
}

func PromptAndMigrate(proj projecter) (bool, error) {
if prompter == nil || out == nil {
return false, errs.New("projectmigration.Register() has not been called")
}

if declined {
return false, nil
}

defaultChoice := false
if migrate, err := prompter.Confirm("", locale.T("projectmigration_confirm"), &defaultChoice); err == nil && !migrate {
if out.Config().Interactive {
out.Notice(locale.Tl("projectmigration_declined", "Migration declined for now"))
}
declined = true
return false, nil
} else if err != nil {
return false, errs.Wrap(err, "Could not confirm migration choice")
}

if err := localcommit.Set(proj.Dir(), proj.LegacyCommitID()); err != nil {
return false, errs.Wrap(err, "Could not create local commit file")
}

if fileutils.DirExists(filepath.Join(proj.Dir(), ".git")) {
Copy link
Member

Choose a reason for hiding this comment

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

Realize this probably wasn't introduced here but we should go up the directory tree to check for this also, since people may not checkout their activestate.yaml inside the same dir as their git repo (eg. TheHomeRepot).

You can use:

func FindFileInPath(dir, filename string) (string, error) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sadly, that's files only, not directories. Still, I'll do something to walk up the tree.

err := localcommit.AddToGitIgnore(proj.Dir())
if err != nil {
multilog.Error("Unable to add local commit file to .gitignore: %v", err)
Copy link
Member

Choose a reason for hiding this comment

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

This would almost certainly be due to something client-side like permission errors I think? We should probably not log this to rollbar. Or maybe add a check to only log if it's not a permission or IO error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't able to find a generic ErrIO variable, but I did find ErrPermission, so we'll go with that.

out.Notice(locale.T("notice_commit_id_gitignore"))
}
}

pf := projectfile.NewProjectField()
if err := pf.LoadProject(proj.URL()); err != nil {
return false, errs.Wrap(err, "Could not load activestate.yaml")
}
pf.StripCommitID()
if err := pf.Save(proj.Path()); err != nil {
return false, errs.Wrap(err, "Could not save activestate.yaml")
}

out.Notice(locale.Tl("projectmigration_success", "Your project was successfully migrated"))

return true, nil
}
6 changes: 3 additions & 3 deletions internal/runners/activate/activate.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ import (
"github.com/ActiveState/cli/internal/process"
"github.com/ActiveState/cli/internal/prompt"
"github.com/ActiveState/cli/internal/runbits/activation"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/internal/runbits/findproject"
"github.com/ActiveState/cli/internal/runbits/runtime"
"github.com/ActiveState/cli/internal/subshell"
"github.com/ActiveState/cli/internal/virtualenvironment"
"github.com/ActiveState/cli/pkg/cmdlets/checker"
"github.com/ActiveState/cli/pkg/cmdlets/checkout"
"github.com/ActiveState/cli/pkg/cmdlets/git"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/pkg/platform/authentication"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/platform/runtime/target"
Expand Down Expand Up @@ -195,8 +195,8 @@ func (r *Activate) Run(params *ActivateParams) (rerr error) {
}
}

commitID, err := localcommit.Get(proj.Dir())
if err != nil && !localcommit.IsFileDoesNotExistError(err) {
commitID, err := commitmediator.Get(proj)
if err != nil {
return errs.Wrap(err, "Unable to get local commit")
}
if commitID == "" {
Expand Down
4 changes: 2 additions & 2 deletions internal/runners/cve/cve.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import (
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/primer"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
medmodel "github.com/ActiveState/cli/pkg/platform/api/mediator/model"
"github.com/ActiveState/cli/pkg/platform/authentication"
"github.com/ActiveState/cli/pkg/platform/model"
Expand Down Expand Up @@ -66,7 +66,7 @@ func (c *Cve) Run() error {
)
}

commitID, err := localcommit.Get(c.proj.Dir())
commitID, err := commitmediator.Get(c.proj)
if err != nil {
return errs.Wrap(err, "Could not get local commit")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/runners/cve/report.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
medmodel "github.com/ActiveState/cli/pkg/platform/api/mediator/model"
"github.com/ActiveState/cli/pkg/platform/authentication"
"github.com/ActiveState/cli/pkg/platform/model"
Expand Down Expand Up @@ -102,7 +102,7 @@ func (r *Report) fetchVulnerabilities(namespaceOverride project.Namespaced) (*me
commitID = namespaceOverride.CommitID.String()
} else {
var err error
commitUUID, err := localcommit.Get(r.proj.Dir())
commitUUID, err := commitmediator.Get(r.proj)
if err != nil {
return nil, errs.Wrap(err, "Unable to get local commit")
}
Expand Down
6 changes: 3 additions & 3 deletions internal/runners/export/recipe.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import (
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/project"
"github.com/ActiveState/cli/pkg/sysinfo"
Expand Down Expand Up @@ -83,8 +83,8 @@ func fetchRecipe(proj *project.Project, commitID strfmt.UUID, platform string) (

if commitID == "" {
var err error
commitID, err = localcommit.Get(proj.Dir())
if err != nil && !localcommit.IsFileDoesNotExistError(err) {
commitID, err = commitmediator.Get(proj)
if err != nil {
return "", errs.Wrap(err, "Unable to get local commit")
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/runners/hello/hello_example.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/primer"
"github.com/ActiveState/cli/internal/runbits"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/internal/runbits/rationalize"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/project"
)
Expand Down Expand Up @@ -145,7 +145,7 @@ func currentCommitMessage(proj *project.Project) (string, error) {
return "", errs.New("Cannot determine which project to use")
}

commitId, err := localcommit.Get(proj.Dir())
commitId, err := commitmediator.Get(proj)
if err != nil {
return "", errs.Wrap(err, "Cannot determine which commit to use")
}
Expand Down
4 changes: 2 additions & 2 deletions internal/runners/history/history.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import (
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/primer"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/cmdlets/commit"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/pkg/platform/api/mono/mono_models"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/project"
Expand Down Expand Up @@ -39,7 +39,7 @@ func (h *History) Run(params *HistoryParams) error {
}
h.out.Notice(locale.Tl("operating_message", "", h.project.NamespaceString(), h.project.Dir()))

localCommitID, err := localcommit.Get(h.project.Dir())
localCommitID, err := commitmediator.Get(h.project)
if err != nil {
return errs.Wrap(err, "Unable to get local commit")
}
Expand Down
5 changes: 3 additions & 2 deletions internal/runners/initialize/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/primer"
"github.com/ActiveState/cli/internal/runbits"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/pkg/platform/authentication"
"github.com/ActiveState/cli/pkg/platform/model"
Expand Down Expand Up @@ -69,8 +70,8 @@ func inferLanguage(config projectfile.ConfigGetter) (string, string, bool) {
if err != nil {
return "", "", false
}
commitID, err := localcommit.Get(defaultProj.Dir())
if err != nil && !localcommit.IsFileDoesNotExistError(err) {
commitID, err := commitmediator.Get(defaultProj)
if err != nil {
multilog.Error("Unable to get local commit: %v", errs.JoinMessage(err))
return "", "", false
}
Expand Down
4 changes: 2 additions & 2 deletions internal/runners/languages/languages.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import (
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/project"
)
Expand All @@ -31,7 +31,7 @@ func (l *Languages) Run() error {
return locale.NewInputError("err_no_project")
}

commitID, err := localcommit.Get(l.project.Dir())
commitID, err := commitmediator.Get(l.project)
if err != nil {
return errs.AddTips(
locale.WrapError(
Expand Down
6 changes: 3 additions & 3 deletions internal/runners/packages/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/project"
)
Expand Down Expand Up @@ -109,8 +109,8 @@ func targetFromProjectFile(proj *project.Project) (*strfmt.UUID, error) {
if proj == nil {
return nil, locale.NewInputError("err_no_project")
}
commit, err := localcommit.Get(proj.Dir())
if err != nil && !localcommit.IsFileDoesNotExistError(err) {
commit, err := commitmediator.Get(proj)
if err != nil {
return nil, errs.Wrap(err, "Unable to get local commit")
}
if commit == "" {
Expand Down
6 changes: 3 additions & 3 deletions internal/runners/packages/search.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/project"
)
Expand Down Expand Up @@ -78,8 +78,8 @@ func targetedLanguage(languageOpt string, proj *project.Project) (string, error)
)
}

commitID, err := localcommit.Get(proj.Dir())
if err != nil && !localcommit.IsFileDoesNotExistError(err) {
commitID, err := commitmediator.Get(proj)
if err != nil {
return "", errs.Wrap(err, "Unable to get local commit")
}
lang, err := model.LanguageByCommit(commitID)
Expand Down
6 changes: 3 additions & 3 deletions internal/runners/platforms/list.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/pkg/localcommit"
"github.com/ActiveState/cli/internal/runbits/commitmediator"
"github.com/ActiveState/cli/pkg/platform/model"
"github.com/ActiveState/cli/pkg/project"
"github.com/go-openapi/strfmt"
Expand Down Expand Up @@ -33,8 +33,8 @@ func (l *List) Run() error {
return locale.NewInputError("err_no_project")
}

commitID, err := localcommit.Get(l.proj.Dir())
if err != nil && !localcommit.IsFileDoesNotExistError(err) {
commitID, err := commitmediator.Get(l.proj)
if err != nil {
return errs.Wrap(err, "Unable to get local commit")
}

Expand Down
Loading