From 65e07082018dec2a47f309258761db760100f5f2 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Mon, 23 Oct 2017 10:11:06 -0400 Subject: Fix include paths in profile multiapp edge case The compiling of OTP applications is done by first topographically sorting them according to their dependencies, deps-first. This allows all compilation to take place in order. In the current code, the same logic extends to top-level applications in an umbrella project. Unfortunately, there are cases where this is not going to be true: when an application has extra_src_dirs entries (or additional directories or files) to conditionally compile under some profiles, it may start depending on another top-level application dedicated to that profile for include files. However, such an app will never make it to production and neither will the compilation artifacts that create the dependency. Under that scenario, current rebar3 is unusable. This patch makes it so that the compilation provider instead changes the logic for top-level apps: rather than copying their directories one by one and compiling them in order, it: 1. copies all top-level apps to the build directory so the files are in their proper locations 2. adds the top-level apps to the path (after the global hooks have run, so the existing scope and env has not changed) 3. runs the compilation as usual. Fixes #1651 --- src/rebar_prv_compile.erl | 19 ++++++++++++++++--- test/rebar_compile_SUITE.erl | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/src/rebar_prv_compile.erl b/src/rebar_prv_compile.erl index 959ecb0..c9a77a5 100644 --- a/src/rebar_prv_compile.erl +++ b/src/rebar_prv_compile.erl @@ -45,13 +45,13 @@ do(State) -> Deps = rebar_state:deps_to_build(State), Cwd = rebar_state:dir(State), - build_apps(State, Providers, Deps), + copy_and_build_apps(State, Providers, Deps), {ok, ProjectApps1} = rebar_digraph:compile_order(ProjectApps), %% Run top level hooks *before* project apps compiled but *after* deps are rebar_hooks:run_all_hooks(Cwd, pre, ?PROVIDER, Providers, State), - ProjectApps2 = build_apps(State, Providers, ProjectApps1), + ProjectApps2 = copy_and_build_project_apps(State, Providers, ProjectApps1), State2 = rebar_state:project_apps(State, ProjectApps2), %% projects with structures like /apps/foo,/apps/bar,/test @@ -77,7 +77,7 @@ format_error({missing_artifact, File}) -> format_error(Reason) -> io_lib:format("~p", [Reason]). -build_apps(State, Providers, Apps) -> +copy_and_build_apps(State, Providers, Apps) -> [build_app(State, Providers, AppInfo) || AppInfo <- Apps]. build_app(State, Providers, AppInfo) -> @@ -86,6 +86,19 @@ build_app(State, Providers, AppInfo) -> copy_app_dirs(AppInfo, AppDir, OutDir), compile(State, Providers, AppInfo). +copy_and_build_project_apps(State, Providers, Apps) -> + %% Top-level apps, because of profile usage and specific orderings (i.e. + %% may require an include file from a profile-specific app for an extra_dirs + %% entry that only exists in a test context), need to be + %% copied and added to the path at once, and not just in compile order. + [copy_app_dirs(AppInfo, + rebar_app_info:dir(AppInfo), + rebar_app_info:out_dir(AppInfo)) + || AppInfo <- Apps], + code:add_pathsa([rebar_app_info:out_dir(AppInfo) || AppInfo <- Apps]), + [compile(State, Providers, AppInfo) || AppInfo <- Apps]. + + build_extra_dirs(State, Apps) -> BaseDir = rebar_state:dir(State), F = fun(App) -> rebar_app_info:dir(App) == BaseDir end, diff --git a/test/rebar_compile_SUITE.erl b/test/rebar_compile_SUITE.erl index 6579617..1655336 100644 --- a/test/rebar_compile_SUITE.erl +++ b/test/rebar_compile_SUITE.erl @@ -45,6 +45,7 @@ include_file_in_src/1, include_file_relative_to_working_directory_test/1, include_file_in_src_test/1, + include_file_in_src_test_multiapp/1, dont_recompile_when_erl_compiler_options_env_does_not_change/1, recompile_when_erl_compiler_options_env_changes/1, always_recompile_when_erl_compiler_options_set/1, @@ -77,6 +78,7 @@ all() -> clean_all, override_deps, profile_override_deps, deps_build_in_prod, include_file_relative_to_working_directory, include_file_in_src, include_file_relative_to_working_directory_test, include_file_in_src_test, + include_file_in_src_test_multiapp, recompile_when_parse_transform_as_opt_changes, recompile_when_parse_transform_inline_changes, regex_filter_skip, regex_filter_regression, @@ -1460,6 +1462,39 @@ include_file_in_src_test(Config) -> ["as", "test", "compile"], {ok, [{app, Name}]}). +%% Same as `include_file_in_src_test/1' but using multiple top-level +%% apps as dependencies. +include_file_in_src_test_multiapp(Config) -> + + Name1 = rebar_test_utils:create_random_name("app2_"), + Name2 = rebar_test_utils:create_random_name("app1_"), + AppDir1 = filename:join([?config(apps, Config), "lib", Name1]), + AppDir2 = filename:join([?config(apps, Config), "lib", Name2]), + Vsn = rebar_test_utils:create_random_vsn(), + rebar_test_utils:create_app(AppDir1, Name1, Vsn, [kernel, stdlib]), + rebar_test_utils:create_app(AppDir2, Name2, Vsn, [kernel, stdlib]), + + Src = "-module(test).\n" +"\n" +"-include_lib(\"" ++ Name2 ++ "/include/test.hrl\").\n" +"\n" +"test() -> ?TEST_MACRO.\n" +"\n", + Include = <<"-define(TEST_MACRO, test).\n">>, + + ok = filelib:ensure_dir(filename:join([AppDir1, "src", "dummy"])), + ok = filelib:ensure_dir(filename:join([AppDir1, "test", "dummy"])), + ok = filelib:ensure_dir(filename:join([AppDir2, "src", "dummy"])), + ok = filelib:ensure_dir(filename:join([AppDir2, "include", "dummy"])), + ok = file:write_file(filename:join([AppDir1, "test", "test.erl"]), Src), + + ok = file:write_file(filename:join([AppDir2, "include", "test.hrl"]), Include), + + RebarConfig = [], + rebar_test_utils:run_and_check(Config, RebarConfig, + ["as", "test", "compile"], + {ok, [{app, Name1}]}). + %% this test sets the env var, compiles, records the file last modified timestamp, %% recompiles and compares the file last modified timestamp to ensure it hasn't %% changed. this test should run on 19.x+ -- cgit v1.1