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

Rustler precompiled fails to verify NIF checksum #35

Closed
inaniyants opened this issue Mar 1, 2023 · 7 comments
Closed

Rustler precompiled fails to verify NIF checksum #35

inaniyants opened this issue Mar 1, 2023 · 7 comments

Comments

@inaniyants
Copy link

inaniyants commented Mar 1, 2023

Hello and thanks for delivering such useful update as precompiled NIFs!

It looks like 0.7.0 release on hex contains wrong checksums file.
You can see, that some dependent packages are failing to update to 0.7.0 , like in this PR
poanetwork/ex_abi#122

I have few suggestions:

  1. add checksums file to .gitignore, cause it doesn't need to be tracked in repo
  2. add release script , which will ensure update of checksums before publish to hex. It can be simple Makefile
release:
    mix rustler_precompiled.download ExKeccak --all --print
    mix hex.publish --yes

then just make release.

In such way won't be needed to commit checksums on each release and won't forget to update it before publishing to hex.

@inaniyants
Copy link
Author

And one side question.
It is still stated in Readme, that "ex_keccak requires Rust to be installed."

I believe with precompiled NIFs it is not required for library users, since rust code is already compiled.
Am I correct ?

@tzumby
Copy link
Collaborator

tzumby commented Mar 1, 2023

Hey @inaniyants,

Thanks for opening this and pointing the problem out! I missed that line in the README.md, I'll update, you definitely don't need Rust unless you want to force compile.

Let me do some digging here on the checksum. I definitely think publishing to hex would be nice as part of the action.

@tzumby
Copy link
Collaborator

tzumby commented Mar 1, 2023

It looks like the authors of the rustler-precompiled did intend for everyone to check-in the checksum file in the repos. That's what I saw in all the libraries that I checked as well. The mismatch is a problem for sure, I'm going to figure out a solution to make sure this doesn't happen in the future.

@inaniyants
Copy link
Author

the authors of the rustler-precompiled did intend for everyone to check-in the checksum file in the repos'

I think there it is meant to include file into hex repo, but not into source code repo.

Here is ref, where I originally found recommendation to not include it to source code repo
https://github.com/philss/rustler_precompiled/blob/main/PRECOMPILATION_GUIDE.md

Note: you don't need to track the checksum file in your version control system (git or other).

@inaniyants
Copy link
Author

I definitely think publishing to hex would be nice as part of the action.

Probably this will help
https://hex.pm/docs/publish#publishing-from-ci
but your hex key should be somewhere stored .
Does github provide this storage for secret keys ?

@tzumby
Copy link
Collaborator

tzumby commented Mar 1, 2023

I just pushed a 0.7.1 update with the right checksums, do you mind re-running your CI with the version bump to see if that fixes it ?

@inaniyants
Copy link
Author

Yeah, new build is ok
poanetwork/ex_abi#123

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

No branches or pull requests

2 participants