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.

Sign up for GitHub

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

Jump to bottom

Target is launched which should not be launched #371

Open
whiterm opened this issue Sep 24, 2021 · 2 comments · May be fixed by #373
Open

Target is launched which should not be launched #371

whiterm opened this issue Sep 24, 2021 · 2 comments · May be fixed by #373
Labels
🐜 bug

Comments

@whiterm
Copy link

whiterm commented Sep 24, 2021

To reproduce this bug, it was necessary to run go run mage.go test4 several times, because this error is floating

// +build mage

package main

import (
	"fmt"

	"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 {
	mg.Deps(Test2)
	fmt.Println("execute test3")
	return nil
}
func Test4() error {
	mg.Deps(Test1, Test2, Test3)
	fmt.Println("execute test4")
	return nil
}

Output:

execute test1
execute test3
Error: error test1
error test1
exit status 1

test3 target is not expected to run
@natefinch
Copy link
Member

Ooh crap, I think I see the problem. If test1 runs and errors out, it's still marked as "run", so test3 sees that it doesn't need to run test1 and assumes it had succeeded. Instead we need to mark test1 as a failure so that if anyone else tries to run it, they fail out.

@whiterm
Copy link
Author

whiterm commented Sep 27, 2021

I think you are right. I just changed the test code a little bit. And now the bug can be reproduced at every run

// +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
}

a-palchikov added a commit to a-palchikov/mage that referenced this issue Sep 27, 2021
@a-palchikov
Persist the panick a dependency had previously failed with to be able
ce4e6c7 
to relay this as non-once result if dependencies are tested
simulataneously.

Fixes magefile#371.
a-palchikov added a commit to a-palchikov/mage that referenced this issue Sep 27, 2021
@a-palchikov
Persist the panick a dependency might have failed with so it can be
bf0c474 
replayed in case a parent depencency is tested simultaneously.

Fixes magefile#371.
a-palchikov added a commit to a-palchikov/mage that referenced this issue Sep 27, 2021
@a-palchikov
Persist the fact that a dependency might have failed and replay it each
14708d6 
time (*onceFun).run is invoked.

Fixes magefile#371.
@a-palchikov a-palchikov linked a pull request Sep 27, 2021 that will close this issue
Open
a-palchikov added a commit to a-palchikov/mage that referenced this issue Oct 4, 2021
@a-palchikov
Persist the fact that a dependency might have failed and replay it each
0e9c134 
time (*onceFun).run is invoked.

Fixes magefile#371.
a-palchikov added a commit to a-palchikov/mage that referenced this issue Feb 17, 2022
@a-palchikov
Persist the fact that a dependency might have failed and replay it each
2859224 
time (*onceFun).run is invoked.

Fixes magefile#371.
a-palchikov added a commit to a-palchikov/mage that referenced this issue Apr 20, 2022
@a-palchikov
Persist the fact that a dependency might have failed and replay it each
fb6efb3 
time (*onceFun).run is invoked.

Fixes magefile#371.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
🐜 bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix dependency error propagation by replaying the panic a-palchikov/mage
2 participants
@natefinch @whiterm