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 cargo-zigbuild #508

Merged
merged 1 commit into from
Jun 6, 2024
Merged

Add cargo-zigbuild #508

merged 1 commit into from
Jun 6, 2024

Conversation

jayvdb
Copy link
Contributor

@jayvdb jayvdb commented Jun 5, 2024

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

You would also need to pip install ziglang before cargo-zigbuild is actually usable.

@jayvdb
Copy link
Contributor Author

jayvdb commented Jun 5, 2024

well the assumption is that https://github.com/goto-bus-stop/setup-zig or similar has been also run, either before or after.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Jun 5, 2024

well the assumption is that https://github.com/goto-bus-stop/setup-zig or similar has been also run, either before or after.

If I am the user, I would assume it has everything installed for cargo-zigbuild to be used.

You can special case cargo-zigbuild by doing a pip install ziglang

@jayvdb
Copy link
Contributor Author

jayvdb commented Jun 5, 2024

My understanding of this action is that it does not do extra installs that might be required to make a tool work.
e.g. installing cargo-valgrind does not also install valgrind ; installing xbuild does not also install adb and other bits of llvm not present on the github runner.

It is up to the user of this action to install dependencies that are missing, and that way they can choose the version they want of those dependencies.

I would support adding an extra tool called "ziglang", which would be a special case also like "valgrind". We could support installing different versions from https://pypi.org/project/ziglang/#history - that looks like a reasonable approach, but Python wheels are a messy format for this given that zig is a single binary, and pip is slow. Importing the binary URLs from https://ziglang.org/download/index.json would be more in tune with the way this action works, allowing runtime verification of the checksums.

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Yeah that makes sense, having a special support for ziglang along with cargo-zigbuild

@NobodyXu NobodyXu merged commit 45b6915 into taiki-e:main Jun 6, 2024
29 checks passed
@jayvdb jayvdb deleted the add-cargo-zigbuild branch June 6, 2024 08:38
@jayvdb
Copy link
Contributor Author

jayvdb commented Jun 6, 2024

Thanks @NobodyXu .

@taiki-e
Copy link
Owner

taiki-e commented Jun 7, 2024

Published in 2.36.0. Thanks @jayvdb for the PR and @NobodyXu for the review!

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.

3 participants