deislabs / wagi

Write HTTP handlers in WebAssembly with a minimal amount of work
Apache License 2.0
889 stars 45 forks source link

ci(release.yaml): add cross-compiled aarch64 config #147

Closed vdice closed 2 years ago

vdice commented 2 years ago

Approach derived from the Krustlet project.

Ref https://github.com/deislabs/wagi/issues/141

Question: Do we want to update the rust.yaml action to add build/compile of this target as well? Changes would be more impactful to that file, so I wanted to ask first.

Tested release job on fork via https://github.com/vdice/wagi/actions/runs/1752378468 and resulting aarch64 binary on arm64 linux VM:

ubuntu@ip-172-31-95-42:~$ uname -a
Linux ip-172-31-95-42 #30~20.04.1-Ubuntu SMP Thu Jan 13 11:49:06 UTC 2022 aarch64 aarch64 aarch64 GNU/Linux
ubuntu@ip-172-31-95-42:~$ ./wagi --version
WAGI Server 0.4.0
vdice commented 2 years ago

@bacongobbler I noticed an unused field that I've removed in 4fb7dfd.

Also, I can push a commit adding the same cross-compiled aarch64 config to the rust.yaml (which runs build/test on PRs/CI) -- or would it be better to issue a follow-up?

bacongobbler commented 2 years ago

I noticed an unused field that I've removed in 4fb7dfd.

Good call... Looks like the --target flag is specified in the args field so everything should be fine there.

Also, I can push a commit adding the same cross-compiled aarch64 config to the rust.yaml (which runs build/test on PRs/CI) -- or would it be better to issue a follow-up?

I'd do it in the same PR IMO, however I'd let the wagi maintainers chime in here - I haven't contributed much code to this project to say whether a change like that may impact something else

bacongobbler commented 2 years ago

That being said I don't see how compiling to aarch64 in a PR would give the project much benefit other than verifying that it successfully compiles on different machine targets. Is that what you're looking to achieve with that change?

vdice commented 2 years ago

That being said I don't see how compiling to aarch64 in a PR would give the project much benefit other than verifying that it successfully compiles on different machine targets. Is that what you're looking to achieve with that change?

Right, the benefit would just be to ensure cross-compilation doesn't break on any PR, thus giving assurance that the same logic during release events should succeed as well... but, agreed, it doesn't add a lot to the existing CI that insures build/test succeeds on Linux/Mac/Windows.

vdice commented 2 years ago

I'll merge this as it stands, but we can revisit https://github.com/deislabs/wagi/pull/147#issuecomment-1022647858 as needed in a follow-up.