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

uclibc: fix O_TMPFILE value, hide some unused fields to fix libc-test #2636

Merged
merged 1 commit into from
Jan 20, 2022

Conversation

skrap
Copy link
Contributor

@skrap skrap commented Jan 19, 2022

This was a bit of a yak-shave.

First, O_TMPFILE was wrong because 020000000 is decimal, not octal. 🤦 This was, of course, causing all sorts of things to be horrible.

That led me to switch to a newer toolchain which actually defined O_TMPFILE, so that the libc tests pass on armv7-unknown-linux-uclibceabihf.

That revealed that certain field names had changed since the last time I ran the libc tests on this architecture, which revealed that some struct fields (e.g. __unused) were being marked as pub where all other architectures left them as private.

So, this patch might seem somewhat sprawling, but without all of these changes it's not possible to run libc-test!

To test:

cd libc-test

PATH=$TOOLCHAIN/bin:$PATH \
  CC_armv7_unknown_linux_uclibc=armv7-buildroot-linux-uclibc-gcc \
  CARGO_TARGET_ARMV7_UNKNOWN_LINUX_UCLIBCEABIHF_LINKER=armv7-buildroot-linux-uclibc-gcc \
  CARGO_TARGET_ARMV7_UNKNOWN_LINUX_UCLIBCEABIHF_RUNNER="qemu-arm -L $TOOLCHAIN/arm-buildroot-linux-uclibcgnueabihf/sysroot/" \
  cargo +nightly test --target armv7-unknown-linux-uclibceabihf -Zbuild-std=core,std    

@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@skrap
Copy link
Contributor Author

skrap commented Jan 19, 2022

(ref nix-rust/nix#1603)

@skrap
Copy link
Contributor Author

skrap commented Jan 19, 2022

@Amanieu I would like to get CI libc-test working for this target, but seeing as it's Tier-3 at the moment, I'm not sure if that's a change which would be accepted. Do you have any opinions or advice there?

@Amanieu
Copy link
Member

Amanieu commented Jan 19, 2022

I'm OK with adding uclibc to CI even though it is tier 3. It seems sorely needed consider the issues found in this PR :)

Could you add it in this PR?

@Amanieu
Copy link
Member

Amanieu commented Jan 19, 2022

Actually I'm going to merge this as it is so that it makes the next libc release. You can add uclibc to CI in a separate PR.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 19, 2022

📌 Commit 0fcf5b3 has been approved by Amanieu

@bors
Copy link
Contributor

bors commented Jan 20, 2022

⌛ Testing commit 0fcf5b3 with merge cf597c2...

@bors
Copy link
Contributor

bors commented Jan 20, 2022

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

@bors bors merged commit cf597c2 into rust-lang:master Jan 20, 2022
bors added a commit that referenced this pull request Jan 21, 2022
add CI for armv7-unknown-linux-uclibceabihf

As discussed in #2636, this is a framework for CI on targets with no prebuilt std.  They will build via `nightly` and `-Zbuild-std`, and run the unit tests via qemu-user (or whatever else the dockerfile specifies).

It seems like I could write some more documentation about how to add more targets of this type, but perhaps I can get a round of feedback on the mechanism here before I go and write that.
bors added a commit that referenced this pull request Jan 21, 2022
add CI for armv7-unknown-linux-uclibceabihf

As discussed in #2636, this is a framework for CI on targets with no prebuilt std.  They will build via `nightly` and `-Zbuild-std`, and run the unit tests via qemu-user (or whatever else the dockerfile specifies).

It seems like I could write some more documentation about how to add more targets of this type, but perhaps I can get a round of feedback on the mechanism here before I go and write that.
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