reset default profile app plugins to empty if at the top level #1885
Conversation
|
This isn't as important since I've realized it still doesn't allow for an override to setup compilation of all mix dependencies. But it seems to clean some cruft up. There is still an issue with if a plugin is included multiple times, like in the project and then in a dependency, and it is a checkout it will be compiled multiple times. The only way to resolve that checkout "issue" (it doesn't break anything, is just confusing and slower) I think is to do proper tracking of plugin's AppInfo. Then in And I'm fine removing this hack from the PR depending on what you think, |
| Opts = reset_hooks(rebar_state:opts(State), CurrentProfiles), | ||
| State1 = rebar_state:opts(State, Opts), | ||
|
|
||
| %% set plugins to empty since this is an app at the top level |
There was a problem hiding this comment.
I guess the implied tail of this comment is "since they have been handled in rebar3:run_aux" ?
There was a problem hiding this comment.
Yea, I can add that.
src/rebar_plugins.erl
Outdated
| StateAcc1 = handle_plugins(Profile, Plugins, StateAcc), | ||
| StateAcc1 = case Profile of | ||
| default -> | ||
| %% default profile state plugins installed in run_aux |
There was a problem hiding this comment.
for all apps or only top-level ones?
There was a problem hiding this comment.
Yea, that is what I meant by "state" instead of "app". I can make it clearer.
src/rebar_plugins.erl
Outdated
| handle_plugins(Profile, Plugins, StateAcc) | ||
| %% hack to keep {add, [{plugins, [<plugin>]}]} from attempting | ||
| %% to install the added plugin to itself, resulting in a never | ||
| %% ending list of plugins to install |
There was a problem hiding this comment.
might be interesting to add a warning as well. This would be the only functionality that does this, but it might be good to let people know "this is starting to be dangerous territory" and maybe they can pick which apps they add the plugin to instead.
There was a problem hiding this comment.
This is being removed, I realized we already are tracking plugin apps. So I can ensure we only do each plugin once instead of doing this.
top level default profile plugins are installed in the run_aux function. This commit removes the plugins from the plugin opts of an application that is also at the top level and skips the default profile when installing top level plugins in project_apps_install so they aren't handled twice. Additionally, before handling a plugin the state's list of known plugin apps is checked and the plugin is skipped if it has already been handled.
tsloughter
force-pushed
the
sub_plugins
branch
from
65dd384
to
f83f68a
Compare
top level default profile plugins are installed in the run_aux
function. This commit removes the plugins from the plugin opts
of an application that is also at the top level and skips the
default profile when installing top level plugins in
project_apps_install so they aren't handled twice.