Merge pull request #965 from ferd/tsloughter-tup_umerge

fix tupple merging for realsies.

3 files changed, 124 insertions, 35 deletions

diff --git a/src/rebar_utils.erl b/src/rebar_utils.erl
index 9fe22ea..fb651c3 100644
--- a/src/rebar_utils.erl
+++ b/src/rebar_utils.erl
@@ -285,7 +285,7 @@ tup_umerge(NewList, OldList) ->
 tup_umerge_([], Olds) ->
     Olds;
 tup_umerge_([New|News], Olds) ->
-    tup_umerge_dedup_(umerge(News, Olds, [], New), []).
+    tup_umerge_dedup_(umerge(new, News, Olds, [], New), []).
 
 %% removes 100% identical duplicate elements so that
 %% `[a,{a,b},a,{a,c},a]' returns `[a,{a,b},{a,c}]'.
@@ -312,35 +312,58 @@ tup_find(Elem, [Elem1 | Elems]) when is_tuple(Elem1) ->
 tup_find(Elem, [_Elem | Elems]) ->
     tup_find(Elem, Elems).
 
-%% This is equivalent to umerge2_2 in the stdlib, except we use the expanded
-%% value/key only to compare
-umerge(News, [Old|Olds], Merged, Cmp) when element(1, Cmp) == element(1, Old);
-                                           element(1, Cmp) == Old andalso not is_tuple(Old);
-                                           Cmp == element(1, Old) andalso not is_tuple(Cmp);
-                                           Cmp =< Old andalso not is_tuple(Cmp) andalso not is_tuple(Old) ->
-    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([New|News], Olds, Merged, CmpMerged, Cmp) when CmpMerged == Cmp ->
-    umerge(News, Olds, Merged, New);
-umerge([New|News], Olds, Merged, _CmpMerged, Cmp) when element(1,New) == element(1, Cmp);
-                                                       element(1,New) == Cmp andalso not is_tuple(Cmp);
-                                                       New == element(1, Cmp) andalso not is_tuple(New);
-                                                       New =< Cmp andalso not is_tuple(New) andalso not is_tuple(Cmp) ->
-    umerge(News, Olds, [New | Merged], New, Cmp);
-umerge([New|News], Olds, Merged, _CmpMerged, Cmp) -> % >
-    umerge(News, Olds, [Cmp | Merged], New);
-umerge([], Olds, Merged, CmpMerged, Cmp) when CmpMerged == Cmp ->
-    lists:reverse(Olds, Merged);
-umerge([], Olds, Merged, _CmpMerged, Cmp) ->
-    lists:reverse(Olds, [Cmp | Merged]).
+-spec umerge(new|old, News, Olds, Acc, Current) -> Merged when
+      News :: [term()],
+      Olds :: [term()],
+      Acc  :: [term()],
+      Current :: term(),
+      Merged :: [term()].
+umerge(_, [], [], Acc, Current) ->
+    [Current | Acc];
+umerge(new, News, [], Acc, Current) ->
+    %% only news left
+    lists:reverse(News, [Current|Acc]);
+umerge(old, [], Olds, Acc, Current) ->
+    %% only olds left
+    lists:reverse(Olds, [Current|Acc]);
+umerge(new, News, [Old|Olds], Acc, Current) ->
+    {Dir, Merged, NewCurrent} = compare({new, Current}, {old, Old}),
+    umerge(Dir, News, Olds, [Merged|Acc], NewCurrent);
+umerge(old, [New|News], Olds, Acc, Current) ->
+    {Dir, Merged, NewCurrent} = compare({new, New}, {old, Current}),
+    umerge(Dir, News, Olds, [Merged|Acc], NewCurrent).
+
+-spec compare({Priority, term()}, {Secondary, term()}) ->
+    {NextPriority, Merged, Larger} when
+      Priority :: new | old,
+      Secondary :: new | old,
+      NextPriority :: new | old,
+      Merged :: term(),
+      Larger :: term().
+compare({Priority, A}, {Secondary, B}) when is_tuple(A), is_tuple(B) ->
+    KA = element(1,A),
+    KB = element(1,B),
+    if KA == KB -> {Secondary, A, B};
+       KA  < KB -> {Secondary, A, B};
+       KA  > KB -> {Priority, B, A}
+    end;
+compare({Priority, A}, {Secondary, B}) when not is_tuple(A), not is_tuple(B) ->
+    if A == B -> {Secondary, A, B};
+       A  < B -> {Secondary, A, B};
+       A  > B -> {Priority, B, A}
+    end;
+compare({Priority, A}, {Secondary, B}) when is_tuple(A), not is_tuple(B) ->
+    KA = element(1,A),
+    if KA == B -> {Secondary, A, B};
+       KA  < B -> {Secondary, A, B};
+       KA  > B -> {Priority, B, A}
+    end;
+compare({Priority, A}, {Secondary, B}) when not is_tuple(A), is_tuple(B) ->
+    KB = element(1,B),
+    if A == KB -> {Secondary, A, B};
+       A  < KB -> {Secondary, A, B};
+       A  > KB -> {Priority, B, A}
+    end.
 
 %% Implements wc -l functionality used to determine patchcount from git output
 line_count(PatchLines) ->
diff --git a/test/rebar_release_SUITE.erl b/test/rebar_release_SUITE.erl
index f6fe8ff..e0fa5a0 100644
--- a/test/rebar_release_SUITE.erl
+++ b/test/rebar_release_SUITE.erl
@@ -4,11 +4,13 @@
 -include_lib("eunit/include/eunit.hrl").
 
 all() -> [release,
-         dev_mode_release,
-         profile_dev_mode_override_release,
-         tar,
-         extend_release,
-         user_output_dir].
+          dev_mode_release,
+          profile_dev_mode_override_release,
+          tar,
+          profile_ordering_sys_config_extend,
+          profile_ordering_sys_config_extend_3_tuple_merge,
+          extend_release,
+          user_output_dir].
 
 init_per_testcase(Case, Config0) ->
     Config = rebar_test_utils:init_rebar_state(Config0),
@@ -111,6 +113,63 @@ extend_release(Config) ->
       {ok, [{release, extended, Vsn, false}]}
      ).
 
+%% Ensure proper ordering of sys_config and extended releases in profiles
+profile_ordering_sys_config_extend(Config) ->
+    AppDir = ?config(apps, Config),
+    Name = ?config(name, Config),
+    Vsn = "1.0.0",
+    TestSysConfig = filename:join(AppDir, "test.config"),
+    OtherSysConfig = filename:join(AppDir, "other.config"),
+    ok = file:write_file(TestSysConfig, "[]."),
+    ok = file:write_file(OtherSysConfig, "[{some, content}]."),
+    {ok, RebarConfig} =
+        file:consult(rebar_test_utils:create_config(AppDir,
+                                                    [{relx, [{release, {list_to_atom(Name), Vsn},
+                                                              [list_to_atom(Name)]},
+                                                             {sys_config, OtherSysConfig},
+                                                             {lib_dirs, [AppDir]}]},
+                                                     {profiles, [{extended,
+                                                                 [{relx, [
+                                                                         {sys_config, TestSysConfig}]}]}]}])),
+    rebar_test_utils:run_and_check(
+      Config, RebarConfig,
+      ["as", "extended", "release"],
+      {ok, [{release, list_to_atom(Name), Vsn, false}]}
+     ),
+
+    ReleaseDir = filename:join([AppDir, "./_build/extended/rel/", Name, "releases", Vsn]),
+    {ok, [[]]} = file:consult(filename:join(ReleaseDir, "sys.config")).
+
+%% test that tup_umerge works with tuples of different sizes
+profile_ordering_sys_config_extend_3_tuple_merge(Config) ->
+    AppDir = ?config(apps, Config),
+    Name = ?config(name, Config),
+    Vsn = "1.0.0",
+    TestSysConfig = filename:join(AppDir, "test.config"),
+    OtherSysConfig = filename:join(AppDir, "other.config"),
+    ok = file:write_file(TestSysConfig, "[]."),
+    ok = file:write_file(OtherSysConfig, "[{some, content}]."),
+    {ok, RebarConfig} =
+        file:consult(rebar_test_utils:create_config(AppDir,
+                                                    [{relx, [{release, {list_to_atom(Name), Vsn},
+                                                              [list_to_atom(Name)]},
+                                                             {sys_config, OtherSysConfig},
+                                                             {lib_dirs, [AppDir]}]},
+                                                     {profiles, [{extended,
+                                                                 [{relx, [
+                                                                         {release, {extended, Vsn, {extend, list_to_atom(Name)}},
+                                                                          []},
+                                                                         {sys_config, TestSysConfig}]}]}]}])),
+
+    rebar_test_utils:run_and_check(
+      Config, RebarConfig,
+      ["as", "extended", "release", "-n", Name],
+      {ok, [{release, list_to_atom(Name), Vsn, false}]}
+     ),
+
+    ReleaseDir = filename:join([AppDir, "./_build/extended/rel/", Name, "releases", Vsn]),
+    {ok, [[]]} = file:consult(filename:join(ReleaseDir, "sys.config")).
+
 user_output_dir(Config) ->
     AppDir = ?config(apps, Config),
     Name = ?config(name, Config),
diff --git a/test/rebar_utils_SUITE.erl b/test/rebar_utils_SUITE.erl
index 42a9551..b32992d 100644
--- a/test/rebar_utils_SUITE.erl
+++ b/test/rebar_utils_SUITE.erl
@@ -259,6 +259,13 @@ tup_merge(_Config) ->
        )
     ),
     ?assertEqual(
+       [{l, a}, {r, a, b}, {s, a}, {s, b}],
+       rebar_utils:tup_umerge(
+         rebar_utils:tup_sort([{r, a, b}, {s, a}, {l, a}]),
+         rebar_utils:tup_sort([{s, b}])
+       )
+    ),
+    ?assertEqual(
        [{a,b,b},{a,b},a,{a,a},{a,a,a},{b,b},{b,b,b},b,{b,a,a},{b,a},{z,b},{z,b,b},z,{z,a},{z,a,a}],
        rebar_utils:tup_umerge(
          rebar_utils:tup_sort([{a,b,b},{b,b},{a,b},{b,b,b},{z,b},{z,b,b},a,b,z]),