changed cyclic dep detection

since interpolation can be used, detection should be a execution time,
and not before

now, to prevent infinite execution, there's a miximum of 100 calls per
task

closes #37

cyclic.go

@@ -1,34 +0,0 @@
-package task
-
-// CheckCyclicDep checks if a task tree has any cyclic dependency
-func (e *Executor) CheckCyclicDep() error {
-	visits := make(map[string]struct{}, len(e.Tasks))
-
-	var checkCyclicDep func(string, *Task) error
-	checkCyclicDep = func(name string, t *Task) error {
-		if _, ok := visits[name]; ok {
-			return ErrCyclicDepDetected
-		}
-		visits[name] = struct{}{}
-		defer delete(visits, name)
-
-		for _, d := range t.Deps {
-			// FIXME: ignoring by now. should return an error instead?
-			task, ok := e.Tasks[d.Task]
-			if !ok {
-				continue
-			}
-			if err := checkCyclicDep(d.Task, task); err != nil {
-				return err
-			}
-		}
-		return nil
-	}
-
-	for k, v := range e.Tasks {
-		if err := checkCyclicDep(k, v); err != nil {
-			return err
-		}
-	}
-	return nil
-}

cyclic_test.go

@@ -1,56 +0,0 @@
-package task_test
-
-import (
-	"testing"
-
-	"github.com/go-task/task"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestCyclicDepCheck(t *testing.T) {
-	isCyclic := &task.Executor{
-		Tasks: task.Tasks{
-			"task-a": &task.Task{
-				Deps: []*task.Dep{&task.Dep{Task: "task-b"}},
-			},
-			"task-b": &task.Task{
-				Deps: []*task.Dep{&task.Dep{Task: "task-a"}},
-			},
-		},
-	}
-
-	assert.Equal(t, task.ErrCyclicDepDetected, isCyclic.CheckCyclicDep(), "task should be cyclic")
-
-	isNotCyclic := &task.Executor{
-		Tasks: task.Tasks{
-			"task-a": &task.Task{
-				Deps: []*task.Dep{&task.Dep{Task: "task-c"}},
-			},
-			"task-b": &task.Task{
-				Deps: []*task.Dep{&task.Dep{Task: "task-c"}},
-			},
-			"task-c": &task.Task{},
-		},
-	}
-
-	assert.NoError(t, isNotCyclic.CheckCyclicDep())
-
-	inexixtentTask := &task.Executor{
-		Tasks: task.Tasks{
-			"task-a": &task.Task{
-				Deps: []*task.Dep{&task.Dep{Task: "invalid-task"}},
-			},
-		},
-	}
-
-	// FIXME: by now Task should ignore non existent tasks
-	// in the future we should improve the detection of
-	// tasks called with interpolation?
-	//     task:
-	//       deps:
-	//         - task: "task{{.VARIABLE}}"
-	//       vars:
-	//         VARIABLE: something
-	assert.NoError(t, inexixtentTask.CheckCyclicDep())
-}

errors.go

@@ -6,8 +6,6 @@ import (
 )
 
 var (
-	// ErrCyclicDepDetected is returned when a cyclic dependency was found in the Taskfile
-	ErrCyclicDepDetected = errors.New("task: cyclic dependency detected")
 	// ErrTaskfileAlreadyExists is returned on creating a Taskfile if one already exists
 	ErrTaskfileAlreadyExists = errors.New("task: A Taskfile already exists")
 )
@@ -61,3 +59,18 @@ type dynamicVarError struct {
 func (err *dynamicVarError) Error() string {
 	return fmt.Sprintf(`task: Command "%s" in taskvars file failed: %s`, err.cmd, err.cause)
 }
+
+// MaximumTaskCallExceededError is returned when a task is called too
+// many times. In this case you probably have a cyclic dependendy or
+// infinite loop
+type MaximumTaskCallExceededError struct {
+	task string
+}
+
+func (e *MaximumTaskCallExceededError) Error() string {
+	return fmt.Sprintf(
+		`task: maximum task call exceeded (%d) for task "%s": probably an cyclic dep or infinite loop`,
+		MaximumTaskCall,
+		e.task,
+	)
+}

task.go

@@ -9,6 +9,7 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 
 	"github.com/go-task/task/execext"
 
@@ -18,6 +19,9 @@ import (
 const (
 	// TaskFilePath is the default Taskfile
 	TaskFilePath = "Taskfile"
+	// MaximumTaskCall is the max number of times a task can be called.
+	// This exists to prevent infinite loops on cyclic dependencies
+	MaximumTaskCall = 100
 )
 
 // Executor executes a Taskfile
@@ -57,14 +61,12 @@ type Task struct {
 	Vars      Vars
 	Set       string
 	Env       Vars
+
+	callCount int32
 }
 
 // Run runs Task
 func (e *Executor) Run(args ...string) error {
-	if err := e.CheckCyclicDep(); err != nil {
-		return err
-	}
-
 	if e.Stdin == nil {
 		e.Stdin = os.Stdin
 	}
@@ -110,6 +112,10 @@ func (e *Executor) RunTask(ctx context.Context, name string, vars Vars) error {
 		return &taskNotFoundError{name}
 	}
 
+	if atomic.AddInt32(&t.callCount, 1) >= MaximumTaskCall {
+		return &MaximumTaskCallExceededError{task: name}
+	}
+
 	if err := e.runDeps(ctx, name, vars); err != nil {
 		return err
 	}

task_test.go

@@ -199,3 +199,15 @@ func TestParams(t *testing.T) {
 		assert.Equal(t, f.content, string(content))
 	}
 }
+
+func TestCyclicDep(t *testing.T) {
+	const dir = "testdata/cyclic"
+
+	e := task.Executor{
+		Dir:    dir,
+		Stdout: ioutil.Discard,
+		Stderr: ioutil.Discard,
+	}
+	assert.NoError(t, e.ReadTaskfile())
+	assert.IsType(t, &task.MaximumTaskCallExceededError{}, e.Run("task-1"))
+}

testdata/cyclic/Taskfile.yml

@@ -0,0 +1,7 @@
+task-1:
+  deps:
+    - task: task-2
+
+task-2:
+  deps:
+    - task: task-1