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.

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

Allow setting kevent_flags on struct sigevent #1731

Merged
merged 2 commits into from Aug 27, 2023

Conversation

asomers
Copy link
Member

@asomers asomers commented Jun 4, 2022

@asomers
Copy link
Member Author

asomers commented Jun 4, 2022

Back when I originally defined struct sigevent, Rust could not represent C unions. So I cheated and defined it as a struct, replacing the union field with what I thought would be its most important member. But now I need to use a different union member. So in the linked libc PR I defined sigevent the correct way, using unions. This PR both fixes the field names and adds a backwards-compatible way to set the new field.
A search through every crate on crates.io shows that Nix was the only currently-maintained crate using the union-as-struct field. So I think we can handle the backwards-compatibility issues in libc by merging the libc PR, then quickly releasing patch releases of Nix.

@asomers asomers force-pushed the eov_oneshot branch 2 times, most recently from 255a6f6 to e83418e Compare June 4, 2022 16:11
@rtzoeller
Copy link
Collaborator

rtzoeller commented Jun 5, 2022

So I think we can handle the backwards-compatibility issues in libc by merging the libc PR, then quickly releasing patch releases of Nix.

This seems reasonable. A few procedural questions:

  • How far back to we want to patch?

    I pulled the download numbers for each recent minor version over the last 14 days:

    Version Downloads in last 14 days
    0.18 18143
    0.19 40880
    0.20 71998
    0.21 21651
    0.22 146464
    0.23 274245
    0.24 160180

    IMO this supports patching back to 0.22, and only back to 0.20 by request.

  • Should we combine this with the existing plans for a 0.24.2 patch, or address this subsequently as a 0.24.3?

@rtzoeller rtzoeller marked this pull request as ready for review June 9, 2022 04:33
@rtzoeller rtzoeller marked this pull request as draft June 9, 2022 04:33
@asomers asomers force-pushed the eov_oneshot branch 2 times, most recently from 48cec20 to 262d5d9 Compare August 11, 2023 21:29
@asomers
Copy link
Member Author

asomers commented Aug 11, 2023

I've rebased. But so far, the libc maintainers still haven't merged the PR there. Trying to be both backwards-compatible AND Rust 1.13.0 compatible AND satisfy that project's style checker is very difficult. If they refuse to merge it, I'm thinking about just copying the FFI definitions into Nix, so we won't be blocked anymore. What do you think of that, @rtzoeller ?

@asomers
Copy link
Member Author

asomers commented Aug 20, 2023

Ok, I pushed a version that inlines the C structures into Nix. But only for FreeBSD, because that's the only platform that really needs this change.

@asomers asomers marked this pull request as ready for review August 26, 2023 19:34
Also, disallow using SigevNotify::SigevThreadId on musl.  I don't think
it ever worked.

Depends on rust-lang/libc#2813
Blocks tokio-rs/tokio#4728
Because the PR to libc is stalled for over one year, with no sign of
progress.
@asomers
Copy link
Member Author

asomers commented Aug 26, 2023

Still no word from upstream. I'll merge this now. Once the upstream PR merges, we can revert the 2nd commit.

@asomers asomers added this pull request to the merge queue Aug 26, 2023
Merged via the queue into nix-rust:master with commit cc73638 Aug 27, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants