Allow top-level apps to take precedence over deps

The use case has been described in issue #1478 where a local application
can exist while being declared as a dependency as well. This allows, for
example, to work on a release where all applications may require to be
published independently, or to provide some form of 'vendoring' with a
local app.

The fix is done by decoupling the dependency source resolution form the
dependency parsing. The reason for this being that the discovery phase
needs to parse apps for their top-level deps, and dep installation needs
to resolve the packages with accuracy. In the current implementation,
both code paths call to the same function.

This patch splits up the precise discovery and makes it happen *only*
when installing dependencies, and only if a top-level app does not
already define the application needing resolving.

One weakness of this fix is that it necessarily breaks cycle detection
in dependencies that involve a root application depending on itself
since its own version as a dep will not be expanded. There appears to be
no possible way to prevent this, but should be rare enough to be worth
the tradeoff for the common case.

src/rebar_app_utils.erl

@@ -34,6 +34,7 @@
          validate_application_info/2,
          parse_deps/5,
          parse_deps/6,
+         expand_deps_sources/2,
          dep_to_app/7,
          format_error/1]).
 
@@ -225,7 +226,7 @@ dep_to_app(Parent, DepsDir, Name, Vsn, Source, IsLock, State) ->
                                     not_found ->
                                         rebar_app_info:new(Parent, Name, Vsn, Dir, [])
                                 end,
-                                update_source(AppInfo0, Source, State)
+                                rebar_app_info:source(AppInfo0, Source)
                     end,
     C = rebar_config:consult(rebar_app_info:dir(AppInfo)),
     AppInfo1 = rebar_app_info:update_opts(AppInfo, rebar_app_info:opts(AppInfo), C),
@@ -236,6 +237,13 @@ dep_to_app(Parent, DepsDir, Name, Vsn, Source, IsLock, State) ->
     AppInfo5 = rebar_app_info:profiles(AppInfo4, [default]),
     rebar_app_info:is_lock(AppInfo5, IsLock).
 
+%% @doc Takes a given application app_info record along with the project.
+%% If the app is a package, resolve and expand the package definition.
+-spec expand_deps_sources(rebar_app_info:t(), rebar_state:t()) ->
+    rebar_app_info:t().
+expand_deps_sources(Dep, State) ->
+    update_source(Dep, rebar_app_info:source(Dep), State).
+
 %% @doc sets the source for a given dependency or app along with metadata
 %% around version if required.
 -spec update_source(rebar_app_info:t(), Source, rebar_state:t()) ->

src/rebar_prv_install_deps.erl

@@ -140,7 +140,9 @@ handle_deps_as_profile(Profile, State, Deps, Upgrade) ->
     DepsDir = profile_dep_dir(State, Profile),
     Deps1 = rebar_app_utils:parse_deps(DepsDir, Deps, State, Locks, Level),
     ProfileLevelDeps = [{Profile, Deps1, Level}],
-    handle_profile_level(ProfileLevelDeps, [], sets:new(), Upgrade, Locks, State).
+    RootSeen = sets:from_list([rebar_app_info:name(AppInfo)
+                               || AppInfo <- rebar_state:project_apps(State)]),
+    handle_profile_level(ProfileLevelDeps, [], RootSeen, RootSeen, Upgrade, Locks, State).
 
 %% ===================================================================
 %% Internal functions
@@ -153,7 +155,9 @@ deps_per_profile(Profiles, Upgrade, State) ->
     Deps = lists:foldl(fun(Profile, DepAcc) ->
                                [parsed_profile_deps(State, Profile, Level) | DepAcc]
                        end, [], Profiles),
-    handle_profile_level(Deps, [], sets:new(), Upgrade, Locks, State).
+    RootSeen = sets:from_list([rebar_app_info:name(AppInfo)
+                               || AppInfo <- rebar_state:project_apps(State)]),
+    handle_profile_level(Deps, [], RootSeen, RootSeen, Upgrade, Locks, State).
 
 parsed_profile_deps(State, Profile, Level) ->
     ParsedDeps = rebar_state:get(State, {parsed_deps, Profile}, []),
@@ -162,17 +166,27 @@ parsed_profile_deps(State, Profile, Level) ->
 %% Level-order traversal of all dependencies, across profiles.
 %% If profiles x,y,z are present, then the traversal will go:
 %% x0, y0, z0, x1, y1, z1, ..., xN, yN, zN.
-handle_profile_level([], Apps, _Seen, _Upgrade, _Locks, State) ->
+%%
+%% There are two 'seen' sets: one for the top-level apps (`RootSeen') and
+%% one for all dependencies (`Seen'). The former is used to know when
+%% to skip the resolving of dependencies altogether (since they're already
+%% top-level apps), while the latter is used to prevent reprocessing
+%% deps more than one.
+handle_profile_level([], Apps, _RootSeen, _Seen, _Upgrade, _Locks, State) ->
     {Apps, State};
-handle_profile_level([{Profile, Deps, Level} | Rest], Apps, Seen, Upgrade, Locks, State) ->
+handle_profile_level([{Profile, Deps, Level} | Rest], Apps, RootSeen, Seen, Upgrade, Locks, State) ->
+    Deps0 = [rebar_app_utils:expand_deps_sources(Dep, State)
+             || Dep <- Deps,
+                %% skip top-level apps being double-declared
+                not sets:is_element(rebar_app_info:name(Dep), RootSeen)],
     {Deps1, Apps1, State1, Seen1} =
-        update_deps(Profile, Level, Deps, Apps
+        update_deps(Profile, Level, Deps0, Apps
                    ,State, Upgrade, Seen, Locks),
     Deps2 = case Deps1 of
         [] -> Rest;
         _ -> Rest ++ [{Profile, Deps1, Level+1}]
     end,
-    handle_profile_level(Deps2, Apps1, sets:union(Seen, Seen1), Upgrade, Locks, State1).
+    handle_profile_level(Deps2, Apps1, RootSeen, sets:union(Seen, Seen1), Upgrade, Locks, State1).
 
 find_cycles(Apps) ->
     case rebar_digraph:compile_order(Apps) of

test/rebar_deps_SUITE.erl

@@ -7,7 +7,7 @@ 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, {group, git}, {group, pkg}].
+          valid_version, top_override, {group, git}, {group, pkg}].
 
 groups() ->
     [{all, [], [flat, pick_highest_left, pick_highest_right,
@@ -47,6 +47,8 @@ init_per_testcase(newly_added_dep, Config) ->
     rebar_test_utils:init_rebar_state(Config);
 init_per_testcase(sub_app_deps, Config) ->
     rebar_test_utils:init_rebar_state(Config);
+init_per_testcase(top_override, Config) ->
+    rebar_test_utils:init_rebar_state(Config);
 init_per_testcase(http_proxy_settings, Config) ->
     %% Create private rebar.config
     Priv = ?config(priv_dir, Config),
@@ -255,6 +257,32 @@ circular1(Config) -> run(Config).
 circular2(Config) -> run(Config).
 circular_skip(Config) -> run(Config).
 
+%% Test that a top-level application overtakes dependencies, and
+%% works even if said deps do not exist.
+top_override(Config) ->
+    AppDir = ?config(apps, Config),
+    ct:pal("dir: ~p", [AppDir]),
+    Name1 = rebar_test_utils:create_random_name("sub_app1_"),
+    Name2 = rebar_test_utils:create_random_name("sub_app2_"),
+    SubAppsDir1 = filename:join([AppDir, "apps", Name1]),
+    SubAppsDir2 = filename:join([AppDir, "apps", Name2]),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(SubAppsDir1, Name1, Vsn, [kernel, stdlib]),
+    rebar_test_utils:create_app(SubAppsDir2, Name2, Vsn, [kernel, stdlib]),
+    rebar_test_utils:create_config(
+      SubAppsDir1,
+      [{deps, [list_to_atom(Name2)]}]
+     ),
+    rebar_test_utils:create_config(
+      SubAppsDir2,
+      [{deps, [{list_to_atom(Name1),
+                {git, "https://example.org", {branch, "master"}}}]}]
+    ),
+    rebar_test_utils:run_and_check(
+      Config, [], ["compile"],
+      {ok, [{app, Name1}, {app,Name2}]}
+    ).
+
 %% Test that the deps of project apps that have their own rebar.config
 %% are included, but that top level rebar.config deps take precedence
 sub_app_deps(Config) ->