Merge pull request #42 from tsloughter/validate-app-file

Check presence of each beam listed in .app

3 files changed, 111 insertions, 43 deletions

diff --git a/src/rlx_app_discovery.erl b/src/rlx_app_discovery.erl
index 033b9e7..39f3921 100644
--- a/src/rlx_app_discovery.erl
+++ b/src/rlx_app_discovery.erl
@@ -39,8 +39,8 @@
 do(State, LibDirs) ->
     rlx_log:info(rlx_state:log(State),
                  fun() ->
-                          ["Resolving OTP Applications from directories:\n",
-                           [[rlx_util:indent(2), LibDir, "\n"] || LibDir <- LibDirs]]
+                         ["Resolving OTP Applications from directories:\n",
+                          [[rlx_util:indent(2), LibDir, "\n"] || LibDir <- LibDirs]]
                  end),
     resolve_app_metadata(State, LibDirs).
 
@@ -58,13 +58,13 @@ resolve_app_metadata(State, LibDirs) ->
               {error, Ret} ->
                   Ret
           end
-          || Err <- AppMeta0,
-             case Err of
-                 {error, _} ->
-                         true;
-                     _ ->
-                         false
-                 end] of
+         || Err <- AppMeta0,
+            case Err of
+                {error, _} ->
+                    true;
+                _ ->
+                    false
+            end] of
         [] ->
             SkipApps = rlx_state:skip_apps(State),
             AppMeta1 = [App || {ok, App} <- setup_overrides(State, AppMeta0),
@@ -94,15 +94,17 @@ resolve_override(AppName, FileName0) ->
     FileName1 = filename:absname(FileName0),
     case is_valid_otp_app(filename:join([FileName1, <<"ebin">>,
                                          erlang:atom_to_list(AppName) ++ ".app"])) of
-         {noresult, false} ->
-             {error, {invalid_override, AppName, FileName1}};
-         Error = {error, _} ->
-             Error;
-         {ok, App} ->
-             {ok, rlx_app_info:link(App, true)}
-     end.
+        {noresult, false} ->
+            {error, {invalid_override, AppName, FileName1}};
+        Error = {error, _} ->
+            Error;
+        {ok, App} ->
+            {ok, rlx_app_info:link(App, true)}
+    end.
 
 -spec format_detail(ErrorDetail::term()) -> iolist().
+format_detail({error, {missing_beam_file, Module, BeamFile}}) ->
+    io_lib:format("Missing beam file ~p ~p", [Module, BeamFile]);
 format_detail({error, {invalid_override, AppName, FileName}}) ->
     io_lib:format("Override {~p, ~p} is not a valid OTP App. Perhaps you forgot to build it?",
                   [AppName, FileName]);
@@ -142,7 +144,7 @@ is_valid_otp_app(File) ->
         <<"ebin">> ->
             case filename:extension(File) of
                 <<".app">> ->
-                    has_at_least_one_beam(EbinDir, File);
+                    gather_application_info(EbinDir, File);
                 _ ->
                     {noresult, false}
             end;
@@ -150,20 +152,6 @@ is_valid_otp_app(File) ->
             {noresult, false}
     end.
 
--spec has_at_least_one_beam(file:name(), file:filename()) ->
-                                   {ok, rlx_app_info:t()} | {error, Reason::term()}.
-has_at_least_one_beam(EbinDir, File) ->
-    case file:list_dir(EbinDir) of
-        {ok, List} ->
-            case lists:any(fun(NFile) -> lists:suffix(".beam", NFile) end, List) of
-                true ->
-                    gather_application_info(EbinDir, File);
-                false ->
-                    {error, {no_beam_files, EbinDir}}
-            end;
-        _ ->
-            {error, {not_a_directory, EbinDir}}
-    end.
 
 -spec gather_application_info(file:name(), file:filename()) ->
                                      {ok, rlx_app_info:t()} | {error, Reason::term()}.
@@ -171,13 +159,55 @@ gather_application_info(EbinDir, File) ->
     AppDir = filename:dirname(EbinDir),
     case file:consult(File) of
         {ok, [{application, AppName, AppDetail}]} ->
-            get_vsn(AppDir, AppName, AppDetail);
+            validate_application_info(EbinDir, File, AppName, AppDetail);
         {error, Reason} ->
             {error, {unable_to_load_app, AppDir, Reason}};
         _ ->
             {error, {invalid_app_file, File}}
     end.
 
+-spec validate_application_info(file:name(),
+                                file:name(),
+                                atom(),
+                                proplists:proplist()) ->
+                                       {ok, list()} | {error, Reason::term()}.
+validate_application_info(EbinDir, AppFile, AppName, AppDetail) ->
+    AppDir = filename:dirname(EbinDir),
+    case get_modules_list(AppFile, AppDetail) of
+        {ok, List} ->
+            case has_all_beams(EbinDir, List) of
+                ok ->
+                    get_vsn(AppDir, AppName, AppDetail);
+                Error1 ->
+                    Error1
+            end;
+        Error -> Error
+    end.
+
+-spec get_modules_list(file:name(), proplists:proplist()) ->
+                              {ok, list()} | {error, Reason::term()}.
+get_modules_list(AppFile, AppDetail) ->
+    case proplists:get_value(modules, AppDetail) of
+        undefined ->
+            {error, {invalid_app_file, AppFile}};
+        ModulesList ->
+            {ok, ModulesList}
+    end.
+
+-spec has_all_beams(file:name(), list()) ->
+                           ok | {error, Reason::term()}.
+has_all_beams(EbinDir, [Module | ModuleList]) ->
+    BeamFile = filename:join([EbinDir,
+                              list_to_binary(atom_to_list(Module) ++ ".beam")]),
+    case ec_file:exists(BeamFile) of
+        true ->
+            has_all_beams(EbinDir, ModuleList);
+        false ->
+            {error, {missing_beam_file, Module, BeamFile}}
+    end;
+has_all_beams(_, []) ->
+    ok.
+
 -spec get_vsn(file:name(), atom(), proplists:proplist()) ->
                      {ok, rlx_app_info:t()} | {error, Reason::term()}.
 get_vsn(AppDir, AppName, AppDetail) ->
diff --git a/test/rlx_discover_SUITE.erl b/test/rlx_discover_SUITE.erl
index d404d8c..c4455e1 100644
--- a/test/rlx_discover_SUITE.erl
+++ b/test/rlx_discover_SUITE.erl
@@ -88,7 +88,7 @@ normal_case(Config) ->
     ?assertMatch(Length, erlang:length(rlx_state:available_apps(State2))).
 
 no_beam_case(Config) ->
-    %% We silently ignore apps with no beams
+    %% do not ignore apps with no beam files if no modules in app file
     LibDir1 = proplists:get_value(lib1, Config),
     _Apps1 = [(fun({Name, Vsn}) ->
                       create_app(LibDir1, Name, Vsn)
@@ -111,8 +111,8 @@ no_beam_case(Config) ->
     write_app_file(AppDir, BadName, BadVsn),
     State0 = proplists:get_value(state, Config),
     {DiscoverProvider, {ok, State1}} = rlx_provider:new(rlx_prv_discover, State0),
-    EbinDir = filename:join([LibDir2, BadName, <<"ebin">>]),
-    ?assertMatch({error, {_, [{no_beam_files, EbinDir}]}},
+
+    ?assertMatch({ok, _},
                  rlx_provider:do(DiscoverProvider, State1)).
 
 bad_ebin_case(Config) ->
diff --git a/test/rlx_release_SUITE.erl b/test/rlx_release_SUITE.erl
index aea28f2..b862bfa 100644
--- a/test/rlx_release_SUITE.erl
+++ b/test/rlx_release_SUITE.erl
@@ -28,6 +28,7 @@
          make_scriptless_release/1,
          make_overridden_release/1,
          make_skip_app_release/1,
+         make_auto_skip_empty_app_release/1,
          make_rerun_overridden_release/1,
          make_implicit_config_release/1,
          overlay_release/1,
@@ -63,6 +64,7 @@ init_per_testcase(_, Config) ->
 all() ->
     [make_release, make_scriptless_release, make_overridden_release,
      make_skip_app_release,
+     make_auto_skip_empty_app_release,
      make_implicit_config_release, make_rerun_overridden_release,
      overlay_release, make_goalless_release, make_depfree_release,
      make_invalid_config_release, make_relup_release, make_relup_release2,
@@ -222,8 +224,45 @@ make_overridden_release(Config) ->
     ?assertMatch(OverrideAppDir, Real).
 
 make_skip_app_release(Config) ->
+    LibDir1 = proplists:get_value(lib1, Config),
+    [(fun({Name, Vsn}) ->
+              create_app(LibDir1, Name, Vsn, [kernel, stdlib], [])
+      end)(App)
+     ||
+        App <-
+            [{create_random_name("lib_app1_"), create_random_vsn()}
+             || _ <- lists:seq(1, 100)]],
+
+    create_app(LibDir1, "goal_app_1", "0.0.1", [stdlib,kernel,non_goal_1], []),
+    create_app(LibDir1, "lib_dep_1", "0.0.1", [stdlib,kernel], []),
+    create_app(LibDir1, "goal_app_2", "0.0.1", [stdlib,kernel,goal_app_1,non_goal_2], []),
+    create_app(LibDir1, "non_goal_1", "0.0.1", [stdlib,kernel], [lib_dep_1]),
+    create_app(LibDir1, "non_goal_2", "0.0.1", [stdlib,kernel], []),
+
+    ConfigFile = filename:join([LibDir1, "relx.config"]),
+    write_config(ConfigFile,
+                 [{release, {foo, "0.0.1"},
+                   [goal_app_1]},
+                  {skip_apps, [goal_app_2]}]),
+    OutputDir = filename:join([proplists:get_value(data_dir, Config),
+                               create_random_name("relx-output")]),
+    {ok, Cwd} = file:get_cwd(),
+    {ok, State} = relx:do(Cwd, undefined, undefined, [], [LibDir1], 2,
+                              OutputDir, [],
+                             ConfigFile),
+    [{{foo, "0.0.1"}, Release}] = ec_dictionary:to_list(rlx_state:realized_releases(State)),
+    AppSpecs = rlx_release:applications(Release),
+    ?assert(lists:keymember(stdlib, 1, AppSpecs)),
+    ?assert(lists:keymember(kernel, 1, AppSpecs)),
+    ?assert(lists:member({non_goal_1, "0.0.1"}, AppSpecs)),
+    ?assertNot(lists:member({non_goal_2, "0.0.1"}, AppSpecs)),
+    ?assert(lists:member({goal_app_1, "0.0.1"}, AppSpecs)),
+    ?assertNot(lists:member({goal_app_2, "0.0.1"}, AppSpecs)),
+    ?assert(lists:member({lib_dep_1, "0.0.1", load}, AppSpecs)).
+
+make_auto_skip_empty_app_release(Config) ->
     DataDir = proplists:get_value(data_dir, Config),
-    SkipAppDir1 = filename:join([DataDir, create_random_name("skip_app_dir_")]),
+    EmptyAppDir1 = filename:join([DataDir, create_random_name("skip_app_dir_")]),
     LibDir1 = proplists:get_value(lib1, Config),
     [(fun({Name, Vsn}) ->
               create_app(LibDir1, Name, Vsn, [kernel, stdlib], [])
@@ -232,9 +271,9 @@ make_skip_app_release(Config) ->
         App <-
             [{create_random_name("lib_app1_"), create_random_vsn()}
              || _ <- lists:seq(1, 100)]],
-    SkipAppApp = create_random_name("skip_app_app"),
-    SkipAppVsn = create_random_vsn(),
-    SkipAppAppName = erlang:list_to_atom(SkipAppApp),
+    EmptyAppApp = create_random_name("empty_app_app"),
+    EmptyAppVsn = create_random_vsn(),
+    EmptyAppAppName = erlang:list_to_atom(EmptyAppApp),
 
     create_app(LibDir1, "goal_app_1", "0.0.1", [stdlib,kernel,non_goal_1], []),
     create_app(LibDir1, "lib_dep_1", "0.0.1", [stdlib,kernel], []),
@@ -242,14 +281,13 @@ make_skip_app_release(Config) ->
     create_app(LibDir1, "non_goal_1", "0.0.1", [stdlib,kernel], [lib_dep_1]),
     create_app(LibDir1, "non_goal_2", "0.0.1", [stdlib,kernel], []),
 
-    create_empty_app(SkipAppDir1, SkipAppApp, SkipAppVsn, [stdlib,kernel], []),
+    create_empty_app(EmptyAppDir1, EmptyAppApp, EmptyAppVsn, [stdlib,kernel], []),
 
     ConfigFile = filename:join([LibDir1, "relx.config"]),
     write_config(ConfigFile,
                  [{release, {foo, "0.0.1"},
                    [goal_app_1,
-                    goal_app_2]},
-                  {skip_apps, [erlang:list_to_atom(SkipAppApp)]}]),
+                    goal_app_2]}]),
     OutputDir = filename:join([proplists:get_value(data_dir, Config),
                                create_random_name("relx-output")]),
     {ok, Cwd} = file:get_cwd(),
@@ -264,7 +302,7 @@ make_skip_app_release(Config) ->
     ?assert(lists:member({non_goal_2, "0.0.1"}, AppSpecs)),
     ?assert(lists:member({goal_app_1, "0.0.1"}, AppSpecs)),
     ?assert(lists:member({goal_app_2, "0.0.1"}, AppSpecs)),
-    ?assertNot(lists:member({SkipAppAppName, SkipAppVsn}, AppSpecs)),
+    ?assertNot(lists:member({EmptyAppAppName, EmptyAppVsn}, AppSpecs)),
     ?assert(lists:member({lib_dep_1, "0.0.1", load}, AppSpecs)).
 
 make_implicit_config_release(Config) ->