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

Inverted force_build priority for otp_app? #80

Open
paulgoetze opened this issue Jul 10, 2024 · 3 comments
Open

Inverted force_build priority for otp_app? #80

paulgoetze opened this issue Jul 10, 2024 · 3 comments

Comments

@paulgoetze
Copy link
Contributor

paulgoetze commented Jul 10, 2024

The current implementation prioritizes a force_build option from a NIF project "project_a" (as used in the example project) over the config :rustler_precompiled, :force_build, project_a: true config defined in "project_b" (which uses "project_a" as a dependency).

Hence, if I understand it correctly, configuring config :rustler_precompiled, :force_build, project_a: true in this case currently does not have any effect, because as soon as a force_build option is present, it can not be overwritten anymore (using Keyword.put_new here https://github.com/philss/rustler_precompiled/blob/main/lib/rustler_precompiled.ex#L161)

Is this behaviour on purpose?

@philss
Copy link
Owner

philss commented Jul 11, 2024

The intention was to prioritize the config that is set directly from the module definition. But thinking now, your proposal makes sense. We could make the application config a priority in case it is set, otherwise we leave as it is. WDYT?
And would you mind to send a PR?

@paulgoetze
Copy link
Contributor Author

👍 Yes, I think that makes sense, I'll send a PR.

@paulgoetze
Copy link
Contributor Author

Hm, now that I thought about it again, I must say that your current implementation actually appears to be more intuitive to me. Using the option if provided in the module and else falling back to the application config might actually be more predictable in how the force_build option works.

So I'll backpedal here and advocate for that it should be solved on the NIF Module level. 😅

Just as a reference, for the rustler_precompiled example app that would be something like:

# lib/rustler_precompilation_example/native.ex

  opts = [
    otp_app: :rustler_precompilation_example,
    # ...other rustler_precompiled options
  ]

  use RustlerPrecompiled,
    (if System.get_env("RUSTLER_PRECOMPILATION_EXAMPLE_BUILD") in ["1", "true"] do
      Keyword.put(opts, :force_build, true)
     else
      opts
    end)

which only passes the force_build option if the ENV var is actually given (here as "1" or "true", but this logic is then up to the NIF creator).

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