From a044c41c665928193b4f98e6514f2696fd099cd0 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Sat, 28 Mar 2020 11:27:49 -0300 Subject: [PATCH 1/4] Upgrade github.com/go-yaml/yaml to v3 --- go.mod | 2 +- go.sum | 2 ++ internal/taskfile/precondition_test.go | 2 +- internal/taskfile/read/taskfile.go | 2 +- internal/taskfile/read/taskvars.go | 2 +- internal/taskfile/taskfile_test.go | 2 +- 6 files changed, 7 insertions(+), 5 deletions(-) diff --git a/go.mod b/go.mod index 26e32939..23ae138f 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/spf13/pflag v1.0.3 github.com/stretchr/testify v1.4.0 golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e - gopkg.in/yaml.v2 v2.2.2 + gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c mvdan.cc/sh/v3 v3.0.2 ) diff --git a/go.sum b/go.sum index 1c067477..e56701b9 100644 --- a/go.sum +++ b/go.sum @@ -52,6 +52,8 @@ gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8 gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= mvdan.cc/editorconfig v0.1.1-0.20191109213504-890940e3f00e/go.mod h1:Ge4atmRUYqueGppvJ7JNrtqpqokoJEFxYbP0Z+WeKS8= mvdan.cc/sh/v3 v3.0.2 h1:NU+UpTZHRdIZ9igRo8zi/7+5/NpetYlhlyDhz1/AdMM= mvdan.cc/sh/v3 v3.0.2/go.mod h1:rBIndNJFYPp8xSppiZcGIk6B5d1g3OEARxEaXjPxwVI= diff --git a/internal/taskfile/precondition_test.go b/internal/taskfile/precondition_test.go index 799e9ac4..dd202bd0 100644 --- a/internal/taskfile/precondition_test.go +++ b/internal/taskfile/precondition_test.go @@ -6,7 +6,7 @@ import ( "github.com/go-task/task/v2/internal/taskfile" "github.com/stretchr/testify/assert" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" ) func TestPreconditionParse(t *testing.T) { diff --git a/internal/taskfile/read/taskfile.go b/internal/taskfile/read/taskfile.go index 7b1db509..5ee7e4e5 100644 --- a/internal/taskfile/read/taskfile.go +++ b/internal/taskfile/read/taskfile.go @@ -9,7 +9,7 @@ import ( "github.com/go-task/task/v2/internal/taskfile" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" ) var ( diff --git a/internal/taskfile/read/taskvars.go b/internal/taskfile/read/taskvars.go index b2022140..f664cf04 100644 --- a/internal/taskfile/read/taskvars.go +++ b/internal/taskfile/read/taskvars.go @@ -8,7 +8,7 @@ import ( "github.com/go-task/task/v2/internal/taskfile" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" ) // Taskvars reads a Taskvars for a given directory diff --git a/internal/taskfile/taskfile_test.go b/internal/taskfile/taskfile_test.go index 0fed779d..9b229a07 100644 --- a/internal/taskfile/taskfile_test.go +++ b/internal/taskfile/taskfile_test.go @@ -6,7 +6,7 @@ import ( "github.com/go-task/task/v2/internal/taskfile" "github.com/stretchr/testify/assert" - "gopkg.in/yaml.v2" + "gopkg.in/yaml.v3" ) func TestCmdParse(t *testing.T) { From 6ed30f1addf85dcf015e3586008e3f9bdf59041d Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Sun, 29 Mar 2020 16:54:59 -0300 Subject: [PATCH 2/4] Refactor variables: Keep order of declaration This shouldn't have any behavior changes for now. This is a code refactor that should allow us to do further improvements on how variables are handled, specially regarding respecting the declaration order in Taskfiles, which should make it easier for the users. Initial work on #218 --- cmd/task/task.go | 4 +- internal/args/args.go | 16 +++--- internal/args/args_test.go | 41 +++++++++----- internal/compiler/compiler.go | 2 +- internal/compiler/env.go | 12 ++--- internal/compiler/v2/compiler_v2.go | 23 ++++---- internal/taskfile/call.go | 2 +- internal/taskfile/cmd.go | 8 +-- internal/taskfile/merge.go | 13 ++--- internal/taskfile/read/taskvars.go | 17 ++---- internal/taskfile/task.go | 8 +-- internal/taskfile/taskfile.go | 11 ++-- internal/taskfile/taskfile_test.go | 18 ++++--- internal/taskfile/var.go | 84 ++++++++++++++++++++++++++--- internal/templater/templater.go | 20 +++---- task.go | 2 +- variables.go | 22 ++++---- 17 files changed, 188 insertions(+), 115 deletions(-) diff --git a/cmd/task/task.go b/cmd/task/task.go index d99db44d..dde290c2 100644 --- a/cmd/task/task.go +++ b/cmd/task/task.go @@ -140,9 +140,7 @@ func main() { } calls, globals := args.Parse(pflag.Args()...) - for name, value := range globals { - e.Taskfile.Vars[name] = value - } + e.Taskfile.Vars.Merge(globals) ctx := context.Background() if !watch { diff --git a/internal/args/args.go b/internal/args/args.go index acea9c29..e157db32 100644 --- a/internal/args/args.go +++ b/internal/args/args.go @@ -7,9 +7,9 @@ import ( ) // Parse parses command line argument: tasks and vars of each task -func Parse(args ...string) ([]taskfile.Call, taskfile.Vars) { +func Parse(args ...string) ([]taskfile.Call, *taskfile.Vars) { var calls []taskfile.Call - var globals taskfile.Vars + var globals *taskfile.Vars for _, arg := range args { if !strings.Contains(arg, "=") { @@ -19,18 +19,16 @@ func Parse(args ...string) ([]taskfile.Call, taskfile.Vars) { if len(calls) < 1 { if globals == nil { - globals = taskfile.Vars{} + globals = &taskfile.Vars{} } - name, value := splitVar(arg) - globals[name] = taskfile.Var{Static: value} + globals.Set(name, taskfile.Var{Static: value}) } else { if calls[len(calls)-1].Vars == nil { - calls[len(calls)-1].Vars = make(taskfile.Vars) + calls[len(calls)-1].Vars = &taskfile.Vars{} } - - name, value := splitVar((arg)) - calls[len(calls)-1].Vars[name] = taskfile.Var{Static: value} + name, value := splitVar(arg) + calls[len(calls)-1].Vars.Set(name, taskfile.Var{Static: value}) } } diff --git a/internal/args/args_test.go b/internal/args/args_test.go index 8dc96e4c..779b37f9 100644 --- a/internal/args/args_test.go +++ b/internal/args/args_test.go @@ -14,7 +14,7 @@ func TestArgs(t *testing.T) { tests := []struct { Args []string ExpectedCalls []taskfile.Call - ExpectedGlobals taskfile.Vars + ExpectedGlobals *taskfile.Vars }{ { Args: []string{"task-a", "task-b", "task-c"}, @@ -29,16 +29,22 @@ func TestArgs(t *testing.T) { ExpectedCalls: []taskfile.Call{ { Task: "task-a", - Vars: taskfile.Vars{ - "FOO": taskfile.Var{Static: "bar"}, + Vars: &taskfile.Vars{ + Keys: []string{"FOO"}, + Mapping: map[string]taskfile.Var{ + "FOO": taskfile.Var{Static: "bar"}, + }, }, }, {Task: "task-b"}, { Task: "task-c", - Vars: taskfile.Vars{ - "BAR": taskfile.Var{Static: "baz"}, - "BAZ": taskfile.Var{Static: "foo"}, + Vars: &taskfile.Vars{ + Keys: []string{"BAR", "BAZ"}, + Mapping: map[string]taskfile.Var{ + "BAR": taskfile.Var{Static: "baz"}, + "BAZ": taskfile.Var{Static: "foo"}, + }, }, }, }, @@ -48,8 +54,11 @@ func TestArgs(t *testing.T) { ExpectedCalls: []taskfile.Call{ { Task: "task-a", - Vars: taskfile.Vars{ - "CONTENT": taskfile.Var{Static: "with some spaces"}, + Vars: &taskfile.Vars{ + Keys: []string{"CONTENT"}, + Mapping: map[string]taskfile.Var{ + "CONTENT": taskfile.Var{Static: "with some spaces"}, + }, }, }, }, @@ -60,8 +69,11 @@ func TestArgs(t *testing.T) { {Task: "task-a"}, {Task: "task-b"}, }, - ExpectedGlobals: taskfile.Vars{ - "FOO": {Static: "bar"}, + ExpectedGlobals: &taskfile.Vars{ + Keys: []string{"FOO"}, + Mapping: map[string]taskfile.Var{ + "FOO": {Static: "bar"}, + }, }, }, { @@ -81,9 +93,12 @@ func TestArgs(t *testing.T) { ExpectedCalls: []taskfile.Call{ {Task: "default"}, }, - ExpectedGlobals: taskfile.Vars{ - "FOO": {Static: "bar"}, - "BAR": {Static: "baz"}, + ExpectedGlobals: &taskfile.Vars{ + Keys: []string{"FOO", "BAR"}, + Mapping: map[string]taskfile.Var{ + "FOO": {Static: "bar"}, + "BAR": {Static: "baz"}, + }, }, }, } diff --git a/internal/compiler/compiler.go b/internal/compiler/compiler.go index aa31d893..75dcb3ee 100644 --- a/internal/compiler/compiler.go +++ b/internal/compiler/compiler.go @@ -7,6 +7,6 @@ import ( // Compiler handles compilation of a task before its execution. // E.g. variable merger, template processing, etc. type Compiler interface { - GetVariables(t *taskfile.Task, call taskfile.Call) (taskfile.Vars, error) + GetVariables(t *taskfile.Task, call taskfile.Call) (*taskfile.Vars, error) HandleDynamicVar(v taskfile.Var) (string, error) } diff --git a/internal/compiler/env.go b/internal/compiler/env.go index 4a9ecf4c..283577cc 100644 --- a/internal/compiler/env.go +++ b/internal/compiler/env.go @@ -9,16 +9,12 @@ import ( // GetEnviron the all return all environment variables encapsulated on a // taskfile.Vars -func GetEnviron() taskfile.Vars { - var ( - env = os.Environ() - m = make(taskfile.Vars, len(env)) - ) - - for _, e := range env { +func GetEnviron() *taskfile.Vars { + m := &taskfile.Vars{} + for _, e := range os.Environ() { keyVal := strings.SplitN(e, "=", 2) key, val := keyVal[0], keyVal[1] - m[key] = taskfile.Var{Static: val} + m.Set(key, taskfile.Var{Static: val}) } return m } diff --git a/internal/compiler/v2/compiler_v2.go b/internal/compiler/v2/compiler_v2.go index f42b5fbe..6bcbfaf2 100644 --- a/internal/compiler/v2/compiler_v2.go +++ b/internal/compiler/v2/compiler_v2.go @@ -19,8 +19,8 @@ var _ compiler.Compiler = &CompilerV2{} type CompilerV2 struct { Dir string - Taskvars taskfile.Vars - TaskfileVars taskfile.Vars + Taskvars *taskfile.Vars + TaskfileVars *taskfile.Vars Expansions int @@ -36,10 +36,10 @@ type CompilerV2 struct { // 3. Taskfile variables // 4. Taskvars file variables // 5. Environment variables -func (c *CompilerV2) GetVariables(t *taskfile.Task, call taskfile.Call) (taskfile.Vars, error) { +func (c *CompilerV2) GetVariables(t *taskfile.Task, call taskfile.Call) (*taskfile.Vars, error) { vr := varResolver{c: c, vars: compiler.GetEnviron()} - vr.vars["TASK"] = taskfile.Var{Static: t.Task} - for _, vars := range []taskfile.Vars{c.Taskvars, c.TaskfileVars, call.Vars, t.Vars} { + vr.vars.Set("TASK", taskfile.Var{Static: t.Task}) + for _, vars := range []*taskfile.Vars{c.Taskvars, c.TaskfileVars, call.Vars, t.Vars} { for i := 0; i < c.Expansions; i++ { vr.merge(vars) } @@ -49,16 +49,16 @@ func (c *CompilerV2) GetVariables(t *taskfile.Task, call taskfile.Call) (taskfil type varResolver struct { c *CompilerV2 - vars taskfile.Vars + vars *taskfile.Vars err error } -func (vr *varResolver) merge(vars taskfile.Vars) { +func (vr *varResolver) merge(vars *taskfile.Vars) { if vr.err != nil { return } tr := templater.Templater{Vars: vr.vars} - for k, v := range vars { + vars.Range(func(k string, v taskfile.Var) error { v = taskfile.Var{ Static: tr.Replace(v.Static), Sh: tr.Replace(v.Sh), @@ -66,10 +66,11 @@ func (vr *varResolver) merge(vars taskfile.Vars) { static, err := vr.c.HandleDynamicVar(v) if err != nil { vr.err = err - return + return err } - vr.vars[k] = taskfile.Var{Static: static} - } + vr.vars.Set(k, taskfile.Var{Static: static}) + return nil + }) vr.err = tr.Err() } diff --git a/internal/taskfile/call.go b/internal/taskfile/call.go index eec41031..c1ca1083 100644 --- a/internal/taskfile/call.go +++ b/internal/taskfile/call.go @@ -3,5 +3,5 @@ package taskfile // Call is the parameters to a task call type Call struct { Task string - Vars Vars + Vars *Vars } diff --git a/internal/taskfile/cmd.go b/internal/taskfile/cmd.go index ea267811..c992dd30 100644 --- a/internal/taskfile/cmd.go +++ b/internal/taskfile/cmd.go @@ -10,14 +10,14 @@ type Cmd struct { Cmd string Silent bool Task string - Vars Vars + Vars *Vars IgnoreError bool } // Dep is a task dependency type Dep struct { Task string - Vars Vars + Vars *Vars } var ( @@ -51,7 +51,7 @@ func (c *Cmd) UnmarshalYAML(unmarshal func(interface{}) error) error { } var taskCall struct { Task string - Vars Vars + Vars *Vars } if err := unmarshal(&taskCall); err == nil { c.Task = taskCall.Task @@ -70,7 +70,7 @@ func (d *Dep) UnmarshalYAML(unmarshal func(interface{}) error) error { } var taskCall struct { Task string - Vars Vars + Vars *Vars } if err := unmarshal(&taskCall); err == nil { d.Task = taskCall.Task diff --git a/internal/taskfile/merge.go b/internal/taskfile/merge.go index d350e4b9..24b3ebb0 100644 --- a/internal/taskfile/merge.go +++ b/internal/taskfile/merge.go @@ -29,18 +29,13 @@ func Merge(t1, t2 *Taskfile, namespaces ...string) error { } if t1.Vars == nil { - t1.Vars = make(Vars) + t1.Vars = &Vars{} } - for k, v := range t2.Vars { - t1.Vars[k] = v - } - if t1.Env == nil { - t1.Env = make(Vars) - } - for k, v := range t2.Env { - t1.Env[k] = v + t1.Env = &Vars{} } + t1.Vars.Merge(t2.Vars) + t1.Env.Merge(t2.Env) if t1.Tasks == nil { t1.Tasks = make(Tasks) diff --git a/internal/taskfile/read/taskvars.go b/internal/taskfile/read/taskvars.go index f664cf04..cc5bcaa8 100644 --- a/internal/taskfile/read/taskvars.go +++ b/internal/taskfile/read/taskvars.go @@ -12,8 +12,8 @@ import ( ) // Taskvars reads a Taskvars for a given directory -func Taskvars(dir string) (taskfile.Vars, error) { - vars := make(taskfile.Vars) +func Taskvars(dir string) (*taskfile.Vars, error) { + vars := &taskfile.Vars{} path := filepath.Join(dir, "Taskvars.yml") if _, err := os.Stat(path); err == nil { @@ -29,24 +29,17 @@ func Taskvars(dir string) (taskfile.Vars, error) { if err != nil { return nil, err } - - if vars == nil { - vars = osVars - } else { - for k, v := range osVars { - vars[k] = v - } - } + vars.Merge(osVars) } return vars, nil } -func readTaskvars(file string) (taskfile.Vars, error) { +func readTaskvars(file string) (*taskfile.Vars, error) { f, err := os.Open(file) if err != nil { return nil, err } var vars taskfile.Vars - return vars, yaml.NewDecoder(f).Decode(&vars) + return &vars, yaml.NewDecoder(f).Decode(&vars) } diff --git a/internal/taskfile/task.go b/internal/taskfile/task.go index f00ee947..d39999cf 100644 --- a/internal/taskfile/task.go +++ b/internal/taskfile/task.go @@ -19,8 +19,8 @@ type Task struct { Status []string Preconditions []*Precondition Dir string - Vars Vars - Env Vars + Vars *Vars + Env *Vars Silent bool Method string Prefix string @@ -55,8 +55,8 @@ func (t *Task) UnmarshalYAML(unmarshal func(interface{}) error) error { Status []string Preconditions []*Precondition Dir string - Vars Vars - Env Vars + Vars *Vars + Env *Vars Silent bool Method string Prefix string diff --git a/internal/taskfile/taskfile.go b/internal/taskfile/taskfile.go index bcb7653c..b8cf5da9 100644 --- a/internal/taskfile/taskfile.go +++ b/internal/taskfile/taskfile.go @@ -7,8 +7,8 @@ type Taskfile struct { Output string Method string Includes IncludedTaskfiles - Vars Vars - Env Vars + Vars *Vars + Env *Vars Tasks Tasks Silent bool } @@ -21,8 +21,8 @@ func (tf *Taskfile) UnmarshalYAML(unmarshal func(interface{}) error) error { Output string Method string Includes IncludedTaskfiles - Vars Vars - Env Vars + Vars *Vars + Env *Vars Tasks Tasks Silent bool } @@ -41,8 +41,5 @@ func (tf *Taskfile) UnmarshalYAML(unmarshal func(interface{}) error) error { if tf.Expansions <= 0 { tf.Expansions = 2 } - if tf.Vars == nil { - tf.Vars = make(Vars) - } return nil } diff --git a/internal/taskfile/taskfile_test.go b/internal/taskfile/taskfile_test.go index 9b229a07..fcdd4bb1 100644 --- a/internal/taskfile/taskfile_test.go +++ b/internal/taskfile/taskfile_test.go @@ -33,9 +33,12 @@ vars: { yamlTaskCall, &taskfile.Cmd{}, - &taskfile.Cmd{Task: "another-task", Vars: taskfile.Vars{ - "PARAM1": taskfile.Var{Static: "VALUE1"}, - "PARAM2": taskfile.Var{Static: "VALUE2"}, + &taskfile.Cmd{Task: "another-task", Vars: &taskfile.Vars{ + Keys: []string{"PARAM1", "PARAM2"}, + Mapping: map[string]taskfile.Var{ + "PARAM1": taskfile.Var{Static: "VALUE1"}, + "PARAM2": taskfile.Var{Static: "VALUE2"}, + }, }}, }, { @@ -46,9 +49,12 @@ vars: { yamlTaskCall, &taskfile.Dep{}, - &taskfile.Dep{Task: "another-task", Vars: taskfile.Vars{ - "PARAM1": taskfile.Var{Static: "VALUE1"}, - "PARAM2": taskfile.Var{Static: "VALUE2"}, + &taskfile.Dep{Task: "another-task", Vars: &taskfile.Vars{ + Keys: []string{"PARAM1", "PARAM2"}, + Mapping: map[string]taskfile.Var{ + "PARAM1": taskfile.Var{Static: "VALUE1"}, + "PARAM2": taskfile.Var{Static: "VALUE2"}, + }, }}, }, } diff --git a/internal/taskfile/var.go b/internal/taskfile/var.go index 225aed1e..99946f8a 100644 --- a/internal/taskfile/var.go +++ b/internal/taskfile/var.go @@ -3,6 +3,8 @@ package taskfile import ( "errors" "strings" + + "gopkg.in/yaml.v3" ) var ( @@ -11,17 +13,86 @@ var ( ) // Vars is a string[string] variables map. -type Vars map[string]Var +type Vars struct { + Keys []string + Mapping map[string]Var +} + +// UnmarshalYAML implements the yaml.Unmarshaler interface. +func (vs *Vars) UnmarshalYAML(node *yaml.Node) error { + if node.Kind != yaml.MappingNode { + return errors.New("task: vars is not a map") + } + + // NOTE(@andreynering): on this style of custom unmarsheling, + // even number contains the keys, while odd numbers contains + // the values. + for i := 0; i < len(node.Content); i += 2 { + keyNode := node.Content[i] + valueNode := node.Content[i+1] + + var v Var + if err := valueNode.Decode(&v); err != nil { + return err + } + vs.Set(keyNode.Value, v) + } + return nil +} + +// Merge merges the given Vars into the caller one +func (vs *Vars) Merge(other *Vars) { + other.Range(func(key string, value Var) error { + vs.Set(key, value) + return nil + }) +} + +// Set sets a value to a given key +func (vs *Vars) Set(key string, value Var) { + if vs == nil { + vs = &Vars{} + } + if vs.Mapping == nil { + vs.Mapping = make(map[string]Var, 1) + } + if !strSliceContains(vs.Keys, key) { + vs.Keys = append(vs.Keys, key) + } + vs.Mapping[key] = value +} + +func strSliceContains(s []string, str string) bool { + for _, v := range s { + if v == str { + return true + } + } + return false +} + +// Range allows you to loop into the vars in its right order +func (vs *Vars) Range(yield func(key string, value Var) error) error { + if vs == nil { + return nil + } + for _, k := range vs.Keys { + if err := yield(k, vs.Mapping[k]); err != nil { + return err + } + } + return nil +} // ToCacheMap converts Vars to a map containing only the static // variables -func (vs Vars) ToCacheMap() (m map[string]interface{}) { - m = make(map[string]interface{}, len(vs)) - for k, v := range vs { +func (vs *Vars) ToCacheMap() (m map[string]interface{}) { + m = make(map[string]interface{}, len(vs.Keys)) + vs.Range(func(k string, v Var) error { if v.Sh != "" { // Dynamic variable is not yet resolved; trigger // to be used in templates. - continue + return nil } if v.Live != nil { @@ -29,7 +100,8 @@ func (vs Vars) ToCacheMap() (m map[string]interface{}) { } else { m[k] = v.Static } - } + return nil + }) return } diff --git a/internal/templater/templater.go b/internal/templater/templater.go index b395070a..b905b1ba 100644 --- a/internal/templater/templater.go +++ b/internal/templater/templater.go @@ -12,7 +12,7 @@ import ( // happen will be assigned to r.err, and consecutive calls to funcs will just // return the zero value. type Templater struct { - Vars taskfile.Vars + Vars *taskfile.Vars cacheMap map[string]interface{} err error @@ -57,20 +57,22 @@ func (r *Templater) ReplaceSlice(strs []string) []string { return new } -func (r *Templater) ReplaceVars(vars taskfile.Vars) taskfile.Vars { - if r.err != nil || len(vars) == 0 { +func (r *Templater) ReplaceVars(vars *taskfile.Vars) *taskfile.Vars { + if r.err != nil || vars == nil || len(vars.Keys) == 0 { return nil } - new := make(taskfile.Vars, len(vars)) - for k, v := range vars { - new[k] = taskfile.Var{ + var new taskfile.Vars + vars.Range(func(k string, v taskfile.Var) error { + new.Set(k, taskfile.Var{ Static: r.Replace(v.Static), Live: v.Live, Sh: r.Replace(v.Sh), - } - } - return new + }) + return nil + }) + + return &new } func (r *Templater) Err() error { diff --git a/task.go b/task.go index be6740ca..0e3eb327 100644 --- a/task.go +++ b/task.go @@ -52,7 +52,7 @@ type Executor struct { Output output.Output OutputStyle string - taskvars taskfile.Vars + taskvars *taskfile.Vars taskCallCount map[string]*int32 mkdirMutexMap map[string]*sync.Mutex diff --git a/variables.go b/variables.go index d3160171..a2ef97d8 100644 --- a/variables.go +++ b/variables.go @@ -50,19 +50,19 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { new.Prefix = new.Task } - new.Env = make(taskfile.Vars, len(e.Taskfile.Env)+len(origTask.Env)) - for k, v := range r.ReplaceVars(e.Taskfile.Env) { - new.Env[k] = v - } - for k, v := range r.ReplaceVars(origTask.Env) { - new.Env[k] = v - } - for k, v := range new.Env { + new.Env = &taskfile.Vars{} + new.Env.Merge(r.ReplaceVars(e.Taskfile.Env)) + new.Env.Merge(r.ReplaceVars(origTask.Env)) + err = new.Env.Range(func(k string, v taskfile.Var) error { static, err := e.Compiler.HandleDynamicVar(v) if err != nil { - return nil, err + return err } - new.Env[k] = taskfile.Var{Static: static} + new.Env.Set(k, taskfile.Var{Static: static}) + return nil + }) + if err != nil { + return nil, err } if len(origTask.Cmds) > 0 { @@ -103,7 +103,7 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { if err != nil { return nil, err } - vars[strings.ToUpper(checker.Kind())] = taskfile.Var{Live: value} + vars.Set(strings.ToUpper(checker.Kind()), taskfile.Var{Live: value}) } // Adding new variables, requires us to refresh the templaters From cbb12b29bd30cfc779e59126fd607fa229497c3e Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Sun, 5 Apr 2020 11:08:49 -0300 Subject: [PATCH 3/4] v3: Fix bug where global vars were not being considered --- internal/taskfile/taskfile.go | 6 ++++++ internal/taskfile/var.go | 3 --- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/internal/taskfile/taskfile.go b/internal/taskfile/taskfile.go index b8cf5da9..fc6b1299 100644 --- a/internal/taskfile/taskfile.go +++ b/internal/taskfile/taskfile.go @@ -41,5 +41,11 @@ func (tf *Taskfile) UnmarshalYAML(unmarshal func(interface{}) error) error { if tf.Expansions <= 0 { tf.Expansions = 2 } + if tf.Vars == nil { + tf.Vars = &Vars{} + } + if tf.Env == nil { + tf.Env = &Vars{} + } return nil } diff --git a/internal/taskfile/var.go b/internal/taskfile/var.go index 99946f8a..2466e93c 100644 --- a/internal/taskfile/var.go +++ b/internal/taskfile/var.go @@ -50,9 +50,6 @@ func (vs *Vars) Merge(other *Vars) { // Set sets a value to a given key func (vs *Vars) Set(key string, value Var) { - if vs == nil { - vs = &Vars{} - } if vs.Mapping == nil { vs.Mapping = make(map[string]Var, 1) } From 68ce8642b16cc972464cd1419a8cf57e976345be Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Sat, 16 May 2020 15:45:41 -0300 Subject: [PATCH 4/4] Create v3 compiler which respects declaration order of variables Also, fix "" been printed when a non-existing variable is printed. --- internal/compiler/v3/compiler_v3.go | 98 +++++++++++++++++++ internal/templater/templater.go | 7 +- task.go | 33 +++++-- task_test.go | 14 +++ testdata/checksum/Taskfile.yml | 2 +- testdata/cyclic/Taskfile.yml | 2 +- testdata/deps/Taskfile.yml | 2 +- testdata/dir/Taskfile.yml | 2 +- .../dir/explicit_doesnt_exist/Taskfile.yml | 2 +- testdata/dir/explicit_exists/Taskfile.yml | 2 +- testdata/dry/Taskfile.yml | 2 +- testdata/dry_checksum/Taskfile.yml | 2 +- testdata/env/Taskfile.yml | 2 +- testdata/expand/Taskfile.yml | 2 +- testdata/generates/Taskfile.yml | 5 +- testdata/generates/Taskvars.yml | 1 - testdata/ignore_errors/Taskfile.yml | 2 +- testdata/includes_call_root_task/Taskfile.yml | 2 +- .../includes_call_root_task/Taskfile2.yml | 2 +- testdata/includes_deps/Taskfile.yml | 2 +- testdata/includes_deps/Taskfile2.yml | 2 +- testdata/includes_empty/Taskfile.yml | 2 +- testdata/includes_empty/Taskfile2.yml | 2 +- testdata/params/Taskfile.yml | 6 +- testdata/params/Taskvars.yml | 2 - testdata/precondition/Taskfile.yml | 2 +- testdata/status/Taskfile.yml | 2 +- testdata/summary/Taskfile.yml | 2 +- testdata/vars/v3/.gitignore | 1 + testdata/vars/v3/Taskfile.yml | 46 +++++++++ variables.go | 7 +- 31 files changed, 225 insertions(+), 35 deletions(-) create mode 100644 internal/compiler/v3/compiler_v3.go delete mode 100644 testdata/generates/Taskvars.yml delete mode 100644 testdata/params/Taskvars.yml create mode 100644 testdata/vars/v3/.gitignore create mode 100644 testdata/vars/v3/Taskfile.yml diff --git a/internal/compiler/v3/compiler_v3.go b/internal/compiler/v3/compiler_v3.go new file mode 100644 index 00000000..85e68d90 --- /dev/null +++ b/internal/compiler/v3/compiler_v3.go @@ -0,0 +1,98 @@ +package v3 + +import ( + "bytes" + "context" + "fmt" + "strings" + "sync" + + "github.com/go-task/task/v2/internal/compiler" + "github.com/go-task/task/v2/internal/execext" + "github.com/go-task/task/v2/internal/logger" + "github.com/go-task/task/v2/internal/taskfile" + "github.com/go-task/task/v2/internal/templater" +) + +var _ compiler.Compiler = &CompilerV3{} + +type CompilerV3 struct { + Dir string + + TaskfileVars *taskfile.Vars + + Logger *logger.Logger + + dynamicCache map[string]string + muDynamicCache sync.Mutex +} + +func (c *CompilerV3) GetVariables(t *taskfile.Task, call taskfile.Call) (*taskfile.Vars, error) { + result := compiler.GetEnviron() + result.Set("TASK", taskfile.Var{Static: t.Task}) + + rangeFunc := func(k string, v taskfile.Var) error { + tr := templater.Templater{Vars: result, RemoveNoValue: true} + v = taskfile.Var{ + Static: tr.Replace(v.Static), + Sh: tr.Replace(v.Sh), + } + if err := tr.Err(); err != nil { + return err + } + static, err := c.HandleDynamicVar(v) + if err != nil { + return err + } + result.Set(k, taskfile.Var{Static: static}) + return nil + } + + if err := c.TaskfileVars.Range(rangeFunc); err != nil { + return nil, err + } + if err := call.Vars.Range(rangeFunc); err != nil { + return nil, err + } + if err := t.Vars.Range(rangeFunc); err != nil { + return nil, err + } + + return result, nil +} + +func (c *CompilerV3) HandleDynamicVar(v taskfile.Var) (string, error) { + if v.Static != "" || v.Sh == "" { + return v.Static, nil + } + + c.muDynamicCache.Lock() + defer c.muDynamicCache.Unlock() + + if c.dynamicCache == nil { + c.dynamicCache = make(map[string]string, 30) + } + if result, ok := c.dynamicCache[v.Sh]; ok { + return result, nil + } + + var stdout bytes.Buffer + opts := &execext.RunCommandOptions{ + Command: v.Sh, + Dir: c.Dir, + Stdout: &stdout, + Stderr: c.Logger.Stderr, + } + if err := execext.RunCommand(context.Background(), opts); err != nil { + return "", fmt.Errorf(`task: Command "%s" in taskvars file failed: %s`, opts.Command, err) + } + + // Trim a single trailing newline from the result to make most command + // output easier to use in shell commands. + result := strings.TrimSuffix(stdout.String(), "\n") + + c.dynamicCache[v.Sh] = result + c.Logger.VerboseErrf(logger.Magenta, `task: dynamic variable: '%s' result: '%s'`, v.Sh, result) + + return result, nil +} diff --git a/internal/templater/templater.go b/internal/templater/templater.go index b905b1ba..85de72fb 100644 --- a/internal/templater/templater.go +++ b/internal/templater/templater.go @@ -2,6 +2,7 @@ package templater import ( "bytes" + "strings" "text/template" "github.com/go-task/task/v2/internal/taskfile" @@ -12,7 +13,8 @@ import ( // happen will be assigned to r.err, and consecutive calls to funcs will just // return the zero value. type Templater struct { - Vars *taskfile.Vars + Vars *taskfile.Vars + RemoveNoValue bool cacheMap map[string]interface{} err error @@ -42,6 +44,9 @@ func (r *Templater) Replace(str string) string { r.err = err return "" } + if r.RemoveNoValue { + return strings.ReplaceAll(b.String(), "", "") + } return b.String() } diff --git a/task.go b/task.go index 0e3eb327..836fd027 100644 --- a/task.go +++ b/task.go @@ -12,6 +12,7 @@ import ( "github.com/go-task/task/v2/internal/compiler" compilerv2 "github.com/go-task/task/v2/internal/compiler/v2" + compilerv3 "github.com/go-task/task/v2/internal/compiler/v3" "github.com/go-task/task/v2/internal/execext" "github.com/go-task/task/v2/internal/logger" "github.com/go-task/task/v2/internal/output" @@ -131,9 +132,9 @@ func (e *Executor) Setup() error { Color: e.Color, } - v, err := strconv.ParseFloat(e.Taskfile.Version, 64) + v, err := e.parsedVersion() if err != nil { - return fmt.Errorf(`task: Could not parse taskfile version "%s": %v`, e.Taskfile.Version, err) + } if v < 2 { @@ -154,12 +155,20 @@ func (e *Executor) Setup() error { e.Logger.Color = false } - e.Compiler = &compilerv2.CompilerV2{ - Dir: e.Dir, - Taskvars: e.taskvars, - TaskfileVars: e.Taskfile.Vars, - Expansions: e.Taskfile.Expansions, - Logger: e.Logger, + if v < 3 { + e.Compiler = &compilerv2.CompilerV2{ + Dir: e.Dir, + Taskvars: e.taskvars, + TaskfileVars: e.Taskfile.Vars, + Expansions: e.Taskfile.Expansions, + Logger: e.Logger, + } + } else { + e.Compiler = &compilerv3.CompilerV3{ + Dir: e.Dir, + TaskfileVars: e.Taskfile.Vars, + Logger: e.Logger, + } } if v < 2.1 && e.Taskfile.Output != "" { @@ -231,6 +240,14 @@ func (e *Executor) Setup() error { return nil } +func (e *Executor) parsedVersion() (float64, error) { + v, err := strconv.ParseFloat(e.Taskfile.Version, 64) + if err != nil { + return 0, fmt.Errorf(`task: Could not parse taskfile version "%s": %v`, e.Taskfile.Version, err) + } + return v, nil +} + // RunTask runs a task by its name func (e *Executor) RunTask(ctx context.Context, call taskfile.Call) error { t, err := e.CompiledTask(call) diff --git a/task_test.go b/task_test.go index c752e95b..ef985361 100644 --- a/task_test.go +++ b/task_test.go @@ -107,6 +107,20 @@ func TestVarsV2(t *testing.T) { tt.Run(t) } +func TestVarsV3(t *testing.T) { + tt := fileContentTest{ + Dir: "testdata/vars/v3", + Target: "default", + Files: map[string]string{ + "missing-var.txt": "\n", + "var-order.txt": "ABCDEF\n", + "dependent-sh.txt": "123456\n", + "with-call.txt": "Hi, ABC123!\n", + }, + } + tt.Run(t) +} + func TestMultilineVars(t *testing.T) { for _, dir := range []string{"testdata/vars/v2/multiline"} { tt := fileContentTest{ diff --git a/testdata/checksum/Taskfile.yml b/testdata/checksum/Taskfile.yml index 1c620a71..d2de1086 100644 --- a/testdata/checksum/Taskfile.yml +++ b/testdata/checksum/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: build: diff --git a/testdata/cyclic/Taskfile.yml b/testdata/cyclic/Taskfile.yml index 0da55536..83cf39db 100644 --- a/testdata/cyclic/Taskfile.yml +++ b/testdata/cyclic/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: task-1: diff --git a/testdata/deps/Taskfile.yml b/testdata/deps/Taskfile.yml index 411eeb91..bc2aa71c 100644 --- a/testdata/deps/Taskfile.yml +++ b/testdata/deps/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: default: diff --git a/testdata/dir/Taskfile.yml b/testdata/dir/Taskfile.yml index f17dd9fe..8e740dbe 100644 --- a/testdata/dir/Taskfile.yml +++ b/testdata/dir/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: whereami: diff --git a/testdata/dir/explicit_doesnt_exist/Taskfile.yml b/testdata/dir/explicit_doesnt_exist/Taskfile.yml index 1b6fb7d1..0f0fb52c 100644 --- a/testdata/dir/explicit_doesnt_exist/Taskfile.yml +++ b/testdata/dir/explicit_doesnt_exist/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: whereami: diff --git a/testdata/dir/explicit_exists/Taskfile.yml b/testdata/dir/explicit_exists/Taskfile.yml index 0ab53b26..a1797562 100644 --- a/testdata/dir/explicit_exists/Taskfile.yml +++ b/testdata/dir/explicit_exists/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: whereami: diff --git a/testdata/dry/Taskfile.yml b/testdata/dry/Taskfile.yml index b4e932ed..81c7e0d1 100644 --- a/testdata/dry/Taskfile.yml +++ b/testdata/dry/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: build: diff --git a/testdata/dry_checksum/Taskfile.yml b/testdata/dry_checksum/Taskfile.yml index 057816b3..f58cf0ef 100644 --- a/testdata/dry_checksum/Taskfile.yml +++ b/testdata/dry_checksum/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: default: diff --git a/testdata/env/Taskfile.yml b/testdata/env/Taskfile.yml index d5a7419d..cb88f95e 100644 --- a/testdata/env/Taskfile.yml +++ b/testdata/env/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' vars: BAZ: diff --git a/testdata/expand/Taskfile.yml b/testdata/expand/Taskfile.yml index 7398407c..967490ac 100644 --- a/testdata/expand/Taskfile.yml +++ b/testdata/expand/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: pwd: diff --git a/testdata/generates/Taskfile.yml b/testdata/generates/Taskfile.yml index 9f05cfd0..4dc6e9ec 100644 --- a/testdata/generates/Taskfile.yml +++ b/testdata/generates/Taskfile.yml @@ -1,4 +1,7 @@ -version: '2' +version: '3' + +vars: + BUILD_DIR: $pwd tasks: abs.txt: diff --git a/testdata/generates/Taskvars.yml b/testdata/generates/Taskvars.yml deleted file mode 100644 index 0907174f..00000000 --- a/testdata/generates/Taskvars.yml +++ /dev/null @@ -1 +0,0 @@ -BUILD_DIR: $pwd diff --git a/testdata/ignore_errors/Taskfile.yml b/testdata/ignore_errors/Taskfile.yml index 1d7dc8b5..31b82a70 100644 --- a/testdata/ignore_errors/Taskfile.yml +++ b/testdata/ignore_errors/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: task-should-pass: diff --git a/testdata/includes_call_root_task/Taskfile.yml b/testdata/includes_call_root_task/Taskfile.yml index 41e60ad0..26373378 100644 --- a/testdata/includes_call_root_task/Taskfile.yml +++ b/testdata/includes_call_root_task/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' includes: included: Taskfile2.yml diff --git a/testdata/includes_call_root_task/Taskfile2.yml b/testdata/includes_call_root_task/Taskfile2.yml index 99d5cb3a..b520bc4b 100644 --- a/testdata/includes_call_root_task/Taskfile2.yml +++ b/testdata/includes_call_root_task/Taskfile2.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: call-root: diff --git a/testdata/includes_deps/Taskfile.yml b/testdata/includes_deps/Taskfile.yml index e6d4d186..bed642bb 100644 --- a/testdata/includes_deps/Taskfile.yml +++ b/testdata/includes_deps/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' includes: included: Taskfile2.yml diff --git a/testdata/includes_deps/Taskfile2.yml b/testdata/includes_deps/Taskfile2.yml index 27eb2b97..c5e704a7 100644 --- a/testdata/includes_deps/Taskfile2.yml +++ b/testdata/includes_deps/Taskfile2.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: default: diff --git a/testdata/includes_empty/Taskfile.yml b/testdata/includes_empty/Taskfile.yml index 2d24e533..a58ab424 100644 --- a/testdata/includes_empty/Taskfile.yml +++ b/testdata/includes_empty/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' includes: included: Taskfile2.yml diff --git a/testdata/includes_empty/Taskfile2.yml b/testdata/includes_empty/Taskfile2.yml index 825285aa..7e984ef7 100644 --- a/testdata/includes_empty/Taskfile2.yml +++ b/testdata/includes_empty/Taskfile2.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' vars: FILE: file.txt diff --git a/testdata/params/Taskfile.yml b/testdata/params/Taskfile.yml index f8d9a91a..06cbe488 100644 --- a/testdata/params/Taskfile.yml +++ b/testdata/params/Taskfile.yml @@ -1,4 +1,8 @@ -version: '2' +version: '3' + +vars: + PORTUGUESE_HELLO_WORLD: Olá, mundo! + GERMAN: Hello tasks: default: diff --git a/testdata/params/Taskvars.yml b/testdata/params/Taskvars.yml deleted file mode 100644 index af0a0efb..00000000 --- a/testdata/params/Taskvars.yml +++ /dev/null @@ -1,2 +0,0 @@ -PORTUGUESE_HELLO_WORLD: Olá, mundo! -GERMAN: "Hello" diff --git a/testdata/precondition/Taskfile.yml b/testdata/precondition/Taskfile.yml index eedc588f..669eb9d8 100644 --- a/testdata/precondition/Taskfile.yml +++ b/testdata/precondition/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: foo: diff --git a/testdata/status/Taskfile.yml b/testdata/status/Taskfile.yml index 57900efc..7923c426 100644 --- a/testdata/status/Taskfile.yml +++ b/testdata/status/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: gen-foo: diff --git a/testdata/summary/Taskfile.yml b/testdata/summary/Taskfile.yml index 3e4d4640..7b857f76 100644 --- a/testdata/summary/Taskfile.yml +++ b/testdata/summary/Taskfile.yml @@ -1,4 +1,4 @@ -version: '2' +version: '3' tasks: task-with-summary: diff --git a/testdata/vars/v3/.gitignore b/testdata/vars/v3/.gitignore new file mode 100644 index 00000000..2211df63 --- /dev/null +++ b/testdata/vars/v3/.gitignore @@ -0,0 +1 @@ +*.txt diff --git a/testdata/vars/v3/Taskfile.yml b/testdata/vars/v3/Taskfile.yml new file mode 100644 index 00000000..4c581115 --- /dev/null +++ b/testdata/vars/v3/Taskfile.yml @@ -0,0 +1,46 @@ +version: '3' + +vars: + VAR_A: A + VAR_B: '{{.VAR_A}}B' + VAR_C: '{{.VAR_B}}C' + + VAR_1: {sh: echo 1} + VAR_2: {sh: 'echo "{{.VAR_1}}2"'} + VAR_3: {sh: 'echo "{{.VAR_2}}3"'} + +tasks: + default: + - task: missing-var + - task: var-order + - task: dependent-sh + - task: with-call + + missing-var: echo '{{.NON_EXISTING_VAR}}' > missing-var.txt + + var-order: + vars: + VAR_D: '{{.VAR_C}}D' + VAR_E: '{{.VAR_D}}E' + VAR_F: '{{.VAR_E}}F' + cmds: + - echo '{{.VAR_F}}' > var-order.txt + + dependent-sh: + vars: + VAR_4: {sh: 'echo "{{.VAR_3}}4"'} + VAR_5: {sh: 'echo "{{.VAR_4}}5"'} + VAR_6: {sh: 'echo "{{.VAR_5}}6"'} + cmds: + - echo '{{.VAR_6}}' > dependent-sh.txt + + with-call: + - task: called-task + vars: + ABC123: '{{.VAR_C}}{{.VAR_3}}' + + called-task: + vars: + MESSAGE: Hi, {{.ABC123}}! + cmds: + - echo "{{.MESSAGE}}" > with-call.txt diff --git a/variables.go b/variables.go index a2ef97d8..569f56ed 100644 --- a/variables.go +++ b/variables.go @@ -23,7 +23,12 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { return nil, err } - r := templater.Templater{Vars: vars} + v, err := e.parsedVersion() + if err != nil { + return nil, err + } + + r := templater.Templater{Vars: vars, RemoveNoValue: v >= 3.0} new := taskfile.Task{ Task: origTask.Task,