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.

Sign up for GitHub

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

add support for optional deps in otp24 #885

Closed
wants to merge 1 commit into from
Closed

add support for optional deps in otp24 #885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 13 additions & 7 deletions src/rlx_app_info.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,13 +36,14 @@
%%%
-module(rlx_app_info).

-export([new/5,
new/6,
-export([new/6,
new/7,
name/1,
vsn/1,
dir/1,
applications/1,
included_applications/1,
optional_applications/1,
link/1,
link/2,
format_error/1,
Expand Down Expand Up @@ -71,17 +72,18 @@
-export_type([t/0,
app_type/0]).

-spec new(atom(), string(), file:name(), [atom()], [atom()]) -> t().
new(Name, Vsn, Dir, Applications, IncludedApplications) ->
new(Name, Vsn, Dir, Applications, IncludedApplications, dep).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is technically a backwards-incompatible change since this is the one bit of the public API users of relx must call. I don't know if we want to provide the sort of "match on the app_type() and know which signature this is" backwards-compatible behavior instead.

Like:

new(Name, Vsn, Dir, Applications, IncludedApplications) ->
    new(Name, Vsn, Dir, Applications, IncludedApplications, [], dep).

new(Name, Vsn, Dir, Applications, IncludedApplications, OptionalApplications) when is_list(OptionalApplications) ->
    new(Name, Vsn, Dir, Applications, IncludedApplications, OptionalApplications, dep);
new(Name, Vsn, Dir, Applications, IncludedApplications, AppType) when is_atom(AppType) ->
    new(Name, Vsn, Dir, Applications, IncludedApplications, OptionalApplications, [], AppType).

...

These tend to make the type specs real messy though it'd be fine keeping the new ones only.

-spec new(atom(), string(), file:name(), [atom()], [atom()], [atom()]) -> t().
new(Name, Vsn, Dir, Applications, IncludedApplications, OptionalApplications) ->
new(Name, Vsn, Dir, Applications, IncludedApplications, OptionalApplications, dep).

-spec new(atom(), string(), file:name(), [atom()], [atom()], app_type()) -> t().
new(Name, Vsn, Dir, Applications, IncludedApplications, AppType) ->
-spec new(atom(), string(), file:name(), [atom()], [atom()], [atom()], app_type()) -> t().
new(Name, Vsn, Dir, Applications, IncludedApplications, OptionalApplications, AppType) ->
#{name => Name,
vsn => Vsn,

applications => Applications,
included_applications => IncludedApplications,
optional_applications => OptionalApplications,

dir => Dir,
link => false,
Expand All @@ -108,6 +110,10 @@ applications(#{applications := Deps}) ->
included_applications(#{included_applications := Deps}) ->
Deps.

-spec optional_applications(t()) -> [atom()].
optional_applications(#{included_applications := Deps}) ->
Deps.

-spec link(t()) -> boolean().
link(#{link := Link}) ->
Link.
Expand Down
40 changes: 29 additions & 11 deletions src/rlx_resolve.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,20 +56,32 @@ subset(Apps, World, LibDirs, CheckCodeLibDirs) ->
subset([], _World, _Seen, _LibDirs, _CheckCodeLibDirs, Acc) ->
Acc;
subset([Goal | Rest], World, Seen, LibDirs, CheckCodeLibDirs, Acc) ->
{Name, Vsn} = name_version(Goal),
{Name, Vsn, Optional} = name_version(Goal),
case sets:is_element(Name, Seen) of
true ->
subset(Rest, World, Seen, LibDirs, CheckCodeLibDirs, Acc);
_ ->
AppInfo=#{applications := Applications,
included_applications := IncludedApplications} =
find_app(Name, Vsn, World, LibDirs, CheckCodeLibDirs),
subset(Rest ++ Applications ++ IncludedApplications,
{Rest1, Acc1} =
case find_app(Name, Vsn, World, LibDirs, CheckCodeLibDirs) of
not_found when Optional == true ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
not_found when Optional == true ->
not_found when Optional ->

Note that this omits the little weird case where an included_application can't logically be an optional_application but this validation allows an included app to miss if optional. I don't think we should really bother fixing that aspect yet.

{Rest, Acc};
not_found ->
erlang:error(?RLX_ERROR({app_not_found, Name, Vsn}));
AppInfo = #{applications := Applications,
included_applications := IncludedApplications,
optional_applications := OptionalApplications0
} ->
OptionalApplications = [{optional, OApp} || OApp <- OptionalApplications0],
{Rest ++ (Applications -- OptionalApplications0)
++ IncludedApplications ++ OptionalApplications,
Acc ++ [AppInfo]}
end,
subset(Rest1,
World,
sets:add_element(Name, Seen),
LibDirs,
CheckCodeLibDirs,
Acc ++ [AppInfo])
Acc1)
end.


Expand All @@ -96,9 +108,13 @@ set_resolved(Release0, Pkgs, State) ->
end.

name_version(Name) when is_atom(Name) ->
{Name, undefined};
{Name, undefined, false};
name_version({Name, #{vsn := Vsn}}) ->
{Name, Vsn}.
{Name, Vsn, false};
name_version({optional, Name}) when is_atom(Name) ->
{Name, undefined, true};
name_version({optional, {Name, #{vsn := Vsn}}}) ->
{Name, Vsn, true}.

remove_exclude_apps(AllApps, State) ->
ExcludeApps = rlx_state:exclude_apps(State),
Expand Down Expand Up @@ -144,7 +160,7 @@ search_for_app(Name, Vsn, LibDirs, CheckCodeLibDirs) ->
%% app not found in any lib dir we are configured to search
%% and user set a custom `system_libs' directory so we do
%% not look in `code:lib_dir'
erlang:error(?RLX_ERROR({app_not_found, Name, Vsn}));
not_found;
AppInfo ->
case check_app(Name, Vsn, AppInfo) of
true ->
Expand Down Expand Up @@ -175,13 +191,13 @@ find_app_in_dir(Name, Vsn, [Dir | Rest]) ->
find_app_in_code_path(Name, Vsn) ->
case code:lib_dir(Name) of
{error, bad_name} ->
erlang:error(?RLX_ERROR({app_not_found, Name, Vsn}));
not_found;
Dir ->
case to_app(Name, Vsn, filename:join([Dir, "ebin", [Name, ".app"]])) of
{true, AppInfo} ->
AppInfo;
false ->
erlang:error(?RLX_ERROR({app_not_found, Name, Vsn}))
not_found
end
end.

Expand All @@ -200,6 +216,7 @@ to_app(Name, Vsn, AppFilePath) ->
end,
Applications = proplists:get_value(applications, AppData, []),
IncludedApplications = proplists:get_value(included_applications, AppData, []),
OptionalApplications = proplists:get_value(optional_applications, AppData, []),

case lists:keyfind(vsn, 1, AppData) of
{_, Vsn1} when Vsn =:= undefined ;
Expand All @@ -209,6 +226,7 @@ to_app(Name, Vsn, AppFilePath) ->

applications => Applications,
included_applications => IncludedApplications,
optional_applications => OptionalApplications,

dir => filename:dirname(filename:dirname(AppFilePath)),
link => false}};
Expand Down
6 changes: 3 additions & 3 deletions test/rlx_test_utils.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,20 @@ create_app(Dir, Name, Vsn, Deps, LibDeps) ->
write_src_file(AppDir, Name),
write_priv_file(AppDir),
compile_src_files(AppDir),
rlx_app_info:new(erlang:list_to_atom(Name), Vsn, AppDir, Deps, []).
rlx_app_info:new(erlang:list_to_atom(Name), Vsn, AppDir, Deps, [], []).

create_full_app(Dir, Name, Vsn, Deps, LibDeps) ->
AppDir = filename:join([Dir, Name ++ "-" ++ Vsn]),
write_full_app_files(AppDir, Name, Vsn, Deps, LibDeps),
compile_src_files(AppDir),
rlx_app_info:new(erlang:list_to_atom(Name), Vsn, AppDir,
Deps, []).
Deps, [], []).

create_empty_app(Dir, Name, Vsn, Deps, LibDeps) ->
AppDir = filename:join([Dir, Name ++ "-" ++ Vsn]),
write_app_file(AppDir, Name, Vsn, [], Deps, LibDeps),
rlx_app_info:new(erlang:list_to_atom(Name), Vsn, AppDir,
Deps, []).
Deps, [], []).

app_modules(Name) ->
[list_to_atom(M ++ Name) ||
Expand Down