From eb796229035cc484d06620e83febd236e89e734c Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sat, 28 Mar 2015 12:51:52 +0100 Subject: [PATCH 01/18] Allow running specific retest suites --- Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index ac1cb7e..ed8dd48 100644 --- a/Makefile +++ b/Makefile @@ -6,6 +6,7 @@ OTPVSNCMD='io:fwrite("~s",[rebar_utils:otp_release()]), halt().' OTPVSN=$(shell erl -pa ebin/ -noshell -eval $(OTPVSNCMD)) PLT_FILENAME=~/.dialyzer_rebar_$(OTPVSN)_plt LOG_LEVEL?=debug +RT_TARGETS?=inttest all: ./bootstrap @@ -73,6 +74,6 @@ test_eunit: all @$(REBAR) eunit test_inttest: all deps - @$(RETEST) -l $(LOG_LEVEL) inttest + @$(RETEST) -l $(LOG_LEVEL) $(RT_TARGETS) travis: clean debug xref clean all deps test From f61ca47b23af0f5e19fda440e1c0b34591e3d502 Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Mon, 23 Mar 2015 21:19:44 +0100 Subject: [PATCH 02/18] Remove useless compile/3 Its case statement is noop. --- src/rebar_base_compiler.erl | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/src/rebar_base_compiler.erl b/src/rebar_base_compiler.erl index c38fb11..f49fc85 100644 --- a/src/rebar_base_compiler.erl +++ b/src/rebar_base_compiler.erl @@ -129,21 +129,10 @@ remove_common_path1([Part | RestFilename], [Part | RestPath]) -> remove_common_path1(FilenameParts, _) -> filename:join(FilenameParts). - -compile(Unit, Config, CompileFn) -> - case CompileFn(Unit, Config) of - ok -> - ok; - skipped -> - skipped; - Error -> - Error - end. - compile_each([], _Config, _CompileFn) -> ok; compile_each([Unit | Rest], Config, CompileFn) -> - case compile(Unit, Config, CompileFn) of + case CompileFn(Unit, Config) of ok -> ?CONSOLE("Compiled ~s\n", [unit_source(Unit)]); {ok, Warnings} -> @@ -224,7 +213,7 @@ compile_worker(QueuePid, Config, CompileFn) -> QueuePid ! {next, self()}, receive {compile, Source} -> - case catch(compile(Source, Config, CompileFn)) of + case catch(CompileFn(Source, Config)) of {ok, Ws} -> QueuePid ! {compiled, Source, Ws}, compile_worker(QueuePid, Config, CompileFn); From b0320ce058b0dfba056398b15d3803da755f5d7c Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sat, 28 Mar 2015 12:53:20 +0100 Subject: [PATCH 03/18] Basic test for erlc (re)compiling and erlcinfo --- inttest/erlc_dep_graph/erlc_dep_graph_rt.erl | 86 ++++++++++++++++++++ inttest/erlc_dep_graph/src/foo.app.src | 7 ++ inttest/erlc_dep_graph/src/lisp.erl | 8 ++ 3 files changed, 101 insertions(+) create mode 100644 inttest/erlc_dep_graph/erlc_dep_graph_rt.erl create mode 100644 inttest/erlc_dep_graph/src/foo.app.src create mode 100644 inttest/erlc_dep_graph/src/lisp.erl diff --git a/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl b/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl new file mode 100644 index 0000000..d5f92cf --- /dev/null +++ b/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl @@ -0,0 +1,86 @@ +%% -*- erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 et +%% ------------------------------------------------------------------- +%% +%% rebar: Erlang Build Tools +%% +%% Copyright (c) 2015 David Kubecka +%% +%% Permission is hereby granted, free of charge, to any person obtaining a copy +%% of this software and associated documentation files (the "Software"), to deal +%% in the Software without restriction, including without limitation the rights +%% to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +%% copies of the Software, and to permit persons to whom the Software is +%% furnished to do so, subject to the following conditions: +%% +%% The above copyright notice and this permission notice shall be included in +%% all copies or substantial portions of the Software. +%% +%% THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +%% IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +%% FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +%% AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +%% LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +%% OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +%% THE SOFTWARE. +%% ------------------------------------------------------------------- +-module(erlc_dep_graph_rt). +-export([files/0, + run/1]). + +-include_lib("eunit/include/eunit.hrl"). + +files() -> + [{copy, "../../rebar", "rebar"}, + {copy, "src", "src"}]. + +run(_Dir) -> + compile_all(ok, ""), + check_beams_ok(), + check_beams_untouched(), + modify_and_recompile_ok(), + ok. + +check_beams_ok() -> + F = fun(BeamFile) -> ?assert(filelib:is_regular(BeamFile)) end, + with_erl_beams(F). + +check_beams_untouched() -> + Beams = filelib:wildcard("ebin/*.beam"), + compile_all_and_assert_mtimes(Beams, fun erlang:'=:='/2). + +modify_and_recompile_ok() -> + touch(["src/lisp.erl"]), + compile_all_and_assert_mtimes(["ebin/lisp.beam"], fun erlang:'<'/2). + +compile_all_and_assert_mtimes(Beams, Cmp) -> + BeamsModifiedBefore = mtime_ns(Beams), + compile_all(ok, ""), + BeamsModifiedAfter = mtime_ns(Beams), + lists:zipwith(fun(Before, After) -> ?assert(Cmp(Before, After)) end, + BeamsModifiedBefore, BeamsModifiedAfter). + +with_erl_beams(F) -> + lists:map( + fun(ErlFile) -> + ErlRoot = filename:rootname(filename:basename(ErlFile)), + BeamFile = filename:join("ebin", ErlRoot ++ ".beam"), + F(BeamFile) + end, + filelib:wildcard("src/*.erl")). + +mtime_ns(Files) -> + [os:cmd("stat -c%y " ++ File) || File <- Files]. + +touch(Files) -> + %% Sleep one second so that filelib:last_modified/1 is guaranteed to notice + %% that files have changed. + ok = timer:sleep(1000), + [os:cmd("touch " ++ File) || File <- Files]. + +compile_all(Result, Opts) -> + ?assertMatch({Result, _}, + retest_sh:run("./rebar " ++ Opts ++ " compile", [])). + +clean_all_ok() -> + ?assertMatch({ok, _}, retest_sh:run("./rebar clean", [])). diff --git a/inttest/erlc_dep_graph/src/foo.app.src b/inttest/erlc_dep_graph/src/foo.app.src new file mode 100644 index 0000000..307b1bc --- /dev/null +++ b/inttest/erlc_dep_graph/src/foo.app.src @@ -0,0 +1,7 @@ +{application,foo, + [{description,[]}, + {vsn,"1.0.0"}, + {registered,[]}, + {applications,[kernel,stdlib]}, + {env,[]} + ]}. \ No newline at end of file diff --git a/inttest/erlc_dep_graph/src/lisp.erl b/inttest/erlc_dep_graph/src/lisp.erl new file mode 100644 index 0000000..dd418d2 --- /dev/null +++ b/inttest/erlc_dep_graph/src/lisp.erl @@ -0,0 +1,8 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +-module(lisp). + +-export([parse_transform/2]). + +parse_transform(Forms, _Options) -> + Forms. From fd482b661a5a40a95ef9642ff00567380395e145 Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sun, 22 Mar 2015 20:30:41 +0100 Subject: [PATCH 04/18] Stop passing useless Config into erlcinfo_file Also pas only InclDirs into init_erlcinfo as this is the only thing from Config/ErlOpts needed there. --- src/rebar_erlc_compiler.erl | 48 +++++++++++++++++-------------------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 56f1ebd..a478b2c 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -109,7 +109,7 @@ compile(Config, _AppFile) -> doterl_compile(Config, "ebin"). -spec clean(rebar_config:config(), file:filename()) -> 'ok'. -clean(Config, _AppFile) -> +clean(_Config, _AppFile) -> MibFiles = rebar_utils:find_files_by_ext("mibs", ".mib"), MIBs = [filename:rootname(filename:basename(MIB)) || MIB <- MibFiles], rebar_file_utils:delete_each( @@ -123,7 +123,7 @@ clean(Config, _AppFile) -> || F <- YrlFiles ]), %% Delete the build graph, if any - rebar_file_utils:rm_rf(erlcinfo_file(Config)), + rebar_file_utils:rm_rf(erlcinfo_file()), %% Erlang compilation is recursive, so it's possible that we have a nested %% directory structure in ebin with .beam files within. As such, we want @@ -313,7 +313,7 @@ doterl_compile(Config, OutDir, MoreSources, ErlOpts) -> CurrPath = code:get_path(), true = code:add_path(filename:absname("ebin")), OutDir1 = proplists:get_value(outdir, ErlOpts, OutDir), - G = init_erlcinfo(Config, AllErlFiles), + G = init_erlcinfo(proplists:get_all_values(i, ErlOpts), AllErlFiles), %% Split RestErls so that files which are depended on are treated %% like erl_first_files. {OtherFirstErls, OtherErls} = @@ -387,12 +387,9 @@ u_add_element(Elem, [Elem|_]=Set) -> Set; u_add_element(Elem, [E1|Set]) -> [E1|u_add_element(Elem, Set)]; u_add_element(Elem, []) -> [Elem]. --spec include_path(file:filename(), - rebar_config:config()) -> [file:filename(), ...]. -include_path(Source, Config) -> - ErlOpts = rebar_config:get(Config, erl_opts, []), - lists:usort(["include", filename:dirname(Source)] - ++ proplists:get_all_values(i, ErlOpts)). +-spec include_path(file:filename(), list()) -> [file:filename(), ...]. +include_path(Source, InclDirs) -> + lists:usort(["include", filename:dirname(Source) |InclDirs]). -spec needs_compile(file:filename(), file:filename(), [string()]) -> boolean(). @@ -401,20 +398,20 @@ needs_compile(Source, Target, Parents) -> lists:any(fun(I) -> TargetLastMod < filelib:last_modified(I) end, [Source] ++ Parents). -check_erlcinfo(_Config, #erlcinfo{vsn=?ERLCINFO_VSN}) -> +check_erlcinfo(#erlcinfo{vsn=?ERLCINFO_VSN}) -> ok; -check_erlcinfo(Config, #erlcinfo{vsn=Vsn}) -> +check_erlcinfo(#erlcinfo{vsn=Vsn}) -> ?ABORT("~s file version is incompatible. expected: ~b got: ~b~n", - [erlcinfo_file(Config), ?ERLCINFO_VSN, Vsn]); -check_erlcinfo(Config, _) -> + [erlcinfo_file(), ?ERLCINFO_VSN, Vsn]); +check_erlcinfo(_) -> ?ABORT("~s file is invalid. Please delete before next run.~n", - [erlcinfo_file(Config)]). + [erlcinfo_file()]). -erlcinfo_file(_Config) -> +erlcinfo_file() -> filename:join([rebar_utils:get_cwd(), ".rebar", ?ERLCINFO_FILE]). -init_erlcinfo(Config, Erls) -> - G = restore_erlcinfo(Config), +init_erlcinfo(InclDirs, Erls) -> + G = restore_erlcinfo(), %% Get a unique list of dirs based on the source files' locations. %% This is used for finding files in sub dirs of the configured %% src_dirs. For example, src/sub_dir/foo.erl. @@ -423,10 +420,10 @@ init_erlcinfo(Config, Erls) -> Dir = filename:dirname(Erl), sets:add_element(Dir, Acc) end, sets:new(), Erls)), - Updates = [update_erlcinfo(G, Erl, include_path(Erl, Config) ++ Dirs) + Updates = [update_erlcinfo(G, Erl, include_path(Erl, InclDirs) ++ Dirs) || Erl <- Erls], Modified = lists:member(modified, Updates), - ok = store_erlcinfo(G, Config, Modified), + ok = store_erlcinfo(G, Modified), G. update_erlcinfo(G, Source, Dirs) -> @@ -463,14 +460,13 @@ modify_erlcinfo(G, Source, Dirs) -> digraph:add_edge(G, Source, Incl) end, AbsIncls). -restore_erlcinfo(Config) -> - File = erlcinfo_file(Config), +restore_erlcinfo() -> G = digraph:new(), - case file:read_file(File) of + case file:read_file(erlcinfo_file()) of {ok, Data} -> try binary_to_term(Data) of Erlcinfo -> - ok = check_erlcinfo(Config, Erlcinfo), + ok = check_erlcinfo(Erlcinfo), #erlcinfo{info=ErlcInfo} = Erlcinfo, {Vs, Es} = ErlcInfo, lists:foreach( @@ -493,9 +489,9 @@ restore_erlcinfo(Config) -> end, G. -store_erlcinfo(_G, _Config, _Modified = false) -> +store_erlcinfo(_G, _Modified = false) -> ok; -store_erlcinfo(G, Config, _Modified) -> +store_erlcinfo(G, _Modified) -> Vs = lists:map( fun(V) -> digraph:vertex(G, V) @@ -508,7 +504,7 @@ store_erlcinfo(G, Config, _Modified) -> {V1, V2} end, digraph:out_edges(G, V)) end, Vs), - File = erlcinfo_file(Config), + File = erlcinfo_file(), ok = filelib:ensure_dir(File), Data = term_to_binary(#erlcinfo{info={Vs, Es}}, [{compressed, 9}]), file:write_file(File, Data). From d61b51bcf8396b90513bfa3bbb36ba8857c4b6ce Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sat, 28 Mar 2015 15:53:06 +0100 Subject: [PATCH 05/18] Stop silently supressing errors from process_attr/2 Otherwise the code cannot be safely refactored, since potential new erros slip unnoticed. --- src/rebar_erlc_compiler.erl | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index a478b2c..e82a1c3 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -672,14 +672,8 @@ parse_attrs(Fd, Includes) -> end. process_attr(Form, Includes) -> - try - AttrName = erl_syntax:atom_value(erl_syntax:attribute_name(Form)), - process_attr(AttrName, Form, Includes) - catch _:_ -> - %% TODO: We should probably try to be more specific here - %% and not suppress all errors. - Includes - end. + AttrName = erl_syntax:atom_value(erl_syntax:attribute_name(Form)), + process_attr(AttrName, Form, Includes). process_attr(import, Form, Includes) -> case erl_syntax_lib:analyze_import_attribute(Form) of @@ -719,8 +713,12 @@ process_attr(compile, Form, Includes) -> [atom_to_list(M) ++ ".erl"|Acc]; (_, Acc) -> Acc - end, Includes, L) - end. + end, Includes, L); + _ -> + Includes + end; +process_attr(_, _Form, Includes) -> + Includes. %% Given the filename from an include_lib attribute, if the path %% exists, return unmodified, or else get the absolute ERL_LIBS From ccdfc1a27045ec5023e4f3c4d569a431f65234ea Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sun, 22 Mar 2015 23:03:36 +0100 Subject: [PATCH 06/18] Extract common pattern from rebar_erlc_compiler:process_attr --- src/rebar_erlc_compiler.erl | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index e82a1c3..91e873c 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -678,9 +678,9 @@ process_attr(Form, Includes) -> process_attr(import, Form, Includes) -> case erl_syntax_lib:analyze_import_attribute(Form) of {Mod, _Funs} -> - [atom_to_list(Mod) ++ ".erl"|Includes]; + [module_to_erl(Mod)|Includes]; Mod -> - [atom_to_list(Mod) ++ ".erl"|Includes] + [module_to_erl(Mod)|Includes] end; process_attr(file, Form, Includes) -> {File, _} = erl_syntax_lib:analyze_file_attribute(Form), @@ -696,21 +696,21 @@ process_attr(include_lib, Form, Includes) -> [File|Includes]; process_attr(behaviour, Form, Includes) -> [FileNode] = erl_syntax:attribute_arguments(Form), - File = erl_syntax:atom_name(FileNode) ++ ".erl", + File = module_to_erl(erl_syntax:atom_value(FileNode)), [File|Includes]; process_attr(compile, Form, Includes) -> [Arg] = erl_syntax:attribute_arguments(Form), case erl_syntax:concrete(Arg) of {parse_transform, Mod} -> - [atom_to_list(Mod) ++ ".erl"|Includes]; + [module_to_erl(Mod)|Includes]; {core_transform, Mod} -> - [atom_to_list(Mod) ++ ".erl"|Includes]; + [module_to_erl(Mod)|Includes]; L when is_list(L) -> lists:foldl( - fun({parse_transform, M}, Acc) -> - [atom_to_list(M) ++ ".erl"|Acc]; - ({core_transform, M}, Acc) -> - [atom_to_list(M) ++ ".erl"|Acc]; + fun({parse_transform, Mod}, Acc) -> + [module_to_erl(Mod)|Acc]; + ({core_transform, Mod}, Acc) -> + [module_to_erl(Mod)|Acc]; (_, Acc) -> Acc end, Includes, L); @@ -720,6 +720,9 @@ process_attr(compile, Form, Includes) -> process_attr(_, _Form, Includes) -> Includes. +module_to_erl(Mod) -> + atom_to_list(Mod) ++ ".erl". + %% Given the filename from an include_lib attribute, if the path %% exists, return unmodified, or else get the absolute ERL_LIBS %% path. From df97a49ce717375cfc7aff45a29026b1751115d1 Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Mon, 23 Mar 2015 03:10:28 +0100 Subject: [PATCH 07/18] Sanitize erlcinfo checking As erlcinfo is only an internal optimization mechanism, user should be only minimally bothered by issues with it. Especially it shouldn't ever cause a fatal error and if it cannot be restored properly, only a warning (not error) message should be emitted (if any). Also it probably doesn't make sense for this warning message to be too detailed - just state that something went wrong and silently delete the erlcinfo file. --- src/rebar_erlc_compiler.erl | 54 +++++++++++++------------------------ 1 file changed, 19 insertions(+), 35 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 91e873c..af7cf64 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -398,20 +398,17 @@ needs_compile(Source, Target, Parents) -> lists:any(fun(I) -> TargetLastMod < filelib:last_modified(I) end, [Source] ++ Parents). -check_erlcinfo(#erlcinfo{vsn=?ERLCINFO_VSN}) -> - ok; -check_erlcinfo(#erlcinfo{vsn=Vsn}) -> - ?ABORT("~s file version is incompatible. expected: ~b got: ~b~n", - [erlcinfo_file(), ?ERLCINFO_VSN, Vsn]); -check_erlcinfo(_) -> - ?ABORT("~s file is invalid. Please delete before next run.~n", - [erlcinfo_file()]). - erlcinfo_file() -> filename:join([rebar_utils:get_cwd(), ".rebar", ?ERLCINFO_FILE]). init_erlcinfo(InclDirs, Erls) -> - G = restore_erlcinfo(), + G = digraph:new(), + try restore_erlcinfo(G) + catch + _ -> + ?WARN("Failed to restore ~s file. Discarding it.~n", [erlcinfo_file()]), + ok = file:delete(erlcinfo_file()) + end, %% Get a unique list of dirs based on the source files' locations. %% This is used for finding files in sub dirs of the configured %% src_dirs. For example, src/sub_dir/foo.erl. @@ -460,34 +457,21 @@ modify_erlcinfo(G, Source, Dirs) -> digraph:add_edge(G, Source, Incl) end, AbsIncls). -restore_erlcinfo() -> - G = digraph:new(), +restore_erlcinfo(G) -> case file:read_file(erlcinfo_file()) of {ok, Data} -> - try binary_to_term(Data) of - Erlcinfo -> - ok = check_erlcinfo(Erlcinfo), - #erlcinfo{info=ErlcInfo} = Erlcinfo, - {Vs, Es} = ErlcInfo, - lists:foreach( - fun({V, LastUpdated}) -> - digraph:add_vertex(G, V, LastUpdated) - end, Vs), - lists:foreach( - fun({V1, V2}) -> - digraph:add_edge(G, V1, V2) - end, Es) - catch - error:badarg -> - ?ERROR( - "Failed (binary_to_term) to restore rebar info file." - " Discard file.~n", []), - ok - end; - _Err -> + #erlcinfo{vsn=?ERLCINFO_VSN, info={Vs, Es}} = binary_to_term(Data), + lists:foreach( + fun({V, LastUpdated}) -> + digraph:add_vertex(G, V, LastUpdated) + end, Vs), + lists:foreach( + fun({V1, V2}) -> + digraph:add_edge(G, V1, V2) + end, Es); + {error, _} -> ok - end, - G. + end. store_erlcinfo(_G, _Modified = false) -> ok; From 22acf8db67b01e25f10ae3fdf19e6199f1012a8b Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Mon, 23 Mar 2015 04:10:11 +0100 Subject: [PATCH 08/18] Assemble Dirs passed to update_erlcinfo in one place Current separation of part of the logic into include_path obscures the purpose of Dirs (see expand_file_names). Moreover for each particular Erl its source file was included twice in Dirs, which is now corrected. Also InclDirs (specified in erl_opts) as part of erlcinfo since they can affect resulting graph, which needs to be therefore regenerated when InclDirs change. See added test as an example. --- inttest/erlc_dep_graph/erlc_dep_graph_rt.erl | 19 +++++--- .../erlc_dep_graph/extra_include/extra.hrl | 3 ++ inttest/erlc_dep_graph/include/lambda.hrl | 3 ++ inttest/erlc_dep_graph/rebar.config | 6 +++ inttest/erlc_dep_graph/src/java.erl | 11 +++++ src/rebar_erlc_compiler.erl | 43 ++++++++----------- 6 files changed, 56 insertions(+), 29 deletions(-) create mode 100644 inttest/erlc_dep_graph/extra_include/extra.hrl create mode 100644 inttest/erlc_dep_graph/include/lambda.hrl create mode 100644 inttest/erlc_dep_graph/rebar.config create mode 100644 inttest/erlc_dep_graph/src/java.erl diff --git a/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl b/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl index d5f92cf..ee3907e 100644 --- a/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl +++ b/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl @@ -32,13 +32,22 @@ files() -> [{copy, "../../rebar", "rebar"}, - {copy, "src", "src"}]. + {copy, "rebar.config", "rebar.config"}, + {copy, "src", "src"}, + {copy, "include", "include"}, + {copy, "extra_include", "extra_include"}]. run(_Dir) -> compile_all(ok, ""), check_beams_ok(), check_beams_untouched(), - modify_and_recompile_ok(), + modify_and_recompile_ok("src/lisp.erl", "ebin/lisp.beam"), + + clean_all_ok(), + compile_all(error, "-C rebar.config.non-existing"), + compile_all(ok, ""), + modify_and_recompile_ok("extra_include/extra.hrl", "ebin/java.beam"), + ok. check_beams_ok() -> @@ -49,9 +58,9 @@ check_beams_untouched() -> Beams = filelib:wildcard("ebin/*.beam"), compile_all_and_assert_mtimes(Beams, fun erlang:'=:='/2). -modify_and_recompile_ok() -> - touch(["src/lisp.erl"]), - compile_all_and_assert_mtimes(["ebin/lisp.beam"], fun erlang:'<'/2). +modify_and_recompile_ok(TouchFile, CheckFile) -> + touch([TouchFile]), + compile_all_and_assert_mtimes([CheckFile], fun erlang:'<'/2). compile_all_and_assert_mtimes(Beams, Cmp) -> BeamsModifiedBefore = mtime_ns(Beams), diff --git a/inttest/erlc_dep_graph/extra_include/extra.hrl b/inttest/erlc_dep_graph/extra_include/extra.hrl new file mode 100644 index 0000000..9d034c9 --- /dev/null +++ b/inttest/erlc_dep_graph/extra_include/extra.hrl @@ -0,0 +1,3 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +-define(CONCISE, impossible). diff --git a/inttest/erlc_dep_graph/include/lambda.hrl b/inttest/erlc_dep_graph/include/lambda.hrl new file mode 100644 index 0000000..6b1622c --- /dev/null +++ b/inttest/erlc_dep_graph/include/lambda.hrl @@ -0,0 +1,3 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +-define(FUN, fake). diff --git a/inttest/erlc_dep_graph/rebar.config b/inttest/erlc_dep_graph/rebar.config new file mode 100644 index 0000000..5002bcd --- /dev/null +++ b/inttest/erlc_dep_graph/rebar.config @@ -0,0 +1,6 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +{erl_opts, + [ + {i, "extra_include"} + ]}. diff --git a/inttest/erlc_dep_graph/src/java.erl b/inttest/erlc_dep_graph/src/java.erl new file mode 100644 index 0000000..2e3f281 --- /dev/null +++ b/inttest/erlc_dep_graph/src/java.erl @@ -0,0 +1,11 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +-module(java). + +-export([factory/0]). + +-include("lambda.hrl"). +-include("extra.hrl"). + +factory() -> + ?FUN. diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index af7cf64..e8b77a6 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -40,11 +40,11 @@ -define(ERLCINFO_FILE, "erlcinfo"). -type erlc_info_v() :: {digraph:vertex(), term()} | 'false'. -type erlc_info_e() :: {digraph:vertex(), digraph:vertex()}. --type erlc_info() :: {list(erlc_info_v()), list(erlc_info_e())}. +-type erlc_info() :: {list(erlc_info_v()), list(erlc_info_e()), list(string())}. -record(erlcinfo, { vsn = ?ERLCINFO_VSN :: pos_integer(), - info = {[], []} :: erlc_info() + info = {[], [], []} :: erlc_info() }). -ifdef(namespaced_types). @@ -387,10 +387,6 @@ u_add_element(Elem, [Elem|_]=Set) -> Set; u_add_element(Elem, [E1|Set]) -> [E1|u_add_element(Elem, Set)]; u_add_element(Elem, []) -> [Elem]. --spec include_path(file:filename(), list()) -> [file:filename(), ...]. -include_path(Source, InclDirs) -> - lists:usort(["include", filename:dirname(Source) |InclDirs]). - -spec needs_compile(file:filename(), file:filename(), [string()]) -> boolean(). needs_compile(Source, Target, Parents) -> @@ -403,26 +399,22 @@ erlcinfo_file() -> init_erlcinfo(InclDirs, Erls) -> G = digraph:new(), - try restore_erlcinfo(G) + try restore_erlcinfo(G, InclDirs) catch - _ -> + _:_ -> ?WARN("Failed to restore ~s file. Discarding it.~n", [erlcinfo_file()]), ok = file:delete(erlcinfo_file()) end, - %% Get a unique list of dirs based on the source files' locations. - %% This is used for finding files in sub dirs of the configured - %% src_dirs. For example, src/sub_dir/foo.erl. - Dirs = sets:to_list(lists:foldl( - fun(Erl, Acc) -> - Dir = filename:dirname(Erl), - sets:add_element(Dir, Acc) - end, sets:new(), Erls)), - Updates = [update_erlcinfo(G, Erl, include_path(Erl, InclDirs) ++ Dirs) - || Erl <- Erls], + Dirs = source_and_include_dirs(InclDirs, Erls), + Updates = [update_erlcinfo(G, Erl, Dirs) || Erl <- Erls], Modified = lists:member(modified, Updates), - ok = store_erlcinfo(G, Modified), + ok = store_erlcinfo(G, Modified, InclDirs), G. +source_and_include_dirs(InclDirs, Erls) -> + SourceDirs = lists:map(fun filename:dirname/1, Erls), + lists:usort(["include" | InclDirs ++ SourceDirs]). + update_erlcinfo(G, Source, Dirs) -> case digraph:vertex(G, Source) of {_, LastUpdated} -> @@ -457,10 +449,13 @@ modify_erlcinfo(G, Source, Dirs) -> digraph:add_edge(G, Source, Incl) end, AbsIncls). -restore_erlcinfo(G) -> +restore_erlcinfo(G, InclDirs) -> case file:read_file(erlcinfo_file()) of {ok, Data} -> - #erlcinfo{vsn=?ERLCINFO_VSN, info={Vs, Es}} = binary_to_term(Data), + %% Since externally passed InclDirs can influence erlcinfo graph (see + %% modify_erlcinfo), we have to check here that they didn't change. + #erlcinfo{vsn=?ERLCINFO_VSN, info={Vs, Es, InclDirs}} = + binary_to_term(Data), lists:foreach( fun({V, LastUpdated}) -> digraph:add_vertex(G, V, LastUpdated) @@ -473,9 +468,9 @@ restore_erlcinfo(G) -> ok end. -store_erlcinfo(_G, _Modified = false) -> +store_erlcinfo(_G, _Modified = false, _InclDirs) -> ok; -store_erlcinfo(G, _Modified) -> +store_erlcinfo(G, _Modified, InclDirs) -> Vs = lists:map( fun(V) -> digraph:vertex(G, V) @@ -490,7 +485,7 @@ store_erlcinfo(G, _Modified) -> end, Vs), File = erlcinfo_file(), ok = filelib:ensure_dir(File), - Data = term_to_binary(#erlcinfo{info={Vs, Es}}, [{compressed, 9}]), + Data = term_to_binary(#erlcinfo{info={Vs, Es, InclDirs}}, [{compressed, 9}]), file:write_file(File, Data). %% NOTE: If, for example, one of the entries in Files refers to From 13fe77d3f5a5e0fb4521d2fbab3280b6d864e82a Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Mon, 23 Mar 2015 04:40:52 +0100 Subject: [PATCH 09/18] Avoid extra list traversal when checking for modified source files --- src/rebar_erlc_compiler.erl | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index e8b77a6..19dc40a 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -406,16 +406,23 @@ init_erlcinfo(InclDirs, Erls) -> ok = file:delete(erlcinfo_file()) end, Dirs = source_and_include_dirs(InclDirs, Erls), - Updates = [update_erlcinfo(G, Erl, Dirs) || Erl <- Erls], - Modified = lists:member(modified, Updates), - ok = store_erlcinfo(G, Modified, InclDirs), + Modified = lists:foldl(update_erlcinfo_fun(G, Dirs), false, Erls), + if Modified -> store_erlcinfo(G, InclDirs); not Modified -> ok end, G. source_and_include_dirs(InclDirs, Erls) -> SourceDirs = lists:map(fun filename:dirname/1, Erls), lists:usort(["include" | InclDirs ++ SourceDirs]). -update_erlcinfo(G, Source, Dirs) -> +update_erlcinfo_fun(G, Dirs) -> + fun(Erl, Modified) -> + case update_erlcinfo(G, Dirs, Erl) of + modified -> true; + unmodified -> Modified + end + end. + +update_erlcinfo(G, Dirs, Source) -> case digraph:vertex(G, Source) of {_, LastUpdated} -> case filelib:last_modified(Source) of @@ -426,14 +433,12 @@ update_erlcinfo(G, Source, Dirs) -> digraph:del_vertex(G, Source), modified; LastModified when LastUpdated < LastModified -> - modify_erlcinfo(G, Source, Dirs), - modified; + modify_erlcinfo(G, Source, Dirs); _ -> unmodified end; false -> - modify_erlcinfo(G, Source, Dirs), - modified + modify_erlcinfo(G, Source, Dirs) end. modify_erlcinfo(G, Source, Dirs) -> @@ -445,9 +450,10 @@ modify_erlcinfo(G, Source, Dirs) -> digraph:add_vertex(G, Source, LastUpdated), lists:foreach( fun(Incl) -> - update_erlcinfo(G, Incl, Dirs), + update_erlcinfo(G, Dirs, Incl), digraph:add_edge(G, Source, Incl) - end, AbsIncls). + end, AbsIncls), + modified. restore_erlcinfo(G, InclDirs) -> case file:read_file(erlcinfo_file()) of @@ -468,9 +474,7 @@ restore_erlcinfo(G, InclDirs) -> ok end. -store_erlcinfo(_G, _Modified = false, _InclDirs) -> - ok; -store_erlcinfo(G, _Modified, InclDirs) -> +store_erlcinfo(G, InclDirs) -> Vs = lists:map( fun(V) -> digraph:vertex(G, V) @@ -486,7 +490,7 @@ store_erlcinfo(G, _Modified, InclDirs) -> File = erlcinfo_file(), ok = filelib:ensure_dir(File), Data = term_to_binary(#erlcinfo{info={Vs, Es, InclDirs}}, [{compressed, 9}]), - file:write_file(File, Data). + ok = file:write_file(File, Data). %% NOTE: If, for example, one of the entries in Files refers to %% gen_server.erl, that entry will be dropped. It is dropped because From 462962ebab154362ac87445550b9590f6f5840ce Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Wed, 25 Mar 2015 09:57:42 +0100 Subject: [PATCH 10/18] Delete existing out-edges before adding new ones in modify_erlcinfo When the source file changes it could happen that some of its dependecies get removed. In that case we should remove these former dependencies from the graph, so that they don't influence recompilation of the source file anymore. --- inttest/erlc_dep_graph/erlc_dep_graph_rt.erl | 19 ++++++++++++++++--- inttest/erlc_dep_graph/src/java.erl.no_extra | 10 ++++++++++ src/rebar_erlc_compiler.erl | 1 + 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 inttest/erlc_dep_graph/src/java.erl.no_extra diff --git a/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl b/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl index ee3907e..fb128c6 100644 --- a/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl +++ b/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl @@ -40,7 +40,7 @@ files() -> run(_Dir) -> compile_all(ok, ""), check_beams_ok(), - check_beams_untouched(), + check_beams_untouched(filelib:wildcard("ebin/*.beam")), modify_and_recompile_ok("src/lisp.erl", "ebin/lisp.beam"), clean_all_ok(), @@ -48,14 +48,27 @@ run(_Dir) -> compile_all(ok, ""), modify_and_recompile_ok("extra_include/extra.hrl", "ebin/java.beam"), + Java = "src/java.erl", + {ok, OrigContent} = file:read_file(Java), + %% Remove header file inclusion + {ok, _} = file:copy("src/java.erl.no_extra", Java), + %% Ensure recompilation + touch([Java]), + compile_all(ok, ""), + %% Modify that header file + touch(["extra_include/extra.hrl"]), + %% Ensure we don't have to recompile anything + check_beams_untouched(["ebin/java.beam"]), + %% Clean up + ok = file:write_file(Java, OrigContent), + ok. check_beams_ok() -> F = fun(BeamFile) -> ?assert(filelib:is_regular(BeamFile)) end, with_erl_beams(F). -check_beams_untouched() -> - Beams = filelib:wildcard("ebin/*.beam"), +check_beams_untouched(Beams) -> compile_all_and_assert_mtimes(Beams, fun erlang:'=:='/2). modify_and_recompile_ok(TouchFile, CheckFile) -> diff --git a/inttest/erlc_dep_graph/src/java.erl.no_extra b/inttest/erlc_dep_graph/src/java.erl.no_extra new file mode 100644 index 0000000..7a8fc04 --- /dev/null +++ b/inttest/erlc_dep_graph/src/java.erl.no_extra @@ -0,0 +1,10 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +-module(java). + +-export([factory/0]). + +-include("lambda.hrl"). + +factory() -> + ?FUN. diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 19dc40a..7c3f6db 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -448,6 +448,7 @@ modify_erlcinfo(G, Source, Dirs) -> ok = file:close(Fd), LastUpdated = {date(), time()}, digraph:add_vertex(G, Source, LastUpdated), + digraph:del_edges(G, digraph:out_edges(G, Source)), lists:foreach( fun(Incl) -> update_erlcinfo(G, Dirs, Incl), From 9e33daa6fa8ec083eefd99adb985a1f9b855a0e5 Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Mon, 23 Mar 2015 04:47:54 +0100 Subject: [PATCH 11/18] Additional minor refactor in erlcinfo - use filelib:last_modified/1 instead of date/0 and time/0 combo in source file vertices - simplify store_erlcinfo/2 - document init_erlcinfo/2 --- src/rebar_erlc_compiler.erl | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 7c3f6db..7a21df6 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -397,6 +397,12 @@ needs_compile(Source, Target, Parents) -> erlcinfo_file() -> filename:join([rebar_utils:get_cwd(), ".rebar", ?ERLCINFO_FILE]). +%% Get dependency graph of given Erls files and their dependencies (header files, +%% parse transforms, behaviours etc.) located in their directories or given +%% InclDirs. Note that last modification times stored in vertices are only for +%% internal optimization and cannot be directly used for deciding whether to +%% recompile a file, since when the file itself doesn't change we don't check its +%% dependencies which might change. init_erlcinfo(InclDirs, Erls) -> G = digraph:new(), try restore_erlcinfo(G, InclDirs) @@ -433,21 +439,20 @@ update_erlcinfo(G, Dirs, Source) -> digraph:del_vertex(G, Source), modified; LastModified when LastUpdated < LastModified -> - modify_erlcinfo(G, Source, Dirs); + modify_erlcinfo(G, Source, LastModified, Dirs); _ -> unmodified end; false -> - modify_erlcinfo(G, Source, Dirs) + modify_erlcinfo(G, Source, filelib:last_modified(Source), Dirs) end. -modify_erlcinfo(G, Source, Dirs) -> +modify_erlcinfo(G, Source, LastModified, Dirs) -> {ok, Fd} = file:open(Source, [read]), Incls = parse_attrs(Fd, []), AbsIncls = expand_file_names(Incls, Dirs), ok = file:close(Fd), - LastUpdated = {date(), time()}, - digraph:add_vertex(G, Source, LastUpdated), + digraph:add_vertex(G, Source, LastModified), digraph:del_edges(G, digraph:out_edges(G, Source)), lists:foreach( fun(Incl) -> @@ -468,7 +473,7 @@ restore_erlcinfo(G, InclDirs) -> digraph:add_vertex(G, V, LastUpdated) end, Vs), lists:foreach( - fun({V1, V2}) -> + fun({_, V1, V2, _}) -> digraph:add_edge(G, V1, V2) end, Es); {error, _} -> @@ -476,18 +481,8 @@ restore_erlcinfo(G, InclDirs) -> end. store_erlcinfo(G, InclDirs) -> - Vs = lists:map( - fun(V) -> - digraph:vertex(G, V) - end, digraph:vertices(G)), - Es = lists:flatmap( - fun({V, _}) -> - lists:map( - fun(E) -> - {_, V1, V2, _} = digraph:edge(G, E), - {V1, V2} - end, digraph:out_edges(G, V)) - end, Vs), + Vs = lists:map(fun(V) -> digraph:vertex(G, V) end, digraph:vertices(G)), + Es = lists:map(fun(E) -> digraph:edge(G, E) end, digraph:edges(G)), File = erlcinfo_file(), ok = filelib:ensure_dir(File), Data = term_to_binary(#erlcinfo{info={Vs, Es, InclDirs}}, [{compressed, 9}]), From a25932ba6aa84f036e850fc42599b65b3e22945c Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Mon, 23 Mar 2015 11:17:01 +0100 Subject: [PATCH 12/18] Compress erlcinfo file less agressively Optimizing for best compression could be very slow for large projects, so optimize rather for the speed. erlcinfo file isn't that huge anyway and moreover difference between levels 2 and 9 isn't that big in practice (although there's quite big difference between level 2 and no compression at all). --- src/rebar_erlc_compiler.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 7a21df6..5db1eca 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -485,7 +485,7 @@ store_erlcinfo(G, InclDirs) -> Es = lists:map(fun(E) -> digraph:edge(G, E) end, digraph:edges(G)), File = erlcinfo_file(), ok = filelib:ensure_dir(File), - Data = term_to_binary(#erlcinfo{info={Vs, Es, InclDirs}}, [{compressed, 9}]), + Data = term_to_binary(#erlcinfo{info={Vs, Es, InclDirs}}, [{compressed, 2}]), ok = file:write_file(File, Data). %% NOTE: If, for example, one of the entries in Files refers to From bd29560ce1fcb16c1d22a05fe27a6965cf016b8c Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Mon, 23 Mar 2015 20:42:16 +0100 Subject: [PATCH 13/18] Use toposort of erlcinfo to decide the order of compilation The precise algorithm is now following: - Decide which files need to be compiled first (ErlFirstFiles). - Split AllErlFiles which are not in ErlFirstFiles into two groups, one consisting of files some other file depends on (DepErls) and the rest (OtherErls). - Final FirstErls are obtained as ErlFirstFiles (in original order) plus toposorted DepErls. OtherErls are left as is since they can be compiled in any (random) order. Also create the erlcinfo graph as acyclic, since otherwise the toposort could not work and we don't want to have cycles among dependencies anyway. --- src/rebar_erlc_compiler.erl | 97 +++++++------------------------------ 1 file changed, 18 insertions(+), 79 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 5db1eca..32e1083 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -292,61 +292,28 @@ doterl_compile(Config, OutDir) -> doterl_compile(Config, OutDir, [], ErlOpts). doterl_compile(Config, OutDir, MoreSources, ErlOpts) -> - 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)), 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. - RestErls = [File || File <- AllErlFiles, - not lists:member(File, ErlFirstFilesConf)], - %% NOTE: order of files in ErlFirstFiles is important! - ErlFirstFiles = [File || File <- ErlFirstFilesConf, - lists:member(File, 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(proplists:get_all_values(i, ErlOpts), AllErlFiles), - %% Split RestErls so that files which are depended on are treated - %% like erl_first_files. - {OtherFirstErls, OtherErls} = - lists:partition( - fun(F) -> - Children = get_children(G, F), - log_files(?FMT("Files dependent on ~s", [F]), Children), - case erls(Children) of - [] -> - %% There are no files dependent on this file. - false; - _ -> - %% There are some files dependent on the file. - %% Thus the file has higher priority - %% and should be compiled in the first place. - true - end - end, RestErls), - %% Dependencies of OtherFirstErls that must be compiled first. - OtherFirstErlsDeps = lists:flatmap( - fun(Erl) -> erls(get_parents(G, Erl)) end, - OtherFirstErls), - %% NOTE: In case the way we retrieve OtherFirstErlsDeps or merge - %% it with OtherFirstErls does not result in the correct compile - %% priorities, or the method in use proves to be too slow for - %% certain projects, consider using a more elaborate method (maybe - %% digraph_utils) or alternatively getting and compiling the .erl - %% parents of an individual Source in internal_erl_compile. By not - %% handling this in internal_erl_compile, we also avoid extra - %% needs_compile/2 calls. - FirstErls = ErlFirstFiles ++ uo_merge(OtherFirstErlsDeps, OtherFirstErls), + G = init_erlcinfo(proplists:get_all_values(i, ErlOpts), AllErlFiles), + ErlFirstFiles = erl_first_files(Config, AllErlFiles), + {DepErls, OtherErls} = lists:partition( + fun(Source) -> digraph:in_degree(G, Source) > 0 end, + [File || File <- AllErlFiles, not lists:member(File, ErlFirstFiles)]), + DepErlsOrdered = digraph_utils:topsort(digraph_utils:subgraph(G, DepErls)), + FirstErls = ErlFirstFiles ++ lists:reverse(DepErlsOrdered), ?DEBUG("Files to compile first: ~p~n", [FirstErls]), + rebar_base_compiler:run( Config, FirstErls, OtherErls, fun(S, C) -> @@ -355,37 +322,14 @@ doterl_compile(Config, OutDir, MoreSources, ErlOpts) -> true = rebar_utils:cleanup_code_path(CurrPath), ok. -%% -%% Return all .erl files from a list of files -%% -erls(Files) -> - [Erl || Erl <- Files, filename:extension(Erl) =:= ".erl"]. - -%% -%% Return a list without duplicates while preserving order -%% -ulist(L) -> - ulist(L, []). - -ulist([H|T], Acc) -> - case lists:member(H, T) of - true -> - ulist(T, Acc); - false -> - ulist(T, [H|Acc]) - end; -ulist([], Acc) -> - lists:reverse(Acc). - -%% -%% Merge two lists without duplicates while preserving order -%% -uo_merge(L1, L2) -> - lists:foldl(fun(E, Acc) -> u_add_element(E, Acc) end, ulist(L1), L2). - -u_add_element(Elem, [Elem|_]=Set) -> Set; -u_add_element(Elem, [E1|Set]) -> [E1|u_add_element(Elem, Set)]; -u_add_element(Elem, []) -> [Elem]. +erl_first_files(Config, AllErlFiles) -> + ErlFirstFilesConf = rebar_config:get_list(Config, erl_first_files, []), + %% 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 + %% is not in AllErlFiles. + %% NOTE: order of files in ErlFirstFiles is important! + [File || File <- ErlFirstFilesConf, lists:member(File, AllErlFiles)]. -spec needs_compile(file:filename(), file:filename(), [string()]) -> boolean(). @@ -404,7 +348,7 @@ erlcinfo_file() -> %% recompile a file, since when the file itself doesn't change we don't check its %% dependencies which might change. init_erlcinfo(InclDirs, Erls) -> - G = digraph:new(), + G = digraph:new([acyclic]), try restore_erlcinfo(G, InclDirs) catch _:_ -> @@ -525,11 +469,6 @@ get_parents(G, Source) -> %% Return all files which the Source depends upon. digraph_utils:reachable_neighbours([Source], G). --spec get_children(rebar_digraph(), file:filename()) -> [file:filename()]. -get_children(G, Source) -> - %% Return all files dependent on the Source. - digraph_utils:reaching_neighbours([Source], G). - -spec internal_erl_compile(rebar_config:config(), file:filename(), file:filename(), list(), rebar_digraph()) -> 'ok' | 'skipped'. From c7a2b450bb26ed3fee2a6f23266ca4e336bce62a Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sat, 28 Mar 2015 23:37:37 +0100 Subject: [PATCH 14/18] Store also max modified times in erlcinfo These times already contain max modified info of their dependencies. Therefore we no longer have to check in internal_erl_compile whether the file really needs recompiling, which simplifies the flow somewhat, because the work with dependency graph is now localized to much smaller space then before. --- inttest/erlc_dep_graph/erlc_dep_graph_rt.erl | 3 + inttest/erlc_dep_graph/src/lisp.erl | 2 + inttest/erlc_dep_graph/src/perl.erl | 10 ++ src/rebar_erlc_compiler.erl | 126 ++++++++----------- 4 files changed, 66 insertions(+), 75 deletions(-) create mode 100644 inttest/erlc_dep_graph/src/perl.erl diff --git a/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl b/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl index fb128c6..384ce87 100644 --- a/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl +++ b/inttest/erlc_dep_graph/erlc_dep_graph_rt.erl @@ -62,6 +62,9 @@ run(_Dir) -> %% Clean up ok = file:write_file(Java, OrigContent), + %% Check that changes propagate deeply through the dependency tree + modify_and_recompile_ok("include/lambda.hrl", "ebin/perl.beam"), + ok. check_beams_ok() -> diff --git a/inttest/erlc_dep_graph/src/lisp.erl b/inttest/erlc_dep_graph/src/lisp.erl index dd418d2..eceb157 100644 --- a/inttest/erlc_dep_graph/src/lisp.erl +++ b/inttest/erlc_dep_graph/src/lisp.erl @@ -4,5 +4,7 @@ -export([parse_transform/2]). +-include("lambda.hrl"). + parse_transform(Forms, _Options) -> Forms. diff --git a/inttest/erlc_dep_graph/src/perl.erl b/inttest/erlc_dep_graph/src/perl.erl new file mode 100644 index 0000000..9687948 --- /dev/null +++ b/inttest/erlc_dep_graph/src/perl.erl @@ -0,0 +1,10 @@ +%% -*- mode: erlang;erlang-indent-level: 4;indent-tabs-mode: nil -*- +%% ex: ts=4 sw=4 ft=erlang et +-module(perl). + +-export(['$_'/0]). + +-compile({parse_transform, lisp}). + +'$_'() -> + anything. diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 32e1083..31ed23e 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -47,14 +47,6 @@ info = {[], [], []} :: erlc_info() }). --ifdef(namespaced_types). -%% digraph:graph() exists starting from Erlang 17. --type rebar_digraph() :: digraph:graph(). --else. -%% digraph() has been obsoleted in Erlang 17 and deprecated in 18. --type rebar_digraph() :: digraph(). --endif. - %% =================================================================== %% Public API %% =================================================================== @@ -306,47 +298,42 @@ doterl_compile(Config, OutDir, MoreSources, ErlOpts) -> OutDir1 = proplists:get_value(outdir, ErlOpts, OutDir), G = init_erlcinfo(proplists:get_all_values(i, ErlOpts), AllErlFiles), - ErlFirstFiles = erl_first_files(Config, AllErlFiles), + NeededErlFiles = needed_files(G, OutDir1, AllErlFiles), + ErlFirstFiles = erl_first_files(Config, NeededErlFiles), {DepErls, OtherErls} = lists:partition( fun(Source) -> digraph:in_degree(G, Source) > 0 end, - [File || File <- AllErlFiles, not lists:member(File, ErlFirstFiles)]), + [File || File <- NeededErlFiles, not lists:member(File, ErlFirstFiles)]), DepErlsOrdered = digraph_utils:topsort(digraph_utils:subgraph(G, DepErls)), FirstErls = ErlFirstFiles ++ lists:reverse(DepErlsOrdered), ?DEBUG("Files to compile first: ~p~n", [FirstErls]), rebar_base_compiler:run( Config, FirstErls, OtherErls, - fun(S, C) -> - internal_erl_compile(C, S, OutDir1, ErlOpts, G) - end), + fun(S, C) -> internal_erl_compile(C, S, OutDir1, ErlOpts) end), true = rebar_utils:cleanup_code_path(CurrPath), ok. -erl_first_files(Config, AllErlFiles) -> +erl_first_files(Config, NeededErlFiles) -> + %% NOTE: rebar_config:get_local perhaps? ErlFirstFilesConf = rebar_config:get_list(Config, erl_first_files, []), - %% 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 - %% is not in AllErlFiles. %% NOTE: order of files in ErlFirstFiles is important! - [File || File <- ErlFirstFilesConf, lists:member(File, AllErlFiles)]. + [File || File <- ErlFirstFilesConf, lists:member(File, NeededErlFiles)]. --spec needs_compile(file:filename(), file:filename(), - [string()]) -> boolean(). -needs_compile(Source, Target, Parents) -> - TargetLastMod = filelib:last_modified(Target), - lists:any(fun(I) -> TargetLastMod < filelib:last_modified(I) end, - [Source] ++ Parents). +%% Get subset of SourceFiles which need to be recompiled, respecting +%% dependencies induced by given graph G. +needed_files(G, OutDir, SourceFiles) -> + lists:filter(fun(Source) -> + Target = target_base(OutDir, Source) ++ ".beam", + digraph:vertex(G, Source) > {Source, filelib:last_modified(Target)} + end, SourceFiles). erlcinfo_file() -> filename:join([rebar_utils:get_cwd(), ".rebar", ?ERLCINFO_FILE]). %% Get dependency graph of given Erls files and their dependencies (header files, %% parse transforms, behaviours etc.) located in their directories or given -%% InclDirs. Note that last modification times stored in vertices are only for -%% internal optimization and cannot be directly used for deciding whether to -%% recompile a file, since when the file itself doesn't change we don't check its -%% dependencies which might change. +%% InclDirs. Note that last modification times stored in vertices already respect +%% dependencies induced by given graph G. init_erlcinfo(InclDirs, Erls) -> G = digraph:new([acyclic]), try restore_erlcinfo(G, InclDirs) @@ -385,12 +372,26 @@ update_erlcinfo(G, Dirs, Source) -> LastModified when LastUpdated < LastModified -> modify_erlcinfo(G, Source, LastModified, Dirs); _ -> - unmodified + Modified = lists:foldl( + update_erlcinfo_fun(G, Dirs), + false, digraph:out_neighbours(G, Source)), + MaxModified = update_max_modified_deps(G, Source), + case Modified orelse MaxModified > LastUpdated of + true -> modified; + false -> unmodified + end end; false -> modify_erlcinfo(G, Source, filelib:last_modified(Source), Dirs) end. +update_max_modified_deps(G, Source) -> + MaxModified = lists:max(lists:map( + fun(File) -> {_, MaxModified} = digraph:vertex(G, File), MaxModified end, + [Source|digraph:out_neighbours(G, Source)])), + digraph:add_vertex(G, Source, MaxModified), + MaxModified. + modify_erlcinfo(G, Source, LastModified, Dirs) -> {ok, Fd} = file:open(Source, [read]), Incls = parse_attrs(Fd, []), @@ -464,43 +465,25 @@ expand_file_names(Files, Dirs) -> end end, Files). --spec get_parents(rebar_digraph(), file:filename()) -> [file:filename()]. -get_parents(G, Source) -> - %% Return all files which the Source depends upon. - digraph_utils:reachable_neighbours([Source], G). - -spec internal_erl_compile(rebar_config:config(), file:filename(), - file:filename(), list(), - rebar_digraph()) -> 'ok' | 'skipped'. -internal_erl_compile(Config, Source, OutDir, ErlOpts, G) -> - %% Determine the target name and includes list by inspecting the source file - Module = filename:basename(Source, ".erl"), - Parents = get_parents(G, Source), - log_files(?FMT("Dependencies of ~s", [Source]), Parents), - - %% Construct the target filename - Target = filename:join([OutDir | string:tokens(Module, ".")]) ++ ".beam", - ok = filelib:ensure_dir(Target), - - %% If the file needs compilation, based on last mod date of includes or - %% the target - case needs_compile(Source, Target, Parents) of - true -> - Opts = [{outdir, filename:dirname(Target)}] ++ - ErlOpts ++ [{i, "include"}, return], - case compile:file(Source, Opts) of - {ok, _Mod} -> - ok; - {ok, _Mod, Ws} -> - rebar_base_compiler:ok_tuple(Config, Source, Ws); - {error, Es, Ws} -> - rebar_base_compiler:error_tuple(Config, Source, - Es, Ws, Opts) - end; - false -> - skipped + file:filename(), list()) -> ok | {ok, any()} | {error, any(), any()}. +internal_erl_compile(Config, Source, OutDir, ErlOpts) -> + TargetDir = filename:dirname(target_base(OutDir, Source)), + ok = filelib:ensure_dir(TargetDir), + Opts = [{outdir, TargetDir}] ++ ErlOpts ++ [{i, "include"}, return], + case compile:file(Source, Opts) of + {ok, _Mod} -> + ok; + {ok, _Mod, Ws} -> + rebar_base_compiler:ok_tuple(Config, Source, Ws); + {error, Es, Ws} -> + rebar_base_compiler:error_tuple(Config, Source, Es, Ws, Opts) end. +target_base(OutDir, Source) -> + Module = filename:basename(Source, ".erl"), + filename:join([OutDir|string:tokens(Module, ".")]). + -spec compile_mib(file:filename(), file:filename(), rebar_config:config()) -> 'ok'. compile_mib(Source, Target, Config) -> @@ -541,7 +524,7 @@ compile_yrl(Source, Target, Config) -> -spec compile_xrl_yrl(rebar_config:config(), file:filename(), file:filename(), list(), module()) -> 'ok'. compile_xrl_yrl(Config, Source, Target, Opts, Mod) -> - case needs_compile(Source, Target, []) of + case needs_compile(Source, Target) of true -> case Mod:file(Source, Opts ++ [{return, true}]) of {ok, _} -> @@ -556,6 +539,9 @@ compile_xrl_yrl(Config, Source, Target, Opts, Mod) -> skipped end. +needs_compile(Source, Target) -> + filelib:last_modified(Source) > filelib:last_modified(Target). + gather_src([], Srcs) -> Srcs; gather_src([Dir|Rest], Srcs) -> @@ -677,13 +663,3 @@ check_file(File) -> false -> ?ABORT("File ~p is missing, aborting\n", [File]); true -> File end. - -%% Print prefix followed by list of files. If the list is empty, print -%% on the same line, otherwise use a separate line. -log_files(Prefix, Files) -> - case Files of - [] -> - ?DEBUG("~s: ~p~n", [Prefix, Files]); - _ -> - ?DEBUG("~s:~n~p~n", [Prefix, Files]) - end. From ee5a75cb3398b43ec46752a7d43abe82361bd885 Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Tue, 31 Mar 2015 23:36:30 +0200 Subject: [PATCH 15/18] Remove obsolete way of generating target ebin subdirectories As per @tuncer: "That's most likely a vestige of package module support". --- src/rebar_erlc_compiler.erl | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 31ed23e..dd49b2e 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -327,6 +327,9 @@ needed_files(G, OutDir, SourceFiles) -> digraph:vertex(G, Source) > {Source, filelib:last_modified(Target)} end, SourceFiles). +target_base(OutDir, Source) -> + filename:join(OutDir, filename:basename(Source, ".erl")). + erlcinfo_file() -> filename:join([rebar_utils:get_cwd(), ".rebar", ?ERLCINFO_FILE]). @@ -468,9 +471,8 @@ expand_file_names(Files, Dirs) -> -spec internal_erl_compile(rebar_config:config(), file:filename(), file:filename(), list()) -> ok | {ok, any()} | {error, any(), any()}. internal_erl_compile(Config, Source, OutDir, ErlOpts) -> - TargetDir = filename:dirname(target_base(OutDir, Source)), - ok = filelib:ensure_dir(TargetDir), - Opts = [{outdir, TargetDir}] ++ ErlOpts ++ [{i, "include"}, return], + ok = filelib:ensure_dir(OutDir), + Opts = [{outdir, OutDir}] ++ ErlOpts ++ [{i, "include"}, return], case compile:file(Source, Opts) of {ok, _Mod} -> ok; @@ -480,10 +482,6 @@ internal_erl_compile(Config, Source, OutDir, ErlOpts) -> rebar_base_compiler:error_tuple(Config, Source, Es, Ws, Opts) end. -target_base(OutDir, Source) -> - Module = filename:basename(Source, ".erl"), - filename:join([OutDir|string:tokens(Module, ".")]). - -spec compile_mib(file:filename(), file:filename(), rebar_config:config()) -> 'ok'. compile_mib(Source, Target, Config) -> From 07a4a14af6a88a47bed3af994d1814b3045777c8 Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sun, 5 Apr 2015 01:34:25 +0200 Subject: [PATCH 16/18] Fix error when processing non-existent include_lib file Since process_attr/3 searches source code for attributes, it can happen that it finds an attribute which is eventually not needed by the compilation, e.g. hidden by ifdef macro - see enclosed test. Such attribute can reference a file which is not in the path or which even doesn't exist at all, and current code doesn't expect such a situation. We fix things by simply ignoring such files. --- inttest/erlc_dep_graph/src/lisp.erl | 3 +++ src/rebar_erlc_compiler.erl | 11 ++++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/inttest/erlc_dep_graph/src/lisp.erl b/inttest/erlc_dep_graph/src/lisp.erl index eceb157..31fc8b5 100644 --- a/inttest/erlc_dep_graph/src/lisp.erl +++ b/inttest/erlc_dep_graph/src/lisp.erl @@ -5,6 +5,9 @@ -export([parse_transform/2]). -include("lambda.hrl"). +-ifdef(NOT_DEFINED). +-include_lib("include/non/existent.hrl"). +-endif. parse_transform(Forms, _Options) -> Forms. diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index dd49b2e..6a1bbe5 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -594,8 +594,7 @@ process_attr(include, Form, Includes) -> process_attr(include_lib, Form, Includes) -> [FileNode] = erl_syntax:attribute_arguments(Form), RawFile = erl_syntax:string_value(FileNode), - File = maybe_expand_include_lib_path(RawFile), - [File|Includes]; + maybe_expand_include_lib_path(RawFile) ++ Includes; process_attr(behaviour, Form, Includes) -> [FileNode] = erl_syntax:attribute_arguments(Form), File = module_to_erl(erl_syntax:atom_value(FileNode)), @@ -631,7 +630,7 @@ module_to_erl(Mod) -> maybe_expand_include_lib_path(File) -> case filelib:is_regular(File) of true -> - File; + [File]; false -> expand_include_lib_path(File) end. @@ -646,8 +645,10 @@ expand_include_lib_path(File) -> Split = filename:split(filename:dirname(File)), Lib = hd(Split), SubDir = filename:join(tl(Split)), - Dir = code:lib_dir(list_to_atom(Lib), list_to_atom(SubDir)), - filename:join(Dir, File1). + case code:lib_dir(list_to_atom(Lib), list_to_atom(SubDir)) of + {error, bad_name} -> []; + Dir -> [filename:join(Dir, File1)] + end. %% %% Ensure all files in a list are present and abort if one is missing From d62624c86d29288b5103b2f91014e09c47241307 Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sun, 29 Mar 2015 10:10:18 +0200 Subject: [PATCH 17/18] Raise version of erlcinfo Its format was (repeatedly) changed in previous commits. --- src/rebar_erlc_compiler.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rebar_erlc_compiler.erl b/src/rebar_erlc_compiler.erl index 6a1bbe5..b59cac5 100644 --- a/src/rebar_erlc_compiler.erl +++ b/src/rebar_erlc_compiler.erl @@ -36,7 +36,7 @@ -include("rebar.hrl"). -include_lib("stdlib/include/erl_compile.hrl"). --define(ERLCINFO_VSN, 1). +-define(ERLCINFO_VSN, 2). -define(ERLCINFO_FILE, "erlcinfo"). -type erlc_info_v() :: {digraph:vertex(), term()} | 'false'. -type erlc_info_e() :: {digraph:vertex(), digraph:vertex()}. From e17d30a2dd300ece0521423bf7c1fa25d0219cbf Mon Sep 17 00:00:00 2001 From: David Kubecka Date: Sun, 29 Mar 2015 23:17:22 +0200 Subject: [PATCH 18/18] append myself to THANKS --- THANKS | 1 + 1 file changed, 1 insertion(+) diff --git a/THANKS b/THANKS index 4032fd9..1d9a1e3 100644 --- a/THANKS +++ b/THANKS @@ -135,3 +135,4 @@ Pavel Baturko Igor Savchuk Mark Anderson Brian H. Ward +David Kubecka