Merge pull request #290 from ferd/fancy-profile-merge-fun

Fancy profile merge fun

5 files changed, 120 insertions, 8 deletions

diff --git a/src/rebar_prv_release.erl b/src/rebar_prv_release.erl
index 766bfe4..db012d5 100644
--- a/src/rebar_prv_release.erl
+++ b/src/rebar_prv_release.erl
@@ -47,7 +47,7 @@ do(State) ->
                           ,{caller, Caller}], AllOptions);
             Config ->
                 relx:main([{lib_dirs, LibDirs}
-                          ,{config, Config}
+                          ,{config, lists:reverse(Config)}
                           ,{output_dir, OutputDir}
                           ,{caller, Caller}], AllOptions)
         end,
diff --git a/src/rebar_state.erl b/src/rebar_state.erl
index 008f202..eced383 100644
--- a/src/rebar_state.erl
+++ b/src/rebar_state.erl
@@ -240,7 +240,7 @@ merge_opts(NewOpts, OldOpts) ->
                            true ->
                                NewValue;
                            false ->
-                               lists:umerge(lists:sort(NewValue), lists:sort(OldValue))
+                               tup_umerge(lists:sort(NewValue), lists:sort(OldValue))
                        end;
                   (_Key, NewValue, _OldValue) ->
                        NewValue
@@ -362,3 +362,59 @@ add_hook(pre, {PreHooks, PostHooks}, Hook) ->
     {[Hook | PreHooks], PostHooks};
 add_hook(post, {PreHooks, PostHooks}, Hook) ->
     {PreHooks, [Hook | PostHooks]}.
+
+%% Custom merge functions. The objective is to behave like lists:umerge/2,
+%% except that we also compare the merge elements based on the key if they're a
+%% tuple, such that `{key, val1}' is always prioritized over `{key, val0}' if
+%% the former is from the 'new' list.
+%%
+%% This lets us apply proper overrides to list of elements according to profile
+%% priority.
+tup_umerge([], Olds) ->
+    Olds;
+tup_umerge(News, Olds) ->
+    [ENew|ENews] = expand(News),
+    EOlds = expand(Olds),
+    unexpand(lists:reverse(umerge(ENews, EOlds, [], ENew))).
+
+%% Expand values, so they `key' is now `{key, key}', and so that
+%% `{key, val}' is now `{key, {key, val}'. This allows us to compare
+%% possibly only on the total key or the value itself.
+expand([]) -> [];
+expand([Tup|T]) when is_tuple(Tup) -> [{element(1, Tup), Tup} | expand(T)];
+expand([H|T]) -> [{H,H} | expand(T)].
+
+%% Go back to unexpanded form.
+unexpand(List) -> [element(2, X) || X <- List].
+
+%% This is equivalent to umerge2_2 in the stdlib, except we use the expanded
+%% value/key only to compare
+umerge(News, [{KOld,_}=Old|Olds], Merged, {KCmp, _} = Cmp) when KCmp =< KOld ->
+    umerge(News, Olds, [Cmp | Merged], Cmp, Old);
+umerge(News, [Old|Olds], Merged, Cmp) ->
+    umerge(News, Olds, [Old | Merged], Cmp);
+umerge(News, [], Merged, Cmp) ->
+    lists:reverse(News, [Cmp | Merged]).
+
+%% Similar to stdlib's umerge2_1 in the stdlib, except that when the expanded
+%% value/keys compare equal, we check if the element is a full dupe to clear it
+%% (like the stdlib function does) or otherwise keep the duplicate around in
+%% an order that prioritizes 'New' elements.
+umerge([{KNew,_}=New|News], Olds, Merged, _CmpMerged, {KCmp,_}=Cmp) when KNew =< KCmp ->
+    umerge(News, Olds, [New | Merged], New, Cmp);
+umerge([{KNew,_}=New|News], Olds, Merged, {KCmp,_}=CmpMerged, Cmp) when KNew == KCmp ->
+    if New == CmpMerged ->
+        umerge(News, Olds, Merged, New);
+       New =/= CmpMerged -> % this is where we depart from the stdlib!
+        umerge(News, Olds, [New | Merged], New, Cmp)
+    end;
+umerge([New|News], Olds, Merged, _CmpMerged, Cmp) -> % >
+    umerge(News, Olds, [Cmp | Merged], New);
+umerge([], Olds, Merged, {KCmpM,_}=CmpMerged, {KCmp,_}=Cmp) when KCmpM =:= KCmp ->
+    if CmpMerged == Cmp ->
+        lists:reverse(Olds, Merged);
+       CmpMerged =/= Cmp -> % We depart from stdlib here too!
+        lists:reverse(Olds, [Cmp | Merged])
+    end;
+umerge([], Olds, Merged, _CmpMerged, Cmp) ->
+    lists:reverse(Olds, [Cmp | Merged]).
diff --git a/test/rebar_profiles_SUITE.erl b/test/rebar_profiles_SUITE.erl
index a4f926e..ab37255 100644
--- a/test/rebar_profiles_SUITE.erl
+++ b/test/rebar_profiles_SUITE.erl
@@ -100,12 +100,17 @@ profile_merges(_Config) ->
                    {test2, "hello"},
                    {test3, [key3]},
                    {test4, "oldvalue"},
+                   {test5, [{key5, true}]},
+                   {test6, [{key6, false}]},
                    {profiles,
                     [{profile1,
                       [{test1, [{key3, 5}, key1]}]},
                      {profile2, [{test2, "goodbye"},
                                  {test3, []},
-                                 {test4, []}]}]}],
+                                 {test4, []},
+                                 {test5, [{key5, false}]},
+                                 {test6, [{key6, true}]}
+                                ]}]}],
     State = rebar_state:new(RebarConfig),
     State1 = rebar_state:apply_profiles(State, [profile1, profile2]),
 
@@ -118,7 +123,9 @@ profile_merges(_Config) ->
 
     %% Check that a newvalue of []/"" doesn't override non-string oldvalues
     [key3] = rebar_state:get(State1, test3),
-    [] = rebar_state:get(State1, test4).
+    [] = rebar_state:get(State1, test4),
+    [{key5, false}, {key5, true}] = rebar_state:get(State1, test5),
+    [{key6, true}, {key6, false}] = rebar_state:get(State1, test6).
 
 add_to_profile(_Config) ->
     RebarConfig = [{foo, true}, {bar, false}],
diff --git a/test/rebar_release_SUITE.erl b/test/rebar_release_SUITE.erl
index 92219a5..3809106 100644
--- a/test/rebar_release_SUITE.erl
+++ b/test/rebar_release_SUITE.erl
@@ -3,7 +3,10 @@
 -include_lib("common_test/include/ct.hrl").
 -include_lib("eunit/include/eunit.hrl").
 
-all() -> [release, tar].
+all() -> [release,
+          dev_mode_release,
+          profile_dev_mode_override_release,
+          tar].
 
 init_per_testcase(Case, Config0) ->
     Config = rebar_test_utils:init_rebar_state(Config0),
@@ -33,9 +36,46 @@ release(Config) ->
     rebar_test_utils:run_and_check(
       Config, RebarConfig,
       ["release"],
-      {ok, [{release, list_to_atom(Name), Vsn}]}
+      {ok, [{release, list_to_atom(Name), Vsn, false}]}
      ).
 
+dev_mode_release(Config) ->
+    AppDir = ?config(apps, Config),
+    Name = ?config(name, Config),
+    Vsn = "1.0.0",
+    {ok, RebarConfig} =
+        file:consult(rebar_test_utils:create_config(AppDir,
+                                                    [{relx, [{release, {list_to_atom(Name), Vsn},
+                                                              [list_to_atom(Name)]},
+                                                             {lib_dirs, [AppDir]},
+                                                             {dev_mode, true}]}])),
+    rebar_test_utils:run_and_check(
+      Config, RebarConfig,
+      ["release"],
+      {ok, [{release, list_to_atom(Name), Vsn, true}]}
+     ).
+
+
+profile_dev_mode_override_release(Config) ->
+    AppDir = ?config(apps, Config),
+    Name = ?config(name, Config),
+    Vsn = "1.0.0",
+    {ok, RebarConfig} =
+        file:consult(rebar_test_utils:create_config(AppDir,
+                                                    [{relx, [{release, {list_to_atom(Name), Vsn},
+                                                              [list_to_atom(Name)]},
+                                                             {lib_dirs, [AppDir]},
+                                                             {dev_mode, true}]},
+                                                     {profiles,
+                                                      [{ct,
+                                                        [{relx, [{dev_mode, false}]}]}]}])),
+    rebar_test_utils:run_and_check(
+      Config, RebarConfig,
+      ["as", "ct", "release"],
+      {ok, [{release, list_to_atom(Name), Vsn, false}]}
+     ).
+
+
 tar(Config) ->
     AppDir = ?config(apps, Config),
     Name = ?config(name, Config),
@@ -48,5 +88,5 @@ tar(Config) ->
     rebar_test_utils:run_and_check(
       Config, RebarConfig,
       ["tar"],
-      {ok, [{release, list_to_atom(Name), Vsn}, {tar, Name, Vsn}]}
+      {ok, [{release, list_to_atom(Name), Vsn, false}, {tar, Name, Vsn}]}
      ).
diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl
index 7c52a18..c035b91 100644
--- a/test/rebar_test_utils.erl
+++ b/test/rebar_test_utils.erl
@@ -217,7 +217,7 @@ check_results(AppDir, Expected) ->
                         ?assertEqual(iolist_to_binary(Vsn),
                                      iolist_to_binary(LockVsn))
                 end
-        ;  ({release, Name, Vsn}) ->
+        ;  ({release, Name, Vsn, ExpectedDevMode}) ->
                 ct:pal("Release: ~p-~s", [Name, Vsn]),
                 {ok, Cwd} = file:get_cwd(),
                 try
@@ -228,6 +228,15 @@ check_results(AppDir, Expected) ->
                     {ok, RelxState2} = rlx_prv_app_discover:do(RelxState1),
                     {ok, RelxState3} = rlx_prv_rel_discover:do(RelxState2),
 
+                    LibDir = filename:join([ReleaseDir, Name, "lib"]),
+                    {ok, RelLibs} = file:list_dir(LibDir),
+                    IsSymLinkFun =
+                        fun(X) ->
+                                ec_file:is_symlink(filename:join(LibDir, X))
+                        end,
+                    DevMode = lists:all(IsSymLinkFun, RelLibs),
+                    ?assertEqual(ExpectedDevMode, DevMode),
+
                     %% throws not_found if it doesn't exist
                     rlx_state:get_realized_release(RelxState3, Name, Vsn)
                 catch