From e9e62657c904f7009565c8792866496de72cc87b Mon Sep 17 00:00:00 2001 From: "Heinz N. Gies" Date: Fri, 19 Feb 2016 16:59:35 -0500 Subject: Add more hex rules so they don't throw errors Add more version constraints Allow for any number of whitespaces after compairison opperator Improve updating and error printing Fix failing tests --- src/rebar_packages.erl | 40 ++++++++++----- src/rebar_prv_update.erl | 122 +++++++++++++++++++++++++++++++++++++++++----- test/rebar_deps_SUITE.erl | 112 +++++++++++++++++++++++++++++++++++------- test/rebar_pkg_SUITE.erl | 9 ++-- 4 files changed, 239 insertions(+), 44 deletions(-) diff --git a/src/rebar_packages.erl b/src/rebar_packages.erl index c56009e..b5e0923 100644 --- a/src/rebar_packages.erl +++ b/src/rebar_packages.erl @@ -7,7 +7,9 @@ ,registry_dir/1 ,package_dir/1 ,registry_checksum/2 + ,find_highest_matching/6 ,find_highest_matching/4 + ,find_all/3 ,verify_table/1 ,format_error/1]). @@ -139,16 +141,26 @@ registry_checksum({pkg, Name, Vsn}, State) -> %% `~> 2.0` | `>= 2.0.0 and < 3.0.0` %% `~> 2.1` | `>= 2.1.0 and < 3.0.0` find_highest_matching(Dep, Constraint, Table, State) -> + find_highest_matching(undefined, undefined, Dep, Constraint, Table, State). + +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)} + catch + error:badarg -> + none + end. + +find_all(Dep, Table, State) -> ?MODULE:verify_table(State), try ets:lookup_element(Table, Dep, 2) of - [[HeadVsn | VsnTail]] -> - {ok, handle_vsns(Constraint, HeadVsn, VsnTail)}; - [[Vsn]] -> - handle_single_vsn(Dep, Vsn, Constraint); - [Vsn] -> - handle_single_vsn(Dep, Vsn, Constraint); - [HeadVsn | VsnTail] -> - {ok, handle_vsns(Constraint, HeadVsn, VsnTail)} + [Vsns] when is_list(Vsns)-> + {ok, Vsns}; + Vsns -> + {ok, Vsns} catch error:badarg -> none @@ -165,13 +177,19 @@ handle_vsns(Constraint, HeadVsn, VsnTail) -> end end, HeadVsn, VsnTail). -handle_single_vsn(Dep, Vsn, Constraint) -> +handle_single_vsn(Pkg, PkgVsn, Dep, Vsn, Constraint) -> case ec_semver:pes(Vsn, Constraint) of true -> {ok, Vsn}; false -> - ?WARN("Only existing version of ~s is ~s which does not match constraint ~~> ~s. " - "Using anyway, but it is not guaranteed to work.", [Dep, Vsn, Constraint]), + case {Pkg, PkgVsn} of + {undefined, undefined} -> + ?WARN("Only existing version of ~s is ~s which does not match constraint ~~> ~s. " + "Using anyway, but it is not guaranteed to work.", [Dep, Vsn, Constraint]); + _ -> + ?WARN("[~s:~s] Only existing version of ~s is ~s which does not match constraint ~~> ~s. " + "Using anyway, but it is not guaranteed to work.", [Pkg, PkgVsn, Dep, Vsn, Constraint]) + end, {ok, Vsn} end. diff --git a/src/rebar_prv_update.erl b/src/rebar_prv_update.erl index 0e3b9a0..5e1e253 100644 --- a/src/rebar_prv_update.erl +++ b/src/rebar_prv_update.erl @@ -11,6 +11,10 @@ -export([hex_to_index/1]). +-ifdef(TEST). +-export([cmp_/6, cmpl_/6, valid_vsn/1]). +-endif. + -include("rebar.hrl"). -include_lib("providers/include/providers.hrl"). @@ -99,7 +103,7 @@ hex_to_index(State) -> ets:foldl(fun({{Pkg, PkgVsn}, [Deps, Checksum, BuildTools | _]}, _) when is_list(BuildTools) -> case lists:any(fun is_supported/1, BuildTools) of true -> - DepsList = update_deps_list(Deps, Registry, State), + DepsList = update_deps_list(Pkg, PkgVsn, Deps, Registry, State), ets:insert(?PACKAGE_TABLE, {{Pkg, PkgVsn}, DepsList, Checksum}); false -> true @@ -137,20 +141,114 @@ hex_to_index(State) -> fail end. -update_deps_list(Deps, HexRegistry, State) -> +update_deps_list(Pkg, PkgVsn, Deps, HexRegistry, State) -> lists:foldl(fun([Dep, DepVsn, false, _AppName | _], DepsListAcc) -> - case DepVsn of - <<"~> ", Vsn/binary>> -> - case rebar_packages:find_highest_matching(Dep, Vsn, HexRegistry, State) of - {ok, HighestDepVsn} -> - [{Dep, HighestDepVsn} | DepsListAcc]; - none -> - ?WARN("Missing registry entry for package ~s. Try to fix with `rebar3 update`", [Dep]), - DepsListAcc - end; - Vsn -> + Dep1 = {Pkg, PkgVsn, Dep}, + case {valid_vsn(DepVsn), DepVsn} of + %% Those are all not perfectly implemented! + %% and doubled since spaces seem not to be + %% enforced + {false, Vsn} -> + ?WARN("[~s:~s], Bad dependency version for ~s: ~s.", + [Pkg, PkgVsn, Dep, Vsn]), + DepsListAcc; + {_, <<"~>", Vsn/binary>>} -> + highest_matching(Dep1, rm_ws(Vsn), HexRegistry, + State, DepsListAcc); + {_, <<">=", Vsn/binary>>} -> + cmp(Dep1, rm_ws(Vsn), HexRegistry, State, + DepsListAcc, fun ec_semver:gte/2); + {_, <<">", Vsn/binary>>} -> + cmp(Dep1, rm_ws(Vsn), HexRegistry, State, + DepsListAcc, fun ec_semver:gt/2); + {_, <<"<=", Vsn/binary>>} -> + cmpl(Dep1, rm_ws(Vsn), HexRegistry, State, + DepsListAcc, fun ec_semver:lte/2); + {_, <<"<", Vsn/binary>>} -> + cmpl(Dep1, rm_ws(Vsn), HexRegistry, State, + DepsListAcc, fun ec_semver:lt/2); + {_, <<"==", Vsn/binary>>} -> + [{Dep, Vsn} | DepsListAcc]; + {_, Vsn} -> [{Dep, Vsn} | DepsListAcc] end; ([_Dep, _DepVsn, true, _AppName | _], DepsListAcc) -> DepsListAcc end, [], Deps). + +rm_ws(<<" ", R/binary>>) -> + rm_ws(R); +rm_ws(R) -> + R. + +valid_vsn(Vsn) -> + %% Regepx from https://github.com/sindresorhus/semver-regex/blob/master/index.js + SemVerRegExp = "v?(0|[1-9][0-9]*)\\.(0|[1-9][0-9]*)(\\.(0|[1-9][0-9]*))?" + "(-[0-9a-z-]+(\\.[0-9a-z-]+)*)?(\\+[0-9a-z-]+(\\.[0-9a-z-]+)*)?", + SupportedVersions = "^(>=?|<=?|~>|==)?\\s*" ++ SemVerRegExp ++ "$", + re:run(Vsn, SupportedVersions) =/= nomatch. + +highest_matching({Pkg, PkgVsn, Dep}, Vsn, HexRegistry, State, DepsListAcc) -> + case rebar_packages:find_highest_matching(Pkg, PkgVsn, Dep, Vsn, HexRegistry, State) of + {ok, HighestDepVsn} -> + [{Dep, HighestDepVsn} | DepsListAcc]; + none -> + ?WARN("[~s:~s] Missing registry entry for package ~s. Try to fix with `rebar3 update`", + [Pkg, PkgVsn, Dep]), + DepsListAcc + end. + +cmp({_Pkg, _PkgVsn, Dep} = Dep1, Vsn, HexRegistry, State, DepsListAcc, CmpFun) -> + {ok, Vsns} = rebar_packages:find_all(Dep, HexRegistry, State), + cmp_(undefined, Vsn, Vsns, DepsListAcc, Dep1, CmpFun). + + +cmp_(undefined, _MinVsn, [], DepsListAcc, {Pkg, PkgVsn, Dep}, _CmpFun) -> + ?WARN("[~s:~s] Missing registry entry for package ~s. Try to fix with `rebar3 update`", + [Pkg, PkgVsn, Dep]), + DepsListAcc; +cmp_(HighestDepVsn, _MinVsn, [], DepsListAcc, {_Pkg, _PkgVsn, Dep}, _CmpFun) -> + [{Dep, HighestDepVsn} | DepsListAcc]; + +cmp_(BestMatch, MinVsn, [Vsn | R], DepsListAcc, Dep, CmpFun) -> + case CmpFun(Vsn, MinVsn) of + true -> + cmp_(Vsn, Vsn, R, DepsListAcc, Dep, CmpFun); + false -> + cmp_(BestMatch, MinVsn, R, DepsListAcc, Dep, CmpFun) + end. + +%% We need to treat this differently since we want a version that is LOWER but +%% the higest possible one. +cmpl({_Pkg, _PkgVsn, Dep} = Dep1, Vsn, HexRegistry, State, DepsListAcc, CmpFun) -> + {ok, Vsns} = rebar_packages:find_all(Dep, HexRegistry, State), + cmpl_(undefined, Vsn, Vsns, DepsListAcc, Dep1, CmpFun). + +cmpl_(undefined, _MaxVsn, [], DepsListAcc, {Pkg, PkgVsn, Dep}, _CmpFun) -> + ?WARN("[~s:~s] Missing registry entry for package ~s. Try to fix with `rebar3 update`", + [Pkg, PkgVsn, Dep]), + DepsListAcc; + +cmpl_(HighestDepVsn, _MaxVsn, [], DepsListAcc, {_Pkg, _PkgVsn, Dep}, _CmpFun) -> + [{Dep, HighestDepVsn} | DepsListAcc]; + +cmpl_(undefined, MaxVsn, [Vsn | R], DepsListAcc, Dep, CmpFun) -> + case CmpFun(Vsn, MaxVsn) of + true -> + cmpl_(Vsn, MaxVsn, R, DepsListAcc, Dep, CmpFun); + false -> + cmpl_(undefined, MaxVsn, R, DepsListAcc, Dep, CmpFun) + end; + +cmpl_(BestMatch, MaxVsn, [Vsn | R], DepsListAcc, Dep, CmpFun) -> + case CmpFun(Vsn, MaxVsn) of + true -> + case ec_semver:gte(Vsn, BestMatch) of + true -> + cmpl_(Vsn, MaxVsn, R, DepsListAcc, Dep, CmpFun); + false -> + cmpl_(BestMatch, MaxVsn, R, DepsListAcc, Dep, CmpFun) + end; + false -> + cmpl_(BestMatch, MaxVsn, R, DepsListAcc, Dep, CmpFun) + end. diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl index fcc46c3..c95854a 100644 --- a/test/rebar_deps_SUITE.erl +++ b/test/rebar_deps_SUITE.erl @@ -3,7 +3,7 @@ -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). -all() -> [sub_app_deps, newly_added_dep, newly_added_after_empty_lock, http_proxy_settings, https_proxy_settings, {group, git}, {group, pkg}]. +all() -> [sub_app_deps, newly_added_dep, newly_added_after_empty_lock, http_proxy_settings, https_proxy_settings, semver_matching_lt, semver_matching_lte, semver_matching_gt, valid_version, {group, git}, {group, pkg}]. groups() -> [{all, [], [flat, pick_highest_left, pick_highest_right, @@ -29,6 +29,14 @@ init_per_group(_, Config) -> end_per_group(_, Config) -> Config. +init_per_testcase(valid_version, Config) -> + rebar_test_utils:init_rebar_state(Config); +init_per_testcase(semver_matching_lt, Config) -> + rebar_test_utils:init_rebar_state(Config); +init_per_testcase(semver_matching_lte, Config) -> + rebar_test_utils:init_rebar_state(Config); +init_per_testcase(semver_matching_gt, Config) -> + rebar_test_utils:init_rebar_state(Config); init_per_testcase(newly_added_after_empty_lock, Config) -> rebar_test_utils:init_rebar_state(Config); init_per_testcase(newly_added_dep, Config) -> @@ -49,14 +57,14 @@ init_per_testcase(http_proxy_settings, Config) -> %% Insert proxy variables into config rebar_test_utils:create_config(GlobalConfigDir, [{http_proxy, "http://localhost:1234"} - ]), + ]), rebar_test_utils:init_rebar_state(Config); init_per_testcase(https_proxy_settings, Config) -> SupportsHttpsProxy = case erlang:system_info(otp_release) of - "R16"++_ -> true; - "R"++_ -> false; - _ -> true % 17 and up don't have a "R" in the version - end, + "R16"++_ -> true; + "R"++_ -> false; + _ -> true % 17 and up don't have a "R" in the version + end, if not SupportsHttpsProxy -> {skip, https_proxy_unsupported_before_R16}; SupportsHttpsProxy -> @@ -73,20 +81,20 @@ init_per_testcase(https_proxy_settings, Config) -> %% Insert proxy variables into config rebar_test_utils:create_config(GlobalConfigDir, [{https_proxy, "http://localhost:1234"} - ]), + ]), rebar_test_utils:init_rebar_state(Config) end; init_per_testcase(Case, Config) -> {Deps, Warnings, Expect} = deps(Case), Expected = case Expect of - {ok, List} -> {ok, format_expected_deps(List)}; - {error, Reason} -> {error, Reason} - end, + {ok, List} -> {ok, format_expected_deps(List)}; + {error, Reason} -> {error, Reason} + end, DepsType = ?config(deps_type, Config), mock_warnings(), [{expect, Expected}, {warnings, Warnings} - | setup_project(Case, Config, rebar_test_utils:expand_deps(DepsType, Deps))]. + | setup_project(Case, Config, rebar_test_utils:expand_deps(DepsType, Deps))]. end_per_testcase(https_proxy_settings, Config) -> meck:unload(rebar_dir), @@ -100,8 +108,8 @@ end_per_testcase(_, Config) -> format_expected_deps(Deps) -> [case Dep of - {N,V} -> {dep, N, V}; - N -> {dep, N} + {N,V} -> {dep, N, V}; + N -> {dep, N} end || Dep <- Deps]. %% format: @@ -208,7 +216,7 @@ sub_app_deps(Config) -> SubAppsDir = filename:join([AppDir, "apps", Name]), SubDeps = rebar_test_utils:top_level_deps(rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []} - ,{"b", "2.0.0", []}])), + ,{"b", "2.0.0", []}])), rebar_test_utils:create_app(SubAppsDir, Name, Vsn, [kernel, stdlib]), rebar_test_utils:create_config(SubAppsDir, [{deps, SubDeps}]), @@ -242,12 +250,12 @@ newly_added_dep(Config) -> %% Add a and c to top level TopDeps2 = rebar_test_utils:top_level_deps(rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []} - ,{"c", "2.0.0", []} - ,{"b", "1.0.0", []}])), + ,{"c", "2.0.0", []} + ,{"b", "1.0.0", []}])), {ok, RebarConfig2} = file:consult(rebar_test_utils:create_config(AppDir, [{deps, TopDeps2}])), LockFile = filename:join(AppDir, "rebar.lock"), RebarConfig3 = rebar_config:merge_locks(RebarConfig2, - rebar_config:consult_lock_file(LockFile)), + rebar_config:consult_lock_file(LockFile)), %% a should now be installed and c should not change rebar_test_utils:run_and_check( @@ -277,7 +285,7 @@ newly_added_after_empty_lock(Config) -> {ok, RebarConfig2} = file:consult(rebar_test_utils:create_config(AppDir, [{deps, TopDeps2}])), LockFile = filename:join(AppDir, "rebar.lock"), RebarConfig3 = rebar_config:merge_locks(RebarConfig2, - rebar_config:consult_lock_file(LockFile)), + rebar_config:consult_lock_file(LockFile)), %% a should now be installed and c should not change rebar_test_utils:run_and_check( @@ -304,6 +312,74 @@ https_proxy_settings(_Config) -> httpc:get_option(https_proxy, rebar)). +semver_matching_lt(_Config) -> + Dep = <<"test">>, + Dep1 = {Dep, <<"1.0.0">>, Dep}, + MaxVsn = <<"0.2.0">>, + Vsns = [<<"0.1.7">>, <<"0.1.9">>, <<"0.1.8">>, <<"0.2.0">>, <<"0.2.1">>], + ?assertEqual([{Dep, <<"0.1.9">>}], + rebar_prv_update:cmpl_(undefined, MaxVsn, Vsns, [], Dep1, + fun ec_semver:lt/2)). + +semver_matching_lte(_Config) -> + Dep = <<"test">>, + Dep1 = {Dep, <<"1.0.0">>, Dep}, + MaxVsn = <<"0.2.0">>, + Vsns = [<<"0.1.7">>, <<"0.1.9">>, <<"0.1.8">>, <<"0.2.0">>, <<"0.2.1">>], + ?assertEqual([{Dep, <<"0.2.0">>}], + rebar_prv_update:cmpl_(undefined, MaxVsn, Vsns, [], Dep1, + fun ec_semver:lte/2)). + +semver_matching_gt(_Config) -> + Dep = <<"test">>, + Dep1 = {Dep, <<"1.0.0">>, Dep}, + MaxVsn = <<"0.2.0">>, + Vsns = [<<"0.1.7">>, <<"0.1.9">>, <<"0.1.8">>, <<"0.2.0">>, <<"0.2.1">>], + ?assertEqual([{Dep, <<"0.2.1">>}], + rebar_prv_update:cmp_(undefined, MaxVsn, Vsns, [], Dep1, + fun ec_semver:gt/2)). +semver_matching_gte(_Config) -> + Dep = <<"test">>, + Dep1 = {Dep, <<"1.0.0">>, Dep}, + MaxVsn = <<"0.2.0">>, + Vsns = [<<"0.1.7">>, <<"0.1.9">>, <<"0.1.8">>, <<"0.2.0">>], + ?assertEqual([{Dep, <<"0.2.0">>}], + rebar_prv_update:cmp_(undefined, MaxVsn, Vsns, [], Dep1, + fun ec_semver:gt/2)). + +valid_version(_Config) -> + ?assert(rebar_prv_update:valid_vsn(<<"0.1">>)), + ?assert(rebar_prv_update:valid_vsn(<<"0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<" 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<" 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"<0.1">>)), + ?assert(rebar_prv_update:valid_vsn(<<"<0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"< 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"< 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<">0.1">>)), + ?assert(rebar_prv_update:valid_vsn(<<">0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"> 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"> 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"<=0.1">>)), + ?assert(rebar_prv_update:valid_vsn(<<"<=0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"<= 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"<= 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<">=0.1">>)), + ?assert(rebar_prv_update:valid_vsn(<<">=0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<">= 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<">= 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"==0.1">>)), + ?assert(rebar_prv_update:valid_vsn(<<"==0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"== 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"== 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"~>0.1">>)), + ?assert(rebar_prv_update:valid_vsn(<<"~>0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"~> 0.1.0">>)), + ?assert(rebar_prv_update:valid_vsn(<<"~> 0.1.0">>)), + ?assertNot(rebar_prv_update:valid_vsn(<<"> 0.1.0 and < 0.2.0">>)), + ok. + + run(Config) -> {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), rebar_test_utils:run_and_check( diff --git a/test/rebar_pkg_SUITE.erl b/test/rebar_pkg_SUITE.erl index 9f19e0d..6a75f32 100644 --- a/test/rebar_pkg_SUITE.erl +++ b/test/rebar_pkg_SUITE.erl @@ -180,11 +180,14 @@ pkgs_provider(Config) -> find_highest_matching(_Config) -> State = rebar_state:new(), - {ok, Vsn} = rebar_packages:find_highest_matching(<<"goodpkg">>, <<"1.0.0">>, package_index, State), + {ok, Vsn} = rebar_packages:find_highest_matching( + <<"test">>, <<"1.0.0">>, <<"goodpkg">>, <<"1.0.0">>, package_index, State), ?assertEqual(<<"1.0.1">>, Vsn), - {ok, Vsn1} = rebar_packages:find_highest_matching(<<"goodpkg">>, <<"1.0">>, package_index, State), + {ok, Vsn1} = rebar_packages:find_highest_matching( + <<"test">>, <<"1.0.0">>, <<"goodpkg">>, <<"1.0">>, package_index, State), ?assertEqual(<<"1.1.1">>, Vsn1), - {ok, Vsn2} = rebar_packages:find_highest_matching(<<"goodpkg">>, <<"2.0">>, package_index, State), + {ok, Vsn2} = rebar_packages:find_highest_matching( + <<"test">>, <<"1.0.0">>, <<"goodpkg">>, <<"2.0">>, package_index, State), ?assertEqual(<<"2.0.0">>, Vsn2). -- cgit v1.1