rcl_app_info must support comparible versions

This changes moves rcl_app_info to store the version as a parsed
semver rather then a string, making it comparable.
tmp app info

3 files changed, 67 insertions, 32 deletions

diff --git a/src/rcl_app_info.erl b/src/rcl_app_info.erl
index d53c1aa..4d91961 100644
--- a/src/rcl_app_info.erl
+++ b/src/rcl_app_info.erl
@@ -48,12 +48,14 @@
          active_deps/2,
          library_deps/1,
          library_deps/2,
+         format_error/1,
+         format/2,
          format/1]).
 
 -export_type([t/0]).
 
 -record(app_info_t, {name :: atom(),
-                     vsn :: string(),
+                     vsn :: ec_semver:semver(),
                      dir :: file:name(),
                      active_deps :: [atom()],
                      library_deps :: [atom()]}).
@@ -68,21 +70,26 @@
 %% ============================================================================
 %% @doc Build a new, empty, app info value. This is not of a lot of use and you
 %% probably wont be doing this much.
--spec new() -> t().
+-spec new() -> {ok, t()}.
 new() ->
-    #app_info_t{}.
+    {ok, #app_info_t{}}.
 
 %% @doc build a complete version of the app info with all fields set.
--spec new(atom(), string(), file:name(), [atom()], [atom()]) -> t().
+-spec new(atom(), string(), file:name(), [atom()], [atom()]) ->
+                 {ok, t()} | {error, Reason::term()}.
 new(AppName, Vsn, Dir, ActiveDeps, LibraryDeps)
   when erlang:is_atom(AppName),
-       erlang:is_list(Vsn),
        erlang:is_list(Dir),
        erlang:is_list(ActiveDeps),
        erlang:is_list(LibraryDeps) ->
-    #app_info_t{name=AppName, vsn=Vsn, dir=Dir,
-                active_deps=ActiveDeps,
-                library_deps=LibraryDeps}.
+    case parse_version(Vsn) of
+        {fail, _} ->
+            {error, {vsn_parse, AppName}};
+        ParsedVsn ->
+            {ok, #app_info_t{name=AppName, vsn=ParsedVsn, dir=Dir,
+                             active_deps=ActiveDeps,
+                             library_deps=LibraryDeps}}
+    end.
 
 -spec name(t()) -> atom().
 name(#app_info_t{name=Name}) ->
@@ -93,14 +100,20 @@ name(AppInfo=#app_info_t{}, AppName)
   when erlang:is_atom(AppName) ->
     AppInfo#app_info_t{name=AppName}.
 
--spec vsn(t()) -> string().
+-spec vsn(t()) -> ec_semver:semver().
 vsn(#app_info_t{vsn=Vsn}) ->
     Vsn.
 
--spec vsn(t(), string()) -> t().
-vsn(AppInfo=#app_info_t{}, AppVsn)
+-spec vsn(t(), string()) -> {ok, t()} | {error, Reason::term()}.
+vsn(AppInfo=#app_info_t{name=AppName}, AppVsn)
   when erlang:is_list(AppVsn) ->
-    AppInfo#app_info_t{vsn=AppVsn}.
+    case parse_version(AppVsn) of
+        {fail, _} ->
+            {error, {vsn_parse, AppName}};
+        ParsedVsn ->
+            {ok, AppInfo#app_info_t{vsn=ParsedVsn}}
+    end.
+
 
 -spec dir(t()) -> file:name().
 dir(#app_info_t{dir=Dir}) ->
@@ -126,17 +139,32 @@ library_deps(AppInfo=#app_info_t{}, LibraryDeps)
   when erlang:is_list(LibraryDeps) ->
     AppInfo#app_info_t{library_deps=LibraryDeps}.
 
+-spec format_error({error, Reason::term()}) -> iolist().
+format_error({error, {vsn_parse, AppName, AppDir}}) ->
+    io_lib:format("Error parsing version for ~p at ~s",
+                  [AppName, AppDir]).
+
 -spec format(t()) -> iolist().
-format(#app_info_t{name=Name, vsn=Vsn, dir=Dir,
-                   active_deps=Deps, library_deps=LibDeps}) ->
-    [erlang:atom_to_list(Name), "-", Vsn, ": ", Dir, "\n",
-     rcl_util:indent(1), "Active Dependencies:\n",
-     [[rcl_util:indent(2), erlang:atom_to_list(Dep), ",\n"] || Dep <- Deps],
-     rcl_util:indent(1), "Library Dependencies:\n",
-     [[rcl_util:indent(2), erlang:atom_to_list(LibDep), ",\n"] || LibDep <- LibDeps]].
+format(AppInfo) ->
+    format(0, AppInfo).
+
+-spec format(non_neg_integer(), t()) -> iolist().
+format(Indent, #app_info_t{name=Name, vsn=Vsn, dir=Dir,
+                           active_deps=Deps, library_deps=LibDeps}) ->
+    [rcl_util:indent(Indent), erlang:atom_to_list(Name), "-", depsolver:format_version(Vsn),
+     ": ", Dir, "\n",
+     rcl_util:indent(Indent + 1), "Active Dependencies:\n",
+     [[rcl_util:indent(Indent + 2), erlang:atom_to_list(Dep), ",\n"] || Dep <- Deps],
+     rcl_util:indent(Indent + 1), "Library Dependencies:\n",
+     [[rcl_util:indent(Indent + 2), erlang:atom_to_list(LibDep), ",\n"] || LibDep <- LibDeps]].
 
 
 
 %%%===================================================================
-%%% Test Functions
+%%% Internal Functions
 %%%===================================================================
+parse_version(Vsn)
+  when erlang:is_list(Vsn) ->
+    ec_semver:parse(Vsn);
+parse_version(Vsn = {_, {_, _}}) ->
+    Vsn.
diff --git a/src/rcl_prv_discover.erl b/src/rcl_prv_discover.erl
index 1acf85a..4ac21a6 100644
--- a/src/rcl_prv_discover.erl
+++ b/src/rcl_prv_discover.erl
@@ -44,7 +44,7 @@ do(State) ->
     rcl_log:info(rcl_state:log(State),
                   fun() ->
                           ["Resolving OTP Applications from directories:\n",
-                           [[LibDir, "\n"] || LibDir <- LibDirs]]
+                           [[rcl_util:indent(1), LibDir, "\n"] || LibDir <- LibDirs]]
                   end),
 
     AppMeta0 = lists:flatten(ec_plists:map(fun discover_dir/1, LibDirs)),
@@ -69,7 +69,7 @@ do(State) ->
             rcl_log:debug(rcl_state:log(State),
                           fun() ->
                                   ["Resolved the following OTP Applications from the system: \n",
-                                   [[rcl_app_info:format(App), "\n"] || App <- AppMeta1]]
+                                   [[rcl_app_info:format(1, App), "\n"] || App <- AppMeta1]]
                           end),
             {ok, rcl_state:available_apps(State, AppMeta1)};
         _ ->
@@ -141,7 +141,10 @@ format_detail({error, {invalid_app_file, File}}) ->
                   [File]);
 format_detail({error, {unversioned_app, AppDir, _AppName}}) ->
     io_lib:format("Application metadata exists but version is not available: ~s",
-                  [AppDir]).
+                  [AppDir]);
+format_detail({error, {app_info_error, Detail}}) ->
+    rcl_app_info:format_error({error, Detail}).
+
 
 -spec discover_dir(file:name()) ->
                           [rcl_app_info:t() | {error, Reason::term()}] |
@@ -210,11 +213,16 @@ get_vsn(AppDir, AppName, AppDetail) ->
         undefined ->
             {error, {unversioned_app, AppDir, AppName}};
         AppVsn ->
-            get_deps(AppDir, AppName, AppVsn, AppDetail)
+            case get_deps(AppDir, AppName, AppVsn, AppDetail) of
+                {ok, App} ->
+                    App;
+                {error, Detail} ->
+                    {error, {app_info_error, Detail}}
+            end
     end.
 
 -spec get_deps(file:name(), atom(), string(), proplists:proplist()) ->
-                      rcl_app_info:t().
+                      {ok, rcl_app_info:t()} | {error, Reason::term()}.
 get_deps(AppDir, AppName, AppVsn, AppDetail) ->
     ActiveApps = proplists:get_value(applications, AppDetail, []),
     LibraryApps = proplists:get_value(included_applications, AppDetail, []),
diff --git a/test/rclt_discover_SUITE.erl b/test/rclt_discover_SUITE.erl
index dca683a..0929e4a 100644
--- a/test/rclt_discover_SUITE.erl
+++ b/test/rclt_discover_SUITE.erl
@@ -58,7 +58,7 @@ all() ->
 normal_case(Config) ->
     LibDir1 = proplists:get_value(lib1, Config),
     Apps1 = [(fun({Name, Vsn}) ->
-                     create_app(LibDir1, Name, Vsn)
+                      create_app(LibDir1, Name, Vsn)
               end)(App)
              ||
                 App <-
@@ -113,7 +113,6 @@ no_beam_case(Config) ->
     State0 = proplists:get_value(state, Config),
     {DiscoverProvider, {ok, State1}} = rcl_provider:new(rcl_prv_discover, State0),
     EbinDir = filename:join([LibDir2, BadName, "ebin"]),
-    %% Ignore apps that do not contain any beam files
     ?assertMatch({error, [{no_beam_files, EbinDir}]},
                  rcl_provider:do(DiscoverProvider, State1)).
 
@@ -145,20 +144,20 @@ bad_ebin_case(Config) ->
     State0 = proplists:get_value(state, Config),
     {DiscoverProvider, {ok, State1}} = rcl_provider:new(rcl_prv_discover, State0),
 
-    %% Ignore apps that do not contain any beam files
     ?assertMatch({error, [{invalid_app_file, Filename}]},
                  rcl_provider:do(DiscoverProvider, State1)).
 
 
-
-%%% API
+%%%===================================================================
+%%% Helper functions
 %%%===================================================================
 create_app(Dir, Name, Vsn) ->
     AppDir = filename:join([Dir, Name]),
     write_app_file(AppDir, Name, Vsn),
     write_beam_file(AppDir, Name),
-    rcl_app_info:new(erlang:list_to_atom(Name), Vsn, AppDir,
-                     [kernel, stdlib], []).
+    {ok, App} = rcl_app_info:new(erlang:list_to_atom(Name), Vsn, AppDir,
+                                 [kernel, stdlib], []),
+    App.
 
 write_beam_file(Dir, Name) ->
     Beam = filename:join([Dir, "ebin", "not_a_real_beam" ++ Name ++ ".beam"]),