Make the feature a taskAction property (was a task property) (#275)

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
+}

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
 }

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
+}

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
+}

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
 }

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)
 }

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 {

pkg/tasks/taskapi/task.go

@@ -2,20 +2,13 @@ package taskapi
 
 import (
 	"fmt"
-
-	"github.com/devbuddy/devbuddy/pkg/features"
 )
 
 // Task represents a task created by a taskDefinition.parser and specified by the TaskInfo
 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))

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 {

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
+}

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) {

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
 

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())
+			}
 		}
 	}
 

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"))
+}