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

[gix-tempfile] Revisit connection to signal-hook #339

Closed
Byron opened this issue Feb 20, 2022 · 0 comments · Fixed by #762
Closed

[gix-tempfile] Revisit connection to signal-hook #339

Byron opened this issue Feb 20, 2022 · 0 comments · Fixed by #762
Labels
acknowledged an issue is accepted as shortcoming to be fixed

Comments

@Byron
Copy link
Member

Byron commented Feb 20, 2022 •
edited
Loading

#336 was a reminder on how good intentions can end up having undesirable impact which is why it should be put into question if the bundling with signal-hook is the right choice for a plumbing crate.

Despite the ecosystem being centered around signal-hook, it seems clear that other options are possible that shouldn't have to deal with an unnecessary dependency.

It also appears that more complex applications will want their own signal handlers and free reign on how to integrate the cleanup, for reference, git-repository also integrates the cleanup handler directly instead of calling setup().

Revisit the way the cleanup handler is promoted to users of the library to make it reasonably easy to use while avoiding accidental behaviour to emerge.
@Byron Byron added the acknowledged an issue is accepted as shortcoming to be fixed label Nov 19, 2022
@Byron Byron changed the title [git-tempfile] Revisit connection to signal-hook [gix-tempfile] Revisit connection to signal-hook Feb 26, 2023
Byron added a commit that referenced this issue Feb 26, 2023
@Byron
feat!: gate all signal handling behind the signals feature toggle. (#…
441f5ac 
…339)

This change also consolidates all signal handling into its own module called
`signal` to provide reusable handlers and as well as well as signal initialization.

Note that the functions to cleanup tempfiles don't interact with the signal registry,
hence they still can be called without the `signals` feature enabled.

Note that this change sneakily fixes a bug that could have caused a `write_all()`
on a tempfile that was removed by a signal to enter an infinite loop.
@Byron Byron linked a pull request Feb 26, 2023 that will close this issue
tempfile upgrade #762
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
acknowledged an issue is accepted as shortcoming to be fixed
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

tempfile upgrade GitoxideLabs/gitoxide
1 participant
@Byron