From be1680e443113b0c8432d632c288c92fba428bf0 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Fri, 24 Jul 2020 13:01:26 +0000 Subject: [PATCH] Trigger rebuilds when OTP compiler version changes This fix is done in three parts: 1. Add the compiler version to the data tracked in the DAG, and extract it from source file (when the DAG isn't around) for comparisons. Then on each build job, check for the compiler version as a build option like every other one. A change in compiler version represents a change in build options, which triggers a rebuild 2. Make it work for plugins. This requires more work because plugins that can find their ebin/ dir are assumed to be pre-built. Rather than undoing this, poke at 1 single beam file in their ebin/ dir and check if their compiler version matches ours to do an additional filter round. This is likely to be faster than reanalyzing the whole app, but a bit more brittle in the long run. 3. Make it work for deps. This currently is done with copy/pasted plugin code that needs to be reworked later if this is shown to be an acceptable extra step. This patch also includes a fix on the path pruning for plugins, which mistakenly subtracted AppInfo records from file paths; the intent was to use the ebin dir as a comparison, which this patch fixes. No tests have been added at this point. --- src/rebar_compiler.erl | 1 + src/rebar_compiler_erl.erl | 14 +++++++---- src/rebar_plugins.erl | 45 ++++++++++++++++++++++++++++++++-- src/rebar_prv_install_deps.erl | 33 ++++++++++++++++++++++++- 4 files changed, 85 insertions(+), 8 deletions(-) diff --git a/src/rebar_compiler.erl b/src/rebar_compiler.erl index d7081b00..c9d808e1 100644 --- a/src/rebar_compiler.erl +++ b/src/rebar_compiler.erl @@ -215,6 +215,7 @@ prepare_compiler_env(Compiler, Apps) -> %% necessary for erlang:function_exported/3 to work as expected %% called here for clarity as it's required by both opts_changed/2 %% and erl_compiler_opts_set/0 in needed_files + application:load(compiler), _ = code:ensure_loaded(compile), _ = code:ensure_loaded(Compiler), ok. diff --git a/src/rebar_compiler_erl.erl b/src/rebar_compiler_erl.erl index 5c00ab8a..b366426d 100644 --- a/src/rebar_compiler_erl.erl +++ b/src/rebar_compiler_erl.erl @@ -145,9 +145,10 @@ compile_and_track(Source, [{Ext, OutDir}], Config, ErlOpts) -> rebar_compiler_epp:flush(), BuildOpts = [{outdir, OutDir} | ErlOpts], Target = target_base(OutDir, Source) ++ Ext, + {ok, CompileVsn} = application:get_key(compiler, vsn), AllOpts = case erlang:function_exported(compile, env_compiler_options, 0) of - true -> BuildOpts ++ compile:env_compiler_options(); - false -> BuildOpts + true -> [{compiler_version, CompileVsn}] ++ BuildOpts ++ compile:env_compiler_options(); + false -> [{compiler_version, CompileVsn}] ++ BuildOpts end, case compile:file(Source, BuildOpts) of {ok, _Mod} -> @@ -237,9 +238,10 @@ target_base(OutDir, Source) -> filename:join(OutDir, filename:basename(Source, ".erl")). opts_changed(Graph, NewOpts, Target, TargetBase) -> + {ok, CompileVsn} = application:get_key(compiler, vsn), TotalOpts = case erlang:function_exported(compile, env_compiler_options, 0) of - true -> NewOpts ++ compile:env_compiler_options(); - false -> NewOpts + true -> [{compiler_version, CompileVsn}] ++ NewOpts ++ compile:env_compiler_options(); + false -> [{compiler_version, CompileVsn}] ++ NewOpts end, TargetOpts = case digraph:vertex(Graph, Target) of {_Target, {artifact, Opts}} -> % tracked dep is found @@ -273,7 +275,9 @@ compile_info(Target) -> case beam_lib:chunks(Target, [compile_info]) of {ok, {_mod, Chunks}} -> CompileInfo = proplists:get_value(compile_info, Chunks, []), - {ok, proplists:get_value(options, CompileInfo, [])}; + CompileVsn = proplists:get_value(version, CompileInfo, "unknown"), + {ok, [{compiler_version, CompileVsn} + | proplists:get_value(options, CompileInfo, [])]}; {error, beam_lib, Reason} -> ?WARN("Couldn't read debug info from ~p for reason: ~p", [Target, Reason]), {error, Reason} diff --git a/src/rebar_plugins.erl b/src/rebar_plugins.erl index 43b33db8..3bbd4a88 100644 --- a/src/rebar_plugins.erl +++ b/src/rebar_plugins.erl @@ -113,11 +113,16 @@ handle_plugin(Profile, Plugin, State, Upgrade) -> ToBuild = rebar_prv_install_deps:cull_compile(Sorted, []), %% Add already built plugin deps to the code path - PreBuiltPaths = [rebar_app_info:ebin_dir(A) || A <- Apps] -- ToBuild, + ToBuildPaths = [rebar_app_info:ebin_dir(A) || A <- ToBuild], + PreBuiltApps = [A || A <- Apps, + Ebin <- [rebar_app_info:ebin_dir(A)], + not lists:member(Ebin, ToBuildPaths)], + {PreUnsafe, PreSafe} = lists:partition(fun needs_rebuild/1, PreBuiltApps), + PreBuiltPaths = [rebar_app_info:ebin_dir(A) || A <- PreSafe], code:add_pathsa(PreBuiltPaths), %% Build plugin and its deps - [build_plugin(AppInfo, Apps, State2) || AppInfo <- ToBuild], + [build_plugin(AppInfo, Apps, State2) || AppInfo <- PreUnsafe++ToBuild], %% Add newly built deps and plugin to code path State3 = rebar_state:update_all_plugin_deps(State2, Apps), @@ -165,3 +170,39 @@ validate_plugin(Plugin) -> [Plugin] end end. + +%% @private do a quick validation of whether a plugin is expected to need +%% to be rebuilt; usually this is handled by the compiler, but since this +%% module does quick filtering by detection, we need to discriminate against +%% cases like the compiler version having changed that would otherwise +%% trigger a rebuild. +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/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 34334d51..26caefbe 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -436,7 +436,8 @@ 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 rebar_app_info:has_all_artifacts(App) =:= true + andalso not needs_rebuild(App). find_app_and_level_by_name([], _) -> false; @@ -446,3 +447,33 @@ find_app_and_level_by_name([App|Apps], Name) -> _ -> 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.