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

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

Merged
merged 2 commits into from Jan 28, 2022

Conversation

vdice
Copy link
Member

@vdice vdice commented Jan 26, 2022

  • Adds a cross-compiled linux aarch64 entry (and add'l logic) into the GH release workflow

Approach derived from the Krustlet project.

Ref #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

Signed-off-by: Vaughn Dice <vaughn.dice@fermyon.com>
Copy link

@bacongobbler bacongobbler left a comment

Choose a reason for hiding this comment

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

LGTM!

Signed-off-by: Vaughn Dice <vaughn.dice@fermyon.com>
@vdice
Copy link
Member Author

vdice commented Jan 26, 2022

@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
Copy link

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
Copy link

bacongobbler commented Jan 26, 2022

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
Copy link
Member Author

vdice commented Jan 26, 2022

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
Copy link
Member Author

vdice commented Jan 28, 2022

I'll merge this as it stands, but we can revisit #147 (comment) as needed in a follow-up.

@vdice vdice merged commit 313aeb6 into deislabs:main Jan 28, 2022
@vdice vdice deleted the ci/release-add-arm64 branch January 28, 2022 17:45
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.

None yet

2 participants