Skip to content

Add struct sock_txtime and related flags#2415

Merged
bors merged 1 commit into
rust-lang:masterfrom
ghedo:sock_txtime
Oct 12, 2021
Merged

Add struct sock_txtime and related flags#2415
bors merged 1 commit into
rust-lang:masterfrom
ghedo:sock_txtime

Conversation

@ghedo

@ghedo ghedo commented Sep 21, 2021

Copy link
Copy Markdown
Contributor

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

@rust-highfive

rust-highfive commented Sep 21, 2021

Copy link
Copy Markdown

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

JohnTitor commented Sep 24, 2021

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Sep 24, 2021

Copy link
Copy Markdown
Contributor

📌 Commit a09bc47 has been approved by JohnTitor

bors added a commit that referenced this pull request Sep 24, 2021
@bors
Auto merge of #2415 - ghedo:sock_txtime, r=JohnTitor
fb6e1e5 
Add struct sock_txtime and related flags

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

bors commented Sep 24, 2021

Copy link
Copy Markdown
Contributor

⌛ Testing commit a09bc47 with merge fb6e1e5...

@bors

bors commented Sep 24, 2021

Copy link
Copy Markdown
Contributor

💔 Test failed - checks-actions

@ghedo

ghedo commented Sep 24, 2021

Copy link
Copy Markdown
Contributor Author

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

bors commented Sep 24, 2021

Copy link
Copy Markdown
Contributor

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

@JohnTitor

JohnTitor commented Sep 26, 2021

Copy link
Copy Markdown
Member

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

@ghedo

ghedo commented Sep 27, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

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
JohnTitor
JohnTitor approved these changes Oct 6, 2021
View reviewed changes

@JohnTitor JohnTitor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@JohnTitor

JohnTitor commented Oct 6, 2021

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Oct 6, 2021

Copy link
Copy Markdown
Contributor

📌 Commit 0fa63b4 has been approved by JohnTitor

bors added a commit that referenced this pull request Oct 6, 2021
@bors
Auto merge of #2415 - ghedo:sock_txtime, r=JohnTitor
65f906d 
Add struct sock_txtime and related flags

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

bors commented Oct 6, 2021

Copy link
Copy Markdown
Contributor

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

@bors

bors commented Oct 6, 2021

Copy link
Copy Markdown
Contributor

💔 Test failed - checks-actions

@ghedo

ghedo commented Oct 7, 2021

Copy link
Copy Markdown
Contributor Author

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

JohnTitor commented Oct 10, 2021

Copy link
Copy Markdown
Member

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

@bors

bors commented Oct 10, 2021

Copy link
Copy Markdown
Contributor

📌 Commit 2c8a498 has been approved by JohnTitor

@bors

bors commented Oct 10, 2021

Copy link
Copy Markdown
Contributor

⌛ Testing commit 2c8a498 with merge aa7ab90...

bors added a commit that referenced this pull request Oct 10, 2021
@bors
Auto merge of #2415 - ghedo:sock_txtime, r=JohnTitor
aa7ab90 
Add struct sock_txtime and related flags

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

bors commented Oct 10, 2021

Copy link
Copy Markdown
Contributor

💔 Test failed - checks-actions

@ghedo
Add struct sock_txtime and related flags
07e5721 
These are needed to use the SO_TXTIME socket option on Linux, which is
already exposed.
@ghedo

ghedo commented Oct 11, 2021

Copy link
Copy Markdown
Contributor Author

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

JohnTitor commented Oct 11, 2021

Copy link
Copy Markdown
Member

👍, @bors r+

@bors

bors commented Oct 11, 2021

Copy link
Copy Markdown
Contributor

📌 Commit 07e5721 has been approved by JohnTitor

@bors

bors commented Oct 12, 2021

Copy link
Copy Markdown
Contributor

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

@bors

bors commented Oct 12, 2021

Copy link
Copy Markdown
Contributor

☀️ 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
Merged
bors added a commit that referenced this pull request Oct 16, 2021
@bors
Auto merge of #2443 - ghedo:release-0.2.104, r=JohnTitor
20e5295 
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.
@cww0614 cww0614 mentioned this pull request May 2, 2022
Merged
bors added a commit that referenced this pull request May 5, 2022
@bors
Auto merge of #2776 - cww0614:socks_txtime_mips_musl, r=Amanieu
277fdd9 
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
@bors
Auto merge of #2776 - cww0614:socks_txtime_mips_musl, r=Amanieu
7b5afd2 
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

Reviewers

@JohnTitor JohnTitor JohnTitor approved these changes

Assignees

@JohnTitor JohnTitor

Labels

S-waiting-on-bors

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ghedo @rust-highfive @JohnTitor @bors