Skip to content

Commit

Permalink
Persist the panick a dependency had previously failed with to be able
Browse files Browse the repository at this point in the history
to relay this as non-once result if dependencies are tested
simulataneously.

Fixes magefile#371.
  • Loading branch information
a-palchikov committed Sep 27, 2021
1 parent d9e2e41 commit ce4e6c7
Showing 1 changed file with 37 additions and 20 deletions.
57 changes: 37 additions & 20 deletions mg/deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package mg

import (
"context"
"fmt"
"log"
"os"
"reflect"
Expand Down Expand Up @@ -36,7 +35,6 @@ func (o *onceMap) LoadOrStore(f Fn) *onceFun {
return existing
}
one := &onceFun{
once: &sync.Once{},
fn: f,
displayName: displayName(f.Name()),
}
Expand Down Expand Up @@ -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 {

This comment has been minimized.

Copy link
@gpeddle

gpeddle Oct 22, 2021

Why not use a short variable declaration here?

if ok, err := v.error(error); !ok { ... }

This comment has been minimized.

Copy link
@a-palchikov

a-palchikov Oct 22, 2021

Author Owner

This will not work. I'm assuming you meant this:

if err, ok := v.(error); !ok { ... }

The reason this does not use scoped local variable is that if the panic's value is an instance of error, this error vallue is used as a result (see chan send below). With your suggestion, there would be no variable, so it would not even compile.
Sorry if my understanding of your code is not correct.

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")))
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}

0 comments on commit ce4e6c7

Please sign in to comment.