Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Only use SIGTSTP on appropriate OSes #255

Merged
merged 6 commits into from
Oct 13, 2023
Merged

Conversation

kpcraig
Copy link
Contributor

@kpcraig kpcraig commented Oct 12, 2023

In #251, we added os.Kill and syscall.SIGTSTP to the terminal interrupts we handled when terminating the listener. Unfortunately, syscall.SIGTSTP is not defined on all systems (notably, Windows).

The simplest way to fix this is to use build constraints to separate the SIGTSTP-having operating systems and the non-SIGTSTP systems.

It also seems like there isn't an obvious alternative to SIGTSTP in Windows (if anyone knows...), so we just cut the behavior in the windows version.

@kpcraig kpcraig changed the title Only use SIGTSTP on systems that have it Only use SIGTSTP on appropriate OSes Oct 12, 2023
Copy link
Contributor

@maxcoulombe maxcoulombe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks you! Sorry for not catching that in the initial PR.

Pobably worth a changelog entry for a 1.17.2 release.

@kpcraig kpcraig merged commit 402c01d into main Oct 13, 2023
@kpcraig kpcraig deleted the VAULT-20861/sigtstp-but-windows branch October 13, 2023 00:06
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants