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

Add struct sock_txtime and related flags #2415

Merged
merged 1 commit into from
Oct 12, 2021
Merged

Add struct sock_txtime and related flags #2415

merged 1 commit into from
Oct 12, 2021

Conversation

ghedo
Copy link
Contributor

@ghedo ghedo commented Sep 21, 2021

These are needed to use the SO_TXTIME socket option on Linux, which is
already exposed.

@rust-highfive
Copy link

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

Please see the contribution instructions for more information.

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Sep 24, 2021

📌 Commit a09bc47 has been approved by JohnTitor

bors added a commit that referenced this pull request Sep 24, 2021
Add struct sock_txtime and related flags

These are needed to use the SO_TXTIME socket option on Linux, which is
already exposed.
@bors
Copy link
Contributor

bors commented Sep 24, 2021

⌛ Testing commit a09bc47 with merge fb6e1e5...

@bors
Copy link
Contributor

bors commented Sep 24, 2021

💔 Test failed - checks-actions

@ghedo
Copy link
Contributor Author

ghedo commented Sep 24, 2021

Just realized I didn't add the new things to libc-test/semver/linux.txt, not sure if that would explain the failure on mips-musl though...

@bors
Copy link
Contributor

bors commented Sep 24, 2021

☔ The latest upstream changes (presumably #2403) made this pull request unmergeable. Please resolve the merge conflicts.

@JohnTitor
Copy link
Member

The semver check is unrelated, something on MIPS is different.

@ghedo
Copy link
Contributor Author

ghedo commented Sep 27, 2021

According to https://github.com/rust-lang/libc/blob/master/ci/docker/mips-unknown-linux-musl/Dockerfile#L9, it looks like the mips+musl CI builds use Linux 4.14, but sock_txtime was added in 4.19, so I put the new struct and constants behind #[cfg(not(all(target_env = "musl", target_arch = "mips")))] which I think should fix the problem for those builds.

Does this seem reasonable @JohnTitor?

@ghedo ghedo requested a review from JohnTitor September 29, 2021 08:24
Copy link
Member

@JohnTitor JohnTitor left a comment

Choose a reason for hiding this comment

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

Thanks!

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 6, 2021

📌 Commit 0fa63b4 has been approved by JohnTitor

bors added a commit that referenced this pull request Oct 6, 2021
Add struct sock_txtime and related flags

These are needed to use the SO_TXTIME socket option on Linux, which is
already exposed.
@bors
Copy link
Contributor

bors commented Oct 6, 2021

⌛ Testing commit 0fa63b4 with merge 65f906d...

@bors
Copy link
Contributor

bors commented Oct 6, 2021

💔 Test failed - checks-actions

@ghedo
Copy link
Contributor Author

ghedo commented Oct 7, 2021

Ugh, that didn't work either. I managed to run the mips+musl build locally, and apparently what is needed is to wrap the struct and consts in a cfg_if! block instead of just #cfg[...] (there are a few other similar cases in the code AFAICT). But that means that I also had to remove the struct and consts from the semver list, otherwise those tests would fail for mips+musl builds.

The Semver macOS failure seems unrelated though (master also fails that currently).

Hopefully this will work now. Please have a look again when you have time @JohnTitor.

@JohnTitor
Copy link
Member

Thanks for investigating, let's try again :)
@bors r+

@bors
Copy link
Contributor

bors commented Oct 10, 2021

📌 Commit 2c8a498 has been approved by JohnTitor

@bors
Copy link
Contributor

bors commented Oct 10, 2021

⌛ Testing commit 2c8a498 with merge aa7ab90...

bors added a commit that referenced this pull request Oct 10, 2021
Add struct sock_txtime and related flags

These are needed to use the SO_TXTIME socket option on Linux, which is
already exposed.
@bors
Copy link
Contributor

bors commented Oct 10, 2021

💔 Test failed - checks-actions

These are needed to use the SO_TXTIME socket option on Linux, which is
already exposed.
@ghedo
Copy link
Contributor Author

ghedo commented Oct 11, 2021

Ok, so, looks like this latest failure was my fault, as I didn't realize I still had to wrap the struct in s_no_extra_traits!, sorry about that.

This should be fixed now, so yet again, hopefully this will work now 🤞 please try again when you geta chance @JohnTitor (and thanks for being patient).

@JohnTitor
Copy link
Member

👍, @bors r+

@bors
Copy link
Contributor

bors commented Oct 11, 2021

📌 Commit 07e5721 has been approved by JohnTitor

@bors
Copy link
Contributor

bors commented Oct 12, 2021

⌛ Testing commit 07e5721 with merge 032d105...

@bors
Copy link
Contributor

bors commented Oct 12, 2021

☀️ Test successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Approved by: JohnTitor
Pushing 032d105 to master...

@bors bors merged commit 032d105 into rust-lang:master Oct 12, 2021
@ghedo ghedo mentioned this pull request Oct 12, 2021
bors added a commit that referenced this pull request Oct 16, 2021
Bump version to 0.2.104

I'd like to get #2415 into a release so I can use the new API in a change I'm working on for the `nix` crate, please.
bors added a commit that referenced this pull request May 5, 2022
Enable sock_txtime on mips musl target

The struct and related constants were originally added in #2415. But they weren't enabled for mips musl target because the kernel version of the build image was old and they couldn't pass the build. Now the kernel version of the build image is already updated and I think we could enable them for mips musl target
bors added a commit that referenced this pull request May 5, 2022
Enable sock_txtime on mips musl target

The struct and related constants were originally added in #2415. But they weren't enabled for mips musl target because the kernel version of the build image was old and they couldn't pass the build. Now the kernel version of the build image is already updated and I think we could enable them for mips musl target
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants