From 3146a285fb1c9e0028a9c09977c6da11df76d6e6 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sat, 10 Feb 2018 23:02:32 +0100 Subject: [PATCH 1/9] sort-as: force an order on multiple profiles --- src/rebar_dir.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rebar_dir.erl b/src/rebar_dir.erl index d7be42358..ee4d3d4be 100644 --- a/src/rebar_dir.erl +++ b/src/rebar_dir.erl @@ -49,7 +49,7 @@ profile_dir(Opts, Profiles) -> %% of profiles to match order passed to `as` ["default"|Rest] -> {rebar_opts:get(Opts, base_dir, ?DEFAULT_BASE_DIR), Rest} end, - ProfilesDir = rebar_string:join(ProfilesStrings, "+"), + ProfilesDir = rebar_string:join(lists:sort(ProfilesStrings), "+"), filename:join(BaseDir, ProfilesDir). %% @doc returns the directory where dependencies should be placed From 9f0e3a2e5b6ea3eccda14660bc64570738ee7796 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Fri, 16 Feb 2018 17:48:17 +0100 Subject: [PATCH 2/9] Revert "sort-as: force an order on multiple profiles" This reverts commit 3f8dd5eacebb913144f3615fdf44658b6223a791. --- src/rebar_dir.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rebar_dir.erl b/src/rebar_dir.erl index ee4d3d4be..d7be42358 100644 --- a/src/rebar_dir.erl +++ b/src/rebar_dir.erl @@ -49,7 +49,7 @@ profile_dir(Opts, Profiles) -> %% of profiles to match order passed to `as` ["default"|Rest] -> {rebar_opts:get(Opts, base_dir, ?DEFAULT_BASE_DIR), Rest} end, - ProfilesDir = rebar_string:join(lists:sort(ProfilesStrings), "+"), + ProfilesDir = rebar_string:join(ProfilesStrings, "+"), filename:join(BaseDir, ProfilesDir). %% @doc returns the directory where dependencies should be placed From e504ba71e152a1e47719269f0108ad47c8f3f8b9 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sat, 17 Feb 2018 01:20:29 +0100 Subject: [PATCH 3/9] sort-as: found the issue. Will look into tests now --- src/rebar_state.erl | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/rebar_state.erl b/src/rebar_state.erl index 577ed2314..7ebe6dad2 100644 --- a/src/rebar_state.erl +++ b/src/rebar_state.erl @@ -87,8 +87,8 @@ new(Config) when is_list(Config) -> opts = Opts }. -spec new(t() | atom(), list()) -> t(). -new(Profile, Config) when is_atom(Profile) - , is_list(Config) -> +new(Profile, Config) when is_atom(Profile), + is_list(Config) -> BaseState = base_state(), Opts = base_opts(Config), BaseState#state_t { dir = rebar_dir:get_cwd(), @@ -283,11 +283,12 @@ apply_profiles(State=#state_t{default = Defaults, current_profiles=CurrentProfil end, Defaults, AppliedProfiles), State#state_t{current_profiles = AppliedProfiles, opts=NewOpts}. +%% @doc A stable deduplicator. deduplicate(Profiles) -> - do_deduplicate(lists:reverse(Profiles), []). + do_deduplicate(Profiles, []). do_deduplicate([], Acc) -> - Acc; + lists:reverse(Acc); do_deduplicate([Head | Rest], Acc) -> case lists:member(Head, Acc) of true -> do_deduplicate(Rest, Acc); From abad0e14bb5a6ea5df21f2732bd72bf6efb0ca1e Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Fri, 23 Feb 2018 04:07:24 +0100 Subject: [PATCH 4/9] sort-as: show issue more clearly --- src/rebar_state.erl | 4 ++++ test/rebar_profiles_SUITE.erl | 19 ++++++++++++++++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/rebar_state.erl b/src/rebar_state.erl index 7ebe6dad2..8c100aa58 100644 --- a/src/rebar_state.erl +++ b/src/rebar_state.erl @@ -43,6 +43,10 @@ allow_provider_overrides/1, allow_provider_overrides/2 ]). +-ifdef(TEST). +-export([deduplicate/1]). +-endif. + -include("rebar.hrl"). -include_lib("providers/include/providers.hrl"). diff --git a/test/rebar_profiles_SUITE.erl b/test/rebar_profiles_SUITE.erl index 6afdc393b..9f7912d6f 100644 --- a/test/rebar_profiles_SUITE.erl +++ b/test/rebar_profiles_SUITE.erl @@ -28,7 +28,8 @@ test_profile_erl_opts_order_4/1, test_profile_erl_opts_order_5/1, test_erl_opts_debug_info/1, - first_files_exception/1]). + first_files_exception/1, + deduplication_stability/1]). -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). @@ -52,7 +53,8 @@ all() -> test_profile_erl_opts_order_4, test_profile_erl_opts_order_5, test_erl_opts_debug_info, - first_files_exception]. + first_files_exception, + deduplication_stability]. init_per_suite(Config) -> application:start(meck), @@ -133,7 +135,7 @@ profile_merge_umbrella_keys(Config) -> {profiles, [{ct, [{vals, [{a,1},{b,2}]}]}]}], - + SubRebarConfig = [{vals, []}, {profiles, [{ct, [{vals, [{c,1}]}]}]}], @@ -546,6 +548,17 @@ first_files_exception(_Config) -> ?assertEqual(["c","a","b","a","e"], rebar_state:get(State1, mib_first_files)), ok. +deduplication_stability(_Config) -> + ?assertEqual([default,all_deps_test], rebar_state:deduplicate([default,all_deps_test])), + ?assertEqual([default,profile1,profile2], rebar_state:deduplicate([default,profile1,profile2])), + ?assertEqual([default,bar,foo], rebar_state:deduplicate([default,bar,foo,bar])), %% master wants [default,foo,bar] + ?assertEqual([default,test,bar], rebar_state:deduplicate([default,test,bar])), + ?assertEqual([default,test,bar], rebar_state:deduplicate([default,test,bar,test])), + ?assertEqual([default,profile1], rebar_state:deduplicate([default,profile1,profile1,profile1])), + ?assertEqual([default,a,b,c,d,e], rebar_state:deduplicate([default,a,b,c,d,e,a,e,b])), + ?assertEqual([default,test], rebar_state:deduplicate([default,test])), + ok. + get_compiled_profile_erl_opts(Profiles, Config) -> AppDir = ?config(apps, Config), PStrs = [atom_to_list(P) || P <- Profiles], From 6aeff6300bf8986079ac6e8f83fa37150e239810 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Fri, 23 Feb 2018 12:50:29 +0100 Subject: [PATCH 5/9] Revert "sort-as: show issue more clearly" This reverts commit 4ad1db97336a3ac070880876ada07d4c7b769327. --- src/rebar_state.erl | 4 ---- test/rebar_profiles_SUITE.erl | 19 +++---------------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/src/rebar_state.erl b/src/rebar_state.erl index 8c100aa58..7ebe6dad2 100644 --- a/src/rebar_state.erl +++ b/src/rebar_state.erl @@ -43,10 +43,6 @@ allow_provider_overrides/1, allow_provider_overrides/2 ]). --ifdef(TEST). --export([deduplicate/1]). --endif. - -include("rebar.hrl"). -include_lib("providers/include/providers.hrl"). diff --git a/test/rebar_profiles_SUITE.erl b/test/rebar_profiles_SUITE.erl index 9f7912d6f..6afdc393b 100644 --- a/test/rebar_profiles_SUITE.erl +++ b/test/rebar_profiles_SUITE.erl @@ -28,8 +28,7 @@ test_profile_erl_opts_order_4/1, test_profile_erl_opts_order_5/1, test_erl_opts_debug_info/1, - first_files_exception/1, - deduplication_stability/1]). + first_files_exception/1]). -include_lib("common_test/include/ct.hrl"). -include_lib("eunit/include/eunit.hrl"). @@ -53,8 +52,7 @@ all() -> test_profile_erl_opts_order_4, test_profile_erl_opts_order_5, test_erl_opts_debug_info, - first_files_exception, - deduplication_stability]. + first_files_exception]. init_per_suite(Config) -> application:start(meck), @@ -135,7 +133,7 @@ profile_merge_umbrella_keys(Config) -> {profiles, [{ct, [{vals, [{a,1},{b,2}]}]}]}], - + SubRebarConfig = [{vals, []}, {profiles, [{ct, [{vals, [{c,1}]}]}]}], @@ -548,17 +546,6 @@ first_files_exception(_Config) -> ?assertEqual(["c","a","b","a","e"], rebar_state:get(State1, mib_first_files)), ok. -deduplication_stability(_Config) -> - ?assertEqual([default,all_deps_test], rebar_state:deduplicate([default,all_deps_test])), - ?assertEqual([default,profile1,profile2], rebar_state:deduplicate([default,profile1,profile2])), - ?assertEqual([default,bar,foo], rebar_state:deduplicate([default,bar,foo,bar])), %% master wants [default,foo,bar] - ?assertEqual([default,test,bar], rebar_state:deduplicate([default,test,bar])), - ?assertEqual([default,test,bar], rebar_state:deduplicate([default,test,bar,test])), - ?assertEqual([default,profile1], rebar_state:deduplicate([default,profile1,profile1,profile1])), - ?assertEqual([default,a,b,c,d,e], rebar_state:deduplicate([default,a,b,c,d,e,a,e,b])), - ?assertEqual([default,test], rebar_state:deduplicate([default,test])), - ok. - get_compiled_profile_erl_opts(Profiles, Config) -> AppDir = ?config(apps, Config), PStrs = [atom_to_list(P) || P <- Profiles], From 1d2aa24ef0d0bc6d1177cce361d7718f887eb03e Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Fri, 23 Feb 2018 12:50:38 +0100 Subject: [PATCH 6/9] Revert "sort-as: found the issue. Will look into tests now" This reverts commit 0f7e6c31e97c238649e7ae0a1b7087e342174ecc. --- src/rebar_state.erl | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/rebar_state.erl b/src/rebar_state.erl index 7ebe6dad2..577ed2314 100644 --- a/src/rebar_state.erl +++ b/src/rebar_state.erl @@ -87,8 +87,8 @@ new(Config) when is_list(Config) -> opts = Opts }. -spec new(t() | atom(), list()) -> t(). -new(Profile, Config) when is_atom(Profile), - is_list(Config) -> +new(Profile, Config) when is_atom(Profile) + , is_list(Config) -> BaseState = base_state(), Opts = base_opts(Config), BaseState#state_t { dir = rebar_dir:get_cwd(), @@ -283,12 +283,11 @@ apply_profiles(State=#state_t{default = Defaults, current_profiles=CurrentProfil end, Defaults, AppliedProfiles), State#state_t{current_profiles = AppliedProfiles, opts=NewOpts}. -%% @doc A stable deduplicator. deduplicate(Profiles) -> - do_deduplicate(Profiles, []). + do_deduplicate(lists:reverse(Profiles), []). do_deduplicate([], Acc) -> - lists:reverse(Acc); + Acc; do_deduplicate([Head | Rest], Acc) -> case lists:member(Head, Acc) of true -> do_deduplicate(Rest, Acc); From 0d88ff8891d1545721b7b61bc866713a13d0905a Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Fri, 23 Feb 2018 13:06:39 +0100 Subject: [PATCH 7/9] do not append test profile if already there. Note that it comes from the prv list passed to providers:create/1 --- src/rebar_state.erl | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/rebar_state.erl b/src/rebar_state.erl index 577ed2314..ac77325b9 100644 --- a/src/rebar_state.erl +++ b/src/rebar_state.erl @@ -257,12 +257,15 @@ apply_profiles(State, Profile) when not is_list(Profile) -> apply_profiles(State, [default]) -> State; apply_profiles(State=#state_t{default = Defaults, current_profiles=CurrentProfiles}, Profiles) -> + IsTesting = lists:member(test, CurrentProfiles), AppliedProfiles = case Profiles of %% Head of list global profile is special, only for use by rebar3 %% It does not clash if a user does `rebar3 as global...` but when %% it is the head we must make sure not to prepend `default` [global | _] -> Profiles; + [test] when IsTesting -> + deduplicate(CurrentProfiles); _ -> deduplicate(CurrentProfiles ++ Profiles) end, From cfe6dfcca5d6526d835531a81cb5bdc1a21c17b3 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Fri, 23 Feb 2018 13:08:02 +0100 Subject: [PATCH 8/9] sort-as: bar profile specializes dep "b" into a version anterior to what test profile wants --- test/rebar_profiles_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/rebar_profiles_SUITE.erl b/test/rebar_profiles_SUITE.erl index 6afdc393b..ddc3cf1dc 100644 --- a/test/rebar_profiles_SUITE.erl +++ b/test/rebar_profiles_SUITE.erl @@ -211,7 +211,7 @@ implicit_profile_deduplicate_deps(Config) -> rebar_test_utils:run_and_check(Config, RebarConfig, ["as", "test,bar", "eunit"], {ok, [{app, Name} ,{dep, "a", "1.0.0"} - ,{dep, "b", "2.0.0"}]}). + ,{dep, "b", "1.0.0"}]}). all_deps_code_paths(Config) -> AppDir = ?config(apps, Config), From 1271458c511100a5402a7aec94e48904ad792ac7 Mon Sep 17 00:00:00 2001 From: Pierre Fenoll Date: Sun, 25 Feb 2018 13:39:45 +0100 Subject: [PATCH 9/9] sort-as: a more general pattern --- src/rebar_state.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rebar_state.erl b/src/rebar_state.erl index ac77325b9..dd1f43fc0 100644 --- a/src/rebar_state.erl +++ b/src/rebar_state.erl @@ -257,14 +257,14 @@ apply_profiles(State, Profile) when not is_list(Profile) -> apply_profiles(State, [default]) -> State; apply_profiles(State=#state_t{default = Defaults, current_profiles=CurrentProfiles}, Profiles) -> - IsTesting = lists:member(test, CurrentProfiles), + ProvidedProfiles = lists:prefix([default|Profiles], CurrentProfiles), AppliedProfiles = case Profiles of %% Head of list global profile is special, only for use by rebar3 %% It does not clash if a user does `rebar3 as global...` but when %% it is the head we must make sure not to prepend `default` [global | _] -> Profiles; - [test] when IsTesting -> + _ when ProvidedProfiles -> deduplicate(CurrentProfiles); _ -> deduplicate(CurrentProfiles ++ Profiles)