diff --git a/pkg/tasks/apt.go b/pkg/tasks/apt.go index 8c231a5b..2339537d 100644 --- a/pkg/tasks/apt.go +++ b/pkg/tasks/apt.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/devbuddy/devbuddy/pkg/features" "github.com/devbuddy/devbuddy/pkg/tasks/taskapi" ) @@ -71,3 +72,7 @@ func (a *aptInstall) Run(ctx *taskapi.Context) error { return nil } + +func (a *aptInstall) Feature() *features.FeatureInfo { + return nil +} diff --git a/pkg/tasks/golang.go b/pkg/tasks/golang.go index 7cbe06fe..0ac94c57 100644 --- a/pkg/tasks/golang.go +++ b/pkg/tasks/golang.go @@ -14,9 +14,7 @@ func parseGolang(config *taskapi.TaskConfig, task *taskapi.Task) error { if err != nil { return err } - task.Info = version - task.SetFeature("golang", version) checkPATHVar := func(ctx *taskapi.Context) *taskapi.ActionResult { if ctx.Env.Get("GOPATH") == "" { @@ -39,7 +37,9 @@ func parseGolang(config *taskapi.TaskConfig, task *taskapi.Task) error { installGo := func(ctx *taskapi.Context) error { return helpers.NewGolang(ctx.Cfg, version).Install() } - task.AddActionWithBuilder("install golang distribution", installGo).OnFunc(installNeeded) + task.AddActionWithBuilder("install golang distribution", installGo). + OnFunc(installNeeded). + SetFeature("golang", version) return nil } diff --git a/pkg/tasks/golang_dep.go b/pkg/tasks/golang_dep.go index c7cf4c03..835f857b 100644 --- a/pkg/tasks/golang_dep.go +++ b/pkg/tasks/golang_dep.go @@ -4,6 +4,7 @@ import ( "fmt" "os/exec" + "github.com/devbuddy/devbuddy/pkg/features" "github.com/devbuddy/devbuddy/pkg/tasks/taskapi" ) @@ -41,6 +42,10 @@ func (p *golangDepInstall) Run(ctx *taskapi.Context) error { return nil } +func (p *golangDepInstall) Feature() *features.FeatureInfo { + return nil +} + type golangDepEnsure struct { } @@ -80,3 +85,7 @@ func (p *golangDepEnsure) Run(ctx *taskapi.Context) error { } return nil } + +func (p *golangDepEnsure) Feature() *features.FeatureInfo { + return nil +} diff --git a/pkg/tasks/homebrew.go b/pkg/tasks/homebrew.go index b6aec2ea..8e1772da 100644 --- a/pkg/tasks/homebrew.go +++ b/pkg/tasks/homebrew.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/devbuddy/devbuddy/pkg/features" "github.com/devbuddy/devbuddy/pkg/helpers" "github.com/devbuddy/devbuddy/pkg/tasks/taskapi" ) @@ -56,3 +57,7 @@ func (b *brewInstall) Run(ctx *taskapi.Context) error { return nil } + +func (b *brewInstall) Feature() *features.FeatureInfo { + return nil +} diff --git a/pkg/tasks/node.go b/pkg/tasks/node.go index 52b00bd5..90235903 100644 --- a/pkg/tasks/node.go +++ b/pkg/tasks/node.go @@ -16,7 +16,6 @@ func parseNode(config *taskapi.TaskConfig, task *taskapi.Task) error { } task.Info = version - task.SetFeature("node", version) run := func(ctx *taskapi.Context) error { return helpers.NewNode(ctx.Cfg, version).Install() @@ -27,7 +26,9 @@ func parseNode(config *taskapi.TaskConfig, task *taskapi.Task) error { } return taskapi.ActionNotNeeded() } - task.AddActionWithBuilder("install nodejs from https://nodejs.org", run).OnFunc(condition) + task.AddActionWithBuilder("install nodejs from https://nodejs.org", run). + OnFunc(condition). + SetFeature("node", version) return nil } diff --git a/pkg/tasks/python.go b/pkg/tasks/python.go index 1274dcf6..8a45c78e 100644 --- a/pkg/tasks/python.go +++ b/pkg/tasks/python.go @@ -21,9 +21,7 @@ func parserPython(config *taskapi.TaskConfig, task *taskapi.Task) error { if err != nil { return err } - task.Info = version - task.SetFeature("python", version) parserPythonInstallPyenv(task, version) parserPythonInstallPythonVersion(task, version) @@ -127,5 +125,7 @@ func parserPythonCreateVirtualenv(task *taskapi.Task, version string) { } return nil } - task.AddActionWithBuilder("create virtualenv", run).OnFunc(needed) + task.AddActionWithBuilder("create virtualenv", run). + OnFunc(needed). + SetFeature("python", version) } diff --git a/pkg/tasks/task_utils_test.go b/pkg/tasks/task_utils_test.go index 92900b7d..de7f29e9 100644 --- a/pkg/tasks/task_utils_test.go +++ b/pkg/tasks/task_utils_test.go @@ -14,9 +14,7 @@ func loadTestTask(t *testing.T, payload string) (*taskapi.Task, error) { if err != nil { t.Fatalf("Failed to load a test fixture: %s", err) } - - task, err := taskapi.NewTaskFromDefinition(data) - return task, err + return taskapi.NewTaskFromDefinition(data) } func ensureLoadTestTask(t *testing.T, payload string) *taskapi.Task { diff --git a/pkg/tasks/taskapi/task.go b/pkg/tasks/taskapi/task.go index fc1c7a68..7997e170 100644 --- a/pkg/tasks/taskapi/task.go +++ b/pkg/tasks/taskapi/task.go @@ -2,8 +2,6 @@ package taskapi import ( "fmt" - - "github.com/devbuddy/devbuddy/pkg/features" ) // Task represents a task created by a taskDefinition.parser and specified by the TaskInfo @@ -11,11 +9,6 @@ type Task struct { *TaskDefinition Info string Actions []TaskAction - Feature features.FeatureInfo -} - -func (t *Task) SetFeature(name, param string) { - t.Feature = features.NewFeatureInfo(name, param) } func (t *Task) AddAction(action TaskAction) { @@ -38,8 +31,12 @@ func (t *Task) Describe() string { description += fmt.Sprintf(" required_task=%s", t.RequiredTask) } - if t.Feature.Name != "" { - description += fmt.Sprintf(" feature=%s:%s", t.Feature.Name, t.Feature.Param) + for _, action := range t.Actions { + f := action.Feature() + if action.Feature() != nil { + feature := *f + description += fmt.Sprintf(" feature=%s:%s", feature.Name, feature.Param) + } } description += fmt.Sprintf(" actions=%d", len(t.Actions)) diff --git a/pkg/tasks/taskapi/task_action.go b/pkg/tasks/taskapi/task_action.go index 8372b4c8..2b8e4297 100644 --- a/pkg/tasks/taskapi/task_action.go +++ b/pkg/tasks/taskapi/task_action.go @@ -1,11 +1,16 @@ package taskapi -import "fmt" +import ( + "fmt" + + "github.com/devbuddy/devbuddy/pkg/features" +) type TaskAction interface { Description() string Needed(*Context) *ActionResult Run(*Context) error + Feature() *features.FeatureInfo } type ActionResult struct { diff --git a/pkg/tasks/taskapi/task_action_builder.go b/pkg/tasks/taskapi/task_action_builder.go index 59666dff..c9654f71 100644 --- a/pkg/tasks/taskapi/task_action_builder.go +++ b/pkg/tasks/taskapi/task_action_builder.go @@ -1,5 +1,9 @@ package taskapi +import ( + "github.com/devbuddy/devbuddy/pkg/features" +) + type genericTaskActionBuilder struct { *genericTaskAction } @@ -23,3 +27,10 @@ func (a *genericTaskActionBuilder) OnFileChange(path string) *genericTaskActionB a.monitoredFiles = append(a.monitoredFiles, path) return a } + +// SetFeature defines that the feature specified should be activated. +func (a *genericTaskActionBuilder) SetFeature(name, param string) *genericTaskActionBuilder { + featureInfo := features.NewFeatureInfo(name, param) + a.feature = &featureInfo + return a +} diff --git a/pkg/tasks/taskapi/task_action_generic.go b/pkg/tasks/taskapi/task_action_generic.go index 18d303be..4bc68a60 100644 --- a/pkg/tasks/taskapi/task_action_generic.go +++ b/pkg/tasks/taskapi/task_action_generic.go @@ -3,6 +3,8 @@ package taskapi import ( "path/filepath" + "github.com/devbuddy/devbuddy/pkg/features" + "github.com/devbuddy/devbuddy/pkg/helpers/store" "github.com/devbuddy/devbuddy/pkg/utils" ) @@ -12,6 +14,7 @@ type genericTaskAction struct { conditions []*genericTaskActionCondition monitoredFiles []string runFunc func(*Context) error + feature *features.FeatureInfo runCalled bool } @@ -37,6 +40,10 @@ func (a *genericTaskAction) Run(ctx *Context) error { return a.runFunc(ctx) } +func (a *genericTaskAction) Feature() *features.FeatureInfo { + return a.feature +} + // internals func (a *genericTaskAction) pre(ctx *Context) (result *ActionResult) { diff --git a/pkg/tasks/taskapi/task_action_generic_test.go b/pkg/tasks/taskapi/task_action_generic_test.go index 1494cca5..d09ad9ea 100644 --- a/pkg/tasks/taskapi/task_action_generic_test.go +++ b/pkg/tasks/taskapi/task_action_generic_test.go @@ -43,6 +43,13 @@ func TestTaskActionGenericDescription(t *testing.T) { require.Equal(t, "dummy desc", action.Description()) } +func TestTaskActionGenericFeature(t *testing.T) { + action := newBuilder("dummy desc", nil) + action.SetFeature("name", "param") + require.Equal(t, "name", action.Feature().Name) + require.Equal(t, "param", action.Feature().Param) +} + func TestTaskActionGenericNoConditions(t *testing.T) { action := newBuilder("", func(ctx *Context) error { return nil }).genericTaskAction diff --git a/pkg/tasks/taskapi/task_list.go b/pkg/tasks/taskapi/task_list.go index 3b606561..c38103fd 100644 --- a/pkg/tasks/taskapi/task_list.go +++ b/pkg/tasks/taskapi/task_list.go @@ -58,8 +58,10 @@ func GetFeaturesFromTasks(tasks []*Task) features.FeatureSet { featureSet := features.FeatureSet{} for _, task := range tasks { - if task.Feature.Name != "" { - featureSet = featureSet.With(task.Feature) + for _, action := range task.Actions { + if action.Feature() != nil { + featureSet = featureSet.With(*action.Feature()) + } } } diff --git a/pkg/tasks/taskengine/task_runner.go b/pkg/tasks/taskengine/task_runner.go index 393ac4c7..6919ee92 100644 --- a/pkg/tasks/taskengine/task_runner.go +++ b/pkg/tasks/taskengine/task_runner.go @@ -16,44 +16,15 @@ type TaskRunnerImpl struct{} func (r *TaskRunnerImpl) Run(ctx *taskapi.Context, task *taskapi.Task) (err error) { for _, action := range task.Actions { - err = runAction(ctx, action) + err = r.runAction(ctx, action) if err != nil { return err } } - - err = r.activateFeature(ctx, task) - return err -} - -func (r *TaskRunnerImpl) activateFeature(ctx *taskapi.Context, task *taskapi.Task) error { - if task.Feature.Name == "" { - return nil - } - - def, err := features.Get(task.Feature) - if err != nil { - return err - } - - devUpNeeded, err := def.Activate(task.Feature.Param, ctx.Cfg, ctx.Project, ctx.Env) - if err != nil { - return err - } - if devUpNeeded { - ctx.UI.TaskWarning(fmt.Sprintf("Something is wrong, the feature %s could not be activated", task.Feature)) - } - - // Special case, we want the bud process to get PATH updates from features to call the right processes. - // Like the pip process from the newly activated virtualenv. - // Explanation: exec.Command calls exec.LookPath to find the executable path, which rely on the PATH of - // the process itself. - // There is no problem when executing a shell command since the shell process will do the executable lookup - // itself with the PATH value from the specified Env. - return os.Setenv("PATH", ctx.Env.Get("PATH")) + return nil } -func runAction(ctx *taskapi.Context, action taskapi.TaskAction) error { +func (r *TaskRunnerImpl) runAction(ctx *taskapi.Context, action taskapi.TaskAction) error { desc := action.Description() result := action.Needed(ctx) @@ -82,5 +53,32 @@ func runAction(ctx *taskapi.Context, action taskapi.TaskAction) error { } } + feature := action.Feature() + if feature != nil { + return r.activateFeature(ctx, *feature) + } return nil } + +func (r *TaskRunnerImpl) activateFeature(ctx *taskapi.Context, feature features.FeatureInfo) error { + def, err := features.Get(feature) + if err != nil { + return err + } + + devUpNeeded, err := def.Activate(feature.Param, ctx.Cfg, ctx.Project, ctx.Env) + if err != nil { + return err + } + if devUpNeeded { + ctx.UI.TaskWarning(fmt.Sprintf("Something is wrong, the feature %s could not be activated", feature)) + } + + // Special case, we want the bud process to get PATH updates from features to call the right processes. + // Like the pip process from the newly activated virtualenv. + // Explanation: exec.Command calls exec.LookPath to find the executable path, which rely on the PATH of + // the process itself. + // There is no problem when executing a shell command since the shell process will do the executable lookup + // itself with the PATH value from the specified Env. + return os.Setenv("PATH", ctx.Env.Get("PATH")) +} diff --git a/pkg/tasks/taskengine/task_runner_test.go b/pkg/tasks/taskengine/task_runner_test.go index f2545f5c..fd5e9760 100644 --- a/pkg/tasks/taskengine/task_runner_test.go +++ b/pkg/tasks/taskengine/task_runner_test.go @@ -12,8 +12,10 @@ import ( func TestRunActionNeeded(t *testing.T) { ctx, output := setupTaskTesting() action := newTestingAction("Action X", taskapi.ActionNeeded("some-reason"), taskapi.ActionNotNeeded(), nil) + task := newTaskWithAction("testtask", action) - err := runAction(ctx, action) + taskRunner := &TaskRunnerImpl{} + err := taskRunner.Run(ctx, task) require.NoError(t, err) require.Equal(t, 2, action.neededCallCount) @@ -25,8 +27,10 @@ func TestRunActionNeeded(t *testing.T) { func TestRunActionNotNeeded(t *testing.T) { ctx, output := setupTaskTesting() action := newTestingAction("Action X", taskapi.ActionNotNeeded(), nil, nil) + task := newTaskWithAction("testtask", action) - err := runAction(ctx, action) + taskRunner := &TaskRunnerImpl{} + err := taskRunner.Run(ctx, task) require.NoError(t, err) require.Equal(t, 1, action.neededCallCount) @@ -38,10 +42,11 @@ func TestRunActionNotNeeded(t *testing.T) { func TestRunActionFailureOnNeeded(t *testing.T) { ctx, _ := setupTaskTesting() action := newTestingAction("Action X", taskapi.ActionFailed("ERROR_X"), nil, nil) + task := newTaskWithAction("testtask", action) - err := runAction(ctx, action) - require.Error(t, err) - require.Contains(t, err.Error(), "failed to detect whether it need to run: ERROR_X") + taskRunner := &TaskRunnerImpl{} + err := taskRunner.Run(ctx, task) + require.Error(t, err, "failed to detect whether it need to run: ERROR_X") require.Equal(t, 1, action.neededCallCount) require.Equal(t, 0, action.runCallCount) @@ -50,10 +55,11 @@ func TestRunActionFailureOnNeeded(t *testing.T) { func TestRunActionFailureOnRun(t *testing.T) { ctx, output := setupTaskTesting() action := newTestingAction("Action X", taskapi.ActionNeeded("some-reason"), nil, errors.New("RunFailed")) + task := newTaskWithAction("testtask", action) - err := runAction(ctx, action) - require.Error(t, err) - require.Contains(t, err.Error(), "failed to run: RunFailed") + taskRunner := &TaskRunnerImpl{} + err := taskRunner.Run(ctx, task) + require.EqualError(t, err, "The task action failed to run: RunFailed") require.Equal(t, 1, action.neededCallCount) require.Equal(t, 1, action.runCallCount) @@ -64,10 +70,11 @@ func TestRunActionFailureOnRun(t *testing.T) { func TestRunActionStillNeeded(t *testing.T) { ctx, _ := setupTaskTesting() action := newTestingAction("Action X", taskapi.ActionNeeded("some-reason"), taskapi.ActionNeeded("some-reason"), nil) + task := newTaskWithAction("testtask", action) - err := runAction(ctx, action) - require.Error(t, err) - require.Contains(t, err.Error(), "did not produce the expected result: some-reason") + taskRunner := &TaskRunnerImpl{} + err := taskRunner.Run(ctx, task) + require.EqualError(t, err, "The task action did not produce the expected result: some-reason") require.Equal(t, 2, action.neededCallCount) require.Equal(t, 1, action.runCallCount) diff --git a/pkg/tasks/taskengine/util_test.go b/pkg/tasks/taskengine/util_test.go index 74256b74..eedf6d37 100644 --- a/pkg/tasks/taskengine/util_test.go +++ b/pkg/tasks/taskengine/util_test.go @@ -20,6 +20,8 @@ type testingAction struct { neededResults []*taskapi.ActionResult neededCallCount int + feature *features.FeatureInfo + runResult error runCallCount int } @@ -42,6 +44,10 @@ func (a *testingAction) Run(ctx *taskapi.Context) error { return a.runResult } +func (a *testingAction) Feature() *features.FeatureInfo { + return a.feature +} + func newTestingAction(desc string, resultBefore, resultAfter *taskapi.ActionResult, runResult error) *testingAction { return &testingAction{ desc: desc, @@ -50,6 +56,13 @@ func newTestingAction(desc string, resultBefore, resultAfter *taskapi.ActionResu } } +func newTaskWithAction(name string, action taskapi.TaskAction) *taskapi.Task { + return &taskapi.Task{ + TaskDefinition: &taskapi.TaskDefinition{Name: name}, + Actions: []taskapi.TaskAction{action}, + } +} + func setupTaskTesting() (*taskapi.Context, *bytes.Buffer) { buf, ui := termui.NewTesting(false)