Persist the fact that a dependency might have failed and replay it each

time (*onceFun).run is invoked.

Fixes #371.

Author: a-palchikov

mage/main_test.go

@@ -1615,6 +1615,35 @@ func TestWrongDependency(t *testing.T) {
 	}
 }
 
+// See https://github.com/magefile/mage/issues/371
+func TestRacyDependency(t *testing.T) {
+	var stdout, stderr bytes.Buffer
+	inv := Invocation{
+		Dir:    "./testdata/racy_deps",
+		Args:   []string{"test4"},
+		Stderr: &stderr,
+		Stdout: &stdout,
+	}
+	code := Invoke(inv)
+	if code != 1 {
+		t.Fatalf("expected 1, but got %v", code)
+	}
+	expectedStderr := `Error: error test1
+error test1
+error test1
+`
+	actualStderr := stderr.String()
+	if actualStderr != expectedStderr {
+		t.Fatalf("expected %q, but got %q", expectedStderr, actualStderr)
+	}
+	expectedStdout := `execute test1
+`
+	actualStdout := stdout.String()
+	if actualStdout != expectedStdout {
+		t.Fatalf("expected %q, but got %q", expectedStdout, actualStdout)
+	}
+}
+
 /// This code liberally borrowed from https://github.com/rsc/goversion/blob/master/version/exe.go
 
 type (

mage/testdata/racy_deps/magefile.go

@@ -0,0 +1,33 @@
+// +build mage
+
+package main
+
+import (
+	"fmt"
+	"time"
+
+	"github.com/magefile/mage/mg"
+)
+
+func Test1() error {
+	fmt.Println("execute test1")
+	return fmt.Errorf("error test1")
+}
+
+func Test2() error {
+	mg.Deps(Test1)
+	fmt.Println("execute test2")
+	return nil
+}
+
+func Test3() error {
+	time.Sleep(time.Second)
+	mg.Deps(Test2)
+	fmt.Println("execute test3")
+	return nil
+}
+func Test4() error {
+	mg.Deps(Test1, Test2, Test3)
+	fmt.Println("execute test4")
+	return nil
+}

mg/deps.go

@@ -2,7 +2,6 @@ package mg
 
 import (
 	"context"
-	"fmt"
 	"log"
 	"os"
 	"reflect"
@@ -36,7 +35,6 @@ func (o *onceMap) LoadOrStore(f Fn) *onceFun {
 		return existing
 	}
 	one := &onceFun{
-		once:        &sync.Once{},
 		fn:          f,
 		displayName: displayName(f.Name()),
 	}
@@ -91,37 +89,44 @@ func CtxDeps(ctx context.Context, fns ...interface{}) {
 
 // runDeps assumes you've already called checkFns.
 func runDeps(ctx context.Context, fns []Fn) {
-	mu := &sync.Mutex{}
-	var errs []string
-	var exit int
-	wg := &sync.WaitGroup{}
+	resultCh := make(chan error, len(fns))
+	var wg sync.WaitGroup
+	wg.Add(len(fns))
 	for _, f := range fns {
 		fn := onces.LoadOrStore(f)
-		wg.Add(1)
 		go func() {
 			defer func() {
 				if v := recover(); v != nil {
-					mu.Lock()
-					if err, ok := v.(error); ok {
-						exit = changeExit(exit, ExitStatus(err))
-					} else {
-						exit = changeExit(exit, 1)
+					var err error
+					var ok bool
+					if err, ok = v.(error); !ok {
+						err = Fatal(1)
+					}
+					select {
+					case <-ctx.Done():
+					case resultCh <- err:
 					}
-					errs = append(errs, fmt.Sprint(v))
-					mu.Unlock()
 				}
 				wg.Done()
 			}()
-			if err := fn.run(ctx); err != nil {
-				mu.Lock()
-				errs = append(errs, fmt.Sprint(err))
-				exit = changeExit(exit, ExitStatus(err))
-				mu.Unlock()
+			err := fn.run(ctx)
+			select {
+			case <-ctx.Done():
+			case resultCh <- err:
 			}
 		}()
 	}
 
 	wg.Wait()
+	close(resultCh)
+	var errs []string
+	var exit int
+	for err := range resultCh {
+		if err != nil {
+			errs = append(errs, err.Error())
+			exit = changeExit(exit, ExitStatus(err))
+		}
+	}
 	if len(errs) > 0 {
 		panic(Fatal(exit, strings.Join(errs, "\n")))
 	}
@@ -184,9 +189,12 @@ func displayName(name string) string {
 }
 
 type onceFun struct {
-	once *sync.Once
+	once sync.Once
 	fn   Fn
 	err  error
+	// depPanicked references the panic a dependency had previously
+	// failed with if set
+	depPanicked interface{}
 
 	displayName string
 }
@@ -195,10 +203,19 @@ type onceFun struct {
 // the same error output.
 func (o *onceFun) run(ctx context.Context) error {
 	o.once.Do(func() {
+		defer func() {
+			if v := recover(); v != nil {
+				o.depPanicked = v
+				panic(v)
+			}
+		}()
 		if Verbose() {
 			logger.Println("Running dependency:", displayName(o.fn.Name()))
 		}
 		o.err = o.fn.Run(ctx)
 	})
+	if o.depPanicked != nil {
+		panic(o.depPanicked)
+	}
 	return o.err
 }