Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add global precondition #1993

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion precondition.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
var ErrPreconditionFailed = errors.New("task: precondition not met")

func (e *Executor) areTaskPreconditionsMet(ctx context.Context, t *ast.Task) (bool, error) {
for _, p := range t.Preconditions {
for _, p := range append(t.Preconditions, e.Taskfile.Preconditions.Preconditions...) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect the global preconditions to run first. For example:

version: '3'

preconditions:
  - test -f foo.txt

tasks:
  default:
    preconditions:
      - test -f bar.txt
    cmds:
      - echo "default task"

Assuming that neither file exists, I would expect to see

task: `test -f foo.txt` failed
task: precondition not met

Also prefer slices.Concat over append and ... now that its in the std lib.

Suggested change
for _, p := range append(t.Preconditions, e.Taskfile.Preconditions.Preconditions...) {
for _, p := range slices.Concat(e.Taskfile.Preconditions.Preconditions, t.Preconditions) {

err := execext.RunCommand(ctx, &execext.RunCommandOptions{
Command: p.Sh,
Dir: t.Dir,
Expand Down
46 changes: 44 additions & 2 deletions task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,10 +443,10 @@ func TestStatus(t *testing.T) {
buff.Reset()
}

func TestPrecondition(t *testing.T) {
func TestPreconditionLocal(t *testing.T) {
t.Parallel()

const dir = "testdata/precondition"
const dir = "testdata/precondition/local"

var buff bytes.Buffer
e := &task.Executor{
Expand Down Expand Up @@ -486,6 +486,48 @@ func TestPrecondition(t *testing.T) {
buff.Reset()
}

func TestPreconditionGlobal(t *testing.T) {
t.Parallel()

var buff bytes.Buffer
e := &task.Executor{
Dir: "testdata/precondition/global",
Stdout: &buff,
Stderr: &buff,
}

require.NoError(t, e.Setup())

// A global precondition that was not met
require.Error(t, e.Run(context.Background(), &ast.Call{Task: "impossible"}))

if buff.String() != "task: 1 != 0 obviously!\n" {
t.Errorf("Wrong output message: %s", buff.String())
}
buff.Reset()

e = &task.Executor{
Dir: "testdata/precondition/global/with_local",
Stdout: &buff,
Stderr: &buff,
}

require.NoError(t, e.Setup())

// A global precondition that was met
require.NoError(t, e.Run(context.Background(), &ast.Call{Task: "foo"}))
if buff.String() != "" {
t.Errorf("Got Output when none was expected: %s", buff.String())
}

// A local precondition that was not met
require.Error(t, e.Run(context.Background(), &ast.Call{Task: "impossible"}))

if buff.String() != "task: 1 != 0 obviously!\n" {
t.Errorf("Wrong output message: %s", buff.String())
}
}

func TestGenerates(t *testing.T) {
t.Parallel()

Expand Down
62 changes: 57 additions & 5 deletions taskfile/ast/precondition.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,56 @@ package ast

import (
"fmt"

"gopkg.in/yaml.v3"
"sync"

"github.com/go-task/task/v3/errors"
"github.com/go-task/task/v3/internal/deepcopy"

"gopkg.in/yaml.v3"
)

// Precondition represents a precondition necessary for a task to run
type Precondition struct {
Sh string
Msg string
type (
Preconditions struct {
Preconditions []*Precondition
mutex sync.RWMutex
}

Precondition struct {
Sh string
Msg string
}
Comment on lines +20 to +23
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved Vars and Var into separate files in another PR. Would be good to follow this pattern here.

)

func NewPreconditions() *Preconditions {
return &Preconditions{
Preconditions: make([]*Precondition, 0),
}
}

func (p *Preconditions) DeepCopy() *Preconditions {
if p == nil {
return nil
}
defer p.mutex.RUnlock()
p.mutex.RLock()
return &Preconditions{
Preconditions: deepcopy.Slice(p.Preconditions),
}
}

func (p *Preconditions) Merge(other *Preconditions) {
if p == nil || p.Preconditions == nil || other == nil {
return
}

p.mutex.Lock()
defer p.mutex.Unlock()

other.mutex.RLock()
defer other.mutex.RUnlock()

p.Preconditions = append(p.Preconditions, deepcopy.Slice(other.Preconditions)...)
}

func (p *Precondition) DeepCopy() *Precondition {
Expand Down Expand Up @@ -55,3 +95,15 @@ func (p *Precondition) UnmarshalYAML(node *yaml.Node) error {

return errors.NewTaskfileDecodeError(nil, node).WithTypeMessage("precondition")
}

func (p *Preconditions) UnmarshalYAML(node *yaml.Node) error {
if p == nil || p.Preconditions == nil {
*p = *NewPreconditions()
}

if err := node.Decode(&p.Preconditions); err != nil {
return errors.NewTaskfileDecodeError(err, node).WithTypeMessage("preconditions")
}

return nil
}
64 changes: 37 additions & 27 deletions taskfile/ast/taskfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,21 @@ var ErrIncludedTaskfilesCantHaveDotenvs = errors.New("task: Included Taskfiles c

// Taskfile is the abstract syntax tree for a Taskfile
type Taskfile struct {
Location string
Version *semver.Version
Output Output
Method string
Includes *Includes
Set []string
Shopt []string
Vars *Vars
Env *Vars
Tasks *Tasks
Silent bool
Dotenv []string
Run string
Interval time.Duration
Location string
Version *semver.Version
Output Output
Method string
Includes *Includes
Set []string
Shopt []string
Vars *Vars
Env *Vars
Preconditions *Preconditions
Tasks *Tasks
Silent bool
Dotenv []string
Run string
Interval time.Duration
}

// Merge merges the second Taskfile into the first
Expand All @@ -59,28 +60,33 @@ func (t1 *Taskfile) Merge(t2 *Taskfile, include *Include) error {
if t1.Tasks == nil {
t1.Tasks = NewTasks()
}
if t1.Preconditions == nil {
t1.Preconditions = NewPreconditions()
}
t1.Vars.Merge(t2.Vars, include)
t1.Env.Merge(t2.Env, include)
t1.Preconditions.Merge(t2.Preconditions)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we don't have scoping, this is going to lead to issues where global preconditions in included Taskfiles are going to impact tasks in parent files. I think this is going to be pretty unintuitive to users at the moment. For example:

# Taskfile.yml
version: '3'

includes:
  include: include.yml

tasks:
  default:
    cmds:
      - echo "default task"
# include.yml
version: '3'

preconditions:
  - test -f foo.txt
task: `test -f foo.txt` failed
task: precondition not met

We could:

  1. Leave as it is, vars already behaves like this. We'll fix it later 🤷‍♂️
  2. Not allow global preconditions in included taskfiles for now
  3. Not merge this functionality until scoping is ready

I think 1 is pretty reckless. We're essentially intentionally adding behaviour that we know is confusing and will raise questions.

3 is very conservative while 2 is a good compromise. It depends how urgently we want to roll out this functionality. If we don't think its urgent then maybe we wait. The question is how long will scoping actually take? I've already taken a stab at it and I can tell you - it's significant piece of work.

return t1.Tasks.Merge(t2.Tasks, include, t1.Vars)
}

func (tf *Taskfile) UnmarshalYAML(node *yaml.Node) error {
switch node.Kind {
case yaml.MappingNode:
var taskfile struct {
Version *semver.Version
Output Output
Method string
Includes *Includes
Set []string
Shopt []string
Vars *Vars
Env *Vars
Tasks *Tasks
Silent bool
Dotenv []string
Run string
Interval time.Duration
Version *semver.Version
Output Output
Method string
Includes *Includes
Preconditions *Preconditions
Set []string
Shopt []string
Vars *Vars
Env *Vars
Tasks *Tasks
Silent bool
Dotenv []string
Run string
Interval time.Duration
}
if err := node.Decode(&taskfile); err != nil {
return errors.NewTaskfileDecodeError(err, node)
Expand All @@ -98,6 +104,7 @@ func (tf *Taskfile) UnmarshalYAML(node *yaml.Node) error {
tf.Dotenv = taskfile.Dotenv
tf.Run = taskfile.Run
tf.Interval = taskfile.Interval
tf.Preconditions = taskfile.Preconditions
if tf.Includes == nil {
tf.Includes = NewIncludes()
}
Expand All @@ -110,6 +117,9 @@ func (tf *Taskfile) UnmarshalYAML(node *yaml.Node) error {
if tf.Tasks == nil {
tf.Tasks = NewTasks()
}
if tf.Preconditions == nil {
tf.Preconditions = NewPreconditions()
}
return nil
}

Expand Down
9 changes: 9 additions & 0 deletions testdata/precondition/global/Taskfile.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
version: '3'

preconditions:
- sh: "[ 1 = 0 ]"
msg: "1 != 0 obviously!"

tasks:
impossible:
cmd: echo "won't run"
12 changes: 12 additions & 0 deletions testdata/precondition/global/with_local/Taskfile.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
version: '3'

preconditions:
- test -f foo.txt

tasks:
foo:

impossible:
preconditions:
- sh: "[ 1 = 0 ]"
msg: "1 != 0 obviously!"
Empty file.
15 changes: 15 additions & 0 deletions website/docs/usage.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -1019,6 +1019,21 @@ tasks:
- echo "I will not run"
```

They can be defined at two levels:

- Global Level: Applies to all tasks.
- Task Level: Applies only to a specific task.

```yaml
version: '3'

preconditions:
- sh: 'exit 1'

tasks:
task-will-fail: echo "I will not run"
```

### Limiting when tasks run

If a task executed by multiple `cmds` or multiple `deps` you can control when it
Expand Down
7 changes: 7 additions & 0 deletions website/static/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,13 @@
"description": "A set of global environment variables.",
"$ref": "#/definitions/env"
},
"preconditions": {
"description": "A list of commands to check if any task should run. If a condition is not met, the task will return an error.",
"type": "array",
"items": {
"$ref": "#/definitions/precondition"
}
},
"tasks": {
"description": "A set of task definitions.",
"$ref": "#/definitions/tasks"
Expand Down
Loading