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

Run tests on AArch64 in CI #1980

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Conversation

frewsxcv
Copy link
Member

@frewsxcv frewsxcv commented Jan 31, 2021

Uses run-on-arch-action to run tests in CI on a virtualized AArch64 environment. This slows down CI quite a bit, but if it were up to me I'd say it's worth the trade-off, since this could surface some critical issues on a soon-to-be tier 1 platform. Hopefully GitHub will release native integration with AArch64 in the future such that this slowness is temporary.

@frewsxcv frewsxcv force-pushed the frewsxcv-aarch64 branch 2 times, most recently from 8f542c4 to 5003615 Compare February 2, 2021 02:51
@frewsxcv frewsxcv changed the title Run tests on AArch64 Run tests on AArch64 in CI Feb 2, 2021
@frewsxcv frewsxcv force-pushed the frewsxcv-aarch64 branch 8 times, most recently from 05790b8 to 98d179b Compare February 2, 2021 13:35
@frewsxcv frewsxcv marked this pull request as ready for review February 2, 2021 13:35
Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

I have some questions, but this is great!

BINDGEN_FEATURE_RUNTIME: 0
BINDGEN_FEATURE_EXTRA_ASSERTS: 0
BINDGEN_FEATURE_TESTING_ONLY_DOCS: 0
BINDGEN_NO_DEFAULT_FEATURES: 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we somehow not duplicate all the testing setup? If not, can we document why?

For example, the curl command to install rust seems like it should work with actions-rs/toolchain instead. If it doesn't, why not?

Copy link
Member Author

Choose a reason for hiding this comment

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

The way run-on-arch-action works is by creating a container and the current directory is mounted as a volume within the container. If I installed Rust with actions-rs/toolchain, the binaries would not be copied into the container. Does that answer your question?

@saleemrashid
Copy link
Contributor

saleemrashid commented Feb 4, 2021

Instead of using run-on-arch-action, wouldn't it be possible to install the binfmt-support and qemu-user-static packages and then just run the tests as usual but for the AArch64 target (and they'll transparently execute under QEMU because of binfmt-support)

Edit: On second thoughts, you might need the dynamic linker and libraries; which is what either the container or the multiarch packages are for. You could still build outside the container.

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Looks fine to me, mind squashing? Specially the test / empty commits. Other than that it looks good, thank you,

@frewsxcv
Copy link
Member Author

frewsxcv commented Feb 4, 2021

Looks fine to me, mind squashing? Specially the test / empty commits. Other than that it looks good, thank you,

Squashed!

Instead of using run-on-arch-action, wouldn't it be possible to install the binfmt-support and qemu-user-static packages and then just run the tests as usual but for the AArch64 target (and they'll transparently execute under QEMU because of binfmt-support)

Edit: On second thoughts, you might need the dynamic linker and libraries; which is what either the container or the multiarch packages are for. You could still build outside the container.

This would be an improvement! But I've already spent longer than I planned to on this pull request, so I'm going to leave that for someone else.

Copy link
Contributor

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Looks sensible, thank you!

@emilio emilio merged commit 0f8ceb6 into rust-lang:master Feb 4, 2021
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