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

Rewrite build of NIF APIs in Rust #397

Merged
merged 1 commit into from
Nov 11, 2021
Merged

Rewrite build of NIF APIs in Rust #397

merged 1 commit into from
Nov 11, 2021

Conversation

philss
Copy link
Member

@philss philss commented Oct 22, 2021

This PR changes the way the NIF APIs are generated to be included in the rustler_sys_api.rs file by replacing the script that was written in Erlang to one written in Rust.

We need this change in order to simplify the cross-compilation of Rustler crates for different NIF version targets.

Before this change we were using an Erlang script to get which NIF version to use in the build process, and this required the presence of Erlang installed in the build system.

Now the dependency of Erlang is removed optional and a new requirement was added: in order to build a crate with target to release the environment variable RUSTLER_NIF_VERSION must should be present. EDIT: if RUSTLER_NIF_VERSION is not present, then we fetch that from Erlang or we use the latest available version. In the process of build creates for an Elixir project, the compiler will inject that env var so nothing changes for users.

This new requirement is not a problem if one is building a dependency from the Elixir side using the Rustler compiler, because we set that value when building the lib. (EDIT: is not a requirement anymore.)

Acknowledgements

Most of the rustler_sys/build.rs was work of @hansihe. Thank you for that! :D

I'm a newbie in Rust, but my friend @lsunsi helped me a lot with this PR. Thanks, migo! <2

PS: this PR includes some changes from #395 . After merge that one I should rebase this. done.
PS2: most of the changes are in the rustler_sys/build.rs file.
PS3: open as draft because I want to try adding some tests to that file. EDIT: I couldn't test the build.rs directly, but I think that relying on the compilation is good enough for now..

EDIT: this closes #388

@filmor
Copy link
Member

filmor commented Oct 23, 2021

Why not just default to the respective newest version? That's what the old build.rs did if it couldn't find escript as well. Also, a lot of this "code generation" can just be put into a proper module as it doesn't really change.

@philss
Copy link
Member Author

philss commented Oct 24, 2021

@filmor the reason is because I want to generate the NIF for multiple OTP versions without having to install each OTP in the build machine. I'm working on making the use of precompiled NIFs easier with Rustler. So this change would make easier for setting up the CI.

About the code generation: they are in the build.rs because we need prebuilt the NIF APIs before the lib is compiled. I'm not sure if we can achieve the same thing "dynamically".

@filmor
Copy link
Member

filmor commented Oct 24, 2021

@filmor the reason is because I want to generate the NIF for multiple OTP versions without having to install each OTP in the build machine. I'm working on making the use of precompiled NIFs easier with Rustler. So this change would make easier for setting up the CI.
I understand that, but CI is not the only use-case. And being able to quickly build rustler_sys on a random system using the matching NIF API for an installed Erlang/OTP or falling back to the newest one available is very useful for development. When I build libraries, I usually have the same version of OTP on the build system that I use in production, so the existing behaviour works just fine for me and setting an additional environment variable would be a hassle.

Additionally, having to set the NIF version requires relatively deep knowledge (to map the OTP version to the respective NIF version and even be aware of the difference).

About the code generation: they are in the build.rs because we need prebuilt the NIF APIs before the lib is compiled. I'm not sure if we can achieve the same thing "dynamically".
I'm not suggesting to do anything dynamically here. On the contrary, I'm suggesting moving the bulk of the "generated" APIs into proper Rust modules using normal macros for the "fancier" parts. This could be done as a second step, though, I see the advantage of just getting rid of the existing scripts.

@philss
Copy link
Member Author

philss commented Oct 26, 2021

I understand that, but CI is not the only use-case. And being able to quickly build rustler_sys on a random system using the matching NIF API for an installed Erlang/OTP or falling back to the newest one available is very useful for development. When I build libraries, I usually have the same version of OTP on the build system that I use in production, so the existing behaviour works just fine for me and setting an additional environment variable would be a hassle.

I agree. I will rollback the fallback to the installed version if the env var is not present. In case erl is not available I'm going to use the latest version with a "warning" (eprintln).

I'm not suggesting to do anything dynamically here. On the contrary, I'm suggesting moving the bulk of the "generated" APIs into proper Rust modules using normal macros for the "fancier" parts. This could be done as a second step, though, I see the advantage of just getting rid of the existing scripts.

Got it. I'm a beginner in Rust and honestly I don't know if I can do this with my current knowledge in the lang. I will try to figure it out 😅

rustler_sys/build.rs Outdated Show resolved Hide resolved
@philss philss changed the title Generate NIF APIs in Rust and require RUSTLER_NIF_VERSION env var Rewrite build of NIF APIs in Rust Oct 28, 2021
@philss philss marked this pull request as ready for review October 28, 2021 20:46
@philss philss requested a review from filmor October 28, 2021 20:54
This changes the way NIF APIs are generated by rewriting the build
script in Rust.  The idea is to remove the dependency of having
Erlang installed.

The way we do now is to read the `RUSTLER_NIF_VERSION` env var in
the `build.rs` files and use that version to generate the NIF API file
before compiling the rest of Rustler's code. If that environment var
is not available, then we try to read from the installed Erlang. If the
build machine doesn't have Erlang, then we use the latest supported
version and write a warning to the stderr of the scripts (you can find
the messages at `target/debug/build/<crate-name>-<hash>/stderr`).

Now all crates compiled directly from a project using Rustler will pass
the correct NIF version in the env var `RUSTLER_NIF_VERSION`. See the
Rustler compiler in Elixir for details.

This is a step to make easier the cross-compilation of Rustler
crates to different NIF versions without having to rely on the current
installed OTP.

We also don't need the precompiled snippets because we can always
generate these files now.
@filmor filmor merged commit 1de30df into rusterlium:master Nov 11, 2021
@filmor
Copy link
Member

filmor commented Nov 11, 2021

Thank you very much :)

@philss philss deleted the ps-gen-api-in-rust branch November 11, 2021 06:12
@evnu evnu mentioned this pull request Nov 17, 2021
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.

Port gen_api.erl to Rust so it can be run without escript installed
2 participants