Merge pull request #252 from tsloughter/master

Split app and release discovery and format_error fix

13 files changed, 150 insertions, 86 deletions

diff --git a/src/rlx_app_info.erl b/src/rlx_app_info.erl
index 931c139..ab35195 100644
--- a/src/rlx_app_info.erl
+++ b/src/rlx_app_info.erl
@@ -52,7 +52,7 @@
          library_deps/2,
          link/1,
          link/2,
-         format_error/1,
+         format_error/2,
          format/2,
          format/1]).
 
@@ -170,8 +170,8 @@ link(#app_info_t{link=Link}) ->
 link(AppInfo, NewLink) ->
     AppInfo#app_info_t{link=NewLink}.
 
--spec format_error(Reason::term()) -> iolist().
-format_error({vsn_parse, AppName}) ->
+-spec format_error(Reason::term(), rlx_state:t()) -> iolist().
+format_error({vsn_parse, AppName}, _) ->
     io_lib:format("Error parsing version for ~p",
                   [AppName]).
 
diff --git a/src/rlx_config.erl b/src/rlx_config.erl
index c838c18..307d2b6 100644
--- a/src/rlx_config.erl
+++ b/src/rlx_config.erl
@@ -25,7 +25,7 @@
 
 %% API
 -export([do/1,
-         format_error/1]).
+         format_error/2]).
 
 -include("relx.hrl").
 
@@ -46,11 +46,11 @@ do(State) ->
             load_config(ConfigFile, State)
     end.
 
--spec format_error(Reason::term()) -> iolist().
-format_error({consult, ConfigFile, Reason}) ->
+-spec format_error(Reason::term(), rlx_state:t()) -> iolist().
+format_error({consult, ConfigFile, Reason}, _) ->
     io_lib:format("Unable to read file ~s: ~s", [ConfigFile,
                                                  file:format_error(Reason)]);
-format_error({invalid_term, Term}) ->
+format_error({invalid_term, Term}, _) ->
     io_lib:format("Invalid term in config file: ~p", [Term]).
 
 %%%===================================================================
diff --git a/src/rlx_depsolver.erl b/src/rlx_depsolver.erl
index fd26145..c7f2f5c 100644
--- a/src/rlx_depsolver.erl
+++ b/src/rlx_depsolver.erl
@@ -76,7 +76,7 @@
 -module(rlx_depsolver).
 
 %% Public Api
--export([format_error/1,
+-export([format_error/2,
          format_roots/1,
          format_culprits/1,
          format_constraint/1,
@@ -326,9 +326,9 @@ filter_packages(PVPairs, RawConstraints) ->
 %% could not be satisfied
 -spec format_error({error, {unreachable_package, list()} |
                            {invalid_constraints, [constraint()]} |
-                           list()}) -> iolist().
-format_error(Error) ->
-    rlx_depsolver_culprit:format_error(Error).
+                           list()}, rlx_state:t()) -> iolist().
+format_error(Error, State) ->
+    rlx_depsolver_culprit:format_error(Error, State).
 
 %% @doc Return a formatted list of roots of the dependency trees which
 %% could not be satisified. These may also have versions attached.
diff --git a/src/rlx_depsolver_culprit.erl b/src/rlx_depsolver_culprit.erl
index cf6dcb2..4448777 100644
--- a/src/rlx_depsolver_culprit.erl
+++ b/src/rlx_depsolver_culprit.erl
@@ -31,7 +31,7 @@
 -module(rlx_depsolver_culprit).
 
 -export([search/3,
-        format_error/1,
+        format_error/2,
         format_version/1,
         format_constraint/1,
         format_roots/1,
@@ -68,19 +68,19 @@ search(State, ActiveCons, [NewCon | Constraints]) ->
             search(State, [NewCon | ActiveCons], Constraints)
     end.
 
-format_error({error, {unreachable_package, AppName}}) ->
+format_error({error, {unreachable_package, AppName}}, _) ->
     ["Dependency ", format_constraint(AppName), " is specified as a dependency ",
      "but is not reachable by the system.\n"];
-format_error({error, {invalid_constraints, Constraints}}) ->
+format_error({error, {invalid_constraints, Constraints}}, _) ->
     ["Invalid constraint ", add_s(Constraints), " specified ",
      lists:foldl(fun(Con, "") ->
                          [io_lib:format("~p", [Con])];
                     (Con, Acc) ->
                          [io_lib:format("~p", [Con]), ", " | Acc]
                  end, "", Constraints)];
-format_error({error, Detail}) ->
-    format_error(Detail);
-format_error(Details) when erlang:is_list(Details) ->
+format_error({error, Detail}, State) ->
+    format_error(Detail, State);
+format_error(Details, _) when erlang:is_list(Details) ->
     ["Unable to solve constraints, the following solutions were attempted \n\n",
      [[format_error_path("    ", Detail)] || Detail <- Details]].
 
diff --git a/src/rlx_dscv_util.erl b/src/rlx_dscv_util.erl
index fefdbce..90153e9 100644
--- a/src/rlx_dscv_util.erl
+++ b/src/rlx_dscv_util.erl
@@ -25,7 +25,7 @@
 -module(rlx_dscv_util).
 
 -export([do/2,
-         format_error/1]).
+         format_error/2]).
 
 -include("relx.hrl").
 
@@ -54,8 +54,8 @@ do(ProcessDir, LibDirs) ->
                                                      ec_file:type(LibDir))
                                 end, LibDirs)).
 
--spec format_error([ErrorDetail::term()]) -> iolist().
-format_error(ErrorDetails)
+-spec format_error([ErrorDetail::term()], rlx_state:t()) -> iolist().
+format_error(ErrorDetails, _)
   when erlang:is_list(ErrorDetails) ->
     [[format_detail(ErrorDetail), "\n"] || ErrorDetail <- ErrorDetails].
 
diff --git a/src/rlx_prv_discover.erl b/src/rlx_prv_app_discover.erl
index 41e3993..154908c 100644
--- a/src/rlx_prv_discover.erl
+++ b/src/rlx_prv_app_discover.erl
@@ -22,7 +22,8 @@
 %%% Lib Dirs looking for all OTP Applications that are available. When it finds
 %%% those OTP Applications it loads the information about them and adds them to
 %%% the state of available apps. This implements the provider behaviour.
--module(rlx_prv_discover).
+-module(rlx_prv_app_discover).
+
 -behaviour(provider).
 
 -export([init/1,
@@ -31,7 +32,7 @@
 
 -include("relx.hrl").
 
--define(PROVIDER, discover).
+-define(PROVIDER, app_discover).
 -define(DEPS, []).
 
 %%============================================================================
@@ -57,15 +58,8 @@ do(State0) ->
     LibDirs = get_lib_dirs(State0),
     case rlx_app_discovery:do(State0, LibDirs) of
         {ok, AppMeta} ->
-            case rlx_rel_discovery:do(State0, LibDirs, AppMeta) of
-                {ok, Releases} ->
-                    State1 = rlx_state:available_apps(State0, AppMeta),
-                    {ok, rlx_state:realized_releases(State1, lists:foldl(fun add/2,
-                                                                         ec_dictionary:new(ec_dict),
-                                                                         Releases))};
-                Error ->
-                    Error
-            end;
+            State1 = rlx_state:available_apps(State0, AppMeta),
+            {ok, State1};
         Error ->
             Error
     end.
@@ -79,11 +73,6 @@ format_error(_, _) ->
 %%%===================================================================
 %%% Internal Functions
 %%%===================================================================
-%% @doc only add the release if its not documented in the system
-add(Rel, Dict) ->
-    RelName = rlx_release:name(Rel),
-    RelVsn = rlx_release:vsn(Rel),
-    ec_dictionary:add({RelName, RelVsn}, Rel, Dict).
 
 get_lib_dirs(State) ->
     LibDirs0 = rlx_state:lib_dirs(State),
@@ -93,8 +82,7 @@ get_lib_dirs(State) ->
         true ->
             lists:flatten([LibDirs0,
                            add_common_project_dirs(State),
-                           add_system_lib_dir(State),
-                           add_release_output_dir(State)])
+                           add_system_lib_dir(State)])
     end.
 
 -spec add_common_project_dirs(rlx_state:t()) -> [file:name()].
@@ -135,17 +123,3 @@ add_system_lib_dir(State) ->
         SystemLibs ->
             erlang:iolist_to_binary(SystemLibs)
     end.
-
-add_release_output_dir(State) ->
-    case rlx_state:get(State, disable_discover_release_output, false) of
-        true ->
-            [];
-        false ->
-            Output = erlang:iolist_to_binary(rlx_state:base_output_dir(State)),
-            case ec_file:exists(erlang:binary_to_list(Output)) of
-                true ->
-                    Output;
-                false ->
-                    []
-            end
-    end.
diff --git a/src/rlx_prv_rel_discover.erl b/src/rlx_prv_rel_discover.erl
new file mode 100644
index 0000000..8b6fd88
--- /dev/null
+++ b/src/rlx_prv_rel_discover.erl
@@ -0,0 +1,97 @@
+%% -*- erlang-indent-level: 4; indent-tabs-mode: nil; fill-column: 80 -*-
+%%% Copyright 2012 Erlware, LLC. All Rights Reserved.
+%%%
+%%% This file is provided to you under the Apache License,
+%%% Version 2.0 (the "License"); you may not use this file
+%%% except in compliance with the License.  You may obtain
+%%% a copy of the License at
+%%%
+%%%   http://www.apache.org/licenses/LICENSE-2.0
+%%%
+%%% Unless required by applicable law or agreed to in writing,
+%%% software distributed under the License is distributed on an
+%%% "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+%%% KIND, either express or implied.  See the License for the
+%%% specific language governing permissions and limitations
+%%% under the License.
+%%%---------------------------------------------------------------------------
+%%% @author Eric Merritt <[email protected]>
+%%% @copyright (C) 2012 Erlware, LLC.
+%%%
+-module(rlx_prv_rel_discover).
+-behaviour(provider).
+
+-export([init/1,
+         do/1,
+         format_error/2]).
+
+-include("relx.hrl").
+
+-define(PROVIDER, rel_discover).
+-define(DEPS, [app_discover]).
+
+%%============================================================================
+%% API
+%%============================================================================
+
+-spec init(rlx_state:t()) -> {ok, rlx_state:t()}.
+init(State) ->
+    State1 = rlx_state:add_provider(State, providers:create([{name, ?PROVIDER},
+                                                             {module, ?MODULE},
+                                                             {bare, false},
+                                                             {deps, ?DEPS},
+                                                             {example, "build"},
+                                                             {short_desc, ""},
+                                                             {desc, ""},
+                                                             {opts, []}])),
+    {ok, State1}.
+
+-spec do(rlx_state:t()) -> {ok, rlx_state:t()} | relx:error().
+do(State0) ->
+    LibDirs = get_lib_dirs(State0),
+    AppMeta = rlx_state:available_apps(State0),
+    case rlx_rel_discovery:do(State0, LibDirs, AppMeta) of
+        {ok, Releases} ->
+            {ok, rlx_state:realized_releases(State0, lists:foldl(fun add/2,
+                                                                 ec_dictionary:new(ec_dict),
+                                                                 Releases))};
+        Error ->
+            Error
+    end.
+
+-spec format_error(any(), rlx_state:t()) -> iolist().
+format_error(_, _) ->
+    "".
+
+%%%===================================================================
+%%% Internal Functions
+%%%===================================================================
+%% @doc only add the release if its not documented in the system
+add(Rel, Dict) ->
+    RelName = rlx_release:name(Rel),
+    RelVsn = rlx_release:vsn(Rel),
+    ec_dictionary:add({RelName, RelVsn}, Rel, Dict).
+
+get_lib_dirs(State) ->
+    LibDirs0 = rlx_state:lib_dirs(State),
+    case rlx_state:get(State, default_libs, true) of
+        false ->
+            LibDirs0;
+        true ->
+            lists:flatten([LibDirs0,
+                           add_release_output_dir(State)])
+    end.
+
+add_release_output_dir(State) ->
+    case rlx_state:get(State, disable_discover_release_output, false) of
+        true ->
+            [];
+        false ->
+            Output = erlang:iolist_to_binary(rlx_state:base_output_dir(State)),
+            case ec_file:exists(erlang:binary_to_list(Output)) of
+                true ->
+                    Output;
+                false ->
+                    []
+            end
+    end.
diff --git a/src/rlx_prv_release.erl b/src/rlx_prv_release.erl
index bd5acc4..3eb59d1 100644
--- a/src/rlx_prv_release.erl
+++ b/src/rlx_prv_release.erl
@@ -18,10 +18,6 @@
 %%% @author Eric Merritt <[email protected]>
 %%% @copyright (C) 2012 Erlware, LLC.
 %%%
-%%% @doc This provider uses the lib_dir setting of the state. It searches the
-%%% Lib Dirs looking for all OTP Applications that are available. When it finds
-%%% those OTP Applications it loads the information about them and adds them to
-%%% the state of available apps. This implements the provider behaviour.
 -module(rlx_prv_release).
 
 -behaviour(provider).
@@ -33,7 +29,7 @@
 -include("relx.hrl").
 
 -define(PROVIDER, resolve_release).
--define(DEPS, [discover]).
+-define(DEPS, [app_discover]).
 
 %%============================================================================
 %% API
@@ -75,9 +71,9 @@ format_error({no_releases_for, RelName}, _) ->
     io_lib:format("No releases exist in the system for ~s!", [RelName]);
 format_error({release_not_found, {RelName, RelVsn}}, _) ->
     io_lib:format("No releases exist in the system for ~p:~s!", [RelName, RelVsn]);
-format_error({failed_solve, Error}, _) ->
+format_error({failed_solve, Error}, State) ->
     io_lib:format("Failed to solve release:\n ~s",
-                  [rlx_depsolver:format_error({error, Error})]).
+                  [rlx_depsolver:format_error({error, Error}, State)]).
 
 %%%===================================================================
 %%% Internal Functions
diff --git a/src/rlx_prv_relup.erl b/src/rlx_prv_relup.erl
index df6f831..0888c0c 100644
--- a/src/rlx_prv_relup.erl
+++ b/src/rlx_prv_relup.erl
@@ -31,7 +31,7 @@
 -include("relx.hrl").
 
 -define(PROVIDER, relup).
--define(DEPS, [release]).
+-define(DEPS, [rel_discover, release]).
 
 %%============================================================================
 %% API
diff --git a/src/rlx_rel_discovery.erl b/src/rlx_rel_discovery.erl
index b7c15bc..dc6210f 100644
--- a/src/rlx_rel_discovery.erl
+++ b/src/rlx_rel_discovery.erl
@@ -18,14 +18,10 @@
 %%% @author Eric Merritt <[email protected]>
 %%% @copyright (C) 2012 Erlware, LLC.
 %%%
-%%% @doc This provider uses the lib_dir setting of the state. It searches the
-%%% Lib Dirs looking for all OTP Applications that are available. When it finds
-%%% those OTP Applications it loads the information about them and adds them to
-%%% the state of available apps. This implements the provider behaviour.
 -module(rlx_rel_discovery).
 
 -export([do/3,
-         format_error/1]).
+         format_error/2]).
 
 -include("relx.hrl").
 
@@ -51,8 +47,8 @@ do(State, LibDirs, AppMeta) ->
             resolve_rel_metadata(State, LibDirs, AppMeta)
     end.
 
--spec format_error([ErrorDetail::term()]) -> iolist().
-format_error(ErrorDetails)
+-spec format_error([ErrorDetail::term()], rlx_state:t()) -> iolist().
+format_error(ErrorDetails, _)
   when erlang:is_list(ErrorDetails) ->
     [[format_detail(ErrorDetail), "\n"] || ErrorDetail <- ErrorDetails].
 
diff --git a/src/rlx_state.erl b/src/rlx_state.erl
index 550a44a..b7cb6ba 100644
--- a/src/rlx_state.erl
+++ b/src/rlx_state.erl
@@ -438,7 +438,8 @@ add_hook(post, {PreHooks, PostHooks}, Hook) ->
 
 -spec create_logic_providers(t()) -> t() | relx:error().
 create_logic_providers(State) ->
-    create_all(State, [rlx_prv_discover,
+    create_all(State, [rlx_prv_app_discover,
+                       rlx_prv_rel_discover,
                        rlx_prv_overlay,
                        rlx_prv_release,
                        rlx_prv_assembler,
diff --git a/test/rlx_depsolver_tests.erl b/test/rlx_depsolver_tests.erl
index 7cbe831..8917db7 100644
--- a/test/rlx_depsolver_tests.erl
+++ b/test/rlx_depsolver_tests.erl
@@ -137,7 +137,7 @@ fail_test() ->
 
     Ret = rlx_depsolver:solve(Dom0, [{app1, "0.1"}]),
     %% We do this to make sure all errors can be formated.
-    _ = rlx_depsolver:format_error(Ret),
+    _ = rlx_depsolver:format_error(Ret, []),
     ?assertMatch({error,
                   [{[{[{app1,{{0,1},{[],[]}}}],
                       [{app1,{{0,1},{[],[]}}},[[{app2,{{0,2},{[],[]}}}]]]}],
@@ -215,7 +215,7 @@ conflicting_failing_test() ->
                                                                   {app5, [{"2.0.0", []},
                                                                           {"6.0.0", []}]}]),
     Ret = rlx_depsolver:solve(Dom0, [app1, app3]),
-    _ = rlx_depsolver:format_error(Ret),
+    _ = rlx_depsolver:format_error(Ret, []),
     ?assertMatch({error,
                   [{[{[app1],
                       [{app1,{{3,0},{[],[]}}},
@@ -351,7 +351,7 @@ filter_versions_test() ->
 
     Ret = rlx_depsolver:filter_packages(Packages,
                                         [{"foo", "1.0.0", '~~~~'} | Cons]),
-    _ = rlx_depsolver:format_error(Ret),
+    _ = rlx_depsolver:format_error(Ret, []),
     ?assertMatch({error, {invalid_constraints, [{<<"foo">>,{{1,0,0},{[],[]}},'~~~~'}]}}, Ret).
 
 
@@ -370,11 +370,11 @@ missing_test() ->
                                                                           {"0.2", []},
                                                                           {"0.3", []}]}]),
     Ret1 = rlx_depsolver:solve(Dom0, [{app4, "0.1"}, {app3, "0.1"}]),
-    _ = rlx_depsolver:format_error(Ret1),
+    _ = rlx_depsolver:format_error(Ret1, []),
     ?assertMatch({error,{unreachable_package,app4}}, Ret1),
 
     Ret2 = rlx_depsolver:solve(Dom0, [{app1, "0.1"}]),
-    _ = rlx_depsolver:format_error(Ret2),
+    _ = rlx_depsolver:format_error(Ret2, []),
     ?assertMatch({error,{unreachable_package,app4}},
                  Ret2).
 
@@ -387,7 +387,7 @@ binary_test() ->
                                                          World),
                               [<<"foo">>]),
 
-    _ = rlx_depsolver:format_error(Ret),
+    _ = rlx_depsolver:format_error(Ret, []),
     ?assertMatch({error,
                   [{[{[<<"foo">>],[{<<"foo">>,{{1,2,3},{[],[]}}}]}],
                     [{{<<"foo">>,{{1,2,3},{[],[]}}},
@@ -408,7 +408,7 @@ doesnt_exist_test() ->
     Constraints = [{<<"foo">>,[{<<"1.2.3">>, [{<<"bar">>, <<"2.0.0">>, gt}]}]}],
     World = rlx_depsolver:add_packages(rlx_depsolver:new_graph(), Constraints),
     Ret = rlx_depsolver:solve(World, [<<"foo">>]),
-    _ = rlx_depsolver:format_error(Ret),
+    _ = rlx_depsolver:format_error(Ret, []),
     ?assertMatch({error,{unreachable_package,<<"bar">>}}, Ret).
 
 %%
@@ -428,7 +428,7 @@ not_new_enough_test() ->
                    {<<"bar">>, [{<<"2.0.0">>, []}]}],
     World = rlx_depsolver:add_packages(rlx_depsolver:new_graph(), Constraints),
     Ret = rlx_depsolver:solve(World, [<<"foo">>]),
-    _ = rlx_depsolver:format_error(Ret),
+    _ = rlx_depsolver:format_error(Ret, []),
     ?assertMatch({error,
                   [{[{[<<"foo">>],[{<<"foo">>,{{1,2,3},{[],[]}}}]}],
                     [{{<<"foo">>,{{1,2,3},{[],[]}}},
@@ -449,7 +449,7 @@ impossible_dependency_test() ->
                                        [{<<"foo">>, [{<<"1.2.3">>,[{ <<"bar">>, <<"2.0.0">>, gt}]}]},
                                         {<<"bar">>, [{<<"2.0.0">>, [{ <<"foo">>, <<"3.0.0">>, gt}]}]}]),
     Ret = rlx_depsolver:solve(World, [<<"foo">>]),
-    _ = rlx_depsolver:format_error(Ret),
+    _ = rlx_depsolver:format_error(Ret, []),
     ?assertMatch({error,
                   [{[{[<<"foo">>],[{<<"foo">>,{{1,2,3},{[],[]}}}]}],
                     [{{<<"foo">>,{{1,2,3},{[],[]}}},
diff --git a/test/rlx_discover_SUITE.erl b/test/rlx_discover_SUITE.erl
index 2db8e4d..4a3b817 100644
--- a/test/rlx_discover_SUITE.erl
+++ b/test/rlx_discover_SUITE.erl
@@ -77,8 +77,8 @@ normal_case(Config) ->
                      || _ <- lists:seq(1, 100)]],
     State0 = rlx_state:put(proplists:get_value(state, Config),
                             default_libs, false),
-    {ok, State1} = providers:new(rlx_prv_discover, State0),
-    DiscoverProvider = providers:get_provider(discover, rlx_state:providers(State1)),
+    {ok, State1} = providers:new(rlx_prv_app_discover, State0),
+    DiscoverProvider = providers:get_provider(app_discover, rlx_state:providers(State1)),
     {ok, State2} = providers:do(DiscoverProvider, State1),
 
     lists:foreach(fun(App) ->
@@ -114,10 +114,10 @@ no_beam_case(Config) ->
     AppDir = filename:join([LibDir2, BadName]),
     write_app_file(AppDir, BadName, BadVsn),
     State0 = proplists:get_value(state, Config),
-    %% Deliberately disable release discovery when running `rlx_prv_discover`
+    %% Deliberately disable release discovery when running `rlx_prv_app_discover`
     State1 = rlx_state:put(State0, disable_rel_discovery, true),
-    {ok, State2} = providers:new(rlx_prv_discover, State1),
-    DiscoverProvider = providers:get_provider(discover, rlx_state:providers(State2)),
+    {ok, State2} = providers:new(rlx_prv_app_discover, State1),
+    DiscoverProvider = providers:get_provider(app_discover, rlx_state:providers(State2)),
 
     ?assertMatch({ok, _},
                  providers:do(DiscoverProvider, State2)).
@@ -146,8 +146,8 @@ bad_ebin_case(Config) ->
     ok = filelib:ensure_dir(Filename),
     ok = ec_file:write_term(Filename, get_bad_app_metadata(BadName, BadVsn)),
     State0 = proplists:get_value(state, Config),
-    {ok, State1} = providers:new(rlx_prv_discover, State0),
-    DiscoverProvider = providers:get_provider(discover, rlx_state:providers(State1)),
+    {ok, State1} = providers:new(rlx_prv_app_discover, State0),
+    DiscoverProvider = providers:get_provider(app_discover, rlx_state:providers(State1)),
     {ok, State2} = providers:do(DiscoverProvider, State1),
     ?assertMatch([], [App || App <- rlx_state:available_apps(State2),
                              BadName =:= rlx_app_info:name(App)]).
@@ -172,8 +172,8 @@ shallow_app_discovery(Config) ->
     State0 = rlx_state:put(proplists:get_value(state, Config),
                            default_libs, false),
     State1 = rlx_state:put(State0, enable_shallow_app_discovery, true),
-    {ok, State2} = providers:new(rlx_prv_discover, State1),
-    DiscoverProvider = providers:get_provider(discover, rlx_state:providers(State2)),
+    {ok, State2} = providers:new(rlx_prv_app_discover, State1),
+    DiscoverProvider = providers:get_provider(app_discover, rlx_state:providers(State2)),
     {ok, State3} = providers:do(DiscoverProvider, State2),
     lists:foreach(fun(App) ->
                           ?assertMatch(true, lists:member(App, rlx_state:available_apps(State3)))