Merge pull request #1721 from tsloughter/no-constraint-match

rebar_package: do not return first package version for constraint with no match

2 files changed, 19 insertions, 6 deletions

diff --git a/src/rebar_packages.erl b/src/rebar_packages.erl
index d17b54f..7910fe2 100644
--- a/src/rebar_packages.erl
+++ b/src/rebar_packages.erl
@@ -170,8 +170,13 @@ find_highest_matching_(Pkg, PkgVsn, Dep, Constraint, Table, State) ->
     try find_all(Dep, Table, State) of
         {ok, [Vsn]} ->
             handle_single_vsn(Pkg, PkgVsn, Dep, Vsn, Constraint);
-        {ok, [HeadVsn | VsnTail]} ->
-                            {ok, handle_vsns(Constraint, HeadVsn, VsnTail)}
+        {ok, Vsns} ->
+            case handle_vsns(Constraint, Vsns) of
+                none ->
+                    none;
+                FoundVsn ->
+                    {ok, FoundVsn}
+            end
     catch
         error:badarg ->
             none
@@ -189,16 +194,16 @@ find_all(Dep, Table, State) ->
             none
     end.
 
-handle_vsns(Constraint, HeadVsn, VsnTail) ->
+handle_vsns(Constraint, Vsns) ->
     lists:foldl(fun(Version, Highest) ->
                         case ec_semver:pes(Version, Constraint) andalso
-                            ec_semver:gt(Version, Highest) of
+                            (Highest =:= none orelse ec_semver:gt(Version, Highest)) of
                             true ->
                                 Version;
                             false ->
                                 Highest
                         end
-                end, HeadVsn, VsnTail).
+                end, none, Vsns).
 
 handle_single_vsn(Pkg, PkgVsn, Dep, Vsn, Constraint) ->
     case ec_semver:pes(Vsn, Constraint) of
diff --git a/test/rebar_pkg_SUITE.erl b/test/rebar_pkg_SUITE.erl
index 32873c8..8ddf58f 100644
--- a/test/rebar_pkg_SUITE.erl
+++ b/test/rebar_pkg_SUITE.erl
@@ -226,7 +226,12 @@ find_highest_matching(_Config) ->
     ?assertEqual(<<"1.1.1">>, Vsn1),
     {ok, Vsn2} = rebar_packages:find_highest_matching(
                    <<"test">>, <<"1.0.0">>, <<"goodpkg">>, <<"2.0">>, package_index, State),
-    ?assertEqual(<<"2.0.0">>, Vsn2).
+    ?assertEqual(<<"2.0.0">>, Vsn2),
+
+    %% regression test. ~> constraints higher than the available packages would result
+    %% in returning the first package version instead of 'none'.
+    ?assertEqual(none, rebar_packages:find_highest_matching(<<"test">>, <<"1.0.0">>, <<"goodpkg">>,
+                                                            <<"~> 5.0">>, package_index, State)).
 
 
 %%%%%%%%%%%%%%%
@@ -267,6 +272,9 @@ mock_config(Name, Config) ->
     meck:expect(rebar_packages, package_dir, fun(_) -> {ok, CacheDir} end),
     rebar_prv_update:hex_to_index(rebar_state:new()),
 
+    meck:new(rebar_prv_update, [passthrough]),
+    meck:expect(rebar_prv_update, do, fun(State) -> {ok, State} end),
+
     %% Cache fetches are mocked -- we assume the server and clients are
     %% correctly used.
     GoodCache = ?config(good_cache, Config),