Major Providers Refactoring about rebar3 HOT 87 CLOSED

This sounds like a good evolution of providers to me; my gut says that this work belongs with the providers.

from rebar3.

Cool. So init could be cut down to:

init() ->
    [{name, ?PROVIDER},
     {module, ?MODULE},
     {bare, false},
     {deps, ?DEPS},
     {example, "rebar erlydtl <subtask>"},
     {short_desc, ""},
     {desc, ""},
     {subproviders, [rebar_prv_erlydtl_compiler]},
     {opts, []}].

or we could move the definition of the provider record to providers.hrl and have init return the record. But I don't really like not wrapping records in a module.

from rebar3.

And could get rid of the need for modifying State in the providers module by changing do/1 to do(RawArgs, ParsedArgs, State). Hm...

from rebar3.

Here's an alternative approach. Providers can define an optional namespace they operate in. You have a single provider that behaves like do, but it's a generic one with a hidden name.

Default rebar stuff would put in nothing (and would be wrapped by do). LFE stuff would work within a lfe namespace, elixir in the ex namespace for example.

So when I call rebar3 lfe compile and there is no provider attached to the lfe name in the default namespace, we look up the lfe namespace, and check for the compile provider (letting you have compile in many namespaces!) and run that one instead.

This requires little to no change in provider support (just new options), and allows a nearly unlimited amount of namespaces for custom projects and languages without requiring central coordination from us.

What do you think?

from rebar3.

Oh and a sweet thing this could enable is a python-like 'future' namespace. Want to try a new compile option for default rebar3 in Erlang? rebar3 future compile and it loads a different compile option that people can use and try.

from rebar3.

That could work. Will need a way of defining namespaces such that someone can include the plugin lfe and it's namespace and providers under that namespace get loaded.

from rebar3.

To be more precise, (and I'll try to build a demo test thing ASAP):
in https://github.com/rebar/rebar3/blob/master/src/rebar_core.erl#L39-L46, first we look at the command and try to find it in the default namespace. If it's not found, we use the command as a namespace identifier.

do maps to default, lfe maps to lfe, ex maps to ex, etc. so that plugins can define their namespace and it automatically creates the equivalent do command for them.

https://github.com/tsloughter/providers/blob/master/src/providers.erl#L29 would probably need a namespace option there
https://github.com/tsloughter/providers/blob/master/src/providers.erl#L166-L167 a version of this function could also use a namespace when matching, or have the current one redirect to a default search.

With this stuff in place, we could probably extend the current do to work and take many aliases as plugins decide to define them.

If I have time tomorrow, I'll try to get a proof of concept going.

from rebar3.

Oh, wow. This is the most idiomatic Erlang suggestion I've heard so far.

Questions:

• How amenable would this be to multiple namespace support? (I'm guessing we'd just have to make the namespace a list instead of an atom ...).
• This would allow LFE, for example, to use an Elixir test runner when calling rebar3 lfe tests (as long as the Elixir provider registered itself for both lfe and ex namespaces).
• Hrm, what about preventing a provider from running in a namespace? If Joxa declares a test runner that works for Elixir and LFE, but LFE wants to use a different test runner ...
• A provider could also register itself for multiple tasks, yes? So rebar3 lfe compile and rebar3 lfe tests are registered by a provider ... but what if one is good, and the community likes it, but others don't like the second (e.g., the tests) provider. How would one prevent over-zealous projects from grabbing tasks in a namespace?

Also: bonus points for minimizing the changes :-)

from rebar3.

In my own view, a provider is registered to a single namespace, but namespace creation is dynamic. As soon as a provider declares it belongs to a given namespace, that namespace is created.

I think trying to reuse the same provider for multiple namespaces makes madness lie this way because dependency chains and sequences of calls will not be the same, and debugging it will probably be a nightmare.

I don't think this is flexibility that is needed nearly as much as initial laziness (I'll just reuse this pile of code blindly). If code needs to be shared, extract it to a third-party library and then declare independent providers that depend on it. Again, this is my view so far.

With that simple mapping, your second, third, and fourth questions are now a non-issue. If you don't want a provider, don't install it as a plugin. Problem solved.

from rebar3.

There's also gonna be the tricky way of how to declare dependencies on steps that ran in a different or local namespace. Currently we use Name and {Profile, Name} to specify dependencies. The new form might force us to consider Name, {Profile, Name}, and {Namespace, Profile, Name}, where Name expands to {default, Name}, and {Profile, Name} expands to {CurrentNamespace, Profile, Name}.

from rebar3.

Actually, we can get rid of {Profile, Name}. After we switching to using a list of profiles that combine into a single State it is no longer needed or used. I simply haven't removed it from the providers app yet.

from rebar3.

So it would be just {Namespace, Name} if we need it.

from rebar3.

Cool, that's even nicer then.

from rebar3.

Definitely agree with the therein-lies-madness. I started having flashbacks
to graph-traversals to calculate dependency chains from code 10 years ago.
Would gladly support a party line of "we're not going down that path in
order to protect you from yourself" :-)

On Sat, Dec 20, 2014 at 9:44 PM, Fred Hebert [email protected]
wrote:

In my own view, a provider is registered to a single namespace, but
namespace creation is dynamic. As soon as a provider declares the
namespace, it is created.

I think trying to reuse the same provider for multiple namespaces makes
madness lie this way because dependency chains and sequences of calls will
not be the same, and debugging it will probably be a nightmare.

I don't think this is flexibility that is needed nearly as much as
initial laziness (I'll just reuse this pile of code blindly). If code needs
to be shared, extract it to a third-party library and then declare
independent providers that depend on it. Again, this is my view so far.

With that simple mapping, your second, third, and fourth questions are now
a non-issue. If you don't want a provider, don't install it as a plugin.
Problem solved.

—
Reply to this email directly or view it on GitHub
#69 (comment).

from rebar3.

what about the currently valid rebar clean compile test? does that just not work anymore?

also what does just rebar ex or rebar lfe do? (they should probably do rebar ex help and rebar lfe help respectively)

from rebar3.

I think by default a command with subcommands should register the default command to run when none given, just like make does.

one question, how to specify commands with subcommands inside "meta commands"

something like

rebar do (lfe compile) (lfe test)

or

rebar do lfe:compile lfe:test

from rebar3.

@talentdeficit this is currently supported by do and would remain so. rebar3 do clean compile test.

@marianoguerra my proposal would make lfe act like do within the lfe namespace. Therefore rebar3 lfe compile test would work the same for lfe as rebar3 do compile test does for regular rebar3 Erlang providers.

from rebar3.

It is rebar3 do clean, compile, test

The commas are necessary because each task has its own arguments. Which does need a fix because right now it simply uses string:tokens and breaks if an arg takes a string like, --desc "some, strong".

from rebar3.

Demo ready in #70

from rebar3.

what about a mix like syntax for subproviders? then the following would all be valid:

$ rebar clean compile eunit
$ rebar compile ct --suite rebar_state_SUITE, rebar_core_SUITE, rebar_prv_common_test_SUITE clean
$ rebar lfe.compile lfe.test
$ rebar lfe
$ rebar ex.compile --jobs 10 ex.test --seed "entropy" hex.publish

from rebar3.

why do you want to compile before running eunit? You don't and shouldn't need to anymore, ever. This was always a terribly shitty experience of rebar IMO. Commands know their dependencies.

Not sure what the current format for the second command is, but this should be supported with 'do'.

For the fourth one, this would be 'rebar3 lfe compile test' under my proposal.

The fifth form (mixing namespaces in a single run) wouldn't be supported under my proposal, but would be rebar3 ex compile --jobs 10, test --seed "entropy" && rebar3 hex publish I believe? (@tsloughter can correct me), but one could create a set of 'workflow' providers that can manually order cross-namespaces in their deps.

from rebar3.

@talentdeficit the issue is it doesn't make clear what args go with what command if you don't use commas between the commands. You could say that any args for compile has to come before ct but that complicates parsing and is unclear to a user.

As for use of . I just don't like it :)

from rebar3.

i'm ok with either syntax, i was just offering an alternate that would not be a breaking change

from rebar3.

@ferd it would be rebar3 lfe compile, test, right?

from rebar3.

i did a github search and there's way less instances of rebar foo bar than i expected so breaking backwards compatibility in this case is probably a nonissue

from rebar3.

@tsloughter right.

@talentdeficit thanks for the search, that's good to know.

from rebar3.

Namespaces have been merged in and updated the erlydtl provider for it https://github.com/rebar/rebar3/blob/master/src/rebar_prv_erlydtl_compiler.erl

I think now we need a good way to include a multiple providers via a plugin. Right now it is 1:1 for plugins and providers. We'll want the user to be able to specify the lfe plugin and all of its providers get registered into rebar3.

from rebar3.

Ah yeah. I'm guessing that's different from a multi-provider like having them submit their own do, but would be more about having a quick way to represent a namespace's entire set of plugins as one single dependency?

from rebar3.

Yea, it may just be a function each plugin has to export so rebar3 can saying Plugin:providers() and get a list of the provider modules to init.

from rebar3.

@tsloughter how bad would it be if say the lfe people made a help provider in the lfe namespace, and that one specified all other lfe plugins as dependencies (or as plugins of its own) ?

from rebar3.

On Sun, Dec 21, 2014 at 2:48 PM, Tristan Sloughter <[email protected]

wrote:

Namespaces have been merged in and updated the erlydtl provider for it
https://github.com/rebar/rebar3/blob/master/src/rebar_prv_erlydtl_compiler.erl

Woo-hoo!

I think now we need a good way to include a multiple providers via a
plugin. Right now it is 1:1 for plugins and providers. We'll want the user
to be able to specify the lfe plugin and all of its providers get
registered into rebar3.

+1 :-)

—
Reply to this email directly or view it on GitHub
#69 (comment).

from rebar3.

A bit late to the game here, but the namespace implementation Fred came up with sounds like a pretty good way forward.

What I am curious about is how I'd go about creating "global" plugins that do not care about things like compile, etc. I have been working on one plugin that packages releases and ships them to S3 for future deploys. That could technically work with lfe, erl and ex, but I can't see how that would fit.

rebar lfe compile, release, ship

Unless if a command is not found in the namespaces it goes back and checks the default namespace. That sounds like it could be a can of worms (also wtf why isn't there a can of worms emoji on here?).

from rebar3.

Yeah right now this can't work with global plugins. Providers are scoped to only one namespace. You would need to have a way to register a different ship for each namespace you have.

Another solution is to provide your ship_global_provider in the default namespace, and then ship ship_lfe_provider which depends on {deps, [{default, ship}]} and does nothing itself. Empty wrappers.

I'm guessing we can find smarter ideas if this really becomes a need that happens a lot and we have so many namespaces it would be insane to ship wrappers every time.

from rebar3.

Did we decide how a plugin would register providers? Either simply have Plugin:providers() exported and rebar3 will run that to get a list of atoms or require .app to have {env, [{providers, []}]} and then do application:load(Plugin) to get the list of provider atoms.

from rebar3.

The app file would probably be the proper place to add that metadata, but the env file lets us just load the app and read the config, allowing for overrides and configuration. So to me the question is therefore whether we actually want people to use and depend on the env as a way to manage plugin providers or we'd prefer they don't? Flexibility's cool, but strictness tends to avoid worlds of hurt.

from rebar3.

The env file? Do you mean that if we put it in .app under env it can technically be overridden?

from rebar3.

I might be missing something... so forgive this potential naivety: why
wouldn't we put this in the rebar.config file?

On Tue, Dec 23, 2014 at 12:01 PM, Fred Hebert [email protected]
wrote:

The app file would probably be the proper place to add that metadata, but
the env file lets us just load the app and read the config, allowing for
overrides and configuration. So to me the question is therefore whether we
actually want people to use and depend on the env as a way to manage plugin
providers or we'd prefer they don't? Flexibility's cool, but strictness
tends to avoid worlds of hurt.

—
Reply to this email directly or view it on GitHub
#69 (comment).

from rebar3.

@oubiwann each provider has to be registered. A plugin like lfe has many providers. A user shouldn't have to put every provider in their rebar.config but only like {lfe, {git, "https://github.com/lfe/rebar3_lfe.git", {branch, "master"}}}. Then rebar3 has to get lfe compile and such somehow.

from rebar3.

@tsloughter yeah that's what I meant. the env tuple can be loaded and overriden. If you read it from the .app file itself, then you don't need to put it in env unless you want to risk inconsistencies when people modify or override the env.

@oubiwann Take a look at https://github.com/rebar/rebar3/blob/master/src/rebar.app.src#L25-L45 for an example of how a single plugin could register multiple providers. Rebar3 does it that way, we could extend that to other plugins (say 'lfe') which could then be able to provide many commands as one package.

from rebar3.

On top of this I guess we should keep the existing mechanism of not having to declare anything at all working, because it's pretty damn nifty and useful.

from rebar3.

What do you mean not having to declare anything at all?

from rebar3.

https://bitbucket.org/ferd/rebar3-todo-plugin/src/612fd81ebad0344572398c796f0ae2222f887d6f/src/provider_todo.app.src?at=master this is a currently valid plugin with no provider declared in the app file.

from rebar3.

Right, but this is impossible if a plugin is able to have more than 1 provider. Right now it simply assumes the plugin name and the provider name are the same.

from rebar3.

Yeah. But we have many options:

1. App name same as provider name
2. Provider names in .src.app structure
3. Read all modules from {modules, ...} in app file and find those that respect the behaviour interface

We can use one or more of these as we see fit. Currently we only implement the first one, but we could very well do 2 => 1, or 2 => 3.

from rebar3.

I think we'd have to choose (1) and (2) or (3) not all 3.

If providers is found in .app use it, if not try simply initing a module with the same name as the app.

from rebar3.

Yeah that sounds good to me.
On Dec 23, 2014 9:50 PM, "Tristan Sloughter" [email protected]
wrote:

I think we'd have to choose (1) and (2) or (3) not all 3.

If providers is found in .app use it, if not try simply initing a module
with the same name as the app.

Reply to this email directly or view it on GitHub
#69 (comment).

from rebar3.

Cool, I'll probably put that together tomorrow.

from rebar3.

@oubiwann if you you want to give it a try I've merged in the change so it loads providers that are defined in the plugins env, like https://github.com/tsloughter/dummy/blob/master/src/dummy.app.src#L10

from rebar3.

hi! I'm trying to adapt my plugin to the latest changes (using nightly rebar3) and I can't get it to work like this::

 ./rebar3 efene compile

here is the quickstart I'm trying to write: http://efene.github.io/docs/quickstart.html
here is the plugin: https://github.com/efene/efene_rebar3_plugin

what am I doing it wrong? are all this changes on the last nightly?

from rebar3.

I'll give this a try shortly, it looks correct.

from rebar3.

Ah, I see the issue. The deps, https://github.com/efene/efene_rebar3_plugin/blob/master/src/efene_rebar3_plugin.erl#L9, are taken to be within the efene namespace.

I think don't support {Namespace, Provider} in the deps list yet and will need to add it now.

from rebar3.

Wait, maybe we do :), try

-define(DEPS, [{default, install_deps}, {default, app_discovery}, {default, compile}]).

from rebar3.

Yeah that's likely what you need to do. I made namespaces relative to the caller. If efene namespaced providers depend on non-efene stuff, they have to be explicit about it.

from rebar3.

I namespaced the deps but still doesn't work, if I run

./rebar3 efene compile

I get this error:

➜  myfnapp  ./rebar3 efene compile
===> Due to a filelib bug in Erlang 17.1 it is recommendedyou update to a newer release.
===> Command efene not found
➜  myfnapp  ./rebar3 efene        
===> Due to a filelib bug in Erlang 17.1 it is recommendedyou update to a newer release.
===> Command efene not found

does the filelib bug have anything to do with the issue?

if I run ./rebar3 help the output is:

...
clean           Remove compiled beam files from apps.
compile         Compile apps .app.src and .erl files.
compile         efene rebar3 plugin
....

so it seems it's not detecting the namespace of my command

should I put in the name parameter on init {?NAMESPACE, ?PROVIDER} instead of just ?PROVIDER ? or it means that the namespace handling of providers is wrong?

from rebar3.

if I run ./rebar3 compile, to see what happens since the two are registered under that name I get:

===> Uncaught error in rebar_core. Run with DEBUG=1 to see stacktrace
===> Uncaught error: {'EXIT',
                            {{badrecord,provider},
                             [{providers,process_deps,3,
                                  [{file,"src/providers.erl"},{line,191}]},
                              {providers,process_dep,2,
                                  [{file,"src/providers.erl"},{line,205}]},
                              {lists,foldl,3,
                                  [{file,"lists.erl"},{line,1261}]},
                              {providers,process_deps,3,
                                  [{file,"src/providers.erl"},{line,196}]},
                              {providers,'-process_deps/2-fun-0-',2,
                                  [{file,"src/providers.erl"},{line,179}]},
                              {lists,flatmap,2,
                                  [{file,"lists.erl"},{line,1248}]},
                              {providers,process_deps,2,
                                  [{file,"src/providers.erl"},{line,178}]},
                              {rebar_core,process_command,2,
                                  [{file,"src/rebar_core.erl"},
                                   {line,38}]}]}}

from rebar3.

Are you sure you have the latest rebar3 and providers? That command should only match the compile under default namespace.

from rebar3.

Ah, so I just tried downloading the "nightly" build and using it to make sure. And that fails as you have above. Are you using that?

from rebar3.

I tried and it works fine for me from the master plugin.

Your problem is in the config file in the template. You use "[email protected]:efene/efene_rebar3_plugin.git". Try with "https://github.com/efene/efene_rebar3_plugin.git" instead and the plugin will be fetched correctly and work better I believe.

We should update the tutorial to use that URI form to make sure it reduces confusion too.

from rebar3.

^CC @marianoguerra

from rebar3.

I really don't think that is it. I tried with the download binary of rebar3 and I get the same thing he does.

from rebar3.

I'm saying the problem would be on the plugin download itself, not the rebar3 download. I added a config file by hand before applying the template and used the https:// version of the plugin URL and it worked first try for me.

from rebar3.

Weird, changing the config does not work for me with the downloaded rebar3. Only works with my local rebar3 built from master.

from rebar3.

yes, I'm downloading rebar3 with:

wget https://s3.amazonaws.com/rebar3/rebar3

should I try building it from source?

from rebar3.

yep, building from sources work.

maybe the nightly's job isn't running or something like that?

from rebar3.

hi, now I'm at it again trying to make a efene ct plugin whose only task is to compile efene code on test/ and then delegate to rebar3's own ct task.

is there a way to call another task explicitly from a plugin at a point that is not before? (that would be solved by putting the other task in the dependencies of this task).

from rebar3.

We'll need tests to make sure this is still working and doesn't break, but there is an element of provider called hooks that takes a tuple of 2 lists, by default: {[], []}. The first list is providers to run before and the second is to run after. So you should be able to add to your provider list of attributes: {hooks, {[], [ct]}}

from rebar3.

should I namespace it with default in my case that I'm using another namespace?

from rebar3.

Yea

from rebar3.

I think you should "resolve" tuples to provider records in this line for it to work?

https://github.com/tsloughter/providers/blob/master/src/providers.erl#L86

given how it's called here:

https://github.com/tsloughter/providers/blob/master/src/providers.erl#L93

from rebar3.

yep, getting the provider like this on my plugin and adding DefaultCTProvider to the post hooks works

  Providers = rebar_state:providers(State),                                  
  DefaultCTProvider = providers:get_provider({default, ct}, Providers),

but I think that should be done on the link provided above (or on rebar3 since providers are in state and afaik providers module doesn't know about rebar_state)

from rebar3.

Ah yup, I'll patch that soon.

from rebar3.

@marianoguerra get_provider should work with providers:get_provider(ct, Providers) just fine. That is what you were asking for, right? https://github.com/tsloughter/providers/blob/master/src/providers.erl#L204

I've also got a patch I'll get out today that replaces profile with profiles in providers.

from rebar3.

Can see the profiles change #96

from rebar3.

@tsloughter you mean I shoud do

{hooks, {[], [providers:get_provider({default, ct}, Providers)]}}

instead of

{hooks, {[], [{default, ct}]}}

on my code or that should be done on rebar's side?

from rebar3.

Oh, it isn't resolving them but instead expects the hook list to be of the actual Provider record? That should change if that is the case. I look at that.

from rebar3.

@tsloughter yes, I think that's happening, see the links here #69 (comment)

from rebar3.

So... this turned out to be a pain.

To get a provider from the name it needs the list of providers. While this is contained in the State variable we can't access that from providers since it is rebar_state and we can't call to rebar from providers.

So neither do/2 or run_all/2 can resolve the atom to a provider...

Not sure how to handle this yet...

from rebar3.

Couldn't rebar_hooks take care of expanding that stuff? I'm guessing ideally you wanted to resolve them whenever the provider is loaded in memory or something?

The best option would be to internally expand them when calling providers:get_target_providers(...) If we fear people may use mixed forms, we'd need an internal flag to note when hooks have been 'normalized' or something, or always do it dynamically.

This is not particularly different from being able to define deps of a provider by its {Namespace, Name} combo, so fixing it in providers directly would probably be good for the library's consistency.

(edited to change the reference for a function)

from rebar3.

rebar_hooks doesn't use providers.

We can only use get_target_providers if the list of providers is available. If you look at create and do in providers you'll see that that isn't available. It "is" in the sense that it is in State but we can't call rebar_state:providers from providers.

from rebar3.

I thought get_target_providers is how we could constrain the list of providers we have to run one we know about. It gets a list of providers passed to it, and it should be possible to have a pass over the list of all providers.

Say after process_deps is being called.

get_target_providers(Target, Providers, Namespace) ->
    TargetProviders = lists:filter(fun(#provider{name=T, namespace=NS})
                                         when T =:= Target, NS =:= Namespace ->
                                           true;
                                      (_) ->
                                           false
                                   end, Providers),
    expand_hooks(process_deps(TargetProviders, Providers), Providers).

I'm not exactly sure what can't work there?

from rebar3.

Oh, that will work. It does mean that a provider that has only been created but not going through expand_hooks will fail to run its hooks though. But maybe that is an acceptable restriction for providers to have.

from rebar3.

Ok, mostly got this working. But a new question. Resolving the deps of the hook provider?

Example: We have provider A with hook = {[B], []} and provider B has deps = [C]. Does a run of A become: [C, B, A] or just [B, A], I assume the former. If correct then the complications come if A also defines deps = [C]. Does this mean a single run of C? Or 2? Currently if A has B as a hook and a dep it will run B twice, so that would lead me to say C is run twice.

Thoughts?

from rebar3.

I'd probably go for [B,A] at first and readvise if there's ever a need for more. If A depends on C and has a pre-hook B, then it should be [C,B,A] and we at least have a way to workaround hooks issues if it ever comes to that by just adding them to A.

from rebar3.

I find that weird because it is basically saying that using a provider as a hook is only supported if the provider doesn't rely on any deps.

from rebar3.

Yes. I'm assuming the provider in the hook would have similar requirements to the currently running one, otherwise why call for the hook? Do we expect some kind of do_everything provider that calls to ct, compile, lock, and then new just because?

I'm thinking that if you're writing a pre-processor of some kind, you may depend on install_deps and want to have compile run after you.

I'm just trying hard to think of a good use case where someone goes "I have this here plugin that does something and then calls release but didn't bother compiling files or installing deps first" and I kind of come short on it.

from rebar3.

Does this issue need to stay open?

from rebar3.

I need to update my plugin to the latest version and check if everything works ok, you can close it and I can reopen it or open a new one if you want.

from rebar3.