From d6195601e95527da29dfd17755dbf71b68bb932a Mon Sep 17 00:00:00 2001 From: Serge Aleynikov Date: Mon, 1 Jul 2019 18:21:43 +0600 Subject: Fix 'rebar3 compile' crash when a dependency is missing app file --- src/rebar_app_discover.erl | 45 ++++++++++++++++++++++++++------------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl index 0c97ad7..0d1e427 100644 --- a/src/rebar_app_discover.erl +++ b/src/rebar_app_discover.erl @@ -85,7 +85,10 @@ 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_find_app_file, AppFile}) -> + io_lib:format("Cannot find app file: ~p~n", [AppFile]). + %% @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 +351,28 @@ 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). + case 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); + [] -> + throw({error, {?MODULE, {cannot_find_app_file, AppFile}}}) + end. %% @doc Read in and parse the .app file if it is availabe. Do the same for %% the .app.src file if it exists. -- cgit v1.1 From 185f885f698fd27b51ef58572592ab29cab0e287 Mon Sep 17 00:00:00 2001 From: Serge Aleynikov Date: Tue, 2 Jul 2019 06:27:20 -0400 Subject: Refine the error cause --- src/rebar_app_discover.erl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl index 0d1e427..0b7eeb1 100644 --- a/src/rebar_app_discover.erl +++ b/src/rebar_app_discover.erl @@ -86,8 +86,8 @@ 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]); -format_error({cannot_find_app_file, AppFile}) -> - io_lib:format("Cannot find app file: ~p~n", [AppFile]). +format_error({cannot_read_app_file, AppFile}) -> + io_lib:format("Cannot read app file: ~p~n", [AppFile]). %% @doc merges configuration of a project app and the top level state @@ -371,7 +371,7 @@ create_app_info(AppInfo, AppDir, AppFile) -> end, rebar_app_info:dir(rebar_app_info:valid(AppInfo2, Valid), AppDir); [] -> - throw({error, {?MODULE, {cannot_find_app_file, AppFile}}}) + throw({error, {?MODULE, {cannot_read_app_file, AppFile}}}) end. %% @doc Read in and parse the .app file if it is availabe. Do the same for -- cgit v1.1 From 9e4a191f46bbf86bfe75f19ddfa1048b1660faba Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Thu, 4 Jul 2019 16:01:51 -0400 Subject: Fix crash when a dependency is missing app file Patch up and add tests on #2112 --- src/rebar_app_discover.erl | 11 +++++-- src/rebar_prv_app_discovery.erl | 2 ++ test/rebar_discover_SUITE.erl | 69 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) create mode 100644 test/rebar_discover_SUITE.erl diff --git a/src/rebar_app_discover.erl b/src/rebar_app_discover.erl index 0b7eeb1..0c15bb2 100644 --- a/src/rebar_app_discover.erl +++ b/src/rebar_app_discover.erl @@ -87,7 +87,9 @@ format_error({module_list, File}) -> format_error({missing_module, 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]). + 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 @@ -351,7 +353,7 @@ 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) -> - case rebar_config:consult_app_file(AppFile) of + try rebar_config:consult_app_file(AppFile) of [{application, AppName, AppDetails}] -> AppVsn = proplists:get_value(vsn, AppDetails), Applications = proplists:get_value(applications, AppDetails, []), @@ -370,8 +372,11 @@ create_app_info(AppInfo, AppDir, AppFile) -> 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 diff --git a/src/rebar_prv_app_discovery.erl b/src/rebar_prv_app_discovery.erl index f5bab49..20b7f90 100644 --- a/src/rebar_prv_app_discovery.erl +++ b/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. diff --git a/test/rebar_discover_SUITE.erl b/test/rebar_discover_SUITE.erl new file mode 100644 index 0000000..3fdd34b --- /dev/null +++ b/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. + -- cgit v1.1