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

give top level plugin providers precedence over default providers #1065

Merged
merged 1 commit into from
Feb 21, 2016

Conversation

tsloughter
Copy link
Collaborator

This change allows top level plugins to take the names of default providers. For example, the cuttlefish plugin, which requires control over the release building, will be {default, release} and {default, tar} in order to make this possible.

@ferd
Copy link
Collaborator

ferd commented Feb 19, 2016

I have a tentative +1. I'd like to keep maybe 1 day or 2 before merging it just to think of ways this could be bad or good. I guess we can't have tests for that either, right?

@ferd
Copy link
Collaborator

ferd commented Feb 19, 2016

What's the order of things if two top-level plugins try to take over the same command?

@tsloughter
Copy link
Collaborator Author

:( I wanted to write my cuttlefish blog post today.

Order is the order they are defined in rebar.config plugins.

@ferd
Copy link
Collaborator

ferd commented Feb 19, 2016

Don't we usually use name order for conflict resolution? Would it make sense to use that here instead so we always pick the same rules?

Also would that still be necessary with the relx hooks I saw in another PR?

@tsloughter
Copy link
Collaborator Author

I'd say no, since it is entirely up to the single rebar.config entry, so in the case like this that you can see all that can have an effect in one place i think using the order defined makes sense.

@talentdeficit
Copy link
Contributor

is the override only for the root app's context, or will redefining compile redefine it for all deps?

@tsloughter
Copy link
Collaborator Author

For all deps. There isn't a concept of providers per deps, the compile provider is the one that knows about deps, no the other way around.

@talentdeficit
Copy link
Contributor

so if i have an app that depends on a custom compile provider, it's unuseable as a dep if that compile provider doesn't also work with all my deps and the root project?

@tsloughter
Copy link
Collaborator Author

@talentdeficit yes, providers are not called for apps, they are run on the project. Only hooks may be run per-app/dep. So even if you could override {default, compile} in only that dep it wouldn't mean anything.

@ferd
Copy link
Collaborator

ferd commented Feb 19, 2016

Well it does have the interesting side effect that it breaks composability then, no? What works in an app should work when that app is a dep, and this is no longer true?

@tsloughter
Copy link
Collaborator Author

Yea, that would be the case for for hooks. Someone could have a plugin that overrides some default provider and uses it as a hook and that hook would not use the plugin provider when run as a dep :( Why must people break things!

@talentdeficit
Copy link
Contributor

would it be possible to ignore deps plugins when it comes to downloading, but use them if the root project includes the plugins just in the context of deps that declare use of that plugin?

that way plugins in deps can't break other deps or your project, but a dep that depends on a plugin is still useable if you include the plugin in your project

@tsloughter
Copy link
Collaborator Author

I'm not sure what you are saying.

@talentdeficit
Copy link
Contributor

if a dep declares a plugin and the plugin is declared in the project rebar.config, use the plugin, otherwise fall back to the built in providers (or error or skip as appropriate if it's a plugin that doesn't attempt to change providers)

@tsloughter
Copy link
Collaborator Author

Ok, new idea. The issue has been that we can't differentiate a project that is an umbrella/release and one that is an app. The former can never be a dependency. So in this patch I add the option project_plugins. These are never looked at for a dependency, only for the currently being used project.

@tsloughter
Copy link
Collaborator Author

This can also be used by any plugin a user wants to have available when developing on their app but isn't needed when being used as a dependency. I may use this for neotoma plugin... Since for that I just build the erl file from the peg and commit it and had been placing it in dev profile but that is wonky since then you have a _build/dev/ which is unneeded, you just want the erl file from src.

@talentdeficit
Copy link
Contributor

i like the idea of differentiating between a project and an app

@ferd
Copy link
Collaborator

ferd commented Feb 20, 2016

Yeah +1 on that idea. Makes a distinction between necessary and optional plugins.

@tsloughter
Copy link
Collaborator Author

so is this good to merge now then?

@ferd
Copy link
Collaborator

ferd commented Feb 21, 2016

Yeah. We'll need to update docs about that too.

ferd added a commit that referenced this pull request Feb 21, 2016
give top level plugin providers precedence over default providers
@ferd ferd merged commit ada08b8 into erlang:master Feb 21, 2016
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

Successfully merging this pull request may close these issues.

3 participants