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

Fix tests, linkers, and specifications for various targets. #1028

Merged
merged 5 commits into from
Oct 2, 2022

Conversation

Alexhuszagh
Copy link
Contributor

@Alexhuszagh Alexhuszagh commented Sep 24, 2022

  • Fixes linking to libgcc for armv5te-unknown-linux-musleabi.
  • Adds C++ support for FreeBSD targets by linking to libstdc++.
  • Tests dynamic library support for Android targets in CI.
  • Tests C++ support for mips64el-unknown-linux-muslabi64 in CI.
  • Convert mips64el-unknown-linux-muslabi64 to a hard-float toolchain to match the rust target.
  • Convert mips64el-unknown-linux-muslabi64 to use the mips64r2 architecture.
  • Convert mips-unknown-linux-musl and mipsel-unknown-linux-musl to use the mips32r2 architecture, identical to the rust targets.
  • Document whether MIPS musl targets are hard or soft float.

@Alexhuszagh Alexhuszagh added docs-problem meta issues/PRs related to the maintenance of the crate. no changelog A valid PR without changelog (no-changelog) labels Sep 24, 2022
@Alexhuszagh
Copy link
Contributor Author

Blocking this until #1008 is merged, so we can use the new TOML format for the matrix.

@Alexhuszagh Alexhuszagh force-pushed the ci_tests branch 4 times, most recently from 853779e to 983edbc Compare September 29, 2022 21:24
@Alexhuszagh Alexhuszagh marked this pull request as ready for review September 29, 2022 21:24
@Alexhuszagh Alexhuszagh requested a review from a team as a code owner September 29, 2022 21:24
@Alexhuszagh
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Sep 29, 2022
@Alexhuszagh
Copy link
Contributor Author

We might be able to fix the partial C++ support for mips64el-unknown-linux-muslabi64 by checking the Rustc version in newer releases. If not, we can always link to libgcc like we do for mips64-unknown-linux-muslabi64. This might be the preferable solution.

@Alexhuszagh Alexhuszagh marked this pull request as draft September 29, 2022 21:31
@bors
Copy link
Contributor

bors bot commented Sep 29, 2022

try

Build failed:

@Alexhuszagh
Copy link
Contributor Author

bors try --target mips64el-unknown-linux-muslabi64

bors bot added a commit that referenced this pull request Sep 29, 2022
@bors
Copy link
Contributor

bors bot commented Sep 29, 2022

try

Build succeeded:

@Alexhuszagh Alexhuszagh marked this pull request as ready for review September 29, 2022 22:48
@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Sep 30, 2022

This is technically a breaking change for mips64el-unknown-linux-muslabi64, however, this changes our build to match the Rust target itself, and since we're moving to a 0.3.0 release, we're allowed to have breaking changes here:
https://github.com/cross-rs/cross/pull/1028/files#diff-8541a3a4775c5542e352ca560a66b3bb939d2dc6112554657dfd8930b87e60ff

Similarly, the 32-bit MIPS musl targets should also likely be hard-float.

@Alexhuszagh
Copy link
Contributor Author

Alexhuszagh commented Sep 30, 2022

We should clearly document the target specifications, since we know:

So with this change, all our targets are now correct for soft or hard-float.

@Alexhuszagh Alexhuszagh changed the title Update Docs and Tests for Target Toolchain Support Fix tests, linkers, and specifications for various targets. Sep 30, 2022
@Alexhuszagh
Copy link
Contributor Author

We can probably get away with building the musl toolchain for a generic mips32 and mips64 architecture, however, since the Rust target will use a more specific architecture, I doubt this will have any changes in practice. This will (minorly) affect external C or C++ dependencies, but shouldn't affect any Rust code.

@Alexhuszagh
Copy link
Contributor Author

bors try --target mipsmusl

bors bot added a commit that referenced this pull request Sep 30, 2022
@Alexhuszagh Alexhuszagh removed the no changelog A valid PR without changelog (no-changelog) label Sep 30, 2022
This PR:
- Fixes linking to libgcc for `armv5te-unknown-linux-musleabi`.
- Adds C++ support for FreeBSD targets by linking to `libstdc++`.
- Tests dynamic library support for Android targets in CI.
- Tests C++ support for `mips64el-unknown-linux-muslabi64` in CI.
- Convert mips64el-unknown-linux-muslabi64 to a hard-float toolchain to match the rust target.
- Convert mips64el-unknown-linux-muslabi64 to use the mips64r2 architecture.
- Convert mips-unknown-linux-musl and mipsel-unknown-linux-musl to use the mips32r2 architecture, identical to the rust targets.
- Document whether MIPS musl targets are hard or soft float.
@bors
Copy link
Contributor

bors bot commented Sep 30, 2022

try

Build succeeded:

Comment on lines -26 to +37
ENV CARGO_TARGET_MIPS64EL_UNKNOWN_LINUX_MUSLABI64_LINKER=mips64el-linux-muslsf-gcc \
ENV CARGO_TARGET_MIPS64EL_UNKNOWN_LINUX_MUSLABI64_LINKER=mips64el-linux-musl-gcc.sh \
CARGO_TARGET_MIPS64EL_UNKNOWN_LINUX_MUSLABI64_RUNNER="/qemu-runner mips64el" \
CC_mips64el_unknown_linux_muslabi64=mips64el-linux-muslsf-gcc \
CXX_mips64el_unknown_linux_muslabi64=mips64el-linux-muslsf-g++ \
CC_mips64el_unknown_linux_muslabi64=mips64el-linux-musl-gcc \
CXX_mips64el_unknown_linux_muslabi64=mips64el-linux-musl-g++ \
Copy link
Member

Choose a reason for hiding this comment

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

ping @abcfy2

does this change work for you?
You can build the image with

cargo xtask build-docker-image mips64el-unknown-linux-muslabi64

this is a hard-float target according to rustc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should be hard-float, we're just using a soft float toolchain which is bound to cause problems at some point. A quick test is:

$ cargo init --bin hello
$ cross build --target mips64el-unknown-linux-muslabi64
$ file target/mips64el-unknown-linux-muslabi64/debug/hello
target/mips64el-unknown-linux-muslabi64/debug/hello: ELF 64-bit LSB executable, MIPS, MIPS64 rel2 version 1 (SYSV), statically linked, with debug_info, not stripped
$ readelf -A target/mips64el-unknown-linux-muslabi64/debug/hello | grep Tag_GNU_MIPS_ABI_FP
  Tag_GNU_MIPS_ABI_FP: Hard float (double precision)
$ docker run -it --rm ghcr.io/cross-rs/mips64el-unknown-linux-muslabi64:main bash
# still using the soft-float toolchain
$ echo $CROSS_MUSL_SYSROOT/
/usr/local/mips64el-linux-muslsf/

# show 32-bit is soft-float
$ cross build --target mipsel-unknown-linux-musl
$ file target/mipsel-unknown-linux-musl/debug/hello
target/mipsel-unknown-linux-musl/debug/hello: ELF 32-bit LSB pie executable, MIPS, MIPS32 rel2 version 1 (SYSV), dynamically linked, interpreter /lib/ld-musl-mipsel-sf.so.1, with debug_info, not stripped
$ readelf -A target/mipsel-unknown-linux-musl/debug/hello | grep Tag_GNU_MIPS_ABI_FP
  Tag_GNU_MIPS_ABI_FP: Soft float

So we're linking a hard-float executable with a soft-float toolchain, which isn't exactly ideal, and bound to cause problems later down the line.

@Alexhuszagh
Copy link
Contributor Author

bors try --target mipsmusl --target aarch64-unknown-linux-musl

Copy link
Member

@Emilgardis Emilgardis left a comment

Choose a reason for hiding this comment

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

looks good!

Fix comment.

Co-authored-by: Emil Gardström <[email protected]>
@bors
Copy link
Contributor

bors bot commented Oct 2, 2022

try

Build succeeded:

@Alexhuszagh
Copy link
Contributor Author

bors r=Emilgardis

@bors
Copy link
Contributor

bors bot commented Oct 2, 2022

Build succeeded:

@bors bors bot merged commit ec03af4 into cross-rs:main Oct 2, 2022
@Alexhuszagh Alexhuszagh deleted the ci_tests branch October 2, 2022 18:16
@Emilgardis Emilgardis added this to the v0.3.0 milestone Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change docs-problem meta issues/PRs related to the maintenance of the crate.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants