Merge pull request #2117 from ferd/saleyn-master

Fix crash when a dependency is missing app.src file

src/rebar_app_discover.erl

@@ -85,7 +85,12 @@ define_root_app(Apps, State) ->
 format_error({module_list, File}) ->
     io_lib:format("Error reading module list from ~p~n", [File]);
 format_error({missing_module, Module}) ->
-    io_lib:format("Module defined in app file missing: ~p~n", [Module]).
+    io_lib:format("Module defined in app file missing: ~p~n", [Module]);
+format_error({cannot_read_app_file, AppFile}) ->
+    io_lib:format("Cannot read app file: ~p~n", [AppFile]);
+format_error({bad_term_file, _File, _Reason} = Error) ->
+    rebar_file_utils:format_error(Error).
+
 
 %% @doc merges configuration of a project app and the top level state
 %% some configuration like erl_opts must be merged into a subapp's opts
@@ -348,24 +353,31 @@ app_dir(AppFile) ->
 %% app file.
 -spec create_app_info(rebar_app_info:t(), file:name(), file:name()) -> rebar_app_info:t().
 create_app_info(AppInfo, AppDir, AppFile) ->
-    [{application, AppName, AppDetails}] = rebar_config:consult_app_file(AppFile),
-    AppVsn = proplists:get_value(vsn, AppDetails),
-    Applications = proplists:get_value(applications, AppDetails, []),
-    IncludedApplications = proplists:get_value(included_applications, AppDetails, []),
-    AppInfo1 = rebar_app_info:name(
-                 rebar_app_info:original_vsn(
-                   rebar_app_info:dir(AppInfo, AppDir), AppVsn), AppName),
-    AppInfo2 = rebar_app_info:applications(
-                 rebar_app_info:app_details(AppInfo1, AppDetails),
-                 IncludedApplications++Applications),
-    Valid = case rebar_app_utils:validate_application_info(AppInfo2) =:= true
-                andalso rebar_app_info:has_all_artifacts(AppInfo2) =:= true of
-                true ->
-                    true;
-                _ ->
-                    false
-            end,
-    rebar_app_info:dir(rebar_app_info:valid(AppInfo2, Valid), AppDir).
+    try rebar_config:consult_app_file(AppFile) of
+        [{application, AppName, AppDetails}] ->
+            AppVsn = proplists:get_value(vsn, AppDetails),
+            Applications = proplists:get_value(applications, AppDetails, []),
+            IncludedApplications = proplists:get_value(included_applications, AppDetails, []),
+            AppInfo1 = rebar_app_info:name(
+                         rebar_app_info:original_vsn(
+                           rebar_app_info:dir(AppInfo, AppDir), AppVsn), AppName),
+            AppInfo2 = rebar_app_info:applications(
+                         rebar_app_info:app_details(AppInfo1, AppDetails),
+                         IncludedApplications++Applications),
+            Valid = case rebar_app_utils:validate_application_info(AppInfo2) =:= true
+                        andalso rebar_app_info:has_all_artifacts(AppInfo2) =:= true of
+                        true ->
+                            true;
+                        _ ->
+                            false
+                    end,
+            rebar_app_info:dir(rebar_app_info:valid(AppInfo2, Valid), AppDir);
+        _Invalid ->
+            throw({error, {?MODULE, {cannot_read_app_file, AppFile}}})
+    catch
+        throw:{error, {rebar_file_utils, Err = {bad_term_file, _File, _Reason}}} ->
+            throw({error, {?MODULE, Err}}) % wrap this
+    end.
 
 %% @doc Read in and parse the .app file if it is availabe. Do the same for
 %% the .app.src file if it exists.

src/rebar_prv_app_discovery.erl

@@ -43,6 +43,8 @@ do(State) ->
             {error, {rebar_packages, Error}};
         throw:{error, {rebar_app_utils, Error}} ->
             {error, {rebar_app_utils, Error}};
+        throw:{error, {rebar_app_discover, Error}} ->
+            {error, {rebar_app_discover, Error}};
         throw:{error, Error} ->
             ?PRV_ERROR(Error)
     end.

test/rebar_discover_SUITE.erl

@@ -0,0 +1,69 @@
+-module(rebar_discover_SUITE).
+-compile(export_all).
+
+-include_lib("common_test/include/ct.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+all() ->
+    [empty_app_src, bad_app_src, invalid_app_src].
+     %% note: invalid .app files without a .app.src also present
+     %% has rebar3 just ignoring the directory as not OTP-related.
+
+
+init_per_testcase(_, Config) ->
+    NewConfig = rebar_test_utils:init_rebar_state(Config, "discover_app_"),
+    AppDir = ?config(apps, NewConfig),
+
+    Name = rebar_test_utils:create_random_name("app1"),
+    Vsn = rebar_test_utils:create_random_vsn(),
+    rebar_test_utils:create_app(AppDir, Name, Vsn, [kernel, stdlib]),
+
+    [{app_names, [Name]}, {vsns, [Vsn]}|NewConfig].
+
+end_per_testcase(_, Config) ->
+    Config.
+
+empty_app_src() ->
+    [{doc, "when there's an empty .app.src file, exit with a good error "
+           "message rather than an uncaught exception"}].
+empty_app_src(Config) ->
+    AppDir = ?config(apps, Config),
+    [Name] = ?config(app_names, Config),
+    AppSrc = filename:join([AppDir, "src", Name ++ ".app.src"]),
+    ok = file:write_file(AppSrc, ""),
+    ?assertEqual(
+       {error, {rebar_app_discover, {cannot_read_app_file, AppSrc}}},
+       rebar_test_utils:run_and_check(Config, [], ["compile"], return)
+    ),
+    ok.
+
+bad_app_src() ->
+    [{doc, "when there's a syntactically invalid "
+           ".app.src file, exit with a good error "
+           "message rather than an uncaught exception"}].
+bad_app_src(Config) ->
+    AppDir = ?config(apps, Config),
+    [Name] = ?config(app_names, Config),
+    AppSrc = filename:join([AppDir, "src", Name ++ ".app.src"]),
+    ok = file:write_file(AppSrc, "bad term file :("),
+    ?assertMatch(
+       {error, {rebar_app_discover, {bad_term_file, AppSrc, _}}},
+       rebar_test_utils:run_and_check(Config, [], ["compile"], return)
+    ),
+    ok.
+
+invalid_app_src() ->
+    [{doc, "when there's a syntactically valid but semantically invalid "
+           ".app.src file, exit with a good error "
+           "message rather than an uncaught exception"}].
+invalid_app_src(Config) ->
+    AppDir = ?config(apps, Config),
+    [Name] = ?config(app_names, Config),
+    AppSrc = filename:join([AppDir, "src", Name ++ ".app.src"]),
+    ok = file:write_file(AppSrc, "{applications, name_but_no_args}."),
+    ?assertEqual(
+       {error, {rebar_app_discover, {cannot_read_app_file, AppSrc}}},
+       rebar_test_utils:run_and_check(Config, [], ["compile"], return)
+    ),
+    ok.
+