-
Notifications
You must be signed in to change notification settings - Fork 21
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
docs: Document how to ship pre-built tools #117
Conversation
@@ -28,6 +24,11 @@ Ready to get started? Copy this repo, then | |||
1. (optional) install the [Renovate app](https://github.com/apps/renovate) to get auto-PRs to keep the dependencies up-to-date. | |||
1. delete this section of the README (everything up to the SNIP). | |||
|
|||
Optional: if you write tools for your rules to call, you should avoid toolchain dependencies for those tools leaking to users. | |||
For example, https://github.com/aspect-build/rules_py actions rely on a couple of binaries written in Rust, but we don't want users to be forced to | |||
fetch a working Rust toolchain. Instead we want to ship pre-built binaries on our GH releases, and the ruleset fetches these as toolchains. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add that there should always be an option for users to override pre-built binaries.
Pre-built binaries will not necessarily work on all platforms that a user cares about. E.g. on Linux pre-built binaries often end up being dynamically linked, meaning they likely won't work on NixOS due to mismatching loader paths, and linking against libc with some specific version constraints that might not work on the user's distribution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's implied by the toolchain registration mechanism that end users could always register a matching toolchain first that points to a target that's built from source. I wonder if that requires a longer example in some documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a sentence about it - could you approve if this looks good now pls?
Add note about allowing users to opt-in for built-from-source tools
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! That looks good.
Fixes #57