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 ARM32/ARM64 CI #930

Merged
merged 2 commits into from
May 13, 2021
Merged

Add ARM32/ARM64 CI #930

merged 2 commits into from
May 13, 2021

Conversation

sipa
Copy link
Contributor

@sipa sipa commented May 2, 2021

No description provided.

@sipa sipa force-pushed the 202104_armci branch 3 times, most recently from 1b70c33 to c709c5c Compare May 2, 2021 20:02
@sipa sipa mentioned this pull request May 2, 2021
@sipa sipa force-pushed the 202104_armci branch 3 times, most recently from 6329af9 to 541269f Compare May 2, 2021 20:57
Copy link
Contributor

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

ACK mod nit

.cirrus.yml Outdated Show resolved Hide resolved
@real-or-random
Copy link
Contributor

real-or-random commented May 3, 2021

Wait, the expectation was probably that this should break due to #931.

I think this is related to the "shortcut" in configure.ac that simply uses the native toolchain to build gen_context when we're not cross-compiling. In this case, we are cross compiling, so we don't take that shortcut.

I'll have a look.

@real-or-random
Copy link
Contributor

Wait, the expectation was probably that this should break due to #931.

I think this is related to the "shortcut" in configure.ac that simply uses the native toolchain to build gen_context when we're not cross-compiling. In this case, we are cross compiling, so we don't take that shortcut.

I'll have a look.

Ok I think I was a little confused when I wrote that. The issue #931 appears when compiling gen_context for the build machine. On CI, we cross-compile for ARM, so building gen_context for the CI machine is never an issue.

I think no simple CI test could test for the breakage in #931. We could of course cross-compile gen_context and then run it on qemu.

One possible way to do so without fiddling around with the build system would be to rely on binfmt_misc, a kernel feature which allows you to run commands like ./cmd which then will be automatically wrapped, e.g., by qemu or wine. (This is why my machine will build and run gen_context.exe when cross-compiling for mingw...) But I heard binfmt_misc has some issues with Docker.

Another possible way is to simply add a CI job that manually cross-compiles gen_context for ARM and runs it on qemu. But this doesn't really test if the autoconf build works.

So I'm not convinced that testing this on CI is worth the hassle.

Anyway, my ACK on this PR is still valid.

@sipa
Copy link
Contributor Author

sipa commented May 3, 2021

Indeed, I realized too late that #931 was in fact related to the build environment for gen_context rather than the ARM asm per se.

Still, these are very useful CI targets I think.

@real-or-random
Copy link
Contributor

real-or-random commented May 3, 2021

Still, these are very useful CI targets I think.

For sure, and this fixes #598.

edit: By the way, I'm happy to see that it really wasn't a lot of work to add this build and the mingw build. So it seems that the general structure of the CI is pretty portable and adding qemu buils is simply a matter of installing the right Debian packages and passing the right HOST.

@gmaxwell
Copy link
Contributor

gmaxwell commented May 3, 2021

if this supports a full docker environment could we just run qemu-system and run a whole emulated distribution? :P That said, having a cross compile to arm test is really really good too.

@sipa
Copy link
Contributor Author

sipa commented May 3, 2021

@gmaxwell I think that's an option, but something I'd like to keep for later.

Copy link
Contributor

@real-or-random real-or-random left a comment

Choose a reason for hiding this comment

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

ACK 8bbad7a CI output looks fine

Copy link
Contributor

@jonasnick jonasnick left a comment

Choose a reason for hiding this comment

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

ACK 8bbad7a

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.

4 participants