Move uclibc under linux #1975

skrap · 2020-11-12T18:31:47Z

This is a first cut at moving uclibc under linux, alongside musl and gnu.

All tests pass on a vexpress a9 running in qemu. I am working on testing the other tier 3 uclibc targets: mips-unknown-linux-uclibc and mipsel-unknown-linux-uclibc. ~~Not having access to these platforms, I am working on getting them running in qemu, but it may take a bit.~~

The style check doesn't pass. I would appreciate some direction here. Many of these constants are defined under 2-of-3 out of musl/glibc/uclibc, so I'm not sure whether I should transform:

#[cfg(not(target_env = "uclibc"))]
pub fn foo();

into 2 separate declarations, one each in musl/mod.rs and gnu/mod.rs or use a cfg_if block for each one (which explodes 2 lines into 5).

Help me choose which fix to use for the items defined in musl and gnu but not uclibc.

It's my guess that exploding into the cfg_if block is better for long-term maintainability, but I'd really appreciate opinions from the maintainers.

rust-highfive · 2020-11-12T18:31:50Z

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.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

skrap · 2020-11-13T19:08:53Z

as of the last round of changes, the mipsel-unknown-linux-uclibc target works. The code doesn't seem to distinguish between that and mips-unknown-linux-uclibc so I am going to declare victory over mips+uclibc and await review comments!

skrap · 2020-11-20T18:26:58Z

@kolapapa I see you have other PRs related to uclibc. if you have a real mips or mipsel uclibc system, and don't mind trying to compile this branch for your use case, I would love any feedback. I definitely don't want to break anyone's code!

kolapapa · 2020-11-22T17:30:49Z

@kolapapa I see you have other PRs related to uclibc. if you have a real mips or mipsel uclibc system, and don't mind trying to compile this branch for your use case, I would love any feedback. I definitely don't want to break anyone's code!

Of course, how can I cooperate with you? I have the real system of mips and the target uses mipsel-unknown-linux-uclibc.

skrap · 2020-11-22T21:32:57Z

Of course, how can I cooperate with you? I have the real system of mips and the target uses mipsel-unknown-linux-uclibc.

Would you mind running your code against this branch? I'd be interested in any compilation issues, and definitely any runtime issues you see.

kolapapa · 2020-11-23T03:18:19Z

Would you mind running your code against this branch? I'd be interested in any compilation issues, and definitely any runtime issues you see.

At the moment, it doesn't seem to be a problem.

https://docs.rs/dns-lookup/1.0.5/dns_lookup/fn.getnameinfo.html
I used linux-like as uclibc. It seems that this crate needs to submit a pr.

skrap · 2020-11-23T13:35:11Z

Hmmm, so uclibc defines flags as a uint in <netdb.h>, like so:

extern int getnameinfo (const struct sockaddr *__restrict __sa,
			socklen_t __salen, char *__restrict __host,
			socklen_t __hostlen, char *__restrict __serv,
			socklen_t __servlen, unsigned int __flags);

However this seems to make uclibc the odd one out in terms of defining this parameter, and since there are dependent crates which require the type to be signed, we can just make libc pretend that it's a signed int. Since it's a flags field it won't really matter.

I pushed a change to this branch to address this. Would you mind checking again?

kolapapa · 2020-11-23T14:04:25Z

Hmmm, so uclibc defines flags as a uint in <netdb.h>, like so:
extern int getnameinfo (const struct sockaddr *__restrict __sa,
			socklen_t __salen, char *__restrict __host,
			socklen_t __hostlen, char *__restrict __serv,
			socklen_t __servlen, unsigned int __flags);
However this seems to make uclibc the odd one out in terms of defining this parameter, and since there are dependent crates which require the type to be signed, we can just make libc pretend that it's a signed int. Since it's a flags field it won't really matter.

I pushed a change to this branch to address this. Would you mind checking again?

So far everything is normal.
I used these crates

clap = { version = "2.33.0", features = ["yaml"] }
tokio = { version = "0.2", features = ["full"] }
reqwest = { version = "0.10.4", features = ["json", "blocking"] }
serde = { version = "1.0", features = ["derive"] }
serde_json = "1.0"
regex = "1.3"
telnet = "0.1.4"
glob = "0.3.0"
lazy_static = "1.4.0"
log = "0.4"
pretty_env_logger = "0.4.0"
anyhow = "1.0.27"
crossbeam-queue = "0.2.1"
config = "0.10.1"
futures = "0.3.4"
trust-dns-resolver = "0.19.4"
url = "2.1.1"
dns-lookup = "1.0.5"

skrap · 2020-11-23T15:17:25Z

@kolapapa Great news. That's super helpful, thanks!

bors · 2020-11-25T04:06:42Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

skrap · 2020-11-25T16:14:43Z

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

jyn514 · 2020-11-30T19:38:09Z

Since this is mostly moving code around, this should make the PR a lot easier to review: rust-lang/rust#76268 (comment)

JohnTitor · 2020-11-30T19:58:15Z

(Note that we haven't built any consensus on moving yet.)

skrap · 2020-12-03T17:28:28Z

(Note that we haven't built any consensus on moving yet.)

Hello @JohnTitor! I realize you are quite busy. I wanted to reach out and ask if there is a way that I can help move this conversation forward. I know it's a big change. It's almost entirely out of the tier-1/2 space, and I will have the ability to maintain this area of the code long-term. It would allow crates like nix to operate atop uclibc, which isn't really possible today. I would also be happy to do the work to build some CI tests around this work as well.

JohnTitor · 2020-12-03T19:37:27Z

So, generally, I'd avoid moving the code without a strong reason to keep git-history consistent. If this PR can simplify our codebase and structure, however, I'm happy to accept it. In this case, most changes look fine but I'd like to:

Use cfg_if! as possible to avoid adding #[cfg(not(target_env = "uclibc"))] to each item.
I'd see CI stuff first if you could set up for it, to ensure everything isn't broken.
Squash commits into appropriate numbers (~5 or so) as the current will hurt our history (don't have to do right now but we should do before merging).

skrap · 2020-12-03T20:51:16Z

Thanks much for the reply! That all makes sense, and I'll work on what you suggested.

For CI, there are no official builds of uclibc targets. I think this means that CI will have to build a full rust toolchain plus spin up an image for use within qemu. Do I have that right? Is that amount of CI CPU acceptable?

jyn514 · 2020-12-03T20:58:24Z

@skrap instead of building a self-hosted rustc (which like you mentioned will take forever), can you cross-compile with -Z build-std? That should have reasonable compile times.

skrap · 2020-12-03T21:00:15Z

I will try that!

skrap · 2020-12-03T22:33:00Z

@skrap instead of building a self-hosted rustc (which like you mentioned will take forever), can you cross-compile with -Z build-std? That should have reasonable compile times.

@jyn514 Sadly I don't think that's going to work. There's a chicken & egg problem here:

$ cargo +nightly run -Z build-std --target mips-unknown-linux-uclibc
[...]
error[E0425]: cannot find function `dl_iterate_phdr` in crate `libc`
   --> /home/skrap/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/std/src/../../backtrace/src/symbolize/gimli.rs:378:23
    |
378 |                 libc::dl_iterate_phdr(Some(callback), &mut ret as *mut Vec<_> as *mut _);
    |                       ^^^^^^^^^^^^^^^ not found in `libc`

So, nightly rust can't build-std against uclibc because libc doesn't build against uclibc. This commit would fix that, but only once nightly rust starts using some libc build which includes this change.

There's seemingly no way to get build-std to use a patch-like mechanism, sadly.

jyn514 · 2020-12-03T22:41:30Z

Hmm, you shouldn't need all of libstd, just core - and actually, looking at https://github.com/rust-lang/libc/blob/master/src/lib.rs#L19-L23 you might not even need core. If you remove -Z build-std and add --features rustc-dep-of-std does it work?

skrap · 2020-12-03T22:50:53Z

Sadly no, but I suspect I'm doing it wrong:

skrap@pop-os:/tmp/foo$ cargo +nightly run --features rustc-dep-of-std --target mips-unknown-linux-uclibc
error: Package `foo v0.1.0 (/tmp/foo)` does not have the feature `rustc-dep-of-std`

jyn514 · 2020-12-03T23:11:53Z

@skrap I would rather not debug this on github, the 10 minute back and forth is pretty painful. Feel free to hop on the discord in #contribute or Zulip and I'm happy to help out.

skrap · 2020-12-04T19:56:44Z

So it seems that since rustup does not currently provide nightlies for any of the uclibc targets, I don't see any way to get this into CI without building a full rust in CI. -Z build-std doesn't help, as it does not allow patching of the deps tree, and ends up trying to build libc 0.2.79, which doesn't compile for any uclibc target.

I'm out of answers for how to proceed on CI, so I will work on the other tasks. Once this change lands in nightly, and libc can once again build against uclibc, I will take up the CI issue again.

(Unless someone has ideas for a different workaround.)

JohnTitor · 2020-12-04T20:02:25Z

So, is it difficult to fix a libc build (and bump its version on rust-lang/rust) first then move items? If it's some simple work, I think it's one of the good ways. If not, then I'm fine to review/merge this one.

skrap · 2020-12-04T21:34:08Z

Ok, after some consulting with @jyn514 and others I was informed that I could build libc via build-std! So I'll work on using that to have CI at least build libc. I think it's probably worth it to have the tests run in CI as well but that will have to come later, once libc-test can build against nightly rust.

skrap · 2020-12-05T01:23:12Z

Todo still:

Use cfg_if! as possible to avoid adding #[cfg(not(target_env = "uclibc"))] to each item.
Separate commit for CI first, via cargo +nightly build --target mipsel-unknown-linux-uclibc -Z build-std=core --no-default-features or similar
Squash commits into appropriate numbers (~5 or so)

bors · 2020-12-10T20:48:45Z

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

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

skrap · 2021-01-13T19:01:47Z

I have made a commit to fix the l4re build, but I am not able to test it. It's correct to the best of my knowledge, but hopefully someone from L4Re will speak up. The L4Re target is tier 2, and unfortunately there's no CI set up for running the libc-test tests.

@bors try

Move uclibc under linux This is a first cut at moving uclibc under linux, alongside `musl` and `gnu`. All tests pass on a vexpress a9 running in qemu. I am working on testing the other tier 3 uclibc targets: `mips-unknown-linux-uclibc` and `mipsel-unknown-linux-uclibc`. ~Not having access to these platforms, I am working on getting them running in qemu, but it may take a bit.~ The style check doesn't pass. I would appreciate some direction here. Many of these constants are defined under 2-of-3 out of musl/glibc/uclibc, so I'm not sure whether I should transform: ```rust #[cfg(not(target_env = "uclibc"))] pub fn foo(); ``` into 2 separate declarations, one each in `musl/mod.rs` and `gnu/mod.rs` or use a `cfg_if` block for each one (which explodes 2 lines into 5). - [x] Help me choose which fix to use for the items defined in musl and gnu but not uclibc. It's my guess that exploding into the `cfg_if` block is better for long-term maintainability, but I'd really appreciate opinions from the maintainers.

bors · 2021-01-13T19:01:56Z

⌛ Trying commit 10693b4 with merge c887363...

bors · 2021-01-13T20:01:59Z

☀️ Try build successful - checks-actions, checks-cirrus-freebsd-11, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13
Build commit: c887363 (c8873635701ee21f614ad0fc5dfbdae8a1c1ec37)

humenda · 2021-01-14T06:17:37Z

Thanks for reaching out. I will have a llok at the weekend, but unfortunately I won't make it earlier.

humenda · 2021-01-16T09:20:16Z

I took some time and read the changes. Unfortunately, I am not in a position to test the changes, ATM. L4Re uses a fork of Uclibc, tha has, to my knowledge, not been updated for a while. There's a high chance that assumptions on your uclibc version do not hold. I only have an old snapshot around (2 years old) and lack the time to check out what L4Re is doing these days. I would suggest that you just continue and we hand in a PR in case something breaks in a newer version of L4Re.

skrap · 2021-01-16T14:25:04Z

Thanks @humenda. @JohnTitor sounds like the work here is complete!

…mbols

… and fns.

…ntations

JohnTitor · 2021-02-13T03:59:12Z

Oh sorry, missed the ping. Seems uclibc target doesn't have std component and we cannot run semver check for it (https://github.com/rust-lang/libc/runs/1697567233). So 9aad8ac is now meaningless, r=me once it's dropped.

JohnTitor · 2021-02-13T21:16:14Z

Thanks!
@bors r+

bors · 2021-02-13T21:16:16Z

📌 Commit 3378f0c has been approved by JohnTitor

bors · 2021-02-13T21:16:23Z

⌛ Testing commit 3378f0c with merge d4080d1...

bors · 2021-02-13T22:21:47Z

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

skrap · 2021-02-14T15:53:58Z

Thank you! All of your time reviewing and shepherding this change was very much appreciated, and I am looking forward to seeing this change in production rust! 🥳

rust-highfive assigned JohnTitor Nov 12, 2020

rust-highfive added the S-waiting-on-review label Nov 12, 2020

skrap force-pushed the feature/move-uclibc-under-linux branch from aafc9be to 1e25976 Compare November 25, 2020 16:14

skrap force-pushed the feature/move-uclibc-under-linux branch from 3c5226f to 10693b4 Compare January 13, 2021 18:28

skrap added 6 commits February 12, 2021 20:41

Add ENOTSUP constant for uclibc

c4803a6

uclibc -> linux-like/linux: mostly mechanical removal of redundant sy…

fe8470b

…mbols

uclibc -> linux-like/linux: Manual consolitation of redundant structs…

10c5418

… and fns.

change getnameinfo flags type to int to conform to other libc impleme…

7775ce2

…ntations

uclibc: style fixes

0ac1028

move EPOLLEXCLUSIVE and EPOLLWAKEUP to linux/mod.rs

225363b

skrap force-pushed the feature/move-uclibc-under-linux branch from 10693b4 to cd5c1e7 Compare February 13, 2021 01:50

skrap added 2 commits February 13, 2021 15:04

uclibc: fix x86_64-unknown-l4re-uclibc build

13090df

style fixes to satisfy new rustfmt

3378f0c

skrap force-pushed the feature/move-uclibc-under-linux branch from 936fab0 to 3378f0c Compare February 13, 2021 20:04

bors added S-waiting-on-bors and removed S-waiting-on-review labels Feb 13, 2021

bors merged commit d4080d1 into rust-lang:master Feb 13, 2021

This was referenced Feb 13, 2021

FD_ISSET: take a *const fd_set instead of *mut fd_set #1725

Merged

Add bindings for target_env = "libnx" #1829

Closed

Use Rust union types #2056

Open

Move uclibc under linux #1975

Move uclibc under linux #1975

Conversation

skrap commented Nov 12, 2020 • edited Loading

rust-highfive commented Nov 12, 2020

skrap commented Nov 13, 2020

skrap commented Nov 20, 2020 • edited Loading

kolapapa commented Nov 22, 2020

skrap commented Nov 22, 2020

kolapapa commented Nov 23, 2020 • edited Loading

skrap commented Nov 23, 2020 • edited Loading

kolapapa commented Nov 23, 2020

skrap commented Nov 23, 2020

bors commented Nov 25, 2020

skrap commented Nov 25, 2020

jyn514 commented Nov 30, 2020

JohnTitor commented Nov 30, 2020

skrap commented Dec 3, 2020

JohnTitor commented Dec 3, 2020 • edited Loading

skrap commented Dec 3, 2020

jyn514 commented Dec 3, 2020

skrap commented Dec 3, 2020

skrap commented Dec 3, 2020

jyn514 commented Dec 3, 2020

skrap commented Dec 3, 2020

jyn514 commented Dec 3, 2020

skrap commented Dec 4, 2020

JohnTitor commented Dec 4, 2020

skrap commented Dec 4, 2020

skrap commented Dec 5, 2020 • edited Loading

bors commented Dec 10, 2020

skrap commented Jan 13, 2021

bors commented Jan 13, 2021

bors commented Jan 13, 2021

humenda commented Jan 14, 2021 via email

humenda commented Jan 16, 2021 via email

skrap commented Jan 16, 2021

JohnTitor commented Feb 13, 2021

JohnTitor commented Feb 13, 2021

bors commented Feb 13, 2021

bors commented Feb 13, 2021

bors commented Feb 13, 2021

skrap commented Feb 14, 2021

skrap commented Nov 12, 2020 •

edited

Loading

skrap commented Nov 20, 2020 •

edited

Loading

kolapapa commented Nov 23, 2020 •

edited

Loading

skrap commented Nov 23, 2020 •

edited

Loading

JohnTitor commented Dec 3, 2020 •

edited

Loading

skrap commented Dec 5, 2020 •

edited

Loading