Skip to content
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

Code Path Handling and Rebar3 #1899

Closed
ferd opened this issue Oct 2, 2018 · 3 comments
Closed

Code Path Handling and Rebar3 #1899

ferd opened this issue Oct 2, 2018 · 3 comments

Comments

@ferd
Copy link
Collaborator

ferd commented Oct 2, 2018

Currently, code path handling is rather a mess. This is due to limitations of the compiler, which makes it so the code paths loaded have an impact on how header files and parse transforms are handled by the compiler. Because we want deterministic builds, and that plugins can coexist along with dependencies within the rebar3 instance currently running, we have a constant risk of plugin dependencies clashing with application and dependency paths.

To solve this, almost every task and module in rebar3 that has to deal with compilation, plugins, and hooks ends up having to juggle code paths. Further problems include dependencies that are used by rebar3 itself (such as bbmustache), which cannot be unloaded in the escript version without risking to segfault.

The number of bugs related to this lately have multiplied continuously:

But in short, we're hitting the limits of the current implementation. The current bhaviour is good:

  • only the right modules are loaded according to the code paths: plugins when plugins are wanted, deps when deps are wanted
  • all the right modules are unloaded according to the code paths: an app that mistakenly includes a file in a plugin but for which no dep exists will currently not compile, and it shouldn't.
  • if the paths can't safely be unloaded or changed (due to purging killing the main rebar3 path), we relax the constraints and keep running

However, in order to do this, the system is overly aggressive:

  • it always unloads all deps paths and reloads all plugin paths
  • it always purges code just in case, for every one of these modules

This is not necessary. Most dependencies of most projects never overlap with plugins, and so we could avoid loading operations most of the time. Those would allow to ensure the safety we need, but without having to be extremely aggressive about paths in ways that can cause side issues everywhere. The paths are also handled at a low level, by manually adding and removing some or all desired paths through various calls to rebar_utils functionality.

What I'm proposing is a new scheme for path handling and reloading:

  • operate in terms of target path sets:
    • base paths > dep paths > plugin paths
    • base paths > plugin paths > dep paths
  • build the target path set using these orders and the known project config with the form [{Module, Type :: dep | plugin, Path}]
  • assemble the current path set
  • if duplicate modules or apps exist in the desired set, mark them as potential conflicts in loaded/running modules: it means we have a clash between plugins and deps
  • apply the code paths in the desired order: add the required ones, remove the other ones
  • from the clashing code sets, check all loaded modules (code:is_loaded/1 or code:all_loaded/0) to see if any of them actually needs any kind of soft purging

This would make code purging a whole lot rarer, which would solve all required issues with regards to parse transforms and hooks, while keeping the paths on disk correct for include paths handlings.

The rebar utils can remain untouched, but we may want to add a higher-level module that takes care of the path ordering and management.

I have to say that currently it feels like path handling should work, but it clearly does not. I'm hoping that basically there aren't too many bugs in the logic of how we build things, it's just the over-aggressiveness and the number of low-level path juggles that causes issues. But there's no guarantee this would actually fix all issues we see right now.

@tsloughter
Copy link
Collaborator

Looks good.

The point about if duplicate modules or apps exist though I'd just limit that to apps. The only way we'd be able to check is comparing each module name or running erlang's shadow or whatever it is called function, right? Seems overboard/slow.

@ferd
Copy link
Collaborator Author

ferd commented Oct 2, 2018

Well no, the idea would be to check all loaded modules in apps that exist in apps loaded in two areas of concern (plugins vs deps), to figure out what needs to be purged and reloaded from the right path. I.e. if you have a plugin running lager, but you need its parse transform for a compile job, we have to unload only that app's modules for the correct version.

This can likely be done by taking the app's desired ebin path (highest precedence), the app's "undesirable ebin path" (the low precedence one) and checking over code:all_loaded()'s result set for modules that are in the low precedence path to purge them in favor of those in the high precedence path.

This should ensure, in case of conflict that if an app had 140 modules but only 2 of them were loaded, we only get to purge 2 of them and leave the unloaded stuff to the code paths to resolve.

@ferd
Copy link
Collaborator Author

ferd commented Oct 7, 2018

WIP branch at https://github.com/ferd/rebar3/tree/refactor-env-paths

Seems to fix bugs on most branches. I'll be reworking it and sending a PR somewhere this week probably

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants