Merge pull request #602 from tsloughter/level_fix

pass failing upgrade case where levels are increased

3 files changed, 51 insertions, 34 deletions

diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl
index 7160704..105bbb1 100644
--- a/src/rebar_prv_install_deps.erl
+++ b/src/rebar_prv_install_deps.erl
@@ -75,7 +75,6 @@ do(State) ->
 
         Upgrade = rebar_state:get(State, upgrade, false),
         {Apps, State1} = deps_per_profile(Profiles, Upgrade, State),
-            %lists:foldl(fun deps_per_profile/2, {[], State}, lists:reverse(Profiles)),
 
         State2 = rebar_state:update_all_deps(State1, Apps),
         CodePaths = [rebar_app_info:ebin_dir(A) || A <- Apps],
@@ -322,7 +321,7 @@ profile_dep_dir(State, Profile) ->
         _ -> rebar_dir:deps_dir(State)
     end.
 
-update_seen_src_dep(AppInfo, Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, Seen, BaseLocks, Locks) ->
+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 or user requested an upgrade
     %% don't print warning about skipping
@@ -331,44 +330,32 @@ update_seen_src_dep(AppInfo, Profile, Level, SrcDeps, PkgDeps, SrcApps, State, U
         false when not Upgrade -> warn_skip_deps(AppInfo, State);
         true -> ok
     end,
-    %% scan for app children here if upgrading
-    case Upgrade of
-        false ->
-            {SrcDeps, PkgDeps, SrcApps, State, Seen, Locks};
-        true ->
-            {NewSrcDeps, NewPkgDeps, NewSrcApps, NewState, NewLocks}
-                = handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps,
-                             Level, State, Locks),
-            {NewSrcDeps, NewPkgDeps, NewSrcApps, NewState, Seen, NewLocks}
-    end.
+    {SrcDeps, PkgDeps, SrcApps, State, Seen, Locks}.
 
 update_unseen_src_dep(AppInfo, Profile, Level, SrcDeps, PkgDeps, SrcApps, State, Upgrade, Seen, Locks) ->
     {NewSeen, State1} = maybe_lock(Profile, AppInfo, Seen, State, Level),
     {NewSrcDeps, NewPkgDeps, NewSrcApps, State2, NewLocks}
         = case Upgrade of
-            true ->
-                handle_upgrade(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps,
-                               Level, State1, Locks);
-            _ ->
-                {_, AppInfo1} = maybe_fetch(AppInfo, Profile, false, Seen, State1),
-                handle_dep(AppInfo1, Profile, SrcDeps, PkgDeps, SrcApps,
-                           Level, State1, Locks)
-        end,
+              true ->
+                  handle_upgrade(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps,
+                                 Level, State1, Seen, Locks);
+              _ ->
+                  {_, 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, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
+handle_upgrade(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Seen, Locks) ->
     Name = rebar_app_info:name(AppInfo),
     case lists:keyfind(Name, 1, Locks) of
         false ->
-            case maybe_fetch(AppInfo, Profile, true, sets:new(), State) of
-                {true, AppInfo1} ->
-                    handle_dep(AppInfo1, Profile, SrcDeps, PkgDeps, SrcApps,
-                               Level, State, Locks);
-                {false, AppInfo1} ->
-                    {[AppInfo1|SrcDeps], PkgDeps, SrcApps, State, Locks}
-            end;
+            {_, AppInfo1} = maybe_fetch(AppInfo, Profile, true, Seen, State),
+            handle_dep(AppInfo1, Profile, SrcDeps, PkgDeps, SrcApps,
+                       Level, State, Locks);
         _StillLocked ->
-            {[AppInfo|SrcDeps], PkgDeps, SrcApps, State, Locks}
+            handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps,
+                       Level, State, Locks)
     end.
 
 handle_dep(AppInfo, Profile, SrcDeps, PkgDeps, SrcApps, Level, State, Locks) ->
diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl
index e59ca23..4a13e03 100644
--- a/test/rebar_test_utils.erl
+++ b/test/rebar_test_utils.erl
@@ -333,12 +333,12 @@ write_app_src_file(Dir, Name, Version, Deps) ->
     ok = ec_file:write_term(Filename, get_app_metadata(ec_cnv:to_list(Name), Version, Deps)).
 
 erl_src_file(Name) ->
-    io_lib:format("-module(~s).\n"
+    io_lib:format("-module('~s').\n"
                   "-export([main/0]).\n"
                   "main() -> ok.\n", [filename:basename(Name, ".erl")]).
 
 erl_eunitized_src_file(Name) ->
-    io_lib:format("-module(~s).\n"
+    io_lib:format("-module('~s').\n"
                   "-export([main/0]).\n"
                   "main() -> ok.\n"
                   "-ifdef(TEST).\n"
@@ -348,7 +348,7 @@ erl_eunitized_src_file(Name) ->
 
 erl_eunit_suite_file(Name) ->
     BaseName = filename:basename(Name, ".erl"),
-    io_lib:format("-module(~s_tests).\n"
+    io_lib:format("-module('~s_tests').\n"
                   "-compile(export_all).\n"
                   "-ifndef(some_define).\n"
                   "-define(some_define, false).\n"
diff --git a/test/rebar_upgrade_SUITE.erl b/test/rebar_upgrade_SUITE.erl
index 79cf29e..f2d4133 100644
--- a/test/rebar_upgrade_SUITE.erl
+++ b/test/rebar_upgrade_SUITE.erl
@@ -10,7 +10,8 @@ groups() ->
                 pair_a, pair_b, pair_ab, pair_c, pair_all,
                 triplet_a, triplet_b, triplet_c,
                 tree_a, tree_b, tree_c, tree_c2, tree_ac, tree_all,
-                delete_d, promote, stable_lock, fwd_lock]},
+                delete_d, promote, stable_lock, fwd_lock,
+                compile_upgrade_parity]},
      {git, [], [{group, all}]},
      {pkg, [], [{group, all}]}].
 
@@ -404,7 +405,20 @@ upgrades(fwd_lock) ->
      %% file to include the result post-upgrade, and then
      %% run a regular lock to see that the lock file is respected
      %% in deps.
-     {"any", [{"A","2"},{"C","2"},{"B","2"},{"D","2"}]}}.
+     {"any", [{"A","2"},{"C","2"},{"B","2"},{"D","2"}]}};
+upgrades(compile_upgrade_parity) ->
+    {[{"A", "1", [{"D",[{"J",[]}]},
+                  {"E",[{"I","1",[]}]}]},
+      {"B", "1", [{"F",[]},
+                  {"G",[]}]},
+      {"C", "1", [{"H",[]},
+                  {"I","2",[]}]}
+     ],
+     [],
+     [],
+     {"", [{"A","1"}, "D", "J", "E", {"I","1"},
+           {"B","1"}, "F", "G",
+           {"C","1"}, "H"]}}.
 
 %% TODO: add a test that verifies that unlocking files and then
 %% running the upgrade code is enough to properly upgrade things.
@@ -518,6 +532,22 @@ fwd_lock(Config) ->
         Config, NewRebarConfig, ["lock", App], Expectation
     ).
 
+compile_upgrade_parity(Config) ->
+    AppDir = ?config(apps, Config),
+    apply(?config(mock, Config), []),
+    {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),
+    %% compiling and upgrading should generate the same lockfiles when
+    %% deps are identical
+    Lockfile = filename:join([AppDir, "rebar.lock"]),
+    rebar_test_utils:run_and_check(Config, RebarConfig, ["compile"], {ok, []}),
+    {ok, CompileLockData1} = file:read_file(Lockfile),
+    rebar_test_utils:run_and_check(Config, RebarConfig, ["upgrade"], {ok, []}),
+    {ok, UpgradeLockData} = file:read_file(Lockfile),
+    rebar_test_utils:run_and_check(Config, RebarConfig, ["compile"], {ok, []}),
+    {ok, CompileLockData2} = file:read_file(Lockfile),
+    ?assertEqual(CompileLockData1, CompileLockData2),
+    ?assertEqual(CompileLockData1, UpgradeLockData).
+
 run(Config) ->
     apply(?config(mock, Config), []),
     {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)),