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 new target armv7-unknown-linux-uclibceabihf #79380

Closed
wants to merge 1 commit into from

Conversation

ykoehler
Copy link

Add support for armv7 uclibc to rust.

./x.py dist

was successful, testing of toolchains via rustup as well, and binary on platform ldd output was fine and ran properly.

I am unclear how to get the ./x.py test library/std working, would need assistance.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @kennytm (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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 24, 2020
@ykoehler
Copy link
Author

This change also required rust-lang/backtrace-rs#396

@ykoehler ykoehler force-pushed the master branch 4 times, most recently from 909a8ce to a75d773 Compare November 24, 2020 16:23
@ykoehler
Copy link
Author

I will have to revert the library/backtrace change.

@camelid
Copy link
Member

camelid commented Dec 18, 2020

FYI I think we may not be accepting new targets right now.

@camelid camelid added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Dec 18, 2020
@skrap
Copy link
Contributor

skrap commented Dec 18, 2020

@ykoehler I am working on a libc pull request related to this target. For my use case, I am building my own toolchain via buildroot. The request was made for me to add CI for some uclibc target (e.g. mipsel-unknown-linux-uclibc) but I don't know of a good source for uclibc toolchains, and it's been suggested to me that having CI build its own toolchain may slow the process too much. Do you know of a source for uclibc-based toolchains?

@ykoehler
Copy link
Author

@skrap I believe that buildroot is a proper way to get such toolchain. Another way would be crosstools-NG, but to my knowledge buildroot is a very popular choice amount the embedded space to generate their toolchain. Otherwise vendors will provides their toolchain but we do not know from which method they selected to build theirs. As such, I think using buildroot or assuming people will use buildroot is a very acceptable decision.

@ykoehler
Copy link
Author

Regarding this PR, we are not using the last version of uClibc, and I do understand that inside this project we probably should, as such, I may need to adjust its content related to that. For example, the change made to the submodule backtrace would need to be removed since the last version of uClibc has the proper support which I had to disable for our version.

@ykoehler
Copy link
Author

@camelid sounds totally reasonable, is there are way to mark this PR as dependent on that other one so that this PR gets notified when the other is closed?

@camelid
Copy link
Member

camelid commented Dec 18, 2020

@ykoehler For one, you should add the S-blocked label and add something to the PR description like "Blocked on #NNN". In terms of being notified, you can use GitHub to subscribe to the "merged" and "closed" events. Just click on "Customize" next to the "Notifications" header in the sidebar in the PR you want to subscribe to.

@camelid
Copy link
Member

camelid commented Dec 18, 2020

S-blocked can be added with this command:

@rustbot label S-blocked

@rustbot rustbot added the S-blocked Status: Blocked on something else such as an RFC or other implementation work. label Dec 18, 2020
@Dylan-DPC-zz
Copy link

in terms of being notified, the triage team often checks for s-blocked prs if those are unblocked,

@JohnCSimon
Copy link
Member

Ping from triage -
@ykoehler is this still blocked?

@bors
Copy link
Contributor

bors commented Feb 3, 2021

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

@ykoehler
Copy link
Author

ykoehler commented Feb 3, 2021

@JohnCSimon my understanding from the comment from @camelid is that new target are not being accepted at this time.

@camelid
Copy link
Member

camelid commented Feb 3, 2021

@JohnCSimon my understanding from the comment from @camelid is that new target are not being accepted at this time.

I am not certain about that, but that's my understanding. Although I think @JohnCSimon was referring to whether this PR is still blocked on that libc PR, which I think it is.

@camelid camelid removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 26, 2021
@bors
Copy link
Contributor

bors commented Jun 6, 2021

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

@JohnTitor
Copy link
Member

Triage: RFC 2803 has been merged so now it shouldn't block this, right?

@skrap
Copy link
Contributor

skrap commented Jun 15, 2021

@ykoehler I'm assuming you'll want to be a maintainer of this target (per the new RFC rust-lang/rfcs#2803) . If you want an additional person, I'm happy to be added as well.

@skrap
Copy link
Contributor

skrap commented Jun 17, 2021

I can confirm that, with rust 1.53.0, this patch allows a full build of rust:

Dist rust-std-1.53.0-armv7-unknown-linux-uclibceabihf
	finished in 16.174 seconds

1.52.1 did not build due to missing pwrite64 symbol in libc when building for uclibc targets. (my fault, thx @JohnTitor for the fix!)

@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Aug 26, 2021
@Dylan-DPC-zz
Copy link

Dylan-DPC-zz commented Aug 26, 2021

@kennytm this looks ready for review

err didnt see the conflict

@ykoehler if you can resolve the conflict we can try and push this forward

@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 26, 2021
@skrap
Copy link
Contributor

skrap commented Aug 26, 2021

@ykoehler There's an additional commit I'd like to suggest for your branch, which fixes a crash when accessing std::env::args:

From 221f4b10d421dca92ffb42e079c3ad8f2ff55232 Mon Sep 17 00:00:00 2001
From: Jonah Petri <jonah@petri.us>
Date: Tue, 3 Aug 2021 14:14:53 -0400
Subject: [PATCH] uclibc does not pass argv/argc in a glibc-compatible way

---
 library/std/src/sys/unix/args.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/library/std/src/sys/unix/args.rs b/library/std/src/sys/unix/args.rs
index 2805b2964d1..94364f9c874 100644
--- a/library/std/src/sys/unix/args.rs
+++ b/library/std/src/sys/unix/args.rs
@@ -103,14 +103,14 @@ mod imp {
         // still initialize here.
         #[cfg(any(
             miri,
-            not(all(target_os = "linux", any(target_env = "gnu", target_env = "uclibc")))
+            not(all(target_os = "linux", any(target_env = "gnu")))
         ))]
         really_init(_argc, _argv);
     }
 
     /// glibc passes argc, argv, and envp to functions in .init_array, as a non-standard extension.
     /// This allows `std::env::args` to work even in a `cdylib`, as it does on macOS and Windows.
-    #[cfg(all(target_os = "linux", any(target_env = "gnu", target_env = "uclibc")))]
+    #[cfg(all(target_os = "linux", any(target_env = "gnu")))]
     #[used]
     #[link_section = ".init_array.00099"]
     static ARGV_INIT_ARRAY: extern "C" fn(
-- 
2.32.0

@JohnCSimon
Copy link
Member

Ping again from triage:

@ykoehler if you can resolve the conflict we can try and push this forward

@camelid camelid added the O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state label Sep 12, 2021
@skrap
Copy link
Contributor

skrap commented Sep 13, 2021

@JohnCSimon It seems that this PR's author is not responding. I am interested in pushing this PR forward. Is that possible? If so, how should I proceed? I could file a PR with the same purpose.

@JohnCSimon
Copy link
Member

@skrap not sure about the process, maybe contact the reviewer?

@ykoehler
Copy link
Author

@JohnCSimon The interest for me to pursue this has dropped since our embedded toolchains doesn't have the latest version as it is required in here. From what a gather from this experience, I do not get why rust is not supporting uclibc for all platforms, since basically, it appears to be a matter of building a toolchain with buildroot and enabling the existing exception created for uclibc throughout the code. And the fact that if this PR goes through I suddenly become responsible for this triplet as per rust policy, I found it really strange, since other than compiling this and enabling the uclibc exception there was not much additional work and having the responsibility to then maintain this is outside the time/resources I have available.

I will see if I still have this build environment as to fix the conflicts, but if you want to kill this PR and take ownership (and from their policy, responsibility) then I am totally fine with that.

@skrap
Copy link
Contributor

skrap commented Sep 14, 2021

@ykoehler I'm not affiliated with the Rust project, but I have employer support to maintain this target. If you don't object, I'll submit a new PR based on this one, and I'll be happy to be the contact person for this target triple. (This will still be tier 3 at this point.)

@pgasiorowski-es
Copy link

pgasiorowski-es commented Sep 14, 2021

I am also interested in compiling for armv7-unknown-linux-uclibceabihf but don't have much experience with setting up rust toolchains and the documentation is not clear on that either. I could at least help with testing this PR but after building rust from ykoehler:master and trying to cross compile a simple hello-word thing I'm experiencing several problems:

First when trying add the target:

rustup target add armv7-unknown-linux-uclibceabihf

error: toolchain 'stable-x86_64-unknown-linux-gnu' does not contain
component 'rust-std' for target 'armv7-unknown-linux-uclibceabihf'

note: not all platforms have the standard library pre-compiled:
https://doc.rust-lang.org/nightly/rustc/platform-support.html

So I updated .cargo/config to add it manually:

[build]
target = "armv7-unknown-linux-uclibceabihf"


[target.armv7-unknown-linux-uclibceabihf]
linker = "arm-linux-uclibceabihf-gcc"
ar = "arm-linux-uclibceabihf-ar"

but when running cargo build I'm getting:

   Compiling arm v0.1.0 (/home/woodzu/development/arm)
error[E0463]: can't find crate for `std`
  |
  = note: the `armv7-unknown-linux-uclibceabihf` target may not be
installed

I understand the problem is that it needs a crate / source code of uclibc but it's not clear how to supply that. Any help would be appreciated.

@ykoehler
Copy link
Author

ykoehler commented Sep 14, 2021 via email

@oli-obk
Copy link
Contributor

oli-obk commented Sep 15, 2021

Closing in favour of #88952

@oli-obk oli-obk closed this Sep 15, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 10, 2021
Add new tier-3 target: armv7-unknown-linux-uclibceabihf

This change adds a new tier-3 target: armv7-unknown-linux-uclibceabihf

This target is primarily used in embedded linux devices where system resources are slim and glibc is deemed too heavyweight.  Cross compilation C toolchains are available [here](https://toolchains.bootlin.com/) or via [buildroot](https://buildroot.org).

The change is based largely on a previous PR rust-lang#79380 with a few minor modifications.  The author of that PR was unable to push the PR forward, and graciously allowed me to take it over.

Per the [target tier 3 policy](https://github.com/rust-lang/rfcs/blob/master/text/2803-target-tier-policy.md), I volunteer to be the "target maintainer".

This is my first PR to Rust itself, so I apologize if I've missed things!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.