From 787cd967b632bef4534ade58ab64a51eda838df1 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Fri, 30 Sep 2016 09:27:40 -0400 Subject: Fix private includes when compiling in test profile When an include file is set in a private path (i.e. src/), the rebar3 compiler would not add them to the {i, Path} params -- only include/ and the project root were being added. This meant that when some extra source directories were added to the compile job, such as test/ when running under the test profile, the private include paths could not be shared with the test module. This patch fixes the issues (and adds tests) for such a specific case by adding all the configured include paths to the {i, Path} erl_opts arguments, yielding successful compile runs. --- src/rebar_erlc_compiler.erl | 23 ++++++++------ test/rebar_compile_SUITE.erl | 71 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 84 insertions(+), 10 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index e6f2b71..bc29939 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -203,8 +203,8 @@ compile_dirs(RebarOpts, BaseDir, SrcDirs, OutDir, Opts) -> G = init_erlcinfo(include_abs_dirs(ErlOpts, BaseDir), AllErlFiles, BaseDir, OutDir), {ParseTransforms, Rest} = split_source_files(AllErlFiles, ErlOpts), - NeededErlFiles = case needed_files(G, ErlOpts, BaseDir, OutDir, ParseTransforms) of - [] -> needed_files(G, ErlOpts, BaseDir, OutDir, Rest); + NeededErlFiles = case needed_files(G, ErlOpts, RebarOpts, BaseDir, OutDir, ParseTransforms) of + [] -> needed_files(G, ErlOpts, RebarOpts, BaseDir, OutDir, Rest); %% at least one parse transform in the opts needs updating, so recompile all _ -> AllErlFiles end, @@ -224,7 +224,7 @@ compile_dirs(RebarOpts, BaseDir, SrcDirs, OutDir, Opts) -> true -> ErlOptsFirst; false -> ErlOpts end, - internal_erl_compile(C, BaseDir, S, OutDir, ErlOpts1) + internal_erl_compile(C, BaseDir, S, OutDir, ErlOpts1, RebarOpts) end) after true = digraph:delete(SubGraph), @@ -312,13 +312,15 @@ filename_to_atom(F) -> list_to_atom(filename:rootname(filename:basename(F))). %% Get subset of SourceFiles which need to be recompiled, respecting %% dependencies induced by given graph G. -needed_files(G, ErlOpts, Dir, OutDir, SourceFiles) -> +needed_files(G, ErlOpts, RebarOpts, Dir, OutDir, SourceFiles) -> lists:filter(fun(Source) -> TargetBase = target_base(OutDir, Source), Target = TargetBase ++ ".beam", + PrivIncludes = [{i, filename:join(Dir, Src)} + || Src <- rebar_dir:src_dirs(RebarOpts, ["src"])], AllOpts = [{outdir, filename:dirname(Target)} ,{i, filename:join(Dir, "include")} - ,{i, Dir}] ++ ErlOpts, + ,{i, Dir}] ++ PrivIncludes ++ ErlOpts, digraph:vertex(G, Source) > {Source, filelib:last_modified(Target)} orelse opts_changed(AllOpts, TargetBase) orelse erl_compiler_opts_set() @@ -518,12 +520,15 @@ expand_file_names(Files, Dirs) -> end, Files). -spec internal_erl_compile(rebar_dict(), file:filename(), file:filename(), - file:filename(), list()) -> ok | {ok, any()} | {error, any(), any()}. -internal_erl_compile(Opts, Dir, Module, OutDir, ErlOpts) -> + file:filename(), list(), rebar_dict()) -> + ok | {ok, any()} | {error, any(), any()}. +internal_erl_compile(Opts, Dir, Module, OutDir, ErlOpts, RebarOpts) -> Target = target_base(OutDir, Module) ++ ".beam", ok = filelib:ensure_dir(Target), - AllOpts = [{outdir, filename:dirname(Target)}] ++ ErlOpts ++ - [{i, filename:join(Dir, "include")}, {i, Dir}, return], + PrivIncludes = [{i, filename:join(Dir, Src)} + || Src <- rebar_dir:src_dirs(RebarOpts, ["src"])], + AllOpts = [{outdir, filename:dirname(Target)}] ++ ErlOpts ++ PrivIncludes ++ + [{i, filename:join(Dir, "include")}, {i, Dir}, return], case compile:file(Module, AllOpts) of {ok, _Mod} -> ok; diff --git a/test/rebar_compile_SUITE.erl b/test/rebar_compile_SUITE.erl index cb16304..f31ab39 100644 --- a/test/rebar_compile_SUITE.erl +++ b/test/rebar_compile_SUITE.erl @@ -42,6 +42,8 @@ deps_build_in_prod/1, include_file_relative_to_working_directory/1, include_file_in_src/1, + include_file_relative_to_working_directory_test/1, + include_file_in_src_test/1, always_recompile_when_erl_compiler_options_set/1, recompile_when_parse_transform_inline_changes/1, recompile_when_parse_transform_as_opt_changes/1]). @@ -68,6 +70,7 @@ all() -> umbrella_mib_first_test, only_default_transitive_deps, 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, recompile_when_parse_transform_as_opt_changes, recompile_when_parse_transform_inline_changes] ++ case erlang:function_exported(os, unsetenv, 1) of @@ -769,7 +772,7 @@ dont_recompile_when_opts_dont_change(Config) -> NewModTime = [filelib:last_modified(filename:join([EbinDir, F])) || F <- NewFiles, filename:extension(F) == ".beam"], - ?assert(ModTime == NewModTime). + ?assertEqual(ModTime, NewModTime). dont_recompile_yrl_or_xrl(Config) -> AppDir = ?config(apps, Config), @@ -1314,6 +1317,72 @@ include_file_in_src(Config) -> ["compile"], {ok, [{app, Name}]}). +%% verify that the proper include path is defined +%% according the erlang doc which states: +%% If the filename File is absolute (possibly after variable substitution), +%% the include file with that name is included. Otherwise, the specified file +%% is searched for in the following directories, and in this order: +%% * The current working directory +%% * The directory where the module is being compiled +%% * The directories given by the include option +%% +%% This test ensures that things keep working when additional directories +%% are used for apps, such as the test/ directory within the test profile. +include_file_relative_to_working_directory_test(Config) -> + AppDir = ?config(apps, Config), + + 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]), + + Src = <<"-module(test).\n" +"\n" +"-include(\"include/test.hrl\").\n" +"\n" +"test() -> ?TEST_MACRO.\n" +"\n">>, + Include = <<"-define(TEST_MACRO, test).\n">>, + + ok = filelib:ensure_dir(filename:join([AppDir, "src", "dummy"])), + ok = filelib:ensure_dir(filename:join([AppDir, "test", "dummy"])), + ok = file:write_file(filename:join([AppDir, "test", "test.erl"]), Src), + + ok = filelib:ensure_dir(filename:join([AppDir, "include", "dummy"])), + ok = file:write_file(filename:join([AppDir, "include", "test.hrl"]), Include), + + RebarConfig = [], + rebar_test_utils:run_and_check(Config, RebarConfig, + ["as", "test", "compile"], + {ok, [{app, Name}]}). + +%% Same as `include_file_in_src/1' but using the `test/' directory +%% within the test profile. +include_file_in_src_test(Config) -> + AppDir = ?config(apps, Config), + + 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]), + + Src = <<"-module(test).\n" +"\n" +"-include(\"test.hrl\").\n" +"\n" +"test() -> ?TEST_MACRO.\n" +"\n">>, + Include = <<"-define(TEST_MACRO, test).\n">>, + + ok = filelib:ensure_dir(filename:join([AppDir, "src", "dummy"])), + ok = filelib:ensure_dir(filename:join([AppDir, "test", "dummy"])), + ok = file:write_file(filename:join([AppDir, "test", "test.erl"]), Src), + + ok = file:write_file(filename:join([AppDir, "src", "test.hrl"]), Include), + + RebarConfig = [], + rebar_test_utils:run_and_check(Config, RebarConfig, + ["as", "test", "compile"], + {ok, [{app, Name}]}). + always_recompile_when_erl_compiler_options_set(Config) -> %% save existing env to restore after test ExistingEnv = os:getenv("ERL_COMPILER_OPTIONS"), -- cgit v1.1 From 82e7616745bdb05143b78e1b3d3c15db709a7f0a Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Fri, 30 Sep 2016 13:28:10 -0400 Subject: Use all_src_dirs for include paths Helps cover extra cases. --- src/rebar_erlc_compiler.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index bc29939..36a247e 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -317,7 +317,7 @@ needed_files(G, ErlOpts, RebarOpts, Dir, OutDir, SourceFiles) -> TargetBase = target_base(OutDir, Source), Target = TargetBase ++ ".beam", PrivIncludes = [{i, filename:join(Dir, Src)} - || Src <- rebar_dir:src_dirs(RebarOpts, ["src"])], + || Src <- rebar_dir:all_src_dirs(RebarOpts, ["src"], [])], AllOpts = [{outdir, filename:dirname(Target)} ,{i, filename:join(Dir, "include")} ,{i, Dir}] ++ PrivIncludes ++ ErlOpts, @@ -526,7 +526,7 @@ internal_erl_compile(Opts, Dir, Module, OutDir, ErlOpts, RebarOpts) -> Target = target_base(OutDir, Module) ++ ".beam", ok = filelib:ensure_dir(Target), PrivIncludes = [{i, filename:join(Dir, Src)} - || Src <- rebar_dir:src_dirs(RebarOpts, ["src"])], + || Src <- rebar_dir:all_src_dirs(RebarOpts, ["src"], [])], AllOpts = [{outdir, filename:dirname(Target)}] ++ ErlOpts ++ PrivIncludes ++ [{i, filename:join(Dir, "include")}, {i, Dir}, return], case compile:file(Module, AllOpts) of -- cgit v1.1