From 3e30b824ef74b1f1e49327721cc08e1a364c82fc Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Wed, 28 Oct 2020 14:26:31 +0000 Subject: [PATCH] Move dep compiler version change to DAG check Rather than doing a check on each single file in a dep set (which can grow quite large), do a dynamic rebuild of deps based on whether the DAG mentions the files have been modified. To avoid reloading the whole DAG more than once and then having to still map files, the compiler version is added to rebar_compiler_erl's CritMeta for the DAG. This comparison is only set in place to impact the rebar_prv_install_deps call that handles pruncing and culling deps by the compiler provider. Intra-DAG comparisons still need to be done by rebar_compiler_erl for things to work, and the hope is that since most built-in compiler mods end up generating erl files in deps, that will be transparent enough to work well. A DAG call to check the status of the dag (vsn, critmeta, existing, etc.) has been added to do the faster check there and allowing proper dispatch. Plugins couldn't be updated because they don't really use the standard dep-style facilities for things due to an early filtering. --- src/rebar_compiler_dag.erl | 26 ++++++++- src/rebar_prv_compile.erl | 87 ++++++++++++++++++++++++------- src/rebar_prv_install_deps.erl | 34 +----------- test/rebar_compile_SUITE.erl | 7 ++- test/rebar_compiler_dag_SUITE.erl | 15 +++++- 5 files changed, 113 insertions(+), 56 deletions(-) diff --git a/src/rebar_compiler_dag.erl b/src/rebar_compiler_dag.erl index 6fcc3e1a..d6038e83 100644 --- a/src/rebar_compiler_dag.erl +++ b/src/rebar_compiler_dag.erl @@ -1,7 +1,7 @@ %%% Module handling the directed graph required for the analysis %%% of all top-level applications by the various compiler plugins. -module(rebar_compiler_dag). --export([init/4, maybe_store/5, terminate/1]). +-export([init/4, status/4, maybe_store/5, terminate/1]). -export([prune/5, populate_sources/5, populate_deps/3, propagate_stamps/1, compile_order/2, store_artifact/4]). @@ -39,6 +39,29 @@ init(Dir, Compiler, Label, CritMeta) -> end, G. +%% @doc Quickly validate whether a DAG exists by validating its file name, +%% version, and CritMeta data, without attempting to actually build it. +-spec status(file:filename_all(), atom(), string() | undefined, critical_meta()) -> + valid | bad_format | bad_vsn | bad_meta | not_found. +status(Dir, Compiler, Label, CritMeta) -> + File = dag_file(Dir, Compiler, Label), + case file:read_file(File) of + {ok, Data} -> + %% The CritMeta value is checked and if it doesn't match, we + %% consider things invalid. Same for the version. + try binary_to_term(Data) of + #dag{vsn = ?DAG_VSN, meta = CritMeta} -> valid; + #dag{vsn = ?DAG_VSN} -> bad_meta; + #dag{meta = CritMeta} -> bad_vsn; + _ -> bad_format + catch + _:_ -> + bad_format + end; + {error, _Err} -> + not_found + end. + %% @doc Clear up inactive (deleted) source files from a given project. %% The file must be in one of the directories that may contain source files %% for an OTP application; source files found in the DAG `G' that lie outside @@ -240,6 +263,7 @@ terminate(G) -> true = digraph:delete(G). store_artifact(G, Source, Target, Meta) -> + mark_dirty(G), digraph:add_vertex(G, Target, {artifact, Meta}), digraph:add_edge(G, Target, Source, artifact). diff --git a/src/rebar_prv_compile.erl b/src/rebar_prv_compile.erl index f799cbd0..d4d268d7 100644 --- a/src/rebar_prv_compile.erl +++ b/src/rebar_prv_compile.erl @@ -39,8 +39,9 @@ do(State) -> rebar_paths:set_paths([deps], State), Providers = rebar_state:providers(State), - Deps = rebar_state:deps_to_build(State), - CompiledDeps = copy_and_build_apps(State, Providers, Deps), + MustBuildDeps = rebar_state:deps_to_build(State), + Deps = rebar_state:all_deps(State), + CompiledDeps = copy_and_build_deps(State, Providers, MustBuildDeps, Deps), State0 = rebar_state:merge_all_deps(State, CompiledDeps), State1 = case IsDepsOnly of @@ -97,7 +98,30 @@ format_error({unknown_project_type, Name, Type}) -> format_error(Reason) -> io_lib:format("~p", [Reason]). -copy_and_build_apps(State, Providers, Apps) -> +pick_deps_to_build(State, MustBuild, All, Tag) -> + InvalidDags = lists:any( + fun({_Mod, Status}) -> + %% A DAG that is not found can be valid, it just has no matching + %% source file. However, bad_vsn, bad_format, and bad_meta should + %% all trigger rebuilds. + %% Good: + %% lists:member(Status, [valid, not_found]) + %% Bad: + %% lists:member(Status, [bad_vsn, bad_format, bad_meta]) + %% + %% Since the fastest check is done on smaller lists handling + %% the common case first: + not lists:member(Status, [valid, not_found]) + end, + check_dags(State, Tag) + ), + case InvalidDags of + true -> All; + false -> MustBuild + end. + +copy_and_build_deps(State, Providers, MustBuildApps, AllApps) -> + Apps = pick_deps_to_build(State, MustBuildApps, AllApps, apps), Apps0 = [prepare_app(State, Providers, App) || App <- Apps], compile(State, Providers, Apps0, apps). @@ -155,31 +179,54 @@ prepare_compilers(State, Providers, AppInfo) -> rebar_hooks:run_all_hooks(AppDir, pre, ?ERLC_HOOK, Providers, AppInfo, State). run_compilers(State, _Providers, Apps, Tag) -> - %% Prepare a compiler digraph to be shared by all compiled applications - %% in a given run, providing the ability to combine their dependency - %% ordering and resources. - %% The Tag allows to create a Label when someone cares about a specific - %% run for compilation; - DAGLabel = case Tag of - undefined -> undefined; - _ -> atom_to_list(Tag) - end, - %% The Dir for the DAG is set to deps_dir so builds taking place - %% in different contexts (i.e. plugins) don't risk clobbering regular deps. - Dir = rebar_dir:deps_dir(State), - CritMeta = [], % used to be incldirs per app - DAGs = [{Mod, rebar_compiler_dag:init(Dir, Mod, DAGLabel, CritMeta)} - || Mod <- rebar_state:compilers(State)], + %% Get the compiler graphs for all modules and artifacts, for each + %% compiler we run + DAGsInfo = load_dags(State, Tag), + DAGs = [{Mod, DAG} || {Mod, {DAG, _Meta}} <- DAGsInfo], %% Compile all the apps build_apps(DAGs, Apps, State), %% Potentially store shared compiler DAGs so next runs can easily %% share the base information for easy re-scans. - lists:foreach(fun({Mod, G}) -> + lists:foreach(fun({Mod, {G, {Dir, DAGLabel, CritMeta}}}) -> rebar_compiler_dag:maybe_store(G, Dir, Mod, DAGLabel, CritMeta), rebar_compiler_dag:terminate(G) - end, DAGs), + end, DAGsInfo), Apps. +load_dags(State, Tag) -> + F = fun(Dir, Mod, DAGLabel, CritMeta) -> + {rebar_compiler_dag:init(Dir, Mod, DAGLabel, CritMeta), + {Dir, DAGLabel, CritMeta}} + end, + map_dags(F, State, Tag). + +check_dags(State, Tag) -> + map_dags(fun rebar_compiler_dag:status/4, State, Tag). + +map_dags(F, State, Tag) -> + DAGLabel = format_label(Tag), + %% The Dir for the DAG is set to deps_dir so builds taking place + %% in different contexts (i.e. plugins) don't risk clobbering regular deps. + Dir = rebar_dir:deps_dir(State), + SharedCritMeta = [], + [{Mod, F(Dir, Mod, DAGLabel, mod_meta(Mod, SharedCritMeta))} + || Mod <- rebar_state:compilers(State)]. + +mod_meta(rebar_compiler_erl, CritMeta) -> + application:load(compiler), + {ok, CompileVsn} = application:get_key(compiler, vsn), + [{compiler, CompileVsn} | CritMeta]; +mod_meta(_, CritMeta) -> + CritMeta. + +format_label(Tag) -> + %% The Tag allows to create a Label when someone cares about a specific + %% run for compilation; + case Tag of + undefined -> undefined; + _ -> atom_to_list(Tag) + end. + finalize_compilers(State, Providers, AppInfo) -> AppDir = rebar_app_info:dir(AppInfo), rebar_hooks:run_all_hooks(AppDir, post, ?ERLC_HOOK, Providers, AppInfo, State). diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 26caefbe..5331c4cb 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -436,8 +436,7 @@ warn_skip_deps(AppInfo, State) -> not_needs_compile(App) -> not(rebar_app_info:is_checkout(App)) andalso rebar_app_info:valid(App) - andalso rebar_app_info:has_all_artifacts(App) =:= true - andalso not needs_rebuild(App). + andalso rebar_app_info:has_all_artifacts(App) =:= true. find_app_and_level_by_name([], _) -> false; @@ -446,34 +445,3 @@ find_app_and_level_by_name([App|Apps], Name) -> Name -> {ok, App, rebar_app_info:dep_level(App)}; _ -> find_app_and_level_by_name(Apps, Name) end. - -needs_rebuild(AppInfo) -> - Ebin = rebar_app_info:ebin_dir(AppInfo), - application:load(compiler), - {ok, CurrentCompileVsn} = application:get_key(compiler, vsn), - case find_some_beam(Ebin) of - {ok, Beam} -> - case beam_lib:chunks(Beam, [compile_info]) of - {ok, {_mod, Chunks}} -> - CompileInfo = proplists:get_value(compile_info, Chunks, []), - CompileVsn = proplists:get_value(version, CompileInfo, "unknown"), - CurrentCompileVsn =/= CompileVsn; - _ -> - %% could be built without debug_info - false - end; - _ -> - %% well we would expect a plugin to have a beam file - true - end. - -find_some_beam(Path) -> - case file:list_dir(Path) of - {error, Reason} -> - {error, Reason}; - {ok, Files} -> - case lists:dropwhile(fun(P) -> filename:extension(P) =/= ".beam" end, Files) of - [Beam|_] -> {ok, filename:join(Path, Beam)}; - _ -> {error, no_beam} - end - end. diff --git a/test/rebar_compile_SUITE.erl b/test/rebar_compile_SUITE.erl index 3252a977..feff69ee 100644 --- a/test/rebar_compile_SUITE.erl +++ b/test/rebar_compile_SUITE.erl @@ -987,7 +987,12 @@ recompile_when_dag_opts_change(Config) -> %% change the config in the DAG... [digraph:add_vertex(G, Beam, {artifact, [{d, some_define}]}) || Beam <- Beams], digraph:add_vertex(G, '$r3_dirty_bit', true), % trigger a save - rebar_compiler_dag:maybe_store(G, DepsDir, rebar_compiler_erl, "project_apps", []), + %% the rebar_compiler_erl module is annotated with a compiler version + %% to help rebuild deps + {ok, CompileVsn} = application:get_key(compiler, vsn), + CritMeta = [{compiler, CompileVsn}], + + rebar_compiler_dag:maybe_store(G, DepsDir, rebar_compiler_erl, "project_apps", CritMeta), rebar_compiler_dag:terminate(G), %% ... but don't change the actual rebar3 config... rebar_test_utils:run_and_check(Config, [], ["compile"], {ok, [{app, Name}]}), diff --git a/test/rebar_compiler_dag_SUITE.erl b/test/rebar_compiler_dag_SUITE.erl index ebd90ff8..0fac5b3a 100644 --- a/test/rebar_compiler_dag_SUITE.erl +++ b/test/rebar_compiler_dag_SUITE.erl @@ -6,7 +6,7 @@ -include_lib("kernel/include/file.hrl"). all() -> - [{group, with_project}]. + [exists, {group, with_project}]. groups() -> %% The tests in this group are dirty, the order is specific @@ -55,6 +55,19 @@ init_per_group(_, Config) -> end_per_group(_, Config) -> Config. +exists(Config) -> + %% Create a DAG + Priv = ?config(priv_dir, Config), + G = rebar_compiler_dag:init(Priv, compilermod, "label", [crit_meta]), + rebar_compiler_dag:store_artifact(G, "somefile", "someartifact", [written]), + rebar_compiler_dag:maybe_store(G, Priv, compilermod, "label", [crit_meta]), + rebar_compiler_dag:terminate(G), + + ?assertEqual(valid, rebar_compiler_dag:status(Priv, compilermod, "label", [crit_meta])), + ?assertEqual(not_found, rebar_compiler_dag:status(Priv, compilermad, "label", [crit_meta])), + ?assertEqual(not_found, rebar_compiler_dag:status(Priv, compilermod, "lobel", [crit_meta])), + ?assertEqual(bad_meta, rebar_compiler_dag:status(Priv, compilermod, "label", [crit_zeta])), + ok. project() -> [{app1, [