-
Notifications
You must be signed in to change notification settings - Fork 28
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
Comments
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? |
👍 Yes, I think that makes sense, I'll send a PR. |
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 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 |
The current implementation prioritizes a
force_build
option from a NIF project "project_a" (as used in the example project) over theconfig :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 aforce_build
option is present, it can not be overwritten anymore (usingKeyword.put_new
here https://github.com/philss/rustler_precompiled/blob/main/lib/rustler_precompiled.ex#L161)Is this behaviour on purpose?
The text was updated successfully, but these errors were encountered: