-
Notifications
You must be signed in to change notification settings - Fork 521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Split up the compiler DAG #2217
Conversation
This is another tricky commit towards replacing the current module analysis with EPP. The compiler DAG is now being shared across multiple applications being compiled, rather than a per-application basis, which promises to allow better ordering, parallelism, and more thorough invalidation of runtime dependencies when they are modified. This however required changes: - The compiler DAG is no longer private to `rebar_compiler`, and has been extracted to the `rebar_compiler_dag` module - The compiler DAG is now started by the `rebar_prv_compile` module, which oversees the calls to `rebar_compiler` for each OTP application - The compiler DAG has been refactored to use a "dirty flag" to know if it was modified, rather than just tracking modifications in a functional manner, since the scope change (going multi-app) makes it impossible to cleanly use the functional approach without much larger changes - The DAG used to be cached within each OTP application. This is no longer possible since it is shared. Instead the DAG is stored in the state's deps_dir, which allows to cleanly split caches between regular apps for the user's project and plugins - The DAG supported a "label" mode that was used to store distinct DAGs for extra_src_dir runs and regular modules; this label is now used (and extended to `rebar_prv_compile` internals) to distinguish between "compile runs", such as "project_apps", or just "apps" (deps). The label is optional (i.e. not used by plugins which have no such need) - The extra_src_dirs for each app is now compiled using the main app's DAG, but the run takes place later in the compilation process. This may need changing to detect and prevent dependencies from src_dirs into extra_src_dirs, but this should not technically be a problem for runtime anyway. - Reworked the support for extra_src_dirs that are at the root of an umbrella project (and therefore do not belong to any single app) to use the new structure, also as part of the project_apps DAG. All tests keep passing, and this puts us in a better place to use EPP with cross-app support in the near-future.
prepare_compiler_env(AppInfo), | ||
lists:foreach(fun({Compiler, G}) -> | ||
run(G, Compiler, AppInfo), | ||
%% TODO: disable default recursivity in extra_src_dirs compiling to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we really merge this without this disabled by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it's what we have today. See the complaints about modules auto-compiled by CT. I just added the note in comments to fix this long-standing bug.
ok | ||
end, | ||
DAGs); | ||
compile_all(Compilers, AppInfo) -> % =< 3.13.0 interface; plugins use this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should decide on how many versions back of rebar3 to support and figure out a way to give useful error messages when plugins are outdated.
Doesn't need to be a part of this PR, just bringing it up because this is adding to the already extensive technical debt for plugin support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. To some extent I have never taken the time to update the rebar3_proper plugin myself so I feel in a bad place to force others to do it as well. It still displays the cool warning every time you run it.
This is another tricky commit towards replacing the current module
analysis with EPP. The compiler DAG is now being shared across multiple
applications being compiled, rather than a per-application basis, which
promises to allow better ordering, parallelism, and more thorough
invalidation of runtime dependencies when they are modified.
This however required changes:
rebar_compiler
, and hasbeen extracted to the
rebar_compiler_dag
modulerebar_prv_compile
module,which oversees the calls to
rebar_compiler
for each OTP applicationit was modified, rather than just tracking modifications in a
functional manner, since the scope change (going multi-app) makes it
impossible to cleanly use the functional approach without much larger
changes
longer possible since it is shared. Instead the DAG is stored in the
state's deps_dir, which allows to cleanly split caches between regular
apps for the user's project and plugins
for extra_src_dir runs and regular modules; this label is now used
(and extended to
rebar_prv_compile
internals) to distinguish between"compile runs", such as "project_apps", or just "apps" (deps). The
label is optional (i.e. not used by plugins which have no such need)
DAG, but the run takes place later in the compilation process. This
may need changing to detect and prevent dependencies from src_dirs
into extra_src_dirs, but this should not technically be a problem for
runtime anyway.
umbrella project (and therefore do not belong to any single app) to
use the new structure, also as part of the project_apps DAG.
clean
command no longer clears the DAG since it's not part ofthe app itself anymore; the prune step should handle it.
clearly doesn't make sense anymore. I plan to eventually replace it with
things such as the compiler version instead.
All tests keep passing, and this puts us in a better place to use EPP
with cross-app support in the near-future.