-
Notifications
You must be signed in to change notification settings - Fork 518
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
allow use deps on ERL_LIBS in escriptize #2552
Conversation
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.
Let me know if I misunderstood something about this situation.
error -> case find_external_app(Name) of | ||
error -> throw(?PRV_ERROR({bad_app, binary_to_atom(Name, utf8)})); | ||
App0 -> App0 | ||
end |
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.
I was about to merge this and I had to do a second take. The apps that are being resolved here are apps that have been declared as part of your project, as defined by rebar_state:all_deps(State)++rebar_state:project_apps(State)
(line 120). These should actually never contain anything in ERL_LIBS
because they're built dynamically from what fits within rebar.config and is resolved recursively.
You can actually see the resolving of code:lib_dir/1
dependencies below on line 276 (279 in your patch). Those are dependencies that should be declared in the .app or .app.src files, but not within rebar.config, since rebar.config assumes it's fetching all the deps declared there.
I believe you see the issue you're having here because there's a disconnection between how you're using the dependencies in rebar3's config (for them to be fetched) and where they're supposed to be used for runtime dependencies (in .app or .app.src files).
I believe this PR is the wrong fix for your problem.
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.
Hi @ferd, the code on line 278 (281 here) skips dependencies which are in code:root_dir()
, ie. part of OTP, but not apps which are in other directories but still on ERL_LIBS
. These are included in BinDepNames
but the next call to find_deps_of_deps
fails since these deps are not in rebar_state:all_deps(State)++rebar_state:project_apps(State)
(I haven't added them to rebar.config
).
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.
Hm yeah, the recursive call is re-adding them there, and it makes sense at that point. Yeah the fix would make sense then since the input set is no longer the rebar.config apps anymore. My bad for the bad code reading.
Support ERL_LIBS libraries in escript building
This brings the behaviour of
rebar3 escriptize
in line withrebar3 release
by enabling it to bundle compiled dependencies found on ERL_LIBS (excluding OTP apps).Currently,
rebar3 escriptize
fails when the escript's.app
file specifies a dependency which is available on ERL_LIBS but not specified inrebar3.config
.Fixes #2551