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 ARM MUSL targets #33189

Closed
wants to merge 3 commits into from
Closed

Add ARM MUSL targets #33189

wants to merge 3 commits into from

Conversation

timonvo
Copy link
Contributor

@timonvo timonvo commented Apr 25, 2016

The targets are:

  • arm-unknown-linux-musleabi
  • arm-unknown-linux-musleabihf
  • armv7-unknown-linux-musleabihf

These mirror the existing gnueabi targets.

All of these targets produce fully static binaries, similar to the
x86 MUSL targets.

For now these targets can only be used with --rustbuild builds, as
rust-lang/compiler-rt#22 only made the
necessary compiler-rt changes in the CMake configs, not the plain
GNU Make configs.

I've tested these targets GCC 5.3.0 compiled again musl-1.1.12
(downloaded from http://musl.codu.org/). An example ./configure
invocation is:

./configure \
    --enable-rustbuild
    --target=arm-unknown-linux-musleabi \
    --musl-root="$MUSL_ROOT"

where MUSL_ROOT points to the arm-linux-musleabi prefix.
Usually that path will be of the form
/foobar/arm-linux-musleabi/arm-linux-musleabi.

Usually the cross-compile toolchain will live under
/foobar/arm-linux-musleabi/bin. That path should either by added
to your PATH variable, or you should add a section to your
config.toml as follows:

[target.arm-unknown-linux-musleabi]
cc = "/foobar/arm-linux-musleabi/bin/arm-linux-musleabi-gcc"
cxx = "/foobar/arm-linux-musleabi/bin/arm-linux-musleabi-g++"

As a prerequisite you'll also have to put a cross-compiled static
libunwind.a library in $MUSL_ROOT/lib. This is similar to how
the x86_64 MUSL targets are built
.

r? @alexcrichton

@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 @alexcrichton (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.

@@ -83,7 +83,7 @@ pub fn std_link(build: &Build,
add_to_sysroot(&out_dir, &libdir);

if target.contains("musl") &&
(target.contains("x86_64") || target.contains("i686")) {
(target.contains("x86_64") || target.contains("i686") || target.contains("arm")) {
Copy link
Member

Choose a reason for hiding this comment

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

Hm perhaps this condition could be reversed to say if target.contains("musl") && !target.contains("mips") ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@alexcrichton
Copy link
Member

Thanks for the PR @timonvo, and nice!

Also cc @rust-lang/tools, curious on thoughts of the idea on using target_env = "musleabi" and "musleabihf". On one hand it's unfortunate how musl-specific code now needs three different target_env arguments instead of one, although it's consistent with how we treat the target "triple" parsing. It's also consistent with the behavior of how we treat the glibc targets (e.g. gnueabi and gnueabihf), although I highly doubt anyone's relying on that distinction.

I don't quite know what the best way to expose this would be. We could perhaps add a target_libc or also perhaps repurpose target_env, but curious what others think

@timonvo
Copy link
Contributor Author

timonvo commented Apr 27, 2016

Hi Alex,

I think I've addressed all your comments. Is there anything else you'd like to see before this can land? I was thinking I'd squash the last two commits into the fe4e327 commit once you give the OK that is can be submitted.

Regarding the target_env issue, as I mentioned in an earlier PR I think this is something that should maybe be resolved, but I don't think this is the right place for it now. As for what my preferred fix is: I think a new target_libc field may be the easiest fix. I think it would also be the most clear way of handling the problem, as for any #cfg based on that value it'd become immediately clear why the conditional is there.

@alexcrichton
Copy link
Member

Hm yeah I was thinking on waiting for a resolution of musleabi vs just musl, but I think you're right that it's probably best to duke out elsewhere. If you wanna do some squashes I'll r+. Thanks @timonvo!

This is to pull in changes to support ARM MUSL targets.

This change also commits a couple of other cargo-generated changes
to other dependencies in the various Cargo.toml files.
The targets are:
- `arm-unknown-linux-musleabi`
- `arm-unknown-linux-musleabihf`
- `armv7-unknown-linux-musleabihf`

These mirror the existing `gnueabi` targets.

All of these targets produce fully static binaries, similar to the
x86 MUSL targets.

For now these targets can only be used with `--rustbuild` builds, as
rust-lang/compiler-rt#22 only made the
necessary compiler-rt changes in the CMake configs, not the plain
GNU Make configs.

I've tested these targets GCC 5.3.0 compiled again musl-1.1.12
(downloaded from http://musl.codu.org/). An example `./configure`
invocation is:

```
./configure \
    --enable-rustbuild
    --target=arm-unknown-linux-musleabi \
    --musl-root="$MUSL_ROOT"
```

where `MUSL_ROOT` points to the `arm-linux-musleabi` prefix.
Usually that path will be of the form
`/foobar/arm-linux-musleabi/arm-linux-musleabi`.

Usually the cross-compile toolchain will live under
`/foobar/arm-linux-musleabi/bin`. That path should either by added
to your `PATH` variable, or you should add a section to your
`config.toml` as follows:

```
[target.arm-unknown-linux-musleabi]
cc = "/foobar/arm-linux-musleabi/bin/arm-linux-musleabi-gcc"
cxx = "/foobar/arm-linux-musleabi/bin/arm-linux-musleabi-g++"
```

As a prerequisite you'll also have to put a cross-compiled static
`libunwind.a` library in `$MUSL_ROOT/lib`. This is similar to [how
the x86_64 MUSL targets are built]
(https://doc.rust-lang.org/book/advanced-linking.html).
@timonvo
Copy link
Contributor Author

timonvo commented Apr 28, 2016

Just did the squash of the last two commits.

// It's important we use "gnueabi" and not "musleabi" here. LLVM uses it
// to determine the calling convention and float ABI, and it doesn't
// support the "musleabi" value.
llvm_target: "arm-unknown-linux-gnueabi".to_string(),
Copy link
Member

Choose a reason for hiding this comment

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

Is it intentional to pass "gneuabi" to LLVM instead of "musleabi"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it is. I believe the comment above explains why :)

Copy link
Member

Choose a reason for hiding this comment

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

Wow I can totally read

@alexcrichton
Copy link
Member

Just one minor nit but otherwise r=me

@alexcrichton
Copy link
Member

@bors: r+ 483d805

@bors
Copy link
Contributor

bors commented Apr 28, 2016

⌛ Testing commit 483d805 with merge 9147703...

@bors
Copy link
Contributor

bors commented Apr 28, 2016

💔 Test failed - auto-win-msvc-64-opt-rustbuild

@alexcrichton
Copy link
Member

@timonvo in #33244 we decided that the targets should just use musl as the target_env instead of "musleabi" and "musleabihf", could you make that change as well?

@timonvo
Copy link
Contributor Author

timonvo commented May 5, 2016

Sure, I'll try to make that change. I'm not sure when I'll have time in next week or two, so this may be on hold for a while.

bors added a commit that referenced this pull request May 7, 2016
rustc_back: use a common musl base

extracted from #33327.

cc #33189.

r? @alexcrichton
bors added a commit that referenced this pull request May 7, 2016
rustc_back: use a common musl base

extracted from #33327.

cc #33189.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented May 7, 2016

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

@alexcrichton
Copy link
Member

Closing due to inactivity, but feel free to resubmit with a rebase!

@japaric
Copy link
Member

japaric commented Jul 26, 2016

@timonvo If you don't mind, I'm going to try to rebase this.

@japaric japaric mentioned this pull request Jul 27, 2016
steveklabnik added a commit to steveklabnik/rust that referenced this pull request Jul 30, 2016
Add ARM MUSL targets

Rebase of rust-lang#33189.

I tested this by producing a std for `arm-unknown-linux-musleabi` then I cross compiled Hello world to said target. Checked that the produced binary was statically linked and verified that the binary worked under QEMU.

This depends on rust-lang/libc#341. I'll have to update this PR after that libc PR is merged.

I'm also working on generating ARM musl cross toolchain via crosstool-ng. Once I verified those work, I'll send a PR to rust-buildbot.

r? @alexcrichton
cc @timonvo
bors added a commit that referenced this pull request Jul 31, 2016
Add ARM MUSL targets

Rebase of #33189.

I tested this by producing a std for `arm-unknown-linux-musleabi` then I cross compiled Hello world to said target. Checked that the produced binary was statically linked and verified that the binary worked under QEMU.

This depends on rust-lang/libc#341. I'll have to update this PR after that libc PR is merged.

I'm also working on generating ARM musl cross toolchain via crosstool-ng. Once I verified those work, I'll send a PR to rust-buildbot.

r? @alexcrichton
cc @timonvo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants