Fix recursive profile merging in umbrella apps

When a config file exists at the root of a project, defines a given
configuration value for a given profile, and that a sub-application
(umbrella app) also has the same profile defined with the same key (but
different values), the configuration values of the sub-application's
profile would get silently dropped.

The problem being that when the function to merge profiles is applied
recursively, it is applied to each profile (so it will merge on the keys
test, prod, etc.) rather than to each of the values of each profile.

This patch reworks the profile merging so that the current behaviour is
respected overall (a profile cannot be cancelled by a subdep's
non-existant profile since its value should have been ignored), but
ensures that sub-deps' profiles are otherwise applied recursively with
the proper rules:

- dependencies favor prior values
- plugins favor new values
- erl_first_files combine the lists
- relx uses the tuple merge algorithm
- erl_opts has its own custom merge as well
- otherwise the new value takes precedence

A test has also been added.

There is a risk of breakage in some applications that may have relied on
the buggy behaviour to work, though at this time we are aware of none of
them.

src/rebar3.erl

@@ -396,8 +396,8 @@ safe_define_test_macro(Opts) ->
     %% defining a compile macro twice results in an exception so
     %% make sure 'TEST' is only defined once
     case test_defined(Opts) of
-       true  -> [];
-       false -> [{d, 'TEST'}]
+       true  -> Opts;
+       false -> [{d, 'TEST'}|Opts]
     end.
 
 test_defined([{d, 'TEST'}|_]) -> true;

src/rebar_opts.erl

@@ -117,7 +117,10 @@ merge_opt(plugins, NewValue, _OldValue) ->
 merge_opt({plugins, _}, NewValue, _OldValue) ->
     NewValue;
 merge_opt(profiles, NewValue, OldValue) ->
-    dict:to_list(merge_opts(dict:from_list(NewValue), dict:from_list(OldValue)));
+    ToMerge = fill_profile_gaps(lists:sort(NewValue),
+                                lists:sort(OldValue)),
+    [{K,dict:to_list(merge_opts(dict:from_list(New), dict:from_list(Old)))}
+     || {K,New,Old} <- ToMerge];
 merge_opt(erl_first_files, Value, Value) ->
     Value;
 merge_opt(erl_first_files, NewValue, OldValue) ->
@@ -190,3 +193,16 @@ filter_defines([{platform_define, ArchRegex, Key, Value} | Rest], Acc) ->
     end;
 filter_defines([Opt | Rest], Acc) ->
     filter_defines(Rest, [Opt | Acc]).
+
+fill_profile_gaps([], []) ->
+    [];
+fill_profile_gaps([{P,V}|Ps], []) ->
+    [{P,V,[]} | fill_profile_gaps(Ps, [])];
+fill_profile_gaps([], [{P,V}|Ps]) ->
+    [{P,[],V} | fill_profile_gaps([], Ps)];
+fill_profile_gaps([{P,VA}|PAs], [{P,VB}|PBs]) ->
+    [{P,VA,VB} | fill_profile_gaps(PAs, PBs)];
+fill_profile_gaps([{PA,VA}|PAs], [{PB,VB}|PBs]) when PA < PB ->
+    [{PA,VA,[]} | fill_profile_gaps(PAs, [{PB, VB}|PBs])];
+fill_profile_gaps([{PA,VA}|PAs], [{PB,VB}|PBs]) when PA > PB ->
+    [{PB,[],VB} | fill_profile_gaps([{PA,VA}|PAs], PBs)].

test/rebar_profiles_SUITE.erl

@@ -7,6 +7,7 @@
          all/0,
          profile_new_key/1,
          profile_merge_keys/1,
+         profile_merge_umbrella_keys/1,
          explicit_profile_deduplicate_deps/1,
          implicit_profile_deduplicate_deps/1,
          all_deps_code_paths/1,
@@ -33,7 +34,8 @@
 -include_lib("kernel/include/file.hrl").
 
 all() ->
-    [profile_new_key, profile_merge_keys, all_deps_code_paths, profile_merges,
+    [profile_new_key, profile_merge_keys, profile_merge_umbrella_keys,
+     all_deps_code_paths, profile_merges,
      explicit_profile_deduplicate_deps, implicit_profile_deduplicate_deps,
      same_profile_deduplication, stack_deduplication,
      add_to_profile, add_to_existing_profile,
@@ -118,6 +120,35 @@ profile_merge_keys(Config) ->
                                                                  ,{dep, "a", "1.0.0"}
                                                                  ,{dep, "b", "2.0.0"}]}).
 
+profile_merge_umbrella_keys(Config) ->
+    AppDir = ?config(apps, Config),
+    ct:pal("Path: ~s", [AppDir]),
+    Name = rebar_test_utils:create_random_name("profile_merge_umbrella_keys"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    SubAppDir = filename:join([AppDir, "apps", Name]),
+
+    RebarConfig = [{vals, [{a,1},{b,1}]},
+                   {profiles,
+                    [{ct,
+                      [{vals, [{a,1},{b,2}]}]}]}],
+    
+    SubRebarConfig = [{vals, []},
+                       {profiles, [{ct, [{vals, [{c,1}]}]}]}],
+
+    rebar_test_utils:create_app(SubAppDir, Name, Vsn, [kernel, stdlib]),
+    rebar_test_utils:create_config(SubAppDir, SubRebarConfig),
+    {ok, RebarConfigRead} = file:consult(rebar_test_utils:create_config(AppDir, RebarConfig)),
+
+    {ok, State} = rebar_test_utils:run_and_check(
+        Config, RebarConfigRead, ["as", "ct", "compile"], return
+    ),
+
+    [ProjectApp] = rebar_state:project_apps(State),
+    ?assertEqual(Name, binary_to_list(rebar_app_info:name(ProjectApp))),
+    Opts = rebar_app_info:opts(ProjectApp),
+    ?assertEqual([{a,1},{b,2},{b,1},{c,1}], dict:fetch(vals, Opts)),
+    ok.
+
 explicit_profile_deduplicate_deps(Config) ->
     AppDir = ?config(apps, Config),