Merge pull request #344 from ferd/proper-default-fetch

Default deps always to the default profile

4 files changed, 146 insertions, 38 deletions

diff --git a/src/rebar_dir.erl b/src/rebar_dir.erl
index 7903ed5..7a12eeb 100644
--- a/src/rebar_dir.erl
+++ b/src/rebar_dir.erl
@@ -1,6 +1,7 @@
 -module(rebar_dir).
 
 -export([base_dir/1,
+         profile_dir/2,
          deps_dir/1,
          deps_dir/2,
          checkouts_dir/1,
@@ -24,7 +25,10 @@
 
 -spec base_dir(rebar_state:t()) -> file:filename_all().
 base_dir(State) ->
-    Profiles = rebar_state:current_profiles(State),
+    profile_dir(State, rebar_state:current_profiles(State)).
+
+-spec profile_dir(rebar_state:t(), [atom()]) -> file:filename_all().
+profile_dir(State, Profiles) ->
     ProfilesStrings = case [ec_cnv:to_list(P) || P <- Profiles] of
         ["default"]      -> ["default"];
         %% drop `default` from the profile dir if it's implicit and reverse order
@@ -34,6 +38,7 @@ base_dir(State) ->
     ProfilesDir = string:join(ProfilesStrings, "+"),
     filename:join(rebar_state:get(State, base_dir, ?DEFAULT_BASE_DIR), ProfilesDir).
 
+
 -spec deps_dir(rebar_state:t()) -> file:filename_all().
 deps_dir(State) ->
     filename:join(base_dir(State), rebar_state:get(State, deps_dir, ?DEFAULT_DEPS_DIR)).
diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl
index c535b4d..c39afad 100644
--- a/src/rebar_prv_install_deps.erl
+++ b/src/rebar_prv_install_deps.erl
@@ -137,7 +137,7 @@ handle_deps(Profile, State0, Deps, Upgrade, Locks) ->
     Registry = rebar_packages:registry(State0),
     State = rebar_state:packages(rebar_state:registry(State0, Registry), {Packages, Graph}),
     %% Split source deps from pkg deps, needed to keep backwards compatibility
-    DepsDir = rebar_dir:deps_dir(State),
+    DepsDir = profile_dep_dir(State, Profile),
     {SrcDeps, PkgDeps} = parse_deps(DepsDir, Deps, State, Locks, 0),
 
     %% Fetch transitive src deps
@@ -203,7 +203,7 @@ update_pkg_deps(Profile, Packages, PkgDeps, Graph, Upgrade, Seen, State) ->
 
 update_pkg_deps(Profile, Pkgs, Packages, Upgrade, Seen, State) ->
     %% Create app_info record for each pkg dep
-    DepsDir = rebar_dir:deps_dir(State),
+    DepsDir = profile_dep_dir(State, Profile),
     {Solved, _, State1}
         = lists:foldl(fun(Pkg, {Acc, SeenAcc, StateAcc}) ->
                         handle_pkg_dep(Profile, Pkg, Packages, Upgrade, DepsDir, Acc, SeenAcc, StateAcc)
@@ -214,7 +214,7 @@ handle_pkg_dep(Profile, Pkg, Packages, Upgrade, DepsDir, Fetched, Seen, State) -
     AppInfo = package_to_app(DepsDir, Packages, Pkg),
     Level = rebar_app_info:dep_level(AppInfo),
     {NewSeen, NewState} = maybe_lock(Profile, AppInfo, Seen, State, Level),
-    {_, AppInfo1} = maybe_fetch(AppInfo, Upgrade, Seen, NewState),
+    {_, AppInfo1} = maybe_fetch(AppInfo, Profile, Upgrade, Seen, NewState),
     {[AppInfo1 | Fetched], NewSeen, NewState}.
 
 maybe_lock(Profile, AppInfo, Seen, State, Level) ->
@@ -252,7 +252,7 @@ package_to_app(DepsDir, Packages, {Name, Vsn, Level}) ->
                        || {PkgName,PkgVsn} <- proplists:get_value(<<"deps">>, P, [])],
             {ok, AppInfo} = rebar_app_info:new(Name, Vsn),
             AppInfo1 = rebar_app_info:deps(AppInfo, PkgDeps),
-            AppInfo2 = rebar_app_info:dir(AppInfo1, rebar_dir:deps_dir(DepsDir, Name)),
+            AppInfo2 = rebar_app_info:dir(AppInfo1, filename:join([DepsDir, Name])),
             AppInfo3 = rebar_app_info:dep_level(AppInfo2, Level),
             rebar_app_info:source(AppInfo3, {pkg, Name, Vsn})
     end.
@@ -278,7 +278,7 @@ update_src_dep(AppInfo, Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrad
     Name = rebar_app_info:name(AppInfo),
     case sets:is_element(Name, Seen) of
         true ->
-            update_seen_src_dep(AppInfo, Level,
+            update_seen_src_dep(AppInfo, Profile, Level,
                                 SrcDeps, PkgDeps, SrcApps,
                                 State, Upgrade, Seen, BaseLocks, Locks);
         false ->
@@ -288,7 +288,13 @@ update_src_dep(AppInfo, Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrad
 
     end.
 
-update_seen_src_dep(AppInfo, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, Seen, BaseLocks, Locks) ->
+profile_dep_dir(State, Profile) ->
+    case Profile of
+        default -> filename:join([rebar_dir:profile_dir(State, [default]), rebar_state:get(State, deps_dir, ?DEFAULT_DEPS_DIR)]);
+        _ -> rebar_dir:deps_dir(State)
+    end.
+
+update_seen_src_dep(AppInfo, Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, Seen, BaseLocks, Locks) ->
     Name = rebar_app_info:name(AppInfo),
     %% If seen from lock file don't print warning about skipping
     case lists:keymember(Name, 1, BaseLocks) of
@@ -303,7 +309,7 @@ update_seen_src_dep(AppInfo, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, S
             {SrcDeps, PkgDeps, SrcApps, State, Seen, Locks};
         true ->
             {NewSrcDeps, NewPkgDeps, NewSrcApps, NewState, NewLocks}
-                = handle_dep(AppInfo, SrcDeps, PkgDeps, SrcApps,
+                = handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps,
                              Level, State, Locks),
             {NewSrcDeps, NewPkgDeps, NewSrcApps, NewState, Seen, NewLocks}
     end.
@@ -313,22 +319,22 @@ update_unseen_src_dep(AppInfo, Profile, Level, SrcDeps, PkgDeps, SrcApps, State,
     {NewSrcDeps, NewPkgDeps, NewSrcApps, State2, NewLocks}
         = case Upgrade of
             true ->
-                handle_upgrade(AppInfo, SrcDeps, PkgDeps, SrcApps,
+                handle_upgrade(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps,
                                Level, State1, Locks);
             _ ->
-                {_, AppInfo1} = maybe_fetch(AppInfo, false, Seen, State1),
-                handle_dep(AppInfo1, SrcDeps, PkgDeps, SrcApps,
+                {_, AppInfo1} = maybe_fetch(AppInfo, Profile, false, Seen, State1),
+                handle_dep(AppInfo1, Profile, SrcDeps, PkgDeps, SrcApps,
                            Level, State1, Locks)
         end,
     {NewSrcDeps, NewPkgDeps, NewSrcApps, State2, NewSeen, NewLocks}.
 
-handle_upgrade(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
+handle_upgrade(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
     Name = rebar_app_info:name(AppInfo),
     case lists:keyfind(Name, 1, Locks) of
         false ->
-            case maybe_fetch(AppInfo, true, sets:new(), State) of
+            case maybe_fetch(AppInfo, Profile, true, sets:new(), State) of
                 {true, AppInfo1} ->
-                    handle_dep(AppInfo1, SrcDeps, PkgDeps, SrcApps,
+                    handle_dep(AppInfo1, Profile, SrcDeps, PkgDeps, SrcApps,
                                Level, State, Locks);
 
                 {false, AppInfo1} ->
@@ -338,8 +344,8 @@ handle_upgrade(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
             {[AppInfo|SrcDeps], PkgDeps, SrcApps, State, Locks}
     end.
 
-handle_dep(AppInfo, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
-    DepsDir = rebar_dir:deps_dir(State),
+handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
+    DepsDir = profile_dep_dir(State, Profile),
     {AppInfo1, NewSrcDeps, NewPkgDeps, NewLocks, State1} =
         handle_dep(State, DepsDir, AppInfo, Locks, Level),
     AppInfo2 = rebar_app_info:dep_level(AppInfo1, Level),
@@ -374,9 +380,9 @@ handle_dep(State, DepsDir, AppInfo, Locks, Level) ->
     {SrcDeps, PkgDeps} = parse_deps(DepsDir, Deps, S3, Locks, Level),
     {AppInfo2, SrcDeps, PkgDeps, Locks++NewLocks, State1}.
 
--spec maybe_fetch(rebar_app_info:t(), boolean() | {true, binary(), integer()},
+-spec maybe_fetch(rebar_app_info:t(), atom(), boolean() | {true, binary(), integer()},
                   sets:set(binary()), rebar_state:t()) -> {boolean(), rebar_app_info:t()}.
-maybe_fetch(AppInfo, Upgrade, Seen, State) ->
+maybe_fetch(AppInfo, Profile, Upgrade, Seen, State) ->
     AppDir = ec_cnv:to_list(rebar_app_info:dir(AppInfo)),
     %% Don't fetch dep if it exists in the _checkouts dir
     case rebar_app_info:is_checkout(AppInfo) of
@@ -385,16 +391,26 @@ maybe_fetch(AppInfo, Upgrade, Seen, State) ->
         false ->
             case rebar_app_discover:find_app(AppDir, all) of
                 false ->
-                    case in_default(AppInfo, State) of
+                    case already_in_default(AppInfo, State) of
                         false ->
-                            {fetch_app(AppInfo, AppDir, State), AppInfo};
+                            case fetch_app(AppInfo, AppDir, State) of
+                                true ->
+                                    case needs_symlinking(State, Profile) of
+                                        true ->
+                                            SymDir = filename:join([rebar_dir:deps_dir(State), rebar_app_info:name(AppInfo)]),
+                                            symlink_dep(AppDir, SymDir),
+                                            {true, AppInfo};
+                                        false ->
+                                            {true, AppInfo}
+                                    end;
+                                Other ->
+                                    {Other, AppInfo}
+                            end;
                         {true, FoundApp} ->
                             %% Preserve the state we created with overrides
                             AppState = rebar_app_info:state(AppInfo),
                             FoundApp1 = rebar_app_info:state(FoundApp, AppState),
-                            ?INFO("Linking ~s to ~s", [rebar_app_info:dir(FoundApp1), AppDir]),
-                            filelib:ensure_dir(AppDir),
-                            rebar_file_utils:symlink_or_copy(rebar_app_info:dir(FoundApp1), AppDir),
+                            symlink_dep(rebar_app_info:dir(FoundApp1), AppDir),
                             {true, FoundApp1}
                     end;
                 {true, AppInfo1} ->
@@ -410,11 +426,30 @@ maybe_fetch(AppInfo, Upgrade, Seen, State) ->
             end
     end.
 
-in_default(AppInfo, State) ->
+already_in_default(AppInfo, State) ->
     Name = ec_cnv:to_list(rebar_app_info:name(AppInfo)),
     DefaultAppDir = filename:join([rebar_state:get(State, base_dir), "default", "lib", Name]),
     rebar_app_discover:find_app(DefaultAppDir, all).
 
+needs_symlinking(State, Profile) ->
+    case {rebar_state:current_profiles(State), Profile} of
+        {[default], default} ->
+            %% file will be in default already -- this is the only run we have
+            false;
+        {_, default} ->
+            %% file fetched to default, needs to be linked to the current
+            %% run's directory.
+            true;
+        _ ->
+            %% File fetched to the right directory already
+            false
+    end.
+
+symlink_dep(From, To) ->
+    ?INFO("Linking ~s to ~s", [From, To]),
+    filelib:ensure_dir(To),
+    rebar_file_utils:symlink_or_copy(From, To).
+
 -spec parse_deps(binary(), list(), rebar_state:t(), list(), integer()) -> {[rebar_app_info:t()], [pkg_dep()]}.
 parse_deps(DepsDir, Deps, State, Locks, Level) ->
     lists:foldl(fun(Dep, Acc) ->
diff --git a/src/rebar_prv_lock.erl b/src/rebar_prv_lock.erl
index 5536ec9..f88b1ff 100644
--- a/src/rebar_prv_lock.erl
+++ b/src/rebar_prv_lock.erl
@@ -29,19 +29,10 @@ init(State) ->
 
 -spec do(rebar_state:t()) -> {ok, rebar_state:t()} | {error, string()}.
 do(State) ->
-    AllDeps = rebar_state:lock(State),
-    Locks = [begin
-                 Dir = rebar_app_info:dir(Dep),
-                 Source = rebar_app_info:source(Dep),
-
-                 %% If source is tuple it is a source dep
-                 %% e.g. {git, "git://github.com/ninenines/cowboy.git", "master"}
-                 {rebar_app_info:name(Dep)
-                 ,rebar_fetch:lock_source(Dir, Source, State)
-                 ,rebar_app_info:dep_level(Dep)}
-             end || Dep <- AllDeps, not(rebar_app_info:is_checkout(Dep))],
+    Locks = build_locks(State),
     Dir = rebar_state:dir(State),
-    file:write_file(filename:join(Dir, ?LOCK_FILE), io_lib:format("~p.~n", [Locks])),
+    file:write_file(filename:join(Dir, ?LOCK_FILE),
+                    io_lib:format("~p.~n", [Locks])),
     {ok, State}.
 
 -spec format_error(any()) -> iolist().
@@ -50,3 +41,17 @@ format_error(Reason) ->
 
 info(_) ->
     "".
+
+build_locks(State) ->
+    AllDeps = rebar_state:lock(State),
+    [begin
+        Dir = rebar_app_info:dir(Dep),
+        Source = rebar_app_info:source(Dep),
+
+        %% If source is tuple it is a source dep
+        %% e.g. {git, "git://github.com/ninenines/cowboy.git", "master"}
+        {rebar_app_info:name(Dep)
+        ,rebar_fetch:lock_source(Dir, Source, State)
+        ,rebar_app_info:dep_level(Dep)}
+     end || Dep <- AllDeps, not(rebar_app_info:is_checkout(Dep))].
+
diff --git a/test/rebar_install_deps_SUITE.erl b/test/rebar_install_deps_SUITE.erl
index aebd3e3..68fb9dd 100644
--- a/test/rebar_install_deps_SUITE.erl
+++ b/test/rebar_install_deps_SUITE.erl
@@ -2,6 +2,7 @@
 -compile(export_all).
 -include_lib("common_test/include/ct.hrl").
 -include_lib("eunit/include/eunit.hrl").
+-include_lib("kernel/include/file.hrl").
 
 all() -> [{group, git}, {group, pkg}].
 
@@ -9,7 +10,7 @@ groups() ->
     [{all, [], [flat, pick_highest_left, pick_highest_right,
                 pick_smallest1, pick_smallest2,
                 circular1, circular2, circular_skip,
-                fail_conflict]},
+                fail_conflict, default_profile, nondefault_profile]},
      {git, [], [{group, all}]},
      {pkg, [], [{group, all}]}].
 
@@ -114,7 +115,17 @@ deps(fail_conflict) ->
     {[{"B", [{"C", "2", []}]},
       {"C", "1", []}],
      [{"C","2"}],
-     rebar_abort}.
+     rebar_abort};
+deps(default_profile) ->
+    {[{"B", []},
+      {"C", []}],
+     [],
+     {ok, ["B", "C"]}};
+deps(nondefault_profile) ->
+    {[{"B", []},
+      {"C", []}],
+     [],
+     {ok, ["B", "C"]}}.
 
 setup_project(fail_conflict, Config0, Deps) ->
     DepsType = ?config(deps_type, Config0),
@@ -134,6 +145,25 @@ setup_project(fail_conflict, Config0, Deps) ->
             mock_pkg_resource:mock([{pkgdeps, rebar_test_utils:flat_pkgdeps(Deps)}])
     end,
     [{rebarconfig, RebarConf} | Config];
+setup_project(nondefault_profile, Config0, Deps) ->
+    DepsType = ?config(deps_type, Config0),
+    Config = rebar_test_utils:init_rebar_state(
+            Config0,
+            "nondefault_profile_"++atom_to_list(DepsType)++"_"
+    ),
+    AppDir = ?config(apps, Config),
+    rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]),
+    TopDeps = rebar_test_utils:top_level_deps(Deps),
+    RebarConf = rebar_test_utils:create_config(AppDir, [{profiles, [
+                                                            {nondef, [{deps, TopDeps}]}
+                                                       ]}]),
+    case DepsType of
+        git ->
+            mock_git_resource:mock([{deps, rebar_test_utils:flat_deps(Deps)}]);
+        pkg ->
+            mock_pkg_resource:mock([{pkgdeps, rebar_test_utils:flat_pkgdeps(Deps)}])
+    end,
+    [{rebarconfig, RebarConf} | Config];
 setup_project(Case, Config0, Deps) ->
     DepsType = ?config(deps_type, Config0),
     Config = rebar_test_utils:init_rebar_state(
@@ -174,6 +204,39 @@ fail_conflict(Config) ->
     ),
     check_warnings(error_calls(), ?config(warnings, Config), ?config(deps_type, Config)).
 
+default_profile(Config) ->
+    {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
+    AppDir = ?config(apps, Config),
+    {ok, Apps} = Expect = ?config(expect, Config),
+    rebar_test_utils:run_and_check(
+        Config, RebarConfig, ["as", "profile", "install_deps"], Expect
+    ),
+    check_warnings(error_calls(), ?config(warnings, Config), ?config(deps_type, Config)),
+    BuildDir = filename:join([AppDir, "_build"]),
+    [?assertMatch({ok, #file_info{type=directory}},
+                  file:read_file_info(filename:join([BuildDir, "default", "lib", App])))
+     || {dep, App} <- Apps],
+    [?assertMatch({ok, #file_info{type=directory}}, % somehow symlinks return dirs
+                  file:read_file_info(filename:join([BuildDir, "profile", "lib", App])))
+     || {dep, App} <- Apps].
+
+nondefault_profile(Config) ->
+    {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
+    AppDir = ?config(apps, Config),
+    {ok, Apps} = Expect = ?config(expect, Config),
+    rebar_test_utils:run_and_check(
+        Config, RebarConfig, ["as", "nondef", "install_deps"], Expect
+    ),
+    check_warnings(error_calls(), ?config(warnings, Config), ?config(deps_type, Config)),
+    BuildDir = filename:join([AppDir, "_build"]),
+    [?assertMatch({error, enoent},
+                  file:read_file_info(filename:join([BuildDir, "default", "lib", App])))
+     || {dep, App} <- Apps],
+    [?assertMatch({ok, #file_info{type=directory}},
+                  file:read_file_info(filename:join([BuildDir, "nondef", "lib", App])))
+     || {dep, App} <- Apps].
+
+
 run(Config) ->
     {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
     rebar_test_utils:run_and_check(