From bd5882f0f0a12981734a9c07a3abc7761c9cc8c5 Mon Sep 17 00:00:00 2001 From: Stephen Prater Date: Fri, 17 May 2019 13:13:47 -0700 Subject: [PATCH 1/6] Add Preconditions to Tasks --- .gitignore | 3 ++ docs/taskfile_versions.md | 15 +++++++ docs/usage.md | 47 +++++++++++++++++++ internal/taskfile/precondition.go | 51 +++++++++++++++++++++ internal/taskfile/precondition_test.go | 49 ++++++++++++++++++++ internal/taskfile/task.go | 31 ++++++------- internal/taskfile/version/version.go | 12 +++++ precondition.go | 44 ++++++++++++++++++ status.go | 2 + task.go | 35 ++++++++++++--- task_test.go | 62 ++++++++++++++++++++++++++ testdata/precondition/Taskfile.yml | 34 ++++++++++++++ testdata/precondition/foo.txt | 0 variables.go | 11 +++++ 14 files changed, 375 insertions(+), 21 deletions(-) create mode 100644 internal/taskfile/precondition.go create mode 100644 internal/taskfile/precondition_test.go create mode 100644 precondition.go create mode 100644 testdata/precondition/Taskfile.yml create mode 100644 testdata/precondition/foo.txt diff --git a/.gitignore b/.gitignore index d5340eb2..c5de0bc5 100644 --- a/.gitignore +++ b/.gitignore @@ -21,3 +21,6 @@ dist/ # intellij idea/goland .idea/ + +# exuberant ctags +tags diff --git a/docs/taskfile_versions.md b/docs/taskfile_versions.md index 82c80463..b56bd1ce 100644 --- a/docs/taskfile_versions.md +++ b/docs/taskfile_versions.md @@ -141,6 +141,21 @@ includes: docker: ./DockerTasks.yml ``` +## Version 2.3 + +Version 2.3 comes with `preconditions` stanza in tasks. + +```yaml +version: '2' + +tasks: + upload_environment: + preconditions: + - test -f .env + cmds: + - aws s3 cp .env s3://myenvironment +``` + Please check the [documentation][includes] [output]: usage.md#output-syntax diff --git a/docs/usage.md b/docs/usage.md index 40c3dd4a..27af026a 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -344,6 +344,53 @@ up-to-date. Also, `task --status [tasks]...` will exit with a non-zero exit code if any of the tasks are not up-to-date. +If you need a certain set of conditions to be _true_ you can use the +`preconditions` stanza. `preconditions` are very similar to `status` +lines except they support `sh` expansion and they SHOULD all return 0 + +```yaml +version: '2' + +tasks: + generate-files: + cmds: + - mkdir directory + - touch directory/file1.txt + - touch directory/file2.txt + # test existence of files + preconditions: + - test -f .env + - sh: "[ 1 = 0 ]" + msg: "One doesn't equal Zero, Halting" +``` + +Preconditions can set specific failure messages that can tell +a user what to do using the `msg` field. + +If a task has a dependency on a sub-task with a precondition, and that +precondition is not met - the calling task will fail. Adding `ignore_errors` +to the precondition will cause parent tasks to execute even if the sub task +can not run. Note that a task executed directly with a failing precondition +will not run unless `--force` is given. + +```yaml +version: '2' +tasks: + task_will_fail: + preconditions: + - sh: "exit 1" + ignore_errors: true + + task_will_succeed: + deps: + - task_will_fail + + task_will_succeed: + cmds: + - task: task_will_fail + - echo "I will run" +``` + ## Variables When doing interpolation of variables, Task will look for the below. diff --git a/internal/taskfile/precondition.go b/internal/taskfile/precondition.go new file mode 100644 index 00000000..04b116a5 --- /dev/null +++ b/internal/taskfile/precondition.go @@ -0,0 +1,51 @@ +package taskfile + +import ( + "errors" + "fmt" +) + +var ( + // ErrCantUnmarshalPrecondition is returned for invalid precond YAML. + ErrCantUnmarshalPrecondition = errors.New("task: can't unmarshal precondition value") +) + +// Precondition represents a precondition necessary for a task to run +type Precondition struct { + Sh string + Msg string + IgnoreError bool +} + +// UnmarshalYAML implements yaml.Unmarshaler interface. +func (p *Precondition) UnmarshalYAML(unmarshal func(interface{}) error) error { + var cmd string + + if err := unmarshal(&cmd); err == nil { + p.Sh = cmd + p.Msg = fmt.Sprintf("`%s` failed", cmd) + p.IgnoreError = false + return nil + } + + var sh struct { + Sh string + Msg string + IgnoreError bool `yaml:"ignore_error"` + } + + err := unmarshal(&sh) + + if err == nil { + p.Sh = sh.Sh + p.Msg = sh.Msg + if p.Msg == "" { + p.Msg = fmt.Sprintf("%s failed", sh.Sh) + } + + p.IgnoreError = sh.IgnoreError + return nil + } + + return err +} diff --git a/internal/taskfile/precondition_test.go b/internal/taskfile/precondition_test.go new file mode 100644 index 00000000..acf89f27 --- /dev/null +++ b/internal/taskfile/precondition_test.go @@ -0,0 +1,49 @@ +package taskfile_test + +import ( + "testing" + + "github.com/go-task/task/v2/internal/taskfile" + + "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v2" +) + +func TestPreconditionParse(t *testing.T) { + tests := []struct { + content string + v interface{} + expected interface{} + }{ + { + "test -f foo.txt", + &taskfile.Precondition{}, + &taskfile.Precondition{Sh: `test -f foo.txt`, Msg: "`test -f foo.txt` failed", IgnoreError: false}, + }, + { + "sh: '[ 1 = 0 ]'", + &taskfile.Precondition{}, + &taskfile.Precondition{Sh: "[ 1 = 0 ]", Msg: "[ 1 = 0 ] failed", IgnoreError: false}, + }, + {` +sh: "[ 1 = 2 ]" +msg: "1 is not 2" +`, + &taskfile.Precondition{}, + &taskfile.Precondition{Sh: "[ 1 = 2 ]", Msg: "1 is not 2", IgnoreError: false}, + }, + {` +sh: "[ 1 = 2 ]" +msg: "1 is not 2" +ignore_error: true +`, + &taskfile.Precondition{}, + &taskfile.Precondition{Sh: "[ 1 = 2 ]", Msg: "1 is not 2", IgnoreError: true}, + }, + } + for _, test := range tests { + err := yaml.Unmarshal([]byte(test.content), test.v) + assert.NoError(t, err) + assert.Equal(t, test.expected, test.v) + } +} diff --git a/internal/taskfile/task.go b/internal/taskfile/task.go index 1afcbfa3..9c1cf3b3 100644 --- a/internal/taskfile/task.go +++ b/internal/taskfile/task.go @@ -5,19 +5,20 @@ type Tasks map[string]*Task // Task represents a task type Task struct { - Task string - Cmds []*Cmd - Deps []*Dep - Desc string - Summary string - Sources []string - Generates []string - Status []string - Dir string - Vars Vars - Env Vars - Silent bool - Method string - Prefix string - IgnoreError bool `yaml:"ignore_error"` + Task string + Cmds []*Cmd + Deps []*Dep + Desc string + Summary string + Sources []string + Generates []string + Status []string + Precondition []*Precondition + Dir string + Vars Vars + Env Vars + Silent bool + Method string + Prefix string + IgnoreError bool `yaml:"ignore_error"` } diff --git a/internal/taskfile/version/version.go b/internal/taskfile/version/version.go index bb2176e4..b2776c31 100644 --- a/internal/taskfile/version/version.go +++ b/internal/taskfile/version/version.go @@ -10,6 +10,8 @@ var ( v21 = mustVersion("2.1") v22 = mustVersion("2.2") v23 = mustVersion("2.3") + v24 = mustVersion("2.4") + v25 = mustVersion("2.5") ) // IsV1 returns if is a given Taskfile version is version 1 @@ -37,6 +39,16 @@ func IsV23(v *semver.Constraints) bool { return v.Check(v23) } +// IsV24 returns if is a given Taskfile version is at least version 2.4 +func IsV24(v *semver.Constraints) bool { + return v.Check(v24) +} + +// IsV25 returns if is a given Taskfile version is at least version 2.5 +func IsV25(v *semver.Constraints) bool { + return v.Check(v25) +} + func mustVersion(s string) *semver.Version { v, err := semver.NewVersion(s) if err != nil { diff --git a/precondition.go b/precondition.go new file mode 100644 index 00000000..c8284ab4 --- /dev/null +++ b/precondition.go @@ -0,0 +1,44 @@ +// Package task provides ... +package task + +import ( + "context" + "errors" + + "github.com/go-task/task/v2/internal/execext" + "github.com/go-task/task/v2/internal/taskfile" +) + +var ( + // ErrNecessaryPreconditionFailed is returned when a precondition fails + ErrNecessaryPreconditionFailed = errors.New("task: precondition not met") + // ErrOptionalPreconditionFailed is returned when a precondition fails + // that has ignore_error set to true + ErrOptionalPreconditionFailed = errors.New("task: optional precondition not met") +) + +func (e *Executor) areTaskPreconditionsMet(ctx context.Context, t *taskfile.Task) (bool, error) { + var optionalPreconditionFailed bool + for _, p := range t.Precondition { + err := execext.RunCommand(ctx, &execext.RunCommandOptions{ + Command: p.Sh, + Dir: t.Dir, + Env: getEnviron(t), + }) + + if err != nil { + e.Logger.Outf(p.Msg) + if p.IgnoreError == true { + optionalPreconditionFailed = true + } else { + return false, ErrNecessaryPreconditionFailed + } + } + } + + if optionalPreconditionFailed == true { + return true, ErrOptionalPreconditionFailed + } + + return true, nil +} diff --git a/status.go b/status.go index 173be34b..7ebb5f84 100644 --- a/status.go +++ b/status.go @@ -78,8 +78,10 @@ func (e *Executor) isTaskUpToDateStatus(ctx context.Context, t *taskfile.Task) ( Env: getEnviron(t), }) if err != nil { + e.Logger.VerboseOutf("task: status command %s exited non-zero: %s", s, err) return false, nil } + e.Logger.VerboseOutf("task: status command %s exited zero", s) } return true, nil } diff --git a/task.go b/task.go index 2df08c2c..f0877f08 100644 --- a/task.go +++ b/task.go @@ -119,7 +119,7 @@ func (e *Executor) Setup() error { Vars: e.taskvars, Logger: e.Logger, } - case version.IsV2(v), version.IsV21(v), version.IsV22(v): + case version.IsV2(v), version.IsV21(v), version.IsV22(v), version.IsV23(v): e.Compiler = &compilerv2.CompilerV2{ Dir: e.Dir, Taskvars: e.taskvars, @@ -127,8 +127,9 @@ func (e *Executor) Setup() error { Expansions: e.Taskfile.Expansions, Logger: e.Logger, } - case version.IsV23(v): - return fmt.Errorf(`task: Taskfile versions greater than v2.3 not implemented in the version of Task`) + + case version.IsV24(v): + return fmt.Errorf(`task: Taskfile versions greater than v2.4 not implemented in the version of Task`) } if !version.IsV21(v) && e.Taskfile.Output != "" { @@ -192,7 +193,13 @@ func (e *Executor) RunTask(ctx context.Context, call taskfile.Call) error { if err != nil { return err } - if upToDate { + + preCondMet, err := e.areTaskPreconditionsMet(ctx, t) + if err != nil { + return err + } + + if upToDate && preCondMet { if !e.Silent { e.Logger.Errf(`task: Task "%s" is up to date`, t.Task) } @@ -224,7 +231,15 @@ func (e *Executor) runDeps(ctx context.Context, t *taskfile.Task) error { d := d g.Go(func() error { - return e.RunTask(ctx, taskfile.Call{Task: d.Task, Vars: d.Vars}) + err := e.RunTask(ctx, taskfile.Call{Task: d.Task, Vars: d.Vars}) + if err != nil { + if err == ErrOptionalPreconditionFailed { + e.Logger.Errf("%s", err) + } else { + return err + } + } + return nil }) } @@ -236,7 +251,15 @@ func (e *Executor) runCommand(ctx context.Context, t *taskfile.Task, call taskfi switch { case cmd.Task != "": - return e.RunTask(ctx, taskfile.Call{Task: cmd.Task, Vars: cmd.Vars}) + err := e.RunTask(ctx, taskfile.Call{Task: cmd.Task, Vars: cmd.Vars}) + if err != nil { + if err == ErrOptionalPreconditionFailed { + e.Logger.Errf("%s", err) + } else { + return err + } + } + return nil case cmd.Cmd != "": if e.Verbose || (!cmd.Silent && !t.Silent && !e.Silent) { e.Logger.Errf(cmd.Cmd) diff --git a/task_test.go b/task_test.go index e2278dc5..51dce8c8 100644 --- a/task_test.go +++ b/task_test.go @@ -273,6 +273,68 @@ func TestStatus(t *testing.T) { } } +func TestPrecondition(t *testing.T) { + const dir = "testdata/precondition" + + var buff bytes.Buffer + e := &task.Executor{ + Dir: dir, + Stdout: &buff, + Stderr: &buff, + Silent: false, + } + + // A precondition that has been met + assert.NoError(t, e.Setup()) + assert.NoError(t, e.Run(context.Background(), taskfile.Call{Task: "foo"})) + if buff.String() != "" { + t.Errorf("Got Output when none was expected: %s", buff.String()) + } + + // A precondition that was not met + assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "impossible"})) + + if buff.String() != "1 != 0\n" { + t.Errorf("Wrong output message: %s", buff.String()) + } + buff.Reset() + + // Calling a task with a precondition in a dependency fails the task + assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "depends_on_imposssible"})) + if buff.String() != "1 != 0\n" { + t.Errorf("Wrong output message: %s", buff.String()) + } + buff.Reset() + + // Calling a task with a precondition in a cmd fails the task + assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "executes_failing_task_as_cmd"})) + if buff.String() != "1 != 0\n" { + t.Errorf("Wrong output message: %s", buff.String()) + } + buff.Reset() + + // A task with a failing precondition and ignore_errors on still fails + assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "impossible_but_i_dont_care"})) + if buff.String() != "2 != 1\n" { + t.Errorf("Wrong output message: %s", buff.String()) + } + buff.Reset() + + // If a precondition has ignore errors, then it will allow _dependent_ tasks to execute + assert.NoError(t, e.Run(context.Background(), taskfile.Call{Task: "depends_on_failure_of_impossible"})) + if buff.String() != "2 != 1\ntask: optional precondition not met\n" { + t.Errorf("Wrong output message: %s", buff.String()) + } + buff.Reset() + + // If a precondition has ignore errors, then it will allow tasks calling it to execute + assert.NoError(t, e.Run(context.Background(), taskfile.Call{Task: "executes_failing_task_as_cmd_but_succeeds"})) + if buff.String() != "2 != 1\ntask: optional precondition not met\n" { + t.Errorf("Wrong output message: %s", buff.String()) + } + +} + func TestGenerates(t *testing.T) { const ( srcTask = "sub/src.txt" diff --git a/testdata/precondition/Taskfile.yml b/testdata/precondition/Taskfile.yml new file mode 100644 index 00000000..02b798ac --- /dev/null +++ b/testdata/precondition/Taskfile.yml @@ -0,0 +1,34 @@ +version: '2' + +tasks: + foo: + precondition: + - test -f foo.txt + + impossible: + precondition: + - sh: "[ 1 = 0 ]" + msg: "1 != 0" + + impossible_but_i_dont_care: + precondition: + - sh: "[ 2 = 1 ]" + msg: "2 != 1" + ignore_error: true + + depends_on_imposssible: + deps: + - impossible + + executes_failing_task_as_cmd: + cmds: + - task: impossible + + depends_on_failure_of_impossible: + deps: + - impossible_but_i_dont_care + + executes_failing_task_as_cmd_but_succeeds: + cmds: + - task: impossible_but_i_dont_care + diff --git a/testdata/precondition/foo.txt b/testdata/precondition/foo.txt new file mode 100644 index 00000000..e69de29b diff --git a/variables.go b/variables.go index 8e360073..0f3c7f96 100644 --- a/variables.go +++ b/variables.go @@ -73,6 +73,7 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { IgnoreError: cmd.IgnoreError, } } + } if len(origTask.Deps) > 0 { new.Deps = make([]*taskfile.Dep, len(origTask.Deps)) @@ -83,6 +84,16 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { } } } + if len(origTask.Precondition) > 0 { + new.Precondition = make([]*taskfile.Precondition, len(origTask.Precondition)) + for i, precond := range origTask.Precondition { + new.Precondition[i] = &taskfile.Precondition{ + Sh: r.Replace(precond.Sh), + Msg: r.Replace(precond.Msg), + IgnoreError: precond.IgnoreError, + } + } + } return &new, r.Err() } From 659cae6a4c93eb25e2b356801d09cd2feb4c82d7 Mon Sep 17 00:00:00 2001 From: Stephen Prater Date: Tue, 28 May 2019 12:28:29 -0700 Subject: [PATCH 2/6] Apply suggestions from code review Co-Authored-By: Andrey Nering --- docs/taskfile_versions.md | 4 ++-- internal/taskfile/task.go | 2 +- variables.go | 1 - 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/taskfile_versions.md b/docs/taskfile_versions.md index b56bd1ce..a318f12a 100644 --- a/docs/taskfile_versions.md +++ b/docs/taskfile_versions.md @@ -141,9 +141,9 @@ includes: docker: ./DockerTasks.yml ``` -## Version 2.3 +## Version 2.6 -Version 2.3 comes with `preconditions` stanza in tasks. +Version 2.6 comes with `preconditions` stanza in tasks. ```yaml version: '2' diff --git a/internal/taskfile/task.go b/internal/taskfile/task.go index 9c1cf3b3..bc4bf981 100644 --- a/internal/taskfile/task.go +++ b/internal/taskfile/task.go @@ -13,7 +13,7 @@ type Task struct { Sources []string Generates []string Status []string - Precondition []*Precondition + Preconditions []*Precondition Dir string Vars Vars Env Vars diff --git a/variables.go b/variables.go index 0f3c7f96..5af5f873 100644 --- a/variables.go +++ b/variables.go @@ -73,7 +73,6 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { IgnoreError: cmd.IgnoreError, } } - } if len(origTask.Deps) > 0 { new.Deps = make([]*taskfile.Dep, len(origTask.Deps)) From 044d3a0ff9a425241a2e930dab295e793e483ae7 Mon Sep 17 00:00:00 2001 From: Stephen Prater Date: Tue, 28 May 2019 13:02:59 -0700 Subject: [PATCH 3/6] Remove ignore_errors --- internal/taskfile/precondition.go | 32 +++++++++++--------------- internal/taskfile/precondition_test.go | 9 ++++---- precondition.go | 20 ++++------------ task.go | 18 +++++---------- task_test.go | 26 +++------------------ testdata/precondition/Taskfile.yml | 15 ++---------- variables.go | 14 +++++------ 7 files changed, 39 insertions(+), 95 deletions(-) diff --git a/internal/taskfile/precondition.go b/internal/taskfile/precondition.go index 04b116a5..554cdf12 100644 --- a/internal/taskfile/precondition.go +++ b/internal/taskfile/precondition.go @@ -12,9 +12,8 @@ var ( // Precondition represents a precondition necessary for a task to run type Precondition struct { - Sh string - Msg string - IgnoreError bool + Sh string + Msg string } // UnmarshalYAML implements yaml.Unmarshaler interface. @@ -24,28 +23,23 @@ func (p *Precondition) UnmarshalYAML(unmarshal func(interface{}) error) error { if err := unmarshal(&cmd); err == nil { p.Sh = cmd p.Msg = fmt.Sprintf("`%s` failed", cmd) - p.IgnoreError = false return nil } var sh struct { - Sh string - Msg string - IgnoreError bool `yaml:"ignore_error"` + Sh string + Msg string } - err := unmarshal(&sh) - - if err == nil { - p.Sh = sh.Sh - p.Msg = sh.Msg - if p.Msg == "" { - p.Msg = fmt.Sprintf("%s failed", sh.Sh) - } - - p.IgnoreError = sh.IgnoreError - return nil + if err := unmarshal(&sh); err != nil { + return err } - return err + p.Sh = sh.Sh + p.Msg = sh.Msg + if p.Msg == "" { + p.Msg = fmt.Sprintf("%s failed", sh.Sh) + } + + return nil } diff --git a/internal/taskfile/precondition_test.go b/internal/taskfile/precondition_test.go index acf89f27..799e9ac4 100644 --- a/internal/taskfile/precondition_test.go +++ b/internal/taskfile/precondition_test.go @@ -18,27 +18,26 @@ func TestPreconditionParse(t *testing.T) { { "test -f foo.txt", &taskfile.Precondition{}, - &taskfile.Precondition{Sh: `test -f foo.txt`, Msg: "`test -f foo.txt` failed", IgnoreError: false}, + &taskfile.Precondition{Sh: `test -f foo.txt`, Msg: "`test -f foo.txt` failed"}, }, { "sh: '[ 1 = 0 ]'", &taskfile.Precondition{}, - &taskfile.Precondition{Sh: "[ 1 = 0 ]", Msg: "[ 1 = 0 ] failed", IgnoreError: false}, + &taskfile.Precondition{Sh: "[ 1 = 0 ]", Msg: "[ 1 = 0 ] failed"}, }, {` sh: "[ 1 = 2 ]" msg: "1 is not 2" `, &taskfile.Precondition{}, - &taskfile.Precondition{Sh: "[ 1 = 2 ]", Msg: "1 is not 2", IgnoreError: false}, + &taskfile.Precondition{Sh: "[ 1 = 2 ]", Msg: "1 is not 2"}, }, {` sh: "[ 1 = 2 ]" msg: "1 is not 2" -ignore_error: true `, &taskfile.Precondition{}, - &taskfile.Precondition{Sh: "[ 1 = 2 ]", Msg: "1 is not 2", IgnoreError: true}, + &taskfile.Precondition{Sh: "[ 1 = 2 ]", Msg: "1 is not 2"}, }, } for _, test := range tests { diff --git a/precondition.go b/precondition.go index c8284ab4..115c19dd 100644 --- a/precondition.go +++ b/precondition.go @@ -10,16 +10,12 @@ import ( ) var ( - // ErrNecessaryPreconditionFailed is returned when a precondition fails - ErrNecessaryPreconditionFailed = errors.New("task: precondition not met") - // ErrOptionalPreconditionFailed is returned when a precondition fails - // that has ignore_error set to true - ErrOptionalPreconditionFailed = errors.New("task: optional precondition not met") + // ErrPreconditionFailed is returned when a precondition fails + ErrPreconditionFailed = errors.New("task: precondition not met") ) func (e *Executor) areTaskPreconditionsMet(ctx context.Context, t *taskfile.Task) (bool, error) { - var optionalPreconditionFailed bool - for _, p := range t.Precondition { + for _, p := range t.Preconditions { err := execext.RunCommand(ctx, &execext.RunCommandOptions{ Command: p.Sh, Dir: t.Dir, @@ -28,17 +24,9 @@ func (e *Executor) areTaskPreconditionsMet(ctx context.Context, t *taskfile.Task if err != nil { e.Logger.Outf(p.Msg) - if p.IgnoreError == true { - optionalPreconditionFailed = true - } else { - return false, ErrNecessaryPreconditionFailed - } + return false, ErrPreconditionFailed } } - if optionalPreconditionFailed == true { - return true, ErrOptionalPreconditionFailed - } - return true, nil } diff --git a/task.go b/task.go index f0877f08..608622f8 100644 --- a/task.go +++ b/task.go @@ -189,12 +189,12 @@ func (e *Executor) RunTask(ctx context.Context, call taskfile.Call) error { } if !e.Force { - upToDate, err := e.isTaskUpToDate(ctx, t) + preCondMet, err := e.areTaskPreconditionsMet(ctx, t) if err != nil { return err } - preCondMet, err := e.areTaskPreconditionsMet(ctx, t) + upToDate, err := e.isTaskUpToDate(ctx, t) if err != nil { return err } @@ -233,11 +233,8 @@ func (e *Executor) runDeps(ctx context.Context, t *taskfile.Task) error { g.Go(func() error { err := e.RunTask(ctx, taskfile.Call{Task: d.Task, Vars: d.Vars}) if err != nil { - if err == ErrOptionalPreconditionFailed { - e.Logger.Errf("%s", err) - } else { - return err - } + e.Logger.Errf("%s", err) + return err } return nil }) @@ -253,11 +250,8 @@ func (e *Executor) runCommand(ctx context.Context, t *taskfile.Task, call taskfi case cmd.Task != "": err := e.RunTask(ctx, taskfile.Call{Task: cmd.Task, Vars: cmd.Vars}) if err != nil { - if err == ErrOptionalPreconditionFailed { - e.Logger.Errf("%s", err) - } else { - return err - } + e.Logger.Errf("%s", err) + return err } return nil case cmd.Cmd != "": diff --git a/task_test.go b/task_test.go index 51dce8c8..05290fd5 100644 --- a/task_test.go +++ b/task_test.go @@ -301,38 +301,18 @@ func TestPrecondition(t *testing.T) { // Calling a task with a precondition in a dependency fails the task assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "depends_on_imposssible"})) - if buff.String() != "1 != 0\n" { + + if buff.String() != "1 != 0\ntask: precondition not met\n" { t.Errorf("Wrong output message: %s", buff.String()) } buff.Reset() // Calling a task with a precondition in a cmd fails the task assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "executes_failing_task_as_cmd"})) - if buff.String() != "1 != 0\n" { + if buff.String() != "1 != 0\ntask: precondition not met\n" { t.Errorf("Wrong output message: %s", buff.String()) } buff.Reset() - - // A task with a failing precondition and ignore_errors on still fails - assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "impossible_but_i_dont_care"})) - if buff.String() != "2 != 1\n" { - t.Errorf("Wrong output message: %s", buff.String()) - } - buff.Reset() - - // If a precondition has ignore errors, then it will allow _dependent_ tasks to execute - assert.NoError(t, e.Run(context.Background(), taskfile.Call{Task: "depends_on_failure_of_impossible"})) - if buff.String() != "2 != 1\ntask: optional precondition not met\n" { - t.Errorf("Wrong output message: %s", buff.String()) - } - buff.Reset() - - // If a precondition has ignore errors, then it will allow tasks calling it to execute - assert.NoError(t, e.Run(context.Background(), taskfile.Call{Task: "executes_failing_task_as_cmd_but_succeeds"})) - if buff.String() != "2 != 1\ntask: optional precondition not met\n" { - t.Errorf("Wrong output message: %s", buff.String()) - } - } func TestGenerates(t *testing.T) { diff --git a/testdata/precondition/Taskfile.yml b/testdata/precondition/Taskfile.yml index 02b798ac..b9f2c338 100644 --- a/testdata/precondition/Taskfile.yml +++ b/testdata/precondition/Taskfile.yml @@ -2,20 +2,14 @@ version: '2' tasks: foo: - precondition: + preconditions: - test -f foo.txt impossible: - precondition: + preconditions: - sh: "[ 1 = 0 ]" msg: "1 != 0" - impossible_but_i_dont_care: - precondition: - - sh: "[ 2 = 1 ]" - msg: "2 != 1" - ignore_error: true - depends_on_imposssible: deps: - impossible @@ -27,8 +21,3 @@ tasks: depends_on_failure_of_impossible: deps: - impossible_but_i_dont_care - - executes_failing_task_as_cmd_but_succeeds: - cmds: - - task: impossible_but_i_dont_care - diff --git a/variables.go b/variables.go index 5af5f873..9c227372 100644 --- a/variables.go +++ b/variables.go @@ -83,13 +83,13 @@ func (e *Executor) CompiledTask(call taskfile.Call) (*taskfile.Task, error) { } } } - if len(origTask.Precondition) > 0 { - new.Precondition = make([]*taskfile.Precondition, len(origTask.Precondition)) - for i, precond := range origTask.Precondition { - new.Precondition[i] = &taskfile.Precondition{ - Sh: r.Replace(precond.Sh), - Msg: r.Replace(precond.Msg), - IgnoreError: precond.IgnoreError, + + if len(origTask.Preconditions) > 0 { + new.Preconditions = make([]*taskfile.Precondition, len(origTask.Preconditions)) + for i, precond := range origTask.Preconditions { + new.Preconditions[i] = &taskfile.Precondition{ + Sh: r.Replace(precond.Sh), + Msg: r.Replace(precond.Msg), } } } From 12ab01d5e62528ae3923284921efed0a6d033856 Mon Sep 17 00:00:00 2001 From: Stephen Prater Date: Tue, 28 May 2019 13:16:37 -0700 Subject: [PATCH 4/6] Clarify difference between status and precondition in docs --- docs/usage.md | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index 27af026a..c43c5073 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -365,13 +365,16 @@ tasks: ``` Preconditions can set specific failure messages that can tell -a user what to do using the `msg` field. +a user what steps to take using the `msg` field. If a task has a dependency on a sub-task with a precondition, and that -precondition is not met - the calling task will fail. Adding `ignore_errors` -to the precondition will cause parent tasks to execute even if the sub task -can not run. Note that a task executed directly with a failing precondition -will not run unless `--force` is given. +precondition is not met - the calling task will fail. Note that a task +executed with a failing precondition will not run unless `--force` is +given. + +Unlike `status` which will skip a task if it is up to date, and continue +executing tasks that depenend on it a `precondition` will fail a task, along +with any other tasks that depend on it. ```yaml version: '2' @@ -379,16 +382,15 @@ tasks: task_will_fail: preconditions: - sh: "exit 1" - ignore_errors: true - task_will_succeed: + task_will_also_fail: deps: - task_will_fail - task_will_succeed: + task_will_still_fail: cmds: - task: task_will_fail - - echo "I will run" + - echo "I will not run" ``` ## Variables From d1463b3e242930cc0ae66397dafd40677d212592 Mon Sep 17 00:00:00 2001 From: Stephen Prater Date: Tue, 11 Jun 2019 11:46:22 -0700 Subject: [PATCH 5/6] Fix typos per review --- docs/usage.md | 14 +++++++------- internal/taskfile/precondition.go | 2 +- precondition.go | 3 +-- task.go | 2 -- task_test.go | 1 - 5 files changed, 9 insertions(+), 13 deletions(-) diff --git a/docs/usage.md b/docs/usage.md index c43c5073..e70cbc5a 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -346,7 +346,7 @@ the tasks are not up-to-date. If you need a certain set of conditions to be _true_ you can use the `preconditions` stanza. `preconditions` are very similar to `status` -lines except they support `sh` expansion and they SHOULD all return 0 +lines except they support `sh` expansion and they SHOULD all return 0. ```yaml version: '2' @@ -373,7 +373,7 @@ executed with a failing precondition will not run unless `--force` is given. Unlike `status` which will skip a task if it is up to date, and continue -executing tasks that depenend on it a `precondition` will fail a task, along +executing tasks that depend on it, a `precondition` will fail a task, along with any other tasks that depend on it. ```yaml @@ -384,13 +384,13 @@ tasks: - sh: "exit 1" task_will_also_fail: - deps: - - task_will_fail + deps: + - task_will_fail task_will_still_fail: - cmds: - - task: task_will_fail - - echo "I will not run" + cmds: + - task: task_will_fail + - echo "I will not run" ``` ## Variables diff --git a/internal/taskfile/precondition.go b/internal/taskfile/precondition.go index 554cdf12..04c1e532 100644 --- a/internal/taskfile/precondition.go +++ b/internal/taskfile/precondition.go @@ -7,7 +7,7 @@ import ( var ( // ErrCantUnmarshalPrecondition is returned for invalid precond YAML. - ErrCantUnmarshalPrecondition = errors.New("task: can't unmarshal precondition value") + ErrCantUnmarshalPrecondition = errors.New("task: Can't unmarshal precondition value") ) // Precondition represents a precondition necessary for a task to run diff --git a/precondition.go b/precondition.go index 115c19dd..3f2f066b 100644 --- a/precondition.go +++ b/precondition.go @@ -1,4 +1,3 @@ -// Package task provides ... package task import ( @@ -23,7 +22,7 @@ func (e *Executor) areTaskPreconditionsMet(ctx context.Context, t *taskfile.Task }) if err != nil { - e.Logger.Outf(p.Msg) + e.Logger.Outf("task: %s", p.Msg) return false, ErrPreconditionFailed } } diff --git a/task.go b/task.go index 608622f8..154166aa 100644 --- a/task.go +++ b/task.go @@ -233,7 +233,6 @@ func (e *Executor) runDeps(ctx context.Context, t *taskfile.Task) error { g.Go(func() error { err := e.RunTask(ctx, taskfile.Call{Task: d.Task, Vars: d.Vars}) if err != nil { - e.Logger.Errf("%s", err) return err } return nil @@ -250,7 +249,6 @@ func (e *Executor) runCommand(ctx context.Context, t *taskfile.Task, call taskfi case cmd.Task != "": err := e.RunTask(ctx, taskfile.Call{Task: cmd.Task, Vars: cmd.Vars}) if err != nil { - e.Logger.Errf("%s", err) return err } return nil diff --git a/task_test.go b/task_test.go index 05290fd5..79568abd 100644 --- a/task_test.go +++ b/task_test.go @@ -281,7 +281,6 @@ func TestPrecondition(t *testing.T) { Dir: dir, Stdout: &buff, Stderr: &buff, - Silent: false, } // A precondition that has been met From cc9264854e884d36f97f67cd28b5d8a68220c05f Mon Sep 17 00:00:00 2001 From: Stephen Prater Date: Tue, 11 Jun 2019 12:20:56 -0700 Subject: [PATCH 6/6] Change error output --- precondition.go | 2 +- task_test.go | 6 +++--- testdata/precondition/Taskfile.yml | 6 +----- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/precondition.go b/precondition.go index 3f2f066b..eff123a0 100644 --- a/precondition.go +++ b/precondition.go @@ -22,7 +22,7 @@ func (e *Executor) areTaskPreconditionsMet(ctx context.Context, t *taskfile.Task }) if err != nil { - e.Logger.Outf("task: %s", p.Msg) + e.Logger.Errf("task: %s", p.Msg) return false, ErrPreconditionFailed } } diff --git a/task_test.go b/task_test.go index 79568abd..d42325c2 100644 --- a/task_test.go +++ b/task_test.go @@ -293,7 +293,7 @@ func TestPrecondition(t *testing.T) { // A precondition that was not met assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "impossible"})) - if buff.String() != "1 != 0\n" { + if buff.String() != "task: 1 != 0 obviously!\n" { t.Errorf("Wrong output message: %s", buff.String()) } buff.Reset() @@ -301,14 +301,14 @@ func TestPrecondition(t *testing.T) { // Calling a task with a precondition in a dependency fails the task assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "depends_on_imposssible"})) - if buff.String() != "1 != 0\ntask: precondition not met\n" { + if buff.String() != "task: 1 != 0 obviously!\n" { t.Errorf("Wrong output message: %s", buff.String()) } buff.Reset() // Calling a task with a precondition in a cmd fails the task assert.Error(t, e.Run(context.Background(), taskfile.Call{Task: "executes_failing_task_as_cmd"})) - if buff.String() != "1 != 0\ntask: precondition not met\n" { + if buff.String() != "task: 1 != 0 obviously!\n" { t.Errorf("Wrong output message: %s", buff.String()) } buff.Reset() diff --git a/testdata/precondition/Taskfile.yml b/testdata/precondition/Taskfile.yml index b9f2c338..405140f7 100644 --- a/testdata/precondition/Taskfile.yml +++ b/testdata/precondition/Taskfile.yml @@ -8,7 +8,7 @@ tasks: impossible: preconditions: - sh: "[ 1 = 0 ]" - msg: "1 != 0" + msg: "1 != 0 obviously!" depends_on_imposssible: deps: @@ -17,7 +17,3 @@ tasks: executes_failing_task_as_cmd: cmds: - task: impossible - - depends_on_failure_of_impossible: - deps: - - impossible_but_i_dont_care