Merge pull request #2982 from maennchen/jm/optional_reject

Ignore optional Hex package dependencies

Author: ferd

THANKS

@@ -148,3 +148,4 @@ Manas Chaudhari
 Luís Rascão
 Marko Minđek
 LEdoian
+Jonatan Männchen

apps/rebar/src/rebar.hrl

@@ -29,7 +29,7 @@
 -define(LOCK_FILE, "rebar.lock").
 -define(DEFAULT_COMPILER_SOURCE_FORMAT, relative).
 -define(DEFAULT_COMPILER_ERROR_FORMAT, rich). % 'minimal' for default values as of 3.23.0 and earlier
--define(PACKAGE_INDEX_VERSION, 6).
+-define(PACKAGE_INDEX_VERSION, 7).
 -define(PACKAGE_TABLE, package_index).
 -define(INDEX_FILE, "packages.idx").
 -define(HEX_AUTH_FILE, "hex.config").

apps/rebar/src/rebar_packages.erl

@@ -11,7 +11,7 @@
         ,resolve_version/6]).
 
 -ifdef(TEST).
--export([new_package_table/0, find_highest_matching_/5]).
+-export([new_package_table/0, find_highest_matching_/5, parse_deps/1]).
 -endif.
 
 -export_type([package/0]).
@@ -205,10 +205,16 @@ find_highest_matching_(Dep, DepVsn, #{name := Repo}, Table, State) when is_binar
 verify_table(State) ->
     ets:info(?PACKAGE_TABLE, named_table) =:= true orelse load_and_verify_version(State).
 
+%% Filter out optional dependencies. Rebar3 does not support optional
+%% dependencies, so we simply ignore them. For full parity with Mix, we would
+%% need to check if the dependency is included elsewhere (even transitively)
+%% without the optional flag, and only exclude it if all references are
+%% optional.
 parse_deps(Deps) ->
     [{maps:get(app, D, Name), {pkg, Name, Constraint, undefined, undefined}}
      || D=#{package := Name,
-            requirement := Constraint} <- Deps].
+            requirement := Constraint} <- Deps,
+        not maps:get(optional, D, false)].
 
 parse_checksum(<<Checksum:256/big-unsigned>>) ->
     list_to_binary(

apps/rebar/test/rebar_pkg_SUITE.erl

@@ -16,7 +16,8 @@
 
 all() -> [good_uncached, good_cached, badpkg, badhash_nocache,
           badindexchk, badhash_cache, bad_to_good, good_disconnect,
-          bad_disconnect, pkgs_provider, find_highest_matching].
+          bad_disconnect, pkgs_provider, find_highest_matching,
+          parse_deps_ignores_optional].
 
 init_per_suite(Config) ->
     application:start(meck),
@@ -250,6 +251,23 @@ find_highest_matching(_Config) ->
                                                        #{name => <<"hexpm">>}, ?PACKAGE_TABLE, State),
     ?assertEqual({{3,0,0},{[<<"rc">>,0],[]}}, Vsn3).
 
+parse_deps_ignores_optional(_Config) ->
+    %% Test that optional dependencies are filtered out
+    Deps = [
+        #{package => <<"req">>, requirement => <<"~> 1.0">>},
+        #{package => <<"opt">>, requirement => <<"~> 2.0">>, optional => true},
+        #{package => <<"exp">>, requirement => <<"~> 3.0">>, optional => false},
+        #{package => <<"ali">>, requirement => <<"4.0">>, app => <<"alias">>},
+        #{package => <<"ali_opt">>, requirement => <<"~> 5.0">>,
+          app => <<"alias2">>, optional => true}
+    ],
+    Result = rebar_packages:parse_deps(Deps),
+    ?assertEqual(3, length(Result)),
+    ?assertMatch([{<<"req">>, {pkg, <<"req">>, <<"~> 1.0">>, _, _}},
+                  {<<"exp">>, {pkg, <<"exp">>, <<"~> 3.0">>, _, _}},
+                  {<<"alias">>, {pkg, <<"ali">>, <<"4.0">>, _, _}}],
+                 Result).
+
 %%%%%%%%%%%%%%%
 %%% Helpers %%%
 %%%%%%%%%%%%%%%