Merge pull request #1337 from ferd/support-aliasing

Properly support package aliasing and alt names

4 files changed, 159 insertions, 30 deletions

diff --git a/src/rebar_prv_update.erl b/src/rebar_prv_update.erl
index 54b135e..75c609e 100644
--- a/src/rebar_prv_update.erl
+++ b/src/rebar_prv_update.erl
@@ -104,7 +104,8 @@ hex_to_index(State) ->
                                   case lists:any(fun is_supported/1, BuildTools) of
                                       true ->
                                           DepsList = update_deps_list(Pkg, PkgVsn, Deps, Registry, State),
-                                          ets:insert(?PACKAGE_TABLE, {{Pkg, PkgVsn}, DepsList, Checksum});
+                                          HashedDeps = update_deps_hashes(DepsList),
+                                          ets:insert(?PACKAGE_TABLE, {{Pkg, PkgVsn}, HashedDeps, Checksum});
                                       false ->
                                           true
                                   end;
@@ -142,8 +143,8 @@ hex_to_index(State) ->
     end.
 
 update_deps_list(Pkg, PkgVsn, Deps, HexRegistry, State) ->
-    lists:foldl(fun([Dep, DepVsn, false, _AppName | _], DepsListAcc) ->
-                        Dep1 = {Pkg, PkgVsn, Dep},
+    lists:foldl(fun([Dep, DepVsn, false, AppName | _], DepsListAcc) ->
+                        Dep1 = {Pkg, PkgVsn, Dep, AppName},
                         case {valid_vsn(DepVsn), DepVsn} of
                             %% Those are all not perfectly implemented!
                             %% and doubled since spaces seem not to be
@@ -168,14 +169,29 @@ update_deps_list(Pkg, PkgVsn, Deps, HexRegistry, State) ->
                                 cmpl(Dep1, rm_ws(Vsn), HexRegistry, State,
                                      DepsListAcc, fun ec_semver:lt/2);
                             {_, <<"==", Vsn/binary>>} ->
-                                [{Dep, Vsn} | DepsListAcc];
+                                [{AppName, {pkg, Dep, Vsn, undefined}} | DepsListAcc];
                             {_, Vsn} ->
-                                [{Dep, Vsn} | DepsListAcc]
+                                [{AppName, {pkg, Dep, Vsn, undefined}} | DepsListAcc]
                         end;
                    ([_Dep, _DepVsn, true, _AppName | _], DepsListAcc) ->
                         DepsListAcc
                 end, [], Deps).
 
+update_deps_hashes(List) ->
+    [{Name, {pkg, PkgName, Vsn, lookup_hash(PkgName, Vsn, Hash)}}
+     || {Name, {pkg, PkgName, Vsn, Hash}} <- List].
+
+lookup_hash(Name, Vsn, undefined) ->
+    try
+        ets:lookup_element(?PACKAGE_TABLE, {Name, Vsn}, 3)
+    catch
+        _:_ ->
+            undefined
+    end;
+lookup_hash(_, _, Hash) ->
+    Hash.
+
+
 rm_ws(<<" ", R/binary>>) ->
     rm_ws(R);
 rm_ws(R) ->
@@ -188,27 +204,27 @@ valid_vsn(Vsn) ->
     SupportedVersions = "^(>=?|<=?|~>|==)?\\s*" ++ SemVerRegExp ++ "$",
     re:run(Vsn, SupportedVersions) =/= nomatch.
 
-highest_matching({Pkg, PkgVsn, Dep}, Vsn, HexRegistry, State, DepsListAcc) ->
+highest_matching({Pkg, PkgVsn, Dep, App}, Vsn, HexRegistry, State, DepsListAcc) ->
     case rebar_packages:find_highest_matching(Pkg, PkgVsn, Dep, Vsn, HexRegistry, State) of
         {ok, HighestDepVsn} ->
-            [{Dep, HighestDepVsn} | DepsListAcc];
+            [{App, {pkg, Dep, HighestDepVsn, undefined}} | 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) ->
+cmp({_Pkg, _PkgVsn, Dep, _App} = 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) ->
+cmp_(undefined, _MinVsn, [], DepsListAcc, {Pkg, PkgVsn, Dep, _App}, _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_(HighestDepVsn, _MinVsn, [], DepsListAcc, {_Pkg, _PkgVsn, Dep, App}, _CmpFun) ->
+    [{App, {pkg, Dep, HighestDepVsn, undefined}} | DepsListAcc];
 
 cmp_(BestMatch, MinVsn, [Vsn | R], DepsListAcc, Dep, CmpFun) ->
     case CmpFun(Vsn, MinVsn) of
@@ -224,13 +240,13 @@ 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) ->
+cmpl_(undefined, _MaxVsn, [], DepsListAcc, {Pkg, PkgVsn, Dep, _App}, _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_(HighestDepVsn, _MaxVsn, [], DepsListAcc, {_Pkg, _PkgVsn, Dep, App}, _CmpFun) ->
+    [{App, {pkg, Dep, HighestDepVsn, undefined}} | DepsListAcc];
 
 cmpl_(undefined, MaxVsn, [Vsn | R], DepsListAcc, Dep, CmpFun) ->
     case CmpFun(Vsn, MaxVsn) of
diff --git a/test/mock_pkg_resource.erl b/test/mock_pkg_resource.erl
index f837713..5769988 100644
--- a/test/mock_pkg_resource.erl
+++ b/test/mock_pkg_resource.erl
@@ -149,7 +149,10 @@ to_index(AllDeps, Dict) ->
     ets:new(package_index, [named_table, public]),
     dict:fold(
       fun(K, Deps, _) ->
-              DepsList = [{ec_cnv:to_binary(DK), ec_cnv:to_binary(DV)} || {DK, DV} <- Deps],
+              DepsList = [{DKB, {pkg, DKB, DVB, undefined}}
+                          || {DK, DV} <- Deps,
+                             DKB <- [ec_cnv:to_binary(DK)],
+                             DVB <- [ec_cnv:to_binary(DV)]],
               ets:insert(package_index, {K, DepsList, <<"checksum">>})
       end, ok, Dict),
     ets:insert(package_index, {package_index_version, 3}),
diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl
index 24bf2a0..6b2ecea 100644
--- a/test/rebar_deps_SUITE.erl
+++ b/test/rebar_deps_SUITE.erl
@@ -385,36 +385,36 @@ https_os_proxy_settings(_Config) ->
 
 semver_matching_lt(_Config) ->
     Dep = <<"test">>,
-    Dep1 = {Dep, <<"1.0.0">>, Dep},
+    Dep1 = {Dep, <<"1.0.0">>, Dep, 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">>}],
+    ?assertEqual([{Dep, {pkg, Dep, <<"0.1.9">>, undefined}}],
                  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},
+    Dep1 = {Dep, <<"1.0.0">>, Dep, 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">>}],
+    ?assertEqual([{Dep, {pkg, Dep, <<"0.2.0">>, undefined}}],
                  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},
+    Dep1 = {Dep, <<"1.0.0">>, Dep, 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">>}],
+    ?assertEqual([{Dep, {pkg, Dep, <<"0.2.1">>, undefined}}],
                  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},
+    Dep1 = {Dep, <<"1.0.0">>, Dep, Dep},
     MaxVsn = <<"0.2.0">>,
     Vsns = [<<"0.1.7">>, <<"0.1.9">>, <<"0.1.8">>, <<"0.2.0">>],
-    ?assertEqual([{Dep, <<"0.2.0">>}],
+    ?assertEqual([{Dep, {pkg, Dep, <<"0.2.0">>, undefined}}],
                  rebar_prv_update:cmp_(undefined, MaxVsn, Vsns, [], Dep1,
                                        fun ec_semver:gt/2)).
 
diff --git a/test/rebar_pkg_alias_SUITE.erl b/test/rebar_pkg_alias_SUITE.erl
index 8915357..2b8ccd2 100644
--- a/test/rebar_pkg_alias_SUITE.erl
+++ b/test/rebar_pkg_alias_SUITE.erl
@@ -4,7 +4,8 @@
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("kernel/include/file.hrl").
 
-all() -> [same_alias, diff_alias, diff_alias_vsn].
+all() -> [same_alias, diff_alias, diff_alias_vsn, transitive_alias,
+          transitive_hash_mismatch].
 
 %% {uuid, {pkg, uuid}} = uuid
 %% {uuid, {pkg, alias}} = uuid on disk
@@ -32,6 +33,18 @@ init_per_testcase(diff_alias_vsn, Config0) ->
     AppDir = ?config(apps, Config),
     rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]),
     RebarConf = rebar_test_utils:create_config(AppDir, [{deps, [{fakelib, "1.0.0", {pkg, goodpkg}}]}]),
+    [{rebarconfig, RebarConf} | Config];
+init_per_testcase(transitive_alias, Config0) ->
+    Config = rebar_test_utils:init_rebar_state(Config0,"transitive_alias_vsn_"),
+    AppDir = ?config(apps, Config),
+    rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]),
+    RebarConf = rebar_test_utils:create_config(AppDir, [{deps, [{topdep, "1.0.0", {pkg, topdep}}]}]),
+    [{rebarconfig, RebarConf} | Config];
+init_per_testcase(transitive_hash_mismatch, Config0) ->
+    Config = rebar_test_utils:init_rebar_state(Config0,"transitive_alias_vsn_"),
+    AppDir = ?config(apps, Config),
+    rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]),
+    RebarConf = rebar_test_utils:create_config(AppDir, [{deps, [{topdep, "1.0.0", {pkg, topdep}}]}]),
     [{rebarconfig, RebarConf} | Config].
 
 end_per_testcase(_, Config) ->
@@ -73,23 +86,108 @@ diff_alias(Config) ->
 
 diff_alias_vsn(Config) -> diff_alias(Config).
 
+transitive_alias(Config) ->
+    %% ensure that the apps fetched under transitive aliases are
+    %% locked properly, but also that they are stored in the right
+    %% directory in the build dir to avoid breaking includes and
+    %% static analysis tools that rely on the location to work
+    AppDir = ?config(apps, Config),
+    Lockfile = filename:join([AppDir, "rebar.lock"]),
+    {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
+    rebar_test_utils:run_and_check(
+        Config, RebarConfig, ["lock"],
+        {ok, [{lock, "topdep"},{dep, "topdep"},
+              {lock,"transitive_app"},{dep,"transitive_app"}]}
+    ),
+    {ok, [{_Vsn, LockData}|_]} = file:consult(Lockfile),
+    ?assert(lists:any(fun({<<"transitive_app">>,{pkg,<<"transitive">>,_},_}) -> true
+                      ;  (_) -> false end, LockData)),
+    AppDir = ?config(apps, Config),
+    AliasedName = filename:join([AppDir, "_build", "default", "lib", "transitive_app"]),
+    PkgName = filename:join([AppDir, "_build", "default", "lib", "transitive"]),
+    ?assert(filelib:is_dir(AliasedName)),
+    ?assertNot(filelib:is_dir(PkgName)),
+    %% An second run yields the same
+    rebar_test_utils:run_and_check(
+        Config, RebarConfig, ["lock"],
+        {ok, [{lock, "topdep"},{dep, "topdep"},
+              {lock,"transitive_app"},{dep,"transitive_app"}]}
+    ),
+    {ok, [{_Vsn, LockData}|_]} = file:consult(Lockfile),
+    ?assert(filelib:is_dir(AliasedName)),
+    ?assertNot(filelib:is_dir(PkgName)),
+    %% So does an upgrade
+    rebar_test_utils:run_and_check(
+        Config, RebarConfig, ["upgrade"],
+        {ok, [{lock, "topdep"},{dep, "topdep"},
+              {lock,"transitive_app"},{dep,"transitive_app"}]}
+    ),
+    {ok, [{_Vsn, LockData}|_]} = file:consult(Lockfile),
+    ?assert(filelib:is_dir(AliasedName)),
+    ?assertNot(filelib:is_dir(PkgName)),
+    ok.
+
+transitive_hash_mismatch(Config) ->
+    %% ensure that the apps fetched under transitive aliases are
+    %% locked properly, but also that they are stored in the right
+    %% directory in the build dir to avoid breaking includes and
+    %% static analysis tools that rely on the location to work
+    AppDir = ?config(apps, Config),
+    Lockfile = filename:join([AppDir, "rebar.lock"]),
+    {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
+    rebar_test_utils:run_and_check(
+        Config, RebarConfig, ["lock"],
+        {ok, [{lock, "topdep"},{dep, "topdep"},
+              {lock,"transitive_app"},{dep,"transitive_app"}]}
+    ),
+    {ok, [LockData|Attrs]} = file:consult(Lockfile),
+    %% Change Lock hash data to cause a failure next time, but on transitive
+    %% deps only
+    NewLock = [LockData|lists:map(
+        fun([{pkg_hash, Hashes}|Rest]) ->
+                [{pkg_hash, [{<<"transitive_app">>, <<"fakehash">>}
+                             | lists:keydelete(<<"transitive_app">>, 1, Hashes)]}
+                 | Rest]
+        ;  (Attr) ->
+                Attr
+        end, Attrs)],
+    {ok, Io} = file:open(Lockfile, [write]),
+    [io:format(Io, "~p.~n", [Attr]) || Attr <- NewLock],
+    file:close(Io),
+    ct:pal("lock: ~p", [file:consult(Lockfile)]),
+    ec_file:remove(filename:join([AppDir, "_build"]), [recursive]),
+    ?assertMatch(
+       {error, {rebar_fetch, {unexpected_hash, _, _, _}}},
+       rebar_test_utils:run_and_check(Config, RebarConfig, ["lock"], return)
+    ),
+    ok.
+
 mock_config(Name, Config) ->
+    {ChkFake, Etag} = create_lib(Name, Config, "fakelib"),
+    {ChkTop, _} = create_lib(Name, Config, "topdep"),
+    {ChkTrans, _} = create_lib(Name, Config, "transitive_app", "transitive"),
     Priv = ?config(priv_dir, Config),
+    TmpDir = filename:join([Priv, "tmp", atom_to_list(Name)]),
+    %% Add an alias for goodpkg -> fakelib by hand
     AppDir = filename:join([Priv, "fakelib"]),
     CacheRoot = filename:join([Priv, "cache", atom_to_list(Name)]),
-    TmpDir = filename:join([Priv, "tmp", atom_to_list(Name)]),
     CacheDir = filename:join([CacheRoot, "hex", "com", "test", "packages"]),
-    filelib:ensure_dir(filename:join([CacheDir, "registry"])),
     rebar_test_utils:create_app(AppDir, "fakelib", "1.0.0", [kernel, stdlib]),
-    {Chk,Etag} = rebar_test_utils:package_app(AppDir, CacheDir, "fakelib-1.0.0"),
-    {Chk,Etag} = rebar_test_utils:package_app(AppDir, CacheDir, "goodpkg-1.0.0"),
+    {ChkFake, Etag} = rebar_test_utils:package_app(AppDir, CacheDir, "goodpkg-1.0.0"),
 
     Tid = ets:new(registry_table, [public]),
     ets:insert_new(Tid, [
         {<<"fakelib">>,[[<<"1.0.0">>]]},
         {<<"goodpkg">>,[[<<"1.0.0">>]]},
-        {{<<"fakelib">>,<<"1.0.0">>}, [[], Chk, [<<"rebar3">>]]},
-        {{<<"goodpkg">>,<<"1.0.0">>}, [[], Chk, [<<"rebar3">>]]}
+        {<<"topdep">>,[[<<"1.0.0">>]]},
+        {<<"transitive">>, [[<<"1.0.0">>]]},
+        {{<<"fakelib">>,<<"1.0.0">>}, [[], ChkFake, [<<"rebar3">>]]},
+        {{<<"goodpkg">>,<<"1.0.0">>}, [[], ChkFake, [<<"rebar3">>]]},
+        {{<<"topdep">>,<<"1.0.0">>},
+         [[
+           [<<"transitive">>, <<"1.0.0">>, false, <<"transitive_app">>]
+          ], ChkTop, [<<"rebar3">>]]},
+        {{<<"transitive">>,<<"1.0.0">>}, [[], ChkTrans, [<<"rebar3">>]]}
     ]),
     ok = ets:tab2file(Tid, filename:join([CacheDir, "registry"])),
     ets:delete(Tid),
@@ -119,3 +217,15 @@ mock_config(Name, Config) ->
 unmock_config(Config) ->
     meck:unload(),
     Config.
+
+create_lib(Name, Config, PkgName) ->
+    create_lib(Name, Config, PkgName, PkgName).
+
+create_lib(Name, Config, AppName, PkgName) ->
+    Priv = ?config(priv_dir, Config),
+    AppDir = filename:join([Priv, PkgName]),
+    CacheRoot = filename:join([Priv, "cache", atom_to_list(Name)]),
+    CacheDir = filename:join([CacheRoot, "hex", "com", "test", "packages"]),
+    filelib:ensure_dir(filename:join([CacheDir, "registry"])),
+    rebar_test_utils:create_app(AppDir, AppName, "1.0.0", [kernel, stdlib]),
+    rebar_test_utils:package_app(AppDir, CacheDir, PkgName++"-1.0.0").