upbound / up

The @upbound CLI
Apache License 2.0
52 stars 41 forks source link

Update pterm so it doesn't intercept signals #528

Closed adamwg closed 4 months ago

adamwg commented 4 months ago

Description of your changes

In older versions, the pterm library we use for pretty-printing intercepted SIGINT and SIGTERM. This signal handling was set up in the pacakge's init function, so any code that ran before we set up our own signal handler couldn't be killed with Ctrl-C.

The signal handling was removed from pterm in a more recent version (see [1]). Updating to the latest fixes the issue where we couldn't kill up login with Ctrl-C (#526).

[1] https://github.com/pterm/pterm/issues/562

Fixes #526

Signed-off-by: Adam Wolfe Gordon adam.wolfegordon@upbound.io

I have:

How has this code been tested

adamwg commented 4 months ago

Question: with this change, do we still need the signal handler we set up in main.go? All it does is call os.Exit (via kongCtx.Exit), and looking at git history I suspect it was introduced to work around the fact that pterm was intercepting our signals.

tnthornton commented 4 months ago

Question: with this change, do we still need the signal handler we set up in main.go? All it does is call os.Exit (via kongCtx.Exit), and looking at git history I suspect it was introduced to work around the fact that pterm was intercepting our signals.

My guess is no, but it should be easy enough to test your changes with the up space init flow to double check.

adamwg commented 4 months ago

Confirmed that with our signal handler removed up space init can still be stopped with Ctrl-C. Added a commit to remove our signal handler, since it's not necessary.