From 23c63f7003c3f80fddc8580cfcf208aa0639b80c Mon Sep 17 00:00:00 2001 From: Slava Yurin Date: Wed, 28 May 2014 14:05:26 +0700 Subject: [PATCH 1/2] Regression test for #249 --- inttest/erlc/erlc_rt.erl | 41 +++++++++++++++++++++++- inttest/erlc/extra-include/foo_extra.hrl | 2 ++ inttest/erlc/extra-src/foo_sup.erl | 2 ++ inttest/erlc/foobar.erl | 8 +++++ inttest/erlc/include/foo_core.hrl | 2 ++ inttest/erlc/rebar.config | 4 ++- inttest/erlc/src/first_erl.erl | 10 ++++++ 7 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 inttest/erlc/foobar.erl create mode 100644 inttest/erlc/src/first_erl.erl diff --git a/inttest/erlc/erlc_rt.erl b/inttest/erlc/erlc_rt.erl index 354ad29..50cdb83 100644 --- a/inttest/erlc/erlc_rt.erl +++ b/inttest/erlc/erlc_rt.erl @@ -1,3 +1,5 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et -module(erlc_rt). -export([files/0, run/1]). @@ -7,6 +9,7 @@ -define(MODULES, [first_xrl, first_yrl, + first_erl, foo, foo_app, foo_sup, @@ -17,6 +20,7 @@ -define(BEAM_FILES, ["first_xrl.beam", "first_yrl.beam", + "first_erl.beam", "foo.beam", "foo_app.beam", "foo_sup.beam", @@ -35,7 +39,10 @@ files() -> {copy, "extra-src", "extra-src"}, {copy, "mibs", "mibs"}, {copy, "asn1", "asn1"}, - {create, "ebin/foo.app", app(foo, ?MODULES)} + {create, "ebin/foo.app", app(foo, ?MODULES)}, + %% deps + {create, "deps/foobar/ebin/foobar.app", app(foobar, [foobar])}, + {copy, "foobar.erl", "deps/foobar/src/foobar.erl"} ]. run(_Dir) -> @@ -53,6 +60,38 @@ run(_Dir) -> ok = check_beams(true), ok = check_debug_info(false), ?assertMatch(true, filelib:is_regular(MibResult)), + %% Regression test for https://github.com/rebar/rebar/issues/249 + %% + %% Root cause: We didn't have per-project .rebar/erlcinfo but just one in + %% /.rebar/erlcinfo. + %% + %% Solution: Ensure every project has its own .rebar/erlcinfo + %% + %% For the bug to happen, the following conditions must be met: + %% + %% 1. /rebar.config has erl_first_files + %% 2. one of the 'first' files depends on another file (in this + %% case via -include_lib()) + %% 3. a sub project's rebar.config, if any, has no erl_first_files entry + %% + %% Now because erl_first_files is retrieved via rebar_config:get_list/3, + %% base_dir/rebar.config's erl_first_files is inherited, and because we had + %% a shared /.rebar/erlcinfo instead of one per project, the + %% cached entry was reused. Next, while compiling the sub project + %% rebar_erlc_compiler:needs_compile/3 gets a last modification time of + %% zero for the 'first' file which does not exist inside the sub project. + %% This, and the fact that it has at least one dependency, makes + %% needs_compile/3 return 'true'. The root cause is that we didn't have per + %% project .rebar/erlcinfo. For /.rebar/erlcinfo to be populated, + %% base_dir has to be compiled at least once. Therefore, after the first + %% compile any compile processing the sub project will fail because + %% needs_compile/3 will always return true for the non-existent 'first' + %% file. + ?assertMatch({ok, _}, retest_sh:run("./rebar clean", [])), + ?assertMatch({ok, _}, retest_sh:run("./rebar compile", [])), + ok = check_beams(true), + ?assertMatch({ok, _}, retest_sh:run("./rebar compile", [])), + ok = check_beams(true), ok. check_beams(Exist) -> diff --git a/inttest/erlc/extra-include/foo_extra.hrl b/inttest/erlc/extra-include/foo_extra.hrl index 228affa..19e9f94 100644 --- a/inttest/erlc/extra-include/foo_extra.hrl +++ b/inttest/erlc/extra-include/foo_extra.hrl @@ -1 +1,3 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et -define(FOO_EXTRA, foo_extra). diff --git a/inttest/erlc/extra-src/foo_sup.erl b/inttest/erlc/extra-src/foo_sup.erl index a0f06b6..c68194e 100644 --- a/inttest/erlc/extra-src/foo_sup.erl +++ b/inttest/erlc/extra-src/foo_sup.erl @@ -1,3 +1,5 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et -module(foo_sup). -behavior(supervisor). diff --git a/inttest/erlc/foobar.erl b/inttest/erlc/foobar.erl new file mode 100644 index 0000000..b6d55a8 --- /dev/null +++ b/inttest/erlc/foobar.erl @@ -0,0 +1,8 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +-module(foobar). + +-export([test/0]). + +test() -> + true. diff --git a/inttest/erlc/include/foo_core.hrl b/inttest/erlc/include/foo_core.hrl index 2363140..803f2f0 100644 --- a/inttest/erlc/include/foo_core.hrl +++ b/inttest/erlc/include/foo_core.hrl @@ -1 +1,3 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et -define(FOO_CORE, foo_core). diff --git a/inttest/erlc/rebar.config b/inttest/erlc/rebar.config index 6a1082a..7ea78e1 100644 --- a/inttest/erlc/rebar.config +++ b/inttest/erlc/rebar.config @@ -1,6 +1,8 @@ %% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- %% ex: ts=4 sw=4 ft=erlang et -{erl_first_files, ["first_xrl.erl", "first_yrl.erl"]}. +{erl_first_files, ["first_xrl.erl", "first_yrl.erl", "src/first_erl.erl"]}. + +{deps, [foobar]}. {erl_opts, [ diff --git a/inttest/erlc/src/first_erl.erl b/inttest/erlc/src/first_erl.erl new file mode 100644 index 0000000..4e9ff20 --- /dev/null +++ b/inttest/erlc/src/first_erl.erl @@ -0,0 +1,10 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +-module(first_erl). + +-include_lib("eunit/include/eunit.hrl"). + +-export([test/0]). + +test() -> + ?debugHere. From 49c25642b3da6796c05385b76160178e3c9a2c83 Mon Sep 17 00:00:00 2001 From: Slava Yurin Date: Thu, 29 May 2014 18:43:23 +0700 Subject: [PATCH 2/2] Fix #249 (erlc regression) The combination of changes to rebar_erlc_compiler, and the fact that erl_first_files is inherited, caused a regression. To fix that, ensure every project uses its own .rebar/erlcinfo. While at it, fix the issue that erl_first_files entries were not included when initializing the dep digraph. Reported-by: Louis-Philippe Gauthier Reported-by: Roland Karlsson Thanks: Tuncer Ayaz --- inttest/erlc/rebar-no_debug_info.config | 2 +- inttest/erlc/rebar.config | 3 ++- rebar.config.sample | 2 +- src/rebar_erlc_compiler.erl | 29 +++++++++++++++++++------ 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/inttest/erlc/rebar-no_debug_info.config b/inttest/erlc/rebar-no_debug_info.config index 1a7113a..07b6fed 100644 --- a/inttest/erlc/rebar-no_debug_info.config +++ b/inttest/erlc/rebar-no_debug_info.config @@ -1,6 +1,6 @@ %% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- %% ex: ts=4 sw=4 ft=erlang et -{erl_first_files, ["first_xrl.erl", "first_yrl.erl"]}. +{erl_first_files, ["src/first_xrl.erl", "src/first_yrl.erl"]}. {erl_opts, [ diff --git a/inttest/erlc/rebar.config b/inttest/erlc/rebar.config index 7ea78e1..71d6660 100644 --- a/inttest/erlc/rebar.config +++ b/inttest/erlc/rebar.config @@ -1,6 +1,7 @@ %% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- %% ex: ts=4 sw=4 ft=erlang et -{erl_first_files, ["first_xrl.erl", "first_yrl.erl", "src/first_erl.erl"]}. +{erl_first_files, + ["src/first_xrl.erl", "src/first_yrl.erl", "src/first_erl.erl"]}. {deps, [foobar]}. diff --git a/rebar.config.sample b/rebar.config.sample index 515ed00..03d2e98 100644 --- a/rebar.config.sample +++ b/rebar.config.sample @@ -21,7 +21,7 @@ %% Erlang files to compile before the rest. Rebar automatically compiles %% parse_transforms and custom behaviours before anything other than the files %% in this list. -{erl_first_files, ["mymib1", "mymib2"]}. +{erl_first_files, ["src/mymib1.erl", "src/mymib2.erl"]}. %% Erlang compiler options {erl_opts, [no_debug_info, diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index b797137..7df488a 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -208,7 +208,7 @@ info_help(Description) -> "(linux|solaris|freebsd|darwin)", 'HAVE_SENDFILE'}, {platform_define, "(linux|freebsd)", 'BACKLOG', 128}, {platform_define, "R13", 'old_inets'}]}, - {erl_first_files, ["mymib1", "mymib2"]}, + {erl_first_files, ["src/mymib1.erl", "src/mymib2.erl"]}, {mib_opts, []}, {mib_first_files, []}, {xrl_opts, []}, @@ -222,6 +222,13 @@ test_compile_config_and_opts(Config, ErlOpts, Cmd) -> {Config2, PropErOpts} = proper_opts(Config1), {Config3, EqcOpts} = eqc_opts(Config2), + %% NOTE: For consistency, all *_first_files lists should be + %% retrieved via rebar_config:get_local. Right now + %% erl_first_files, eunit_first_files, and qc_first_files use + %% rebar_config:get_list and are inherited, but xrl_first_files + %% and yrl_first_files use rebar_config:get_local. Inheritance of + %% *_first_files is questionable as the file would need to exist + %% in all project directories for it to work. OptsAtom = list_to_atom(Cmd ++ "_compile_opts"), TestOpts = rebar_config:get_list(Config3, OptsAtom, []), Opts0 = [{d, 'TEST'}] ++ @@ -277,20 +284,28 @@ doterl_compile(Config, OutDir) -> doterl_compile(Config, OutDir, [], ErlOpts). doterl_compile(Config, OutDir, MoreSources, ErlOpts) -> - ErlFirstFiles = rebar_config:get_list(Config, erl_first_files, []), + ErlFirstFilesConf = rebar_config:get_list(Config, erl_first_files, []), ?DEBUG("erl_opts ~p~n", [ErlOpts]), %% Support the src_dirs option allowing multiple directories to %% contain erlang source. This might be used, for example, should %% eunit tests be separated from the core application source. SrcDirs = rebar_utils:src_dirs(proplists:append_values(src_dirs, ErlOpts)), - RestErls = [Source || Source <- gather_src(SrcDirs, []) ++ MoreSources, - not lists:member(Source, ErlFirstFiles)], + AllErlFiles = gather_src(SrcDirs, []) ++ MoreSources, + %% NOTE: If and when erl_first_files is not inherited anymore + %% (rebar_config:get_local instead of rebar_config:get_list), consider + %% logging a warning message for any file listed in erl_first_files which + %% wasn't found via gather_src. + {ErlFirstFiles, RestErls} = + lists:partition( + fun(Source) -> + lists:member(Source, ErlFirstFilesConf) + end, AllErlFiles), %% Make sure that ebin/ exists and is on the path ok = filelib:ensure_dir(filename:join("ebin", "dummy.beam")), CurrPath = code:get_path(), true = code:add_path(filename:absname("ebin")), OutDir1 = proplists:get_value(outdir, ErlOpts, OutDir), - G = init_erlcinfo(Config, RestErls), + G = init_erlcinfo(Config, AllErlFiles), %% Split RestErls so that files which are depended on are treated %% like erl_first_files. {OtherFirstErls, OtherErls} = @@ -387,8 +402,8 @@ check_erlcinfo(Config, _) -> ?ABORT("~s file is invalid. Please delete before next run.~n", [erlcinfo_file(Config)]). -erlcinfo_file(Config) -> - filename:join([rebar_utils:base_dir(Config), ".rebar", ?ERLCINFO_FILE]). +erlcinfo_file(_Config) -> + filename:join([rebar_utils:get_cwd(), ".rebar", ?ERLCINFO_FILE]). init_erlcinfo(Config, Erls) -> G = restore_erlcinfo(Config),