Skip to content

Support testing non-native targets #429

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

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

Conversation

kkysen
Copy link
Contributor

@kkysen kkysen commented Jun 1, 2022

Fixes #410.

Current Description

Added support for cross-compiling and emulation using qemu. The targets to install are determined in docker_build.sh from the target-tuple files and passed to the Dockerfile as an argument. There it installs the gcc-${c_target} apt packages for cross-compiling in provision_deb.sh (as well as qemu) and the rust targets in provision_rust.sh using rustup. Then it modifies test_translator.py to generate a .cargo/config.toml for every cross-compiling test. This sets the correct linker, rustflags, and runner (qemu-${arch} -L /usr/${c_target}, which means we don't need to mess with binfmt_misc either). It also sets the default target, so we don't need to always pass --target ${target} everywhere.

The existing arm tests, asm.arm and asm.aarch64, are currently failing, so they are temporarily fixed or skipped until we figure out how to actually fix them so that we don't cause CI failures. See #431, #430, and #433 (now fixed by #441) for the issues.

Since this PR also changes CI, the current CI tests that are passing don't really mean anything. I've tested the changes locally on ubuntu:focal and they pass. Once we're confident it won't break CI at all, these changes should be merged into feature/ci-dev and/or have ./scripts/docker_build.sh push-all be run.

Initial Description

Added support to scripts/test_translator.py for cross-compiling using cargo zigbuild and zig cc and for emulation using qemu.

More specifically, cargo zigbuild is used in place of cargo for building and running the test crates. It uses zig cc as the linker, which has seamless cross-compilation that just works.

qemu is used to emulate other targets and architectures. These are small test programs, so this should be fine, and doing it this way allows all tests to be run locally, which is very convenient.

I wasn't sure how dependencies should be managed, but for this, I just had them installed in test_translator.py itself, assuming it's running Ubuntu Linux. I assume there's a better way to do this that's more cross-platform, but I wasn't sure how to do that yet, so I just did it inline first to make things quick. We can fix that next. As is, it uses sudo and isn't going to work in CI, but it probably will work locally.

Specifically for dependencies,

  • cargo-zigbuild is installed using cargo install
  • zig cc is installed through pip install ziglang (added it to requirements.txt)
  • rustc targets are installed through rustup target add
  • qemu is installed through apt (qemu-user, qemu-user-static)
  • qemu dynamic libraries are installed through apt (gcc-${triple})
  • update-binfmts (for enabling qemu binfmt_misc support) is installed through apt (binfmt-support)
  • qemu binfmt_misc entries are added through update-binfmts --enable

Note, however, that the asm.arm and asm.aarch64 tests, the ones I was trying to test, don't appear to work out of the box. I'm not sure if the errors in them are due to something wrong in the cross compilation and emulation, are they are simply errors in the tests themselves since I assume they weren't being tested as regularly. Specifically, asm.arm fails at runtime on an assertion (#431), and asm.aarch64 fails to compile, saying an asm argument is unused (#430).

@kkysen
Copy link
Contributor Author

kkysen commented Jun 1, 2022

I moved the installation stuff to scripts/provision_cross_tests.sh, which I only run in scripts/provision_deb.sh now. If installs all the same stuff as before, but since it's not installed at use-site, it installs necessary things for all targets found in target-tuple files.

scripts/test_translator.py doesn't install anything anymore. If cargo-zigbuild is available, it uses that (which uses zig cc), otherwise just normal cargo. And it goes back to the logic of only running tests for targets if those (rustup) targets are already installed.

We can keep discussing whether to use zig cc or not, but I wanted to add these changes first, which are simpler, and see how it works in CI.

@kkysen kkysen marked this pull request as draft June 2, 2022 21:30
@kkysen
Copy link
Contributor Author

kkysen commented Jun 7, 2022

Removed any dependencies on zig cc and cargo-zigbuild now. It now just uses the gcc-${c_target} apt packages for cross-compiling.

It does this by generating a .cargo/config.toml for every cross-compiling test. This sets the correct linker, rustflags, and runner (qemu-${arch}, which means we don't need to mess with binfmt_misc either). It also sets the default target, so we don't need to always pass --target ${target} everywhere.

provision_cross_tests.sh is also now split into just additions to provision_deb.sh and provision_rust.sh.

Also, the arm tests that are failing are temporarily fixed/skipped until we figure out how to actually fix them so that we don't cause CI failures. See #431, #430, and #433.

Speaking of CI, these changes, once approved (@thedataking), need to also be pushed to feature/ci-dev and have ./scripts/docker_build.sh push-all be run.

@kkysen kkysen marked this pull request as ready for review June 7, 2022 04:57
@kkysen
Copy link
Contributor Author

kkysen commented Jun 10, 2022

#433 is now fixed by #441, so that's down to 1/2 issues blocking the tests/asm.aarch64 test from working.

@kkysen kkysen changed the title Add Support for Testing Non-Native Targets Support testing non-native targets Aug 9, 2022
kkysen added 20 commits August 9, 2022 14:03
using `qemu` for emulation and `cargo-zigbuild` and `zig cc` for cross-compilation.

Specifically, ...

`cargo-zigbuild` is used in place of `cargo`
for building and running the test crates.
It uses `zig cc` as the linker, which has seamless cross compilation.

`qemu` is used to emulate other targets and architectures.
These are small test programs, so this should be fine,
and doing it this way allows all tests to be run locally,
which is very convenient.

I wasn't sure how dependencies should be managed,
but for this, I just had them installed in `test_translator.py` itself,
assuming it's running Ubuntu Linux.
If there's a better way to do this, let me know,
but I just did it inline first to make things quick.

Specifically,
* `cargo-zigbuild` is installed using `cargo install`
* `zig cc` is installed through `pip install ziglang` (added it to `requirements.txt`)
* `rustc` targets are installed through `rustup target add`
* `qemu` is installed through `apt` (`qemu-user`, `qemu-user-static`)
* `qemu` dynamic libraries are installed through `apt` (`gcc-${triple}`)
* `update-binfmts` (for enabling `qemu` `binfmt_misc` support) is installed through `apt` (`binfmt-support`)
… matches (since Rust uses `zig cc`, too).
…ts/provision_cross_tests.sh` instead of `scripts/test_translator.py`, which now just assumes they're already there.
And forgot to remove the `rustup target add` stuff in the last commit.
It's nice to be able to run `provision_cross_tests.sh` locally
as well as in CI (where `sudo` doesn't exist but you're running as root (I think)).
But locally, some other commands you might not want to give sudo to.

Also, don't use `cargo quickinstall` actually.
Tested using `./scripts/docker_build.sh ubuntu:focal`.

The installation script (`provision_cross_tests.sh`) is run twice,
once as a normal user, once as root, so that it can install it's different things.
…do it in the script outside the docker build).
…ation now.

Generate a `.cargo/config.toml` file containing the necessary overrides.

Specifically, the `.cargo/config.toml` contains:
* the default target, so `cargo build`/`run`/`test` works
* the cross compiler under `linker`
* rustflags to make sure you're using gold/bfd (not lld/mold)
* the qemu runner (so no `binfmt_misc` support needed anymore either)

This is also useful for running `--keep rust_src`
and being able to run `cargo run` yourself without any flags/overrides needed.
…We can investigate and fix for sure later.
… (unused asm arg) and clang 10 (unsupported arm scalable vector extensions).
kkysen added 13 commits August 9, 2022 14:04
… with a C warning that didn't show up in clang 14, which works.
…function with a C warning that didn't show up in clang 14, which works."

This reverts commit af93630.
…the asm and narrowing down the cause."

This reverts commit 81454b6.
…even all the asm didn't work). I think it might just be the aarch64 target on clang 10 that's the problem.
…ything (even all the asm didn't work). I think it might just be the aarch64 target on clang 10 that's the problem."

This reverts commit 3dac9ef.
…nce it's specified in the generated `.cargo/config.toml` now.
…g.toml`.

Usually this isn't needed, but who knows when it will be.
@kkysen kkysen force-pushed the kkysen/non-native-tests branch from 2a48d5f to e95082b Compare August 9, 2022 21:07
@kkysen kkysen mentioned this pull request Aug 12, 2022
14 tasks
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.

Need support for tests that target ARM
1 participant