mage: cancel context on SIGINT

[pmcatominey]

On receiving an interrupt signal, mage cancels the context allowing the magefile to perform any cleanup before exiting.

A second interrupt signal will kill the magefile process without delay.

The behaviour for a timeout remains unchanged (context is canclled and the magefile exits).

Fixes #269, #266.

[pmcatominey]

Updated to switch test from SIGINFO to SIGHUP as the former does not exist on linux/amd64.

[flowchartsman]

You may want to consider holding off on this until signal.NotifyContext lands: https://github.com/golang/go/issues/37255

[natefinch]

Sorry for the late review.

re: signal.NotifyContext .... I don't want mage to require a new version of Go, because I know the pain of being stuck on an older version. I think we can do fine with the knobs we have.

This PR looks pretty good, but I think we're missing a synchronization effort... we should wait for the targets to clean up, and right now there's no mechanism for that. So, like, if you put a defer in a target that takes a little time to run, it may not always have time to run before mage exits out. I think we need a waitgroup and a timeout... like, first time we get sigint, cancel the context and start waiting for people to clean up. Maybe wait up to 5 seconds. If the user hits ctrl-c again, skip the timeout and just exit.

so something like this:

$ mage runlongthing
^C cancelling mage targets, waiting up to 5 seconds for cleanup...
^C exiting mage
$ 

[pmcatominey]

@natefinch thanks for the review.

I've updated the code to apply a timeout after a SIGINT and tests to cover this, aswell as testing that a deferred function call within a target will be ran during during cleanup.

I'm not sure where a WaitGroup would be useful?

[faiq]

Hi can we get some movement on this PR @natefinch? This feature would be killer for us. Please let me know how I can help make it happen. Thank you

[natefinch]

Thanks for pinging me on this, @faiq ... I'll have to re-review the code and work on the merge conflicts, but it seems like it should be able to be merged soon. I'll try to poke at this some tonight.