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

Fix Zig setup step in CI #1252

Merged
merged 1 commit into from
Jun 13, 2022
Merged

Fix Zig setup step in CI #1252

merged 1 commit into from
Jun 13, 2022

Conversation

hronro
Copy link
Contributor

@hronro hronro commented Jun 7, 2022

Fix the broken CI of building aarch64 binaries.

And I've tested it in my forked repo: https://github.com/hronro/grpc-web/actions/runs/2451424475

Comment on lines 20 to 22
sudo mkdir /zig
curl 'https://ziglang.org/download/0.9.1/zig-linux-x86_64-0.9.1.tar.xz' | sudo tar xJ --strip-components=1 --directory=/zig
sudo ln -s /zig/zig /usr/bin/zig
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for the fix!

Curious does this have to happen under root directory? If not, would sudo still be necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's about installing for all users (need root permission) or installing for the current user (don't need permission). I think both of them are OK in CI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ahh right.. If you don't mind, i wonder if you could install it for the current user? (in general i'd want to minimize root access unless strictly necessary). Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah the change is slightly more complicated than i thought.. Thanks a lot! :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol do you have any simpler solutions?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not necessarily.. I was thinking maybe a local bin already exists and/or can be directly downloaded into.. but this works.. 😄

Copy link
Collaborator

@sampajano sampajano left a comment

Choose a reason for hiding this comment

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

thanks so much for the change!! :)

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.

None yet

2 participants