Merge pull request #2037 from Kuroneer/kuroneer/fix-deps-command

Fix rebar3 deps command

src/rebar_app_utils.erl

@@ -34,6 +34,7 @@
          validate_application_info/2,
          parse_deps/5,
          parse_deps/6,
+         parse_dep/6,
          expand_deps_sources/2,
          dep_to_app/7,
          format_error/1]).

src/rebar_prv_deps.erl

@@ -9,7 +9,7 @@
 -include("rebar.hrl").
 
 -define(PROVIDER, deps).
--define(DEPS, [app_discovery]).
+-define(DEPS, [install_deps]).
 
 -spec init(rebar_state:t()) -> {ok, rebar_state:t()}.
 init(State) ->
@@ -22,98 +22,127 @@ init(State) ->
                     {deps, ?DEPS},
                     {example, "rebar3 deps"},
                     {short_desc, "List dependencies"},
-                    {desc, "List dependencies. Those not matching lock files "
-                           "are followed by an asterisk (*)."},
+                    {desc, "List dependencies. Those not matching "
+                           "the config file are followed by "
+                           "an asterisk (*)."},
                     {opts, []}])),
     {ok, State1}.
 
+
 -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}.
 do(State) ->
     Profiles = rebar_state:current_profiles(State),
-    List = [{Profile, rebar_state:get(State, {deps, Profile}, [])}
-           || Profile <- Profiles],
-    [display(State, Profile, Deps) || {Profile, Deps} <- List],
+    [display(State, Profile) || Profile <- Profiles],
     {ok, State}.
 
+
 -spec format_error(any()) -> iolist().
 format_error(Reason) ->
     io_lib:format("~p", [Reason]).
 
-display(State, default, Deps) ->
-    NewDeps = merge(Deps, rebar_state:get(State, deps, [])),
-    display_deps(State, NewDeps),
+
+display(State, Profile = default) ->
+    display_profile_deps(State, Profile),
     ?CONSOLE("", []);
-display(State, Profile, Deps) ->
+display(State, Profile) ->
     ?CONSOLE("-- ~p --", [Profile]),
-    display_deps(State, Deps),
+    display_profile_deps(State, Profile),
     ?CONSOLE("", []).
 
-merge(Deps, SourceDeps) ->
-    merge1(dedup([normalize(Dep) || Dep <- Deps]),
-           [normalize(Dep) || Dep <- SourceDeps]).
-
-normalize(Name) when is_binary(Name) ->
-    Name;
-normalize(Name) when is_atom(Name) ->
-    atom_to_binary(Name, unicode);
-normalize(Dep) when is_tuple(Dep) ->
-    Name = element(1, Dep),
-    setelement(1, Dep, normalize(Name)).
-
-merge1(Deps, SourceDeps) ->
-    Names = [name(Dep) || Dep <- Deps],
-    ToAdd = [Dep || Dep <- SourceDeps,
-                    not lists:member(name(Dep), Names)],
-    Deps ++ ToAdd.
-
-%% Keep the latter one as locks come after regular deps in the list.
-%% This is totally not safe as an assumption, but it's what we got.
-%% We do this by comparing the current element and looking if a
-%% similar named one happens later. If so, drop the current one.
-dedup(Deps) -> dedup(Deps, [name(Dep) || Dep <- Deps]).
-
-dedup([], []) -> [];
-dedup([Dep|Deps], [Name|DepNames]) ->
-    case lists:member(Name, DepNames) of
-        true -> dedup(Deps, DepNames);
-        false -> [Dep | dedup(Deps, DepNames)]
+
+display_profile_deps(State, Profile) ->
+    DepsDir = rebar_prv_install_deps:profile_dep_dir(State, Profile),
+
+    ProfileDeps = rebar_state:get(State, {deps, Profile}, []),
+    % ProfileDeps include those deps from rebar.lock that have been
+    % removed from rebar.config
+    ConfiguredDeps = [parse_dep_without_locks(DepsDir, Dep, State)
+                      || Dep <- ProfileDeps],
+    LockedDepsMap = locked_deps_map(State, Profile),
+    [display_dep(State, Dep, LockedDepsMap) || Dep <- ConfiguredDeps].
+
+
+parse_dep_without_locks(DepsDir, Dep, State) ->
+    ParsedDep = rebar_app_utils:parse_dep(Dep, root, DepsDir, State, [], 0),
+    case Dep of
+        {_Name, Src, Level} when is_tuple(Src), is_integer(Level) ->
+            % This Dep is not in rebar.config but in rebar.lock
+            rebar_app_info:source(ParsedDep, undefined);
+        _ ->
+            rebar_app_utils:expand_deps_sources(ParsedDep, State)
+    end.
+
+
+locked_deps_map(State, Profile) ->
+    ParsedDeps = rebar_state:get(State, {parsed_deps, Profile}, []),
+    lists:foldl(fun(Dep, DepsIn) ->
+                        case rebar_app_info:is_lock(Dep) of
+                            true ->
+                                DepName = rebar_app_info:name(Dep),
+                                maps:put(rebar_utils:to_binary(DepName), Dep, DepsIn);
+                            _ ->
+                                DepsIn
+                        end
+                end, maps:new(), ParsedDeps).
+
+
+display_dep(State, Dep, LockedDeps) ->
+    Name = rebar_utils:to_binary(rebar_app_info:name(Dep)),
+    NeedsUpdate = rebar_fetch:needs_update(Dep, State),
+    Source = rebar_app_info:source(Dep),
+    LockedSource = case maps:get(Name, LockedDeps, undefined) of
+                       undefined -> undefined;
+                       LockedDep -> rebar_app_info:source(LockedDep)
+                   end,
+
+    display_dep_line(Name, NeedsUpdate, source_text(LockedSource), source_text(Source)).
+
+
+% Dep is a checkout
+display_dep_line(Name, _NeedsUpdate, _LockedSource, Source = checkout) ->
+    ?CONSOLE("~ts* (~ts)", [Name, Source]);
+
+% Dep exists only in lock file
+display_dep_line(Name, _NeedsUpdate, LockedSource, _Source = undefined) ->
+    ?CONSOLE("~ts* (locked ~ts <> none)", [Name, LockedSource]);
+
+% Dep not locked, report whether the disk copy matches the Source
+display_dep_line(Name, true, undefined, Source) ->
+    ?CONSOLE("~ts* (~ts)", [Name, Source]);
+display_dep_line(Name, _, undefined, Source) ->
+    ?CONSOLE("~ts (~ts)", [Name, Source]);
+
+% Dep locked, install_deps provider should have had updated the disk copy with
+% the locked version
+display_dep_line(Name, false, _LockedSource, Source) ->
+    % dep locked and no need to update (LockedSource and Source might not match
+    % because of one using {ref, X} and the other {tag, Y})
+    ?CONSOLE("~ts (locked ~ts)", [Name, Source]);
+display_dep_line(Name, _NeedsUpdate, LockedSource, Source) ->
+    % dep locked with mismatching lock and config files
+    ?CONSOLE("~ts* (locked ~ts <> ~ts)", [Name, LockedSource, Source]).
+
+
+source_text(Source) when is_list(Source); is_atom(Source) ->
+    Source;
+source_text({pkg, _Name, Vsn, _Hash, _RepoConfig}) ->
+    source_text({pkg, _Name, Vsn, _Hash});
+source_text({pkg, _Name, Vsn, _Hash}) ->
+    [<<"package">>, " ", rebar_utils:to_binary(Vsn)];
+source_text(Source) when is_tuple(Source), tuple_size(Source) < 3 ->
+    element(1, Source);
+source_text(Source) when is_tuple(Source) ->
+    Type = element(1, Source),
+    case element(3, Source) of
+        {ref , Ref} ->
+            SmallRef = case rebar_utils:to_binary(Ref) of
+                           <<R:7/binary, _/binary>> -> <<R/binary, "...">>;
+                           R -> R
+                       end,
+            [atom_to_binary(Type, unicode), " source ", SmallRef];
+        {_ , Vsn} ->
+            [atom_to_binary(Type, unicode), " source ", rebar_utils:to_binary(Vsn)];
+        _ ->
+            [atom_to_binary(Type, unicode), " source"]
     end.
 
-name(T) when is_tuple(T) -> element(1, T);
-name(B) when is_binary(B) -> B.
-
-display_deps(State, Deps) ->
-    lists:foreach(fun(Dep) -> display_dep(State, Dep) end, Deps).
-
-%% packages
-display_dep(_State, {Name, Vsn}) when is_list(Vsn) ->
-    ?CONSOLE("~ts* (package ~ts)", [rebar_utils:to_binary(Name), rebar_utils:to_binary(Vsn)]);
-display_dep(_State, Name) when is_binary(Name) ->
-    ?CONSOLE("~ts* (package)", [Name]);
-display_dep(_State, {Name, Source}) when is_tuple(Source) ->
-    ?CONSOLE("~ts* (~ts source)", [rebar_utils:to_binary(Name), type(Source)]);
-display_dep(_State, {Name, _Vsn, Source}) when is_tuple(Source) ->
-    ?CONSOLE("~ts* (~ts source)", [rebar_utils:to_binary(Name), type(Source)]);
-display_dep(_State, {Name, _Vsn, Source, _Opts}) when is_tuple(Source) ->
-    ?CONSOLE("~ts* (~ts source)", [rebar_utils:to_binary(Name), type(Source)]);
-%% Locked
-display_dep(State, {Name, _Source={pkg, _, Vsn}, Level}) when is_integer(Level) ->
-    DepsDir = rebar_dir:deps_dir(State),
-    AppDir = filename:join([DepsDir, rebar_utils:to_binary(Name)]),
-    {ok, AppInfo} = rebar_app_info:discover(AppDir),
-    NeedsUpdate = case rebar_fetch:needs_update(AppInfo, State) of
-        true -> "*";
-        false -> ""
-    end,
-    ?CONSOLE("~ts~ts (locked package ~ts)", [Name, NeedsUpdate, Vsn]);
-display_dep(State, {Name, Source, Level}) when is_tuple(Source), is_integer(Level) ->
-    DepsDir = rebar_dir:deps_dir(State),
-    AppDir = filename:join([DepsDir, rebar_utils:to_binary(Name)]),
-    {ok, AppInfo} = rebar_app_info:discover(AppDir),
-    NeedsUpdate = case rebar_fetch:needs_update(AppInfo, State) of
-        true -> "*";
-        false -> ""
-    end,
-    ?CONSOLE("~ts~ts (locked ~ts source)", [Name, NeedsUpdate, type(Source)]).
-
-type(Source) when is_tuple(Source) -> element(1, Source).

test/rebar_deps_SUITE.erl

@@ -7,7 +7,9 @@ all() -> [sub_app_deps, newly_added_dep, newly_added_after_empty_lock,
           http_proxy_settings, https_proxy_settings,
           http_os_proxy_settings, https_os_proxy_settings,
           semver_matching_lt, semver_matching_lte, semver_matching_gt,
-          valid_version, top_override, {group, git}, {group, pkg}].
+          valid_version, top_override, {group, git}, {group, pkg},
+          deps_cmd_needs_update_called
+         ].
 
 groups() ->
     [{all, [], [flat, pick_highest_left, pick_highest_right,
@@ -131,6 +133,8 @@ init_per_testcase(https_os_proxy_settings, Config) ->
             rebar_test_utils:create_config(GlobalConfigDir, []),
             rebar_test_utils:init_rebar_state(Config)
     end;
+init_per_testcase(deps_cmd_needs_update_called, Config) ->
+    rebar_test_utils:init_rebar_state(Config);
 init_per_testcase(Case, Config) ->
     {Deps, Warnings, Expect} = deps(Case),
     Expected = case Expect of
@@ -249,6 +253,10 @@ mock_warnings() ->
     %% the call log.
     meck:new(rebar_log, [no_link, passthrough]).
 
+mock_rebar_fetch() ->
+    meck:new(rebar_fetch, [no_link, passthrough]).
+
+
 %%% TESTS %%%
 flat(Config) -> run(Config).
 pick_highest_left(Config) -> run(Config).
@@ -437,7 +445,7 @@ semver_matching_gte(_Config) ->
     MaxVsn = <<"0.2.0">>,
     Vsns = [<<"0.1.7">>, <<"0.1.9">>, <<"0.1.8">>, <<"0.2.0">>],
     ?assertEqual({ok, <<"0.2.0">>},
-                 rebar_packages:cmp_(undefined, MaxVsn, Vsns, 
+                 rebar_packages:cmp_(undefined, MaxVsn, Vsns,
                                        fun ec_semver:gt/2)).
 
 valid_version(_Config) ->
@@ -480,6 +488,65 @@ run(Config) ->
     ),
     check_warnings(warning_calls(), ?config(warnings, Config), ?config(deps_type, Config)).
 
+
+deps_cmd_needs_update_called(Config) ->
+    mock_rebar_fetch(),
+    AppDir = ?config(apps, Config),
+    Deps = rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []}
+                                             ,{"b", "1.0.0", [{"c", "1.0.0", []}]}
+                                             ,{"c", "2.0.0", []}]),
+    {SrcDeps, _} = rebar_test_utils:flat_deps(Deps),
+    mock_git_resource:mock([{deps, SrcDeps}]),
+
+    Name = rebar_test_utils:create_random_name("app_"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+
+    SubAppsDir = filename:join([AppDir, "apps", Name]),
+    rebar_test_utils:create_app(SubAppsDir, Name, Vsn, [kernel, stdlib]),
+
+    TopDeps = rebar_test_utils:top_level_deps(
+                rebar_test_utils:expand_deps(git, [{"b", "1.0.0", []}])),
+    {ok, RebarConfig} = file:consult(rebar_test_utils:create_config(AppDir, [{deps, TopDeps}])),
+    rebar_test_utils:run_and_check(Config, RebarConfig, ["deps"], {ok, []}),
+
+    [<<"b">>] = rebar_fetch_needs_update_calls_sorted(),
+
+    %% Add c to top level
+    TopDeps2 = rebar_test_utils:top_level_deps(rebar_test_utils:expand_deps(git, [{"c", "2.0.0", []}
+                                                                                 ,{"b", "1.0.0", []}])),
+    {ok, RebarConfig2} = file:consult(rebar_test_utils:create_config(AppDir, [{deps, TopDeps2}])),
+    rebar_test_utils:run_and_check(Config, RebarConfig2, ["deps"], {ok, []}),
+
+    %% Only top level deps are checked for updates
+    [<<"b">>, <<"b">>, <<"c">>] = rebar_fetch_needs_update_calls_sorted(),
+
+    %% Lock deps
+    rebar_test_utils:run_and_check(Config, RebarConfig2, ["lock"], {ok, []}),
+    NeedsUpdate1 = rebar_fetch_needs_update_calls_sorted(),
+
+    %% Switch c for a as top level deps
+    TopDeps3 = rebar_test_utils:top_level_deps(rebar_test_utils:expand_deps(git, [{"a", "1.0.0", []}
+                                                                                 ,{"b", "1.0.0", []}])),
+
+    {ok, RebarConfig3} = file:consult(rebar_test_utils:create_config(AppDir, [{deps, TopDeps3}])),
+    LockFile = filename:join(AppDir, "rebar.lock"),
+    RebarConfig4 = rebar_config:merge_locks(RebarConfig3,
+                                            rebar_config:consult_lock_file(LockFile)),
+
+    rebar_test_utils:run_and_check(Config, RebarConfig4, ["deps"], {ok, []}),
+
+    NeedsUpdate2 = lists:subtract(rebar_fetch_needs_update_calls_sorted(), NeedsUpdate1),
+
+    %% B and C from lock file  + install_deps and A, B and C from 'deps'
+    [<<"a">>, <<"b">>, <<"b">>, <<"c">>, <<"c">>] = NeedsUpdate2.
+
+
+rebar_fetch_needs_update_calls_sorted() ->
+    History = meck:history(rebar_fetch),
+    DepsNames = [rebar_app_info:name(Dep)
+                 || {_, {rebar_fetch, needs_update, [Dep, _]}, _} <- History],
+    lists:sort(DepsNames).
+
 warning_calls() ->
     History = meck:history(rebar_log),
     [{Str, Args} || {_, {rebar_log, log, [warn, Str, Args]}, _} <- History].