From d6234af49a51aa7d60bc24f0ecf39c80eaca68c4 Mon Sep 17 00:00:00 2001 From: Pete Davison Date: Sun, 23 Feb 2025 18:13:56 +0000 Subject: [PATCH] feat: allow variable references in a matrix (#2069) --- task_test.go | 60 ++++++++++++++++++++++------ taskfile/ast/matrix.go | 72 ++++++++++++++++++++++++---------- testdata/for/cmds/Taskfile.yml | 25 ++++++++++++ testdata/for/deps/Taskfile.yml | 29 ++++++++++++++ variables.go | 38 +++++++++++++++--- 5 files changed, 185 insertions(+), 39 deletions(-) diff --git a/task_test.go b/task_test.go index ff407f18..4455f0b9 100644 --- a/task_test.go +++ b/task_test.go @@ -2975,6 +2975,7 @@ func TestForCmds(t *testing.T) { tests := []struct { name string expectedOutput string + wantErr bool }{ { name: "loop-explicit", @@ -2984,6 +2985,14 @@ func TestForCmds(t *testing.T) { name: "loop-matrix", expectedOutput: "windows/amd64\nwindows/arm64\nlinux/amd64\nlinux/arm64\ndarwin/amd64\ndarwin/arm64\n", }, + { + name: "loop-matrix-ref", + expectedOutput: "windows/amd64\nwindows/arm64\nlinux/amd64\nlinux/arm64\ndarwin/amd64\ndarwin/arm64\n", + }, + { + name: "loop-matrix-ref-error", + wantErr: true, + }, { name: "loop-sources", expectedOutput: "bar\nfoo\n", @@ -3018,18 +3027,22 @@ func TestForCmds(t *testing.T) { t.Run(test.name, func(t *testing.T) { t.Parallel() - var stdOut bytes.Buffer - var stdErr bytes.Buffer - e := task.Executor{ + buf := &bytes.Buffer{} + e := &task.Executor{ Dir: "testdata/for/cmds", - Stdout: &stdOut, - Stderr: &stdErr, + Stdout: buf, + Stderr: buf, Silent: true, Force: true, } require.NoError(t, e.Setup()) - require.NoError(t, e.Run(context.Background(), &ast.Call{Task: test.name})) - assert.Equal(t, test.expectedOutput, stdOut.String()) + err := e.Run(context.Background(), &ast.Call{Task: test.name}) + if test.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + assert.Equal(t, test.expectedOutput, buf.String()) }) } } @@ -3040,6 +3053,7 @@ func TestForDeps(t *testing.T) { tests := []struct { name string expectedOutputContains []string + wantErr bool }{ { name: "loop-explicit", @@ -3056,6 +3070,21 @@ func TestForDeps(t *testing.T) { "darwin/arm64\n", }, }, + { + name: "loop-matrix-ref", + expectedOutputContains: []string{ + "windows/amd64\n", + "windows/arm64\n", + "linux/amd64\n", + "linux/arm64\n", + "darwin/amd64\n", + "darwin/arm64\n", + }, + }, + { + name: "loop-matrix-ref-error", + wantErr: true, + }, { name: "loop-sources", expectedOutputContains: []string{"bar\n", "foo\n"}, @@ -3091,20 +3120,25 @@ func TestForDeps(t *testing.T) { t.Parallel() // We need to use a sync buffer here as deps are run concurrently - var buff SyncBuffer - e := task.Executor{ + buf := &SyncBuffer{} + e := &task.Executor{ Dir: "testdata/for/deps", - Stdout: &buff, - Stderr: &buff, + Stdout: buf, + Stderr: buf, Silent: true, Force: true, // Force output of each dep to be grouped together to prevent interleaving OutputStyle: ast.Output{Name: "group"}, } require.NoError(t, e.Setup()) - require.NoError(t, e.Run(context.Background(), &ast.Call{Task: test.name})) + err := e.Run(context.Background(), &ast.Call{Task: test.name}) + if test.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } for _, expectedOutputContains := range test.expectedOutputContains { - assert.Contains(t, buff.buf.String(), expectedOutputContains) + assert.Contains(t, buf.buf.String(), expectedOutputContains) } }) } diff --git a/taskfile/ast/matrix.go b/taskfile/ast/matrix.go index e6b9f173..379f1781 100644 --- a/taskfile/ast/matrix.go +++ b/taskfile/ast/matrix.go @@ -10,15 +10,25 @@ import ( "github.com/go-task/task/v3/internal/deepcopy" ) -type Matrix struct { - om *orderedmap.OrderedMap[string, []any] -} - -type MatrixElement orderedmap.Element[string, []any] +type ( + // Matrix is an ordered map of variable names to arrays of values. + Matrix struct { + om *orderedmap.OrderedMap[string, *MatrixRow] + } + // A MatrixElement is a key-value pair that is used for initializing a + // Matrix structure. + MatrixElement orderedmap.Element[string, *MatrixRow] + // A MatrixRow list of values for a matrix key or a reference to another + // variable. + MatrixRow struct { + Ref string + Value []any + } +) func NewMatrix(els ...*MatrixElement) *Matrix { matrix := &Matrix{ - om: orderedmap.NewOrderedMap[string, []any](), + om: orderedmap.NewOrderedMap[string, *MatrixRow](), } for _, el := range els { matrix.Set(el.Key, el.Value) @@ -33,27 +43,27 @@ func (matrix *Matrix) Len() int { return matrix.om.Len() } -func (matrix *Matrix) Get(key string) ([]any, bool) { +func (matrix *Matrix) Get(key string) (*MatrixRow, bool) { if matrix == nil || matrix.om == nil { return nil, false } return matrix.om.Get(key) } -func (matrix *Matrix) Set(key string, value []any) bool { +func (matrix *Matrix) Set(key string, value *MatrixRow) bool { if matrix == nil { matrix = NewMatrix() } if matrix.om == nil { - matrix.om = orderedmap.NewOrderedMap[string, []any]() + matrix.om = orderedmap.NewOrderedMap[string, *MatrixRow]() } return matrix.om.Set(key, value) } // All returns an iterator that loops over all task key-value pairs. -func (matrix *Matrix) All() iter.Seq2[string, []any] { +func (matrix *Matrix) All() iter.Seq2[string, *MatrixRow] { if matrix == nil || matrix.om == nil { - return func(yield func(string, []any) bool) {} + return func(yield func(string, *MatrixRow) bool) {} } return matrix.om.AllFromFront() } @@ -67,9 +77,9 @@ func (matrix *Matrix) Keys() iter.Seq[string] { } // Values returns an iterator that loops over all task values. -func (matrix *Matrix) Values() iter.Seq[[]any] { +func (matrix *Matrix) Values() iter.Seq[*MatrixRow] { if matrix == nil || matrix.om == nil { - return func(yield func([]any) bool) {} + return func(yield func(*MatrixRow) bool) {} } return matrix.om.Values() } @@ -93,14 +103,36 @@ func (matrix *Matrix) UnmarshalYAML(node *yaml.Node) error { keyNode := node.Content[i] valueNode := node.Content[i+1] - // Decode the value node into a Matrix struct - var v []any - if err := valueNode.Decode(&v); err != nil { - return errors.NewTaskfileDecodeError(err, node) - } + switch valueNode.Kind { + case yaml.SequenceNode: + // Decode the value node into a Matrix struct + var v []any + if err := valueNode.Decode(&v); err != nil { + return errors.NewTaskfileDecodeError(err, node) + } - // Add the task to the ordered map - matrix.Set(keyNode.Value, v) + // Add the row to the ordered map + matrix.Set(keyNode.Value, &MatrixRow{ + Value: v, + }) + + case yaml.MappingNode: + // Decode the value node into a Matrix struct + var refStruct struct { + Ref string + } + if err := valueNode.Decode(&refStruct); err != nil { + return errors.NewTaskfileDecodeError(err, node) + } + + // Add the reference to the ordered map + matrix.Set(keyNode.Value, &MatrixRow{ + Ref: refStruct.Ref, + }) + + default: + return errors.NewTaskfileDecodeError(nil, node).WithMessage("matrix values must be an array or a reference") + } } return nil } diff --git a/testdata/for/cmds/Taskfile.yml b/testdata/for/cmds/Taskfile.yml index a436a4a3..a4196a96 100644 --- a/testdata/for/cmds/Taskfile.yml +++ b/testdata/for/cmds/Taskfile.yml @@ -1,5 +1,10 @@ version: "3" +vars: + OS_VAR: ["windows", "linux", "darwin"] + ARCH_VAR: ["amd64", "arm64"] + NOT_A_LIST: "not a list" + tasks: # Loop over a list of values loop-explicit: @@ -15,6 +20,26 @@ tasks: ARCH: ["amd64", "arm64"] cmd: echo "{{.ITEM.OS}}/{{.ITEM.ARCH}}" + loop-matrix-ref: + cmds: + - for: + matrix: + OS: + ref: .OS_VAR + ARCH: + ref: .ARCH_VAR + cmd: echo "{{.ITEM.OS}}/{{.ITEM.ARCH}}" + + loop-matrix-ref-error: + cmds: + - for: + matrix: + OS: + ref: .OS_VAR + ARCH: + ref: .NOT_A_LIST + cmd: echo "{{.ITEM.OS}}/{{.ITEM.ARCH}}" + # Loop over the task's sources loop-sources: sources: diff --git a/testdata/for/deps/Taskfile.yml b/testdata/for/deps/Taskfile.yml index 1d70f50d..002ef38c 100644 --- a/testdata/for/deps/Taskfile.yml +++ b/testdata/for/deps/Taskfile.yml @@ -1,5 +1,10 @@ version: "3" +vars: + OS_VAR: ["windows", "linux", "darwin"] + ARCH_VAR: ["amd64", "arm64"] + NOT_A_LIST: "not a list" + tasks: # Loop over a list of values loop-explicit: @@ -19,6 +24,30 @@ tasks: vars: TEXT: "{{.ITEM.OS}}/{{.ITEM.ARCH}}" + loop-matrix-ref: + deps: + - for: + matrix: + OS: + ref: .OS_VAR + ARCH: + ref: .ARCH_VAR + task: echo + vars: + TEXT: "{{.ITEM.OS}}/{{.ITEM.ARCH}}" + + loop-matrix-ref-error: + deps: + - for: + matrix: + OS: + ref: .OS_VAR + ARCH: + ref: .NOT_A_LIST + task: echo + vars: + TEXT: "{{.ITEM.OS}}/{{.ITEM.ARCH}}" + # Loop over the task's sources loop-sources: sources: diff --git a/variables.go b/variables.go index 91c3ad82..d1d1c4e9 100644 --- a/variables.go +++ b/variables.go @@ -1,6 +1,7 @@ package task import ( + "fmt" "os" "path/filepath" "strings" @@ -151,7 +152,7 @@ func (e *Executor) compiledTask(call *ast.Call, evaluateShVars bool) (*ast.Task, continue } if cmd.For != nil { - list, keys, err := itemsFromFor(cmd.For, new.Dir, new.Sources, vars, origTask.Location) + list, keys, err := itemsFromFor(cmd.For, new.Dir, new.Sources, vars, origTask.Location, cache) if err != nil { return nil, err } @@ -198,7 +199,7 @@ func (e *Executor) compiledTask(call *ast.Call, evaluateShVars bool) (*ast.Task, continue } if dep.For != nil { - list, keys, err := itemsFromFor(dep.For, new.Dir, new.Sources, vars, origTask.Location) + list, keys, err := itemsFromFor(dep.For, new.Dir, new.Sources, vars, origTask.Location, cache) if err != nil { return nil, err } @@ -270,11 +271,18 @@ func itemsFromFor( sources []*ast.Glob, vars *ast.Vars, location *ast.Location, + cache *templater.Cache, ) ([]any, []string, error) { var keys []string // The list of keys to loop over (only if looping over a map) var values []any // The list of values to loop over // Get the list from a matrix if f.Matrix.Len() != 0 { + if err := resolveMatrixRefs(f.Matrix, cache); err != nil { + return nil, nil, errors.TaskfileInvalidError{ + URI: location.Taskfile, + Err: err, + } + } return asAnySlice(product(f.Matrix)), nil, nil } // Get the list from the explicit for list @@ -333,9 +341,27 @@ func itemsFromFor( return values, keys, nil } +func resolveMatrixRefs(matrix *ast.Matrix, cache *templater.Cache) error { + if matrix.Len() == 0 { + return nil + } + for _, row := range matrix.All() { + if row.Ref != "" { + v := templater.ResolveRef(row.Ref, cache) + switch value := v.(type) { + case []any: + row.Value = value + default: + return fmt.Errorf("matrix reference %q must resolve to a list", row.Ref) + } + } + } + return nil +} + // product generates the cartesian product of the input map of slices. -func product(inputMap *ast.Matrix) []map[string]any { - if inputMap.Len() == 0 { +func product(matrix *ast.Matrix) []map[string]any { + if matrix.Len() == 0 { return nil } @@ -343,13 +369,13 @@ func product(inputMap *ast.Matrix) []map[string]any { result := []map[string]any{{}} // Iterate over each slice in the slices - for key, slice := range inputMap.All() { + for key, row := range matrix.All() { var newResult []map[string]any // For each combination in the current result for _, combination := range result { // Append each element from the current slice to the combinations - for _, item := range slice { + for _, item := range row.Value { newComb := make(map[string]any, len(combination)) // Copy the existing combination for k, v := range combination {