-
Notifications
You must be signed in to change notification settings - Fork 19
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
Set up rustler_precompiled
#18
Set up rustler_precompiled
#18
Conversation
Create workflow for `release` which will handle using GitHub Actions to pre-compile `fastrss` for us. This was created using the following as examples: - https://github.com/kloeckner-i/mail_parser/blob/fe20dc953ea10f86b1ff3ad8532866a733a406ea/.github/workflows/release.yml - https://github.com/elixir-nx/explorer/blob/7a6c3bf64a31a6ff7cce9ea4f5ef220e8e70a9db/.github/workflows/release.yml
- Change macOS target condition - Add workaround for cross-compiling `x86_64-unknown-linux-musl` - Add config to make smaller builds when releasing
Follow the advice from rustler_precompiled and configure Cross to pass down `RUSTLER_NIF_VERSION`.
c1e1d79
to
673ad86
Compare
The repo is still using `master`, not `main`
f3bde32
to
282b92c
Compare
@@ -0,0 +1,171 @@ | |||
name: Build precompiled NIFs |
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.
This is mostly a copy/paste of the various examples that are already out there
- https://github.com/philss/rustler_precompilation_example/blob/main/.github/workflows/release.yml
- https://github.com/kloeckner-i/mail_parser/blob/main/.github/workflows/release.yml
- https://github.com/elixir-nx/explorer/blob/main/.github/workflows/release.yml
The two main differences:
- The project name (
NIF_DIRECTORY
,working-directory
) - The use of
master
branch instead ofmain
to match the projects default/primary branch
|
||
1. Update [`CHANGELOG.md`](./CHANGELOG.md) to list the changes that will be included in the new version. | ||
2. Update the package version in [`README.md`](./README.md) and [`mix.exs`](./mix.exs). | ||
3. Commit the changes with a version bump commit, `git commit -m "Bump version"` . |
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.
This seems to be the commit phrase you use, so I suggested it to make copy/paste easier. We can drop or change it if there's a different workflow you'd prefer.
5. Create a new release in GitHub, ensuring you are pointing to the version bump commit created above. | ||
6. Wait. Creating a new release will kick off the release action which uses [`rustler_precompiled`](https://hexdocs.pm/rustler_precompiled/RustlerPrecompiled.html) to precompile the `fast_rss` Rust dependency. You must wait until this process is complete before releasing to Hex. | ||
7. Checkout the recently create tag. | ||
8. Once the NIFs are built, use `mix rustler_precompiled.download FastRss.Native --all --print` to download generate the checksum file. |
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.
This project has a step in their checklist to include the checksums in their GitHub release notes:
Copy the output of the mix task and add it to the release notes
You can see the results here. I'm assuming some combination of rustler_precompiled
and/or Hex will validate the checksum file automatically
The file generated will be named checksum-Elixir.RustlerPrecompilationExample.Native.exs and it's extremely important that you include this file in your Hex package (by updating the files: field in your mix.exs). Otherwise your package won't work.[^1]
I'm not sure if folks would feel better if they could see the checksums as well. explorer
didn't seem to do this, so I did not suggest it. If this is a step you would like to take as a part of releasing the project, I can definitely add a note to this checklist.
1. Update [`CHANGELOG.md`](./CHANGELOG.md) to list the changes that will be included in the new version. | ||
2. Update the package version in [`README.md`](./README.md) and [`mix.exs`](./mix.exs). | ||
3. Commit the changes with a version bump commit, `git commit -m "Bump version"` . | ||
4. Push the changes to GitHub. |
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.
Is this your preferred workflow? explorer
suggests opening a PR, but I didn't see any "version bump" PRs, so I'm assuming this is not your current workflow.
# Configuration for cross-compiling with `rustler_precompiled` | ||
# See https://hexdocs.pm/rustler_precompiled/precompilation_guide.html#additional-configuration-before-build | ||
|
||
[target.'cfg(target_os = "macos")'] |
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.
[target.'cfg(target_os = "macos")']
seem to cover both [target.x86_64-apple-darwin]
and [target.aarch64-apple-darwin]
. If you are not comfortable with the change, I can use the existing two entries.
@@ -1,11 +1,23 @@ | |||
[target.x86_64-apple-darwin] | |||
# Configuration for cross-compiling with `rustler_precompiled` | |||
# See https://hexdocs.pm/rustler_precompiled/precompilation_guide.html#additional-configuration-before-build |
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.
Essentially a copy/paste from https://hexdocs.pm/rustler_precompiled/precompilation_guide.html#additional-configuration-before-build
@@ -0,0 +1,6 @@ | |||
# Pass relevant config when cross-compiling as a part of release |
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.
7. Checkout the recently create tag. | ||
8. Once the NIFs are built, use `mix rustler_precompiled.download FastRss.Native --all --print` to download generate the checksum file. | ||
9. Run `mix hex.publish`. | ||
10. Bump the version in the `mix.exs` and add the `-dev` flag to it. |
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.
Adding the -dev
suffix to the version will force the NIF to build locally1. This may not be strictly necessary if you keep your local version matching the precompiled version or use the FORCE_BUILD
flag, but I decided to add this suggestion because:
Footnotes
@praveenperera, no rush, but I wanted to ping you to let you know I have taken this out of draft. If you still think this change could be useful, would you be open to adding a Footnotes |
@tmr08c done! Thanks for this I’ll review this on Monday. Remind me if I don’t by the end of the week |
@praveenperera – I am in no rush on this, but I wanted to ping you as you requested. Let me know if I can provide more annotations or explanations, or if something like a call would be helpful for you. |
@tmr08c thanks for pining me. Looks good. Merged! |
Thank you, @praveenperera! I see you cut a new release, and it looks like precompiled assets were attached, so that's promising. It doesn't look like the release has been pushed to Hex yet, so please let me know if you run into any issues with that. |
@tmr08c thanks for the reminder, I was waiting for the release to complete before publishing, and I guess I forgot to come back and publish. Its published now. Thanks again for the PR. |
My pleasure, @praveenperera; thank you for the project and for being open to trying out
This was definitely a negative side effect that I was worried about. Having to wait for all of the builds on release can definitely be a pain. If you find the cost is too high for the benefit, please let me know and I can revert the work. We could also reduce the number of build targets to reduce the amount of work that needs to be done. fast_rss/.github/workflows/release.yml Lines 32 to 40 in 367eeec
I upgraded the package version on a personal project and it used the precompiled binary.
It looks like everything works! |
FastRSS user here! It seems that the checksums file on hex.pm is called
Maybe it worked on your system because your filesystem is case-insensitive. Anyway, this should be an easy fix. BTW, congrats on the precompilation work! |
@thiagopromano I published 0.4.3 after running the correct command. Can you try it now? |
Still no good, I've checked and the file is still with the wrong name: https://preview.hex.pm/preview/fast_rss/0.4.3/show/checksum-Elixir.FastRss.Native.exs |
Hey @praveenperera, still no luck. I'm creating an issue just so other people find the conversation more easily. |
I’m not sure how the precompiled assets work. I ran the command with the correct case this time. So im not sure why hex has the wrong files. Any ideas? |
Closes #17
What
Adds
rustler_precompiled
as a dependency and creates a GitHub action(
release.yml
) to attach precompiled versions of thefastrss
crate asartifacts to the release.
The value proposition of
rustler_precompiled
is that it can reduce the barrierof entry for package users. By including a pre-compiled version of
fastrss
,users no longer need to have the Rust toolchain installed locally to use this
package.
This change does add a bit more overhead to the release workflow. To reduce some
of this friction, this change also includes a checklist (
RELEASE.md
) to followwhen creating a release.
(Manually) Testing
I did not test the full Hex.pm release, but I locally tested pulling from a release on my fork.
You can see here that the action successfully built all targets.
To use the release, I made the following change to
mix.exs
:and then tried a (clean) compile
On the first pass, they will warn you if you didn't generate the checksum file (this would be uploaded as a part of the flow documented in
release.md
).After following their suggestion, the compilation worked and includes a log, showing it fetched the precompiled asset from GitHub:
if I set
@version
to a fake version we don't have precopmiled assets for, it will fail: