[WIP][DX] Automatic registration of installed bundles

[gnugat]

As discussed here, this PR integrates the Wizard Bundle in the Standard Edition.

On each composer require ..., this library will:

1. check if the package is a bundle
2. convert the package name into a fully qualified classname (FCQN)
3. insert the FQCN in the AppKernel (if not already registered)

It depends on a Bundle which provides a the two following commands:

• wizard:register:bundle: registers the given FCQN in the AppKernel (useful when a bundle has been created manually)
• wizard:register:package: converts the given package name into a FCQN and registers it in the AppKernel (useful when a bundle has been unregistered, but is still installed, and needs to be enabled again)

[hacfi]

👍

Just an idea for that: We could use an extra parameter in the composer.json to make it customizable. Something like "always", "if_specified" (if the composer.json of the package contains an extra parameter like "symfony_auto_install") and "never".

[gnugat]

@hacfi: I like your idea :) . I don't think that people would add a parameter to their bundle's composer.json, but letting the possibility to add a parameter in the application's one seems reasonable

[hacfi]

While we’re at it: Should we consider adding a way to provide a default config file which gets appended to the config.yml?

[gnugat]

That would be the easier way to provide automatic configuration (we've tried many times to implement it using the Config Tree, but to no avail). The only downside of this idea is that it requires the community to update their bundles

[hacfi]

@gnugat As it’s optional I don’t think that’s a problem. Active bundles will eventually add a default config and for the other ones you have to add the config manually as you do these days.

[weaverryan]

Hey @gnugat!

I just tried the bundle commands and they worked great! I like them because (A) they work and (B) they stay out of the way (I choose to use them).

I'm less convinced about the composer plugin part, at least for now. Because:
A) It's a bit magic: it's might be a bit surprising that my code is modified
B) If something goes wrong, my Composer install will blow up in the middle. I don't like that, especially if I'm during deployment
C) If we add bundle configuration stuff later (which I hope we do, and soon), then it couldn't be included in the composer plugin, because if you're installing 5 bundles, it would be odd to configure them all at the same time (and again, there is the problem with deployment). So, we would need a command to do this. And if we're asking the user to run a command to configure a bundle, then the composer plugin hasn't really saved us the step we wanted.

So I'm +1 for the bundle but -1 for the plugin. What do others think? And also, would we need/want to move the bundle into either a sensio or symfony namespace (this is something @gnugat and I were wondering).

Once we agree, then we could continue to work on the bundle - clean up a few small things, and see about the bundle configuration command.

Thanks!

[gnugat]

Hey @weaverryan!

I'm glad the bundle suits you. If @fabpot or any symfony (core?) contributor agree with moving it to sensio or symfony, then it's ok for me :) .

[cordoval]

nice approach @gnugat however the problem starts when you have to have your bundle installed for all of this to work which is a manual step unless installing the plugin also installs the bundle. @weaverryan i don't think we do manipulation of bundles on production so not sure where would this mean problems on deployment.

[wouterj]

How does this work for bundles that depend on other bundles? For instance, the sonata admin bundle needs 4 other bundles in order to work.

[gnugat]

With the plugin, this is handled nicely by composer: it installs the highest dependencies first and fires the event on each installation (so the plugin installs each bundle in the correct order).
With the bundle, everything should be done "semi-manually".

The only case which isn't handled yet: Bundle classes which have a dependency (for e.g. almost every JMS bundle depend on the Kernel).

[wouterj]

What's the status of this PR?

[gnugat]

I'm ready to make any required changes for this to happen. What are the current blockers?

[weaverryan]

Hey @gnugat

Hmm, now I'm -1 on this. But here's why:

1. I don't want to hook into or change the Composer process. Composer is used by all of PHP, and if we hook into that process (e.g. to auto-register the bundle afterwards), it starts to blur things for beginners: they may expect this for all packages and they won't know where this magic is coming from. I've always been -1 on the Composer plugin. I want bundles to feel just like installing a library and for users to think of them as the same idea.

2. If we don't use the Composer plugin, then the only left is the ability to auto-update AppKernel. But to do that, you need to use the FQN of the bundle. And at that point, it's just as easy (and less magic) to go into AppKernel and add it yourself.

So, I'm just not clear now that there is a big enough problem worth solving. I have talked to people before that disagree with me, and say that this functionality (modifying AppKernel) should exist, and that it should also be smart enough to take into account that one bundle depends on another (e.g. HautelookAliceBundle also require DoctrineFixturesBundle), but we don't have that information currently. So, I don't see a path from here - but @gnugat please let me know what you think :).

Thanks!

[gnugat]

Hi @weaverryan,

I agree with you, only the composer plugin is worth the installation, so if in the end you choose not to use it we can close this PR.

I'd like you to know that I really understand your concerns about newcomers, but on the other hand this kind of "magic" behaviour already exists: when composer update is run the cache is updated.
Using the wizard-plugin would feel exactly the same: when composer require is run, the kernel is updated.
Also new beginners don't necessarily (need to) know about the existence of AppKernel, they won't notice that something happened and won't wonder if this kind of behaviour can happen for packages that are not bundle, or for other projects.

Finally, regarding the dependency issue, Composer installs the highest ones first, so in your example it will first install DoctrineFixturesBundle and then HautelookAliceBundle. No worries on this side ;) .

I'll soon have the time to (finally!) take your advices into account and make it way more simple, so I'm putting this PR in a WIP state, if you don't mind :) .

[weaverryan]

@gnugat hmm, nice point about the DotrineFixturesBundle and HautelookAliceBundle - that's interesting. If we did do the plugin, it would need to walk the user through gently. For example:

It looks like you're installing a Symfony bundle. Would you like the bundle to be automatically added to your AppKernel?

I'm still not sold, but maybe .... ;)

[Pierstoval]

And what about bundles that depends on many other bundles ? When you want to install SonataAdminBundle, you need to register so many bundles that it can't work if you "install it automatically from composer" ...

For this feature, maybe we should think about old issues like #598 and #608.
If we could implement something as "bundle dependencies", then registering one bundle at a time may be really great to create tiny repositories that just depends on multiple other repositories in the composer.json file, and that depends on other bundles that are instanciated "dynamically"...

By the way, it would be great to have such a feature, but it's primordial not to force the user to use this feature when it's available. In fact, if the command prompts the user for automatic bundle install if the composer dependency is a symfony-bundle one, I'd set the default answer to "no".

[gnugat]

@Pierstoval oh, I didn't know SonataAdminBundle couldn't be installed via "composer require". I'll have a look.

@weaverryan it would be interesting to know if beginners need to know about the existence of AppKernel, but I suppose we can't use something like A/B testing to find out...
Maybe the interactive question can be a nice compromise to check out how people react?

[Pierstoval]

@gnugat SoantaAdminBundle is like any other Symfony libraries, it's a package that is installed through composer, but the problem is that it depends on SonataCoreBundle and SonataBlockBundle.

And it's becoming even worse if you want the SonataPageBundle, it depends on the Admin, Core, Block, Seo, Cache, Grid bundles from Sonata, but also with FosRestBundle, JMSSerializerBundle and NelmioApiBundle. Do you see ? It depends on no less than 12 bundles, and it's strongly recommended to use a 13th bundle, SonataDoctrineOrmAdminBundle.
13 bundle dependencies. It's tough, and for a beginner, it's a great fall to hell when you need to look at the 13 documentation pages to set up all these bundles.

That's why I'd be more interested in setting up bundle dependencies instead of setting up bundle auto-registration after composer install.
Because with SonataPage, it would only register 1 of the total 14 bundles you need to register for this package :)

[gnugat]

@Pierstoval I've taken a look at SonataAdminBundle, and it says the following:

SonataAdminBundle relies on other bundles to implement some features. Besides the storage layer mentioned on step 2, there are other bundles needed for SonataAdminBundle to work:

• SonataBlockBundle
• KnpMenuBundle (Version 2.*)

These bundles are automatically downloaded by composer as a dependency of SonataAdminBundle. However, you have to enable them in your AppKernel.php, and configure them manually. Don’t forget to enable SonataAdminBundle too

As I've said above, this kind of dependency is absolutely not an issue with wizard-plugin. It is a composer plugin hooked on the "package installed" event, which means that it would proceed as follows:

1. composer downloads first the ighest dependency: KnpMenuBundle
2. once KnpMenuBundle downloaded, composer dispatches a "package installed" event
3. wizard-plugin is called and adds KnpMenuBundle in AppKernel
4. composer downloads then another highest dependency: SonataBlockBundle
5. once SonataBlockBundle downloaded, composer dispatches a "package installed" event
6. wizard-plugin is called and adds SonataBlockBundle in AppKernel
7. composer downloads then the required bundle: SonataAdminBundle
8. once SonataAdminBundle downloaded, composer dispatches a "package installed" event
9. wizard-plugin is called and adds SonataAdminBundle in AppKernel

To be fair Composer did a pretty good job here, so wizard-plugin hasn't to take care of it on its own. The bundle you install can have many dependencies, and those can have even more dependencies on their, it won't be a problem: they'll all be registered in the right order.

[Pierstoval]

Wow, then it's simply AWESOME if it works well 😄

[magnusnordlander]

I think this is a good idea, given, as @weaverryan suggested, that it asks for "permission". Better yet would probably be a y/n/? option, where the ? option shows a small piece of text on the subject of "What is the AppKernel, and why does it need to be updated?", with a link to the docs, before asking again.

Furthermore, I would suggest standardising some way in which bundles could opt-in to show a "post-install" note pointing the user to any other steps required to complete installation of the bundle.

As an example, DoctrineFixturesBundle doesn't require additional steps for installation and wouldn't show anything install notes, but LiipImagineBundle would probably add a note about requiring the user to update their routing.xml, and pointing them to documentation on the subject. The same goes for bundles with mandatory configuration.

[Pierstoval]

A "post-install" note can be very big if packages want to give many informations. imagine installing, as explained before, SonataPageBundle, it'll install more than a dozen of bundles, just imagine how dirty your console would look after installing all these packages.

A simple link to the different READMEs or a link to another documentation might sound better

The "Auto-registration" prompt should probably better be a "yes/no/all/none" , if you select "yes" or "no", it still asks for other packages, unless you answer "all" or "none", I think it's better.

[magnusnordlander]

@Pierstoval Well, terseness would be up to the package author, but unless it's really simple, a link to the appropriate section of the readme is probably best, yes. Hopefully most of the dozen bundles required by SonataPageBundle wouldn't require any install notes at all.

Also, while it is useful to look at the "worst case" scenario, the most common case is a bundle has no dependencies, or like one or two dependencies total, in which case an install note still makes sense.

[gnugat]

As you might guess, the main challenge here is to convert a Composer package name into the bundle's Fully Qualified Classname.
The wizard worked pretty well with PSR-0 because it used Composer's autoload dump, but the same trick cannot be achieve with PSR-4. As I haven't found a solution yet I think I'll just close this PR, and re open it later, if I can find something.

[hacfi]

@gnugat How about using Symfony Finder and look for a file called *Bundle.php? I look into it when I find the time.

[wouterj]

@hacfi that's basing on conventions. Bundle classes don't have to be suffixed with Bundle.

[hacfi]

Ok..wasn’t aware of that as this convention is used so often. Then the only way would be checking if any class of the .php files in the main directory (taken from composer’s PSR-4 path) extends Symfony\Component\HttpKernel\Bundle\Bundle or implements Symfony\Component\HttpKernel\Bundle\BundleInterface. For those projects which don’t have the bundle file in the main directory (example https://github.com/yohang/Finite/tree/master/src/Finite/Bundle/FiniteBundle) we could add a property to composer.json which gets read!?

[gnugat]

@hacfi good idea, I'll try that. It won't handle everything, but it's better than nothing

[hacfi]

It’s kind of the brute force attack but as you said - there is no other way. As this operation doesn’t happen too often we can ignore the performance.

[Pierstoval]

I agree @hacfi for using a composer attribute in composer.json, this is probably the best way to handle it. The only flaw is that one would need to setup his composer.json file with both the type: "symfony-bundle" and "symfony-bundle-fqcn": "Acme\Demo\Bundle\AcmeDemoBundle" . But this is the cleanest way to do it.

The problem with searching automatically is that it can be a problem for a few bundles when they have the bundle instance in their tests directory, so it looks a bit dirty to me :/

[hacfi]

@Pierstoval Good point..but we could try if excluding the directories Tests, tests and test (the common names for the tests directories.. Test is used by Twig for library code so I exclude that) gives decent results!?

[Pierstoval]

Even if it is a good idea, it can be a problem IMO. In fact, what you are expecting to be "usual" is unfortunately not "standard", and it is both non-permissive and too permissive. Too permissive because the user and developer don't have to do anything, which is great, but non-permissive because it would not allow one to use the "Test" keyword in his bundle name.

That said, you can do as you want for this feature, as it's yours :) It's just my opinion ;)