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

Adding --max-runs flag to control number of times a task should run before termination + minor bug fixes #1332

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
12 changes: 12 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# Changelog

## v3.31.0 - 2023-09-14
MohamedAbdeen21 marked this conversation as resolved.
Show resolved Hide resolved

- Added a `--max-runs` flag, which sets the maximum number of times a task
should run before being considered an infinite loop or a cyclic dep,
and therefore killed (#1321 by @bogwro).
- Added a test task in `testdata/for` and test case to test `--max-runs`
behavior.
- Fixed a bug where `TaskCalledTooManyTimes` error always shows 0 as number
of exceeded max runs.
- Fixed a bug where a task will be one run short from the number specified
by`MaximumTaskCall`.

## v3.30.0 - 2023-09-13

- Prep work for Remote Taskfiles (#1316 by @pd93).
Expand Down
7 changes: 7 additions & 0 deletions cmd/task/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ var flags struct {
experiments bool
download bool
offline bool
maxRuns int
}

func main() {
Expand Down Expand Up @@ -135,6 +136,7 @@ func run() error {
pflag.DurationVarP(&flags.interval, "interval", "I", 0, "Interval to watch for changes.")
pflag.BoolVarP(&flags.global, "global", "g", false, "Runs global Taskfile, from $HOME/{T,t}askfile.{yml,yaml}.")
pflag.BoolVar(&flags.experiments, "experiments", false, "Lists all the available experiments and whether or not they are enabled.")
pflag.IntVar(&flags.maxRuns, "max-runs", task.MaximumTaskCall, "Set the maximum number of runs per task before being considered infinte or cyclic and therefore terminated.")
Copy link
Member

@pd93 pd93 Sep 14, 2023

Choose a reason for hiding this comment

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

Not requesting any changes to this now. Just opening this up for thoughts and opinions as naming is always hard. Some other possibilities:

  • --limit - Nice and short, but might be too generic (could clash with something like include-limit if we wanted it)
  • --call-limit
  • --max-task-calls - Very explicit, but longer. I don't think length is a huge issue with these flags as they're most likely to be used in CI or scripts.

Copy link
Author

Choose a reason for hiding this comment

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

Also, I'd like to shorten the description. I couldn't find more concise (yet complete) description.


// Gentle force experiment will override the force flag and add a new force-all flag
if experiments.GentleForce {
Expand Down Expand Up @@ -227,6 +229,10 @@ func run() error {
taskSorter = &sort.AlphaNumeric{}
}

if flags.maxRuns < 1 {
return errors.New("task: You can't set --max-runs to less than 1")
}

e := task.Executor{
Force: flags.force,
ForceAll: flags.forceAll,
Expand All @@ -245,6 +251,7 @@ func run() error {
Color: flags.color,
Concurrency: flags.concurrency,
Interval: flags.interval,
MaxRuns: flags.maxRuns,

Stdin: os.Stdin,
Stdout: os.Stdout,
Expand Down
10 changes: 10 additions & 0 deletions docs/docs/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,16 @@ sidebar_position: 14

# Changelog

## v3.31.0 - 2023-09-14

- Added a `--max-runs` flag, which sets the maximum number of times a task
should run before being considered an infinite loop or a cyclic dep,
and therefore killed ([#1321](https://github.com/go-task/task/issues/1321) by [@bogwro](https://github.com/bogwro)).
- Added a test task in `testdata/for` and a test case in `task_test` to test `--max-runs` behavior.
- Fixed a bug where `TaskCalledTooManyTimes` error always shows 0 as number
of exceeded max runs.
- Fixed a bug where a task will be one run short from the number specified by`MaximumTaskCall`.

MohamedAbdeen21 marked this conversation as resolved.
Show resolved Hide resolved
## v3.30.0 - 2023-09-13

- Prep work for Remote Taskfiles ([#1316](https://github.com/go-task/task/issues/1316) by [@pd93](https://github.com/pd93)).
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@go-task/cli",
"version": "3.30.0",
"version": "3.31.0",
MohamedAbdeen21 marked this conversation as resolved.
Show resolved Hide resolved
"description": "A task runner / simpler Make alternative written in Go",
"scripts": {
"postinstall": "go-npm install",
Expand Down
13 changes: 10 additions & 3 deletions task.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ import (

const (
// MaximumTaskCall is the max number of times a task can be called.
// This exists to prevent infinite loops on cyclic dependencies
// This exists to prevent infinite loops on cyclic dependencies.
// Used as the default value if max-runs flag is not set.
MaximumTaskCall = 100
)

Expand Down Expand Up @@ -64,6 +65,7 @@ type Executor struct {
Color bool
Concurrency int
Interval time.Duration
MaxRuns int
AssumesTerm bool

Stdin io.Reader
Expand Down Expand Up @@ -127,6 +129,11 @@ func (e *Executor) Run(ctx context.Context, calls ...taskfile.Call) error {
return e.watchTasks(calls...)
}

// if executor wasn't created through CLI, (i.e. for testing)
if e.MaxRuns == 0 {
e.MaxRuns = MaximumTaskCall
}

g, ctx := errgroup.WithContext(ctx)
for _, c := range calls {
c := c
Expand All @@ -147,8 +154,8 @@ func (e *Executor) RunTask(ctx context.Context, call taskfile.Call) error {
if err != nil {
return err
}
if !e.Watch && atomic.AddInt32(e.taskCallCount[t.Task], 1) >= MaximumTaskCall {
return &errors.TaskCalledTooManyTimesError{TaskName: t.Task}
if !e.Watch && atomic.AddInt32(e.taskCallCount[t.Task], 1) > int32(e.MaxRuns) {
return &errors.TaskCalledTooManyTimesError{TaskName: t.Task, MaximumTaskCall: e.MaxRuns}
}

release := e.acquireConcurrencyLimit()
Expand Down
29 changes: 29 additions & 0 deletions task_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2289,3 +2289,32 @@ func TestFor(t *testing.T) {
})
}
}

func TestTooManyRuns(t *testing.T) {
tests := []struct {
name string
expectedError string
}{
{
name: "loop-too-many",
expectedError: `task: Failed to run task "loop-too-many": task: Maximum task call exceeded (4) for task "looped-task": probably an cyclic dep or infinite loop`,
},
}

for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
var buff bytes.Buffer
e := task.Executor{
Dir: "testdata/for",
Stdout: &buff,
Stderr: &buff,
Silent: true,
Force: true,
MaxRuns: 4, // task contains 5 loops
}

require.NoError(t, e.Setup())
assert.EqualError(t, e.Run(context.Background(), taskfile.Call{Task: test.name, Direct: true}), test.expectedError)
})
}
}
10 changes: 10 additions & 0 deletions testdata/for/Taskfile.yml
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ tasks:
var: FOO
task: task-{{.ITEM}}

loop-too-many:
vars:
FOO: foo.txt foo.txt foo.txt foo.txt foo.txt
cmds:
- for:
var: FOO
task: looped-task
vars:
FILE: "{{.ITEM}}"

looped-task:
internal: true
cmd: cat "{{.FILE}}"
Expand Down