Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

[FrameworkBundle] bundle:install / remove commands #6082

Open
Seldaek opened this Issue · 37 comments
@Seldaek

I have been thinking about this for a while and I think the best way to have bundles added to the kernel etc when you install one from composer is to do this from the symfony console.

The main issue is that if it's done with a composer hook/script, it'll fire at random when you install an old bundle, or if you upgrade a bundle it will remove/install it if it's installed as a zip. Lots of issues.

On the other hand, a bundle:install command could take a package name, install it via composer, and once installed prompt you if it should be enabled for all envs or only dev/test, and prompt if it should register a config/routing/.. automatically. Similarly bundle:remove would clean up the config, routing and whatnot, instead of leaving users with all that dirty work to do.

The downside is it probably means having composer as a dependency of the command.

@WouterJ

I think it is better to add this in the SensioGeneratorBundle, as it is just a command that makes life easier.

You can use the KernelManipulator class of the SensioGeneratorBundle. The KernelManipulator::addBundle($bundle) command will check if it already exists, if so it will throw an exception which we can catch and ignore. Otherwise it will register the bundle.

The bundle:install command will be a lot more usefull, I think. I don't think we should require composer as an option for this command. What about checking if composer is installed: If so, it will use composer otherwise it assumes that you have downloaded and installed it in the vendor directory and you can use the handy things like register config/routing/... It will look something like this:

$ php app/console bundle:install
...
bundle: FOSUserBundle:2.x
bundle: KnpMenuBundle:1.x
....
register config? yes
register routing? yes
...

Install the bundles

$ php app/console bundle:install --no-composer
...
bundle: FOSUserBundle:2.x
...
register config? yes
register routing? yes

$ php app/console bundle:install --bundles 'FOSUserBundle:2.x KnpMenuBundle:1.x'
@Seldaek

Yeah the advantage of requiring it goes through composer is that you can then use the package name to look up the bundle name in the vendor dir. Unfortunately there is no guaranteed direct mapping between package name and bundle name.

@bamarni

I don't see what config could be automatically added for a bundle, what do you mean by register config?

To me it's a great idea but I'm +1 for using composer package name and only registering the bundle in AppKernel, the rest should be done by the user. This would be a really useful command.

As per the remove command, you might be interested by this issue, your feature would solve it by providing a more generic way : symfony/symfony-standard#412, looking a the discussion we can see it's not so easy to implement.

@Seldaek
@stof
Collaborator

@Seldaek bundlename: ~ is useless since some 2.0PRx release (don't remember which one). The DI extension of the bundle is always called if it exists.

@Seldaek
@rafacouto

Here is a command to a possible bundle:install command: http://stackoverflow.com/questions/13082254/is-there-a-way-to-update-automatically-the-appkernel-in-symfony2 (Touki's courtesy :)

@WouterJ

You can do this easier with the use of Sesios KernelManipulator. This class is already used to register a bundle when creating a bundle with the generate:bundle command.

@rafacouto

@WouterJ, you are right. However KernelManipulator is in the SensioGeneratorBundle.

What do you think is better to implement this new command: in SensioGeneratorBundle or in SymfonyFrameworkBundle or wherever?

@WouterJ

@rafacouo, as I said in the first comment. I think this does not belong to the Symfony core and I think the best option is to place this in the SensioGeneratorBundle, because you can then use the KernelManipulator.

@jakzal
Collaborator

@WouterJ SensioGeneratorBundle provides commands to generate various stuff. bundle:install doesn't belong there. Just because SensioGeneratorBundle already contains a piece of code we could use, doesn't mean this is the right place for any command which needs mentioned code. In this specific case it would rather mean that the existing code is in the wrong place.

@gnugat

Here's a project which has commands that enable a bundle (adds it to the AppKernel): https://github.com/gnugat/GnugatWizardBundle

It depends on the SensioGeneratorBundle's KernelManipulator, and should soon be able to be fired by the composer require command. The roadmap also includes automatic configuration and bundle removal (but removal is far more complex than installation).

@Peter-Rene-Menges

Thanks to Wouter for both referencing the issue that I created and mentioning this issue to me.
As is stated, we got to get rid of the static code registration of bundles in Symfony.
It's disgusting that this got to be done programmatically. It's an configuration issue to register bundles, not an implementation detail of the Kernel. The Addon from Wouter to register additional configurations from that bundle is perfect. Imagine some sort of bundle using annotated Entities or Routes. These could be both easily imported and further steps as creating the entities in the database could be performed, as well as provided service implementations by the bundle.

@Peter-Rene-Menges

But keep in mind, that the kernel will be splitted into komponents, #9406.
So the the Bundle - Komponent would communicated with the kernel using the registry pattern I suggest where the Kernel can look up the installed and in - use Bundles. This registry could implement a cache for metadata like namespaces and the command(s) we are discussing actually would manipulate this cache.

@stof
Collaborator

@Peter-Rene-Menges Automatic routing importing is a bad idea: the order of routes is important, and projects may want to use a different prefix when importing the routes (or to import only some of the bundle routes). the URLs should stay under the control of the project IMO

@Peter-Rene-Menges

Good point, though I would expect a well - behaved bundle to define it's own extension point in the routing system by registering a unique path and not using paths underneath the root path directly. Still, to keep that extension point configure - able to the client , this would be an extension to the command we haven't discussed yet.

@WouterJ

The prefix @stof is talking about is not about naming, it's about URL prefixes. Imagine I installed the SonataAdminBundle and it automatically adds a /admin prefix when registering routes. After that, you'll never be able to change it to /administrator.

Also, when automatically enabling bundles. I'm not sure if I like that. I want some bundles to be only available in certain environments, how would this command be able to do that?
Some bundles needs configuration in order to work, how are we dealing with that?

Some parts of the framework can't just be enabled, and registering bundles, routing and config are three of those imo.

@Peter-Rene-Menges

Yes, and that's why I stated: First, a well -behaved bundle named SonataAdminBundle should register it's path under /sonata/admin ,
Second: The subcommand that I mentioned given the prefix by configuration as:
Under which context -path the Controler - Routes should be installed [/admin]? /administration
e.g.
The problem arising from the first - machting -route wins situation can be overcome by providing more parsing whilst importing routes and analyzing the route - tree, equal path's raise collisions obviously and should therefore handeld by the command in an pre - analysis step.

@WouterJ

To me, it's a lot of code changing/additions for such a simple task by hand... Doing it yourself gives you more freedom, flexibility and gives you the ability to do things different than standard. E.g. when using the FosUserBundle, you do not want to load all roads, only login and registration routes are enough for a simple system.

@Peter-Rene-Menges

Yes , it is perfectly well a lot of code changes. But modifying the Kernel of a Framework just to add components is just inacceptable.

@stof
Collaborator

@Peter-Rene-Menges It is not the kernel of the framework. It is the kernel of your app. This is why it lives in app/, not in vendor/symfony

@Peter-Rene-Menges

So it is still a component of the framework as I didn't write it but it was provided by the framework.

@WouterJ

No, it's not provided by the framework. It's the kernel of your application. It extends a class from the core, but almost all classes in your application do that.

The AppKernel is created for project specific things. It determines things like the file structure (where do config files live, where should the cache and log file be placed, etc.) and it determines which bundles should be registered. And a lot of other stuff, like the charset.

@stof
Collaborator

but almost all classes in your application do that.

@WouterJ I hope it is not the case. The classes implementing the business logic should not be extending the framework classes.
However, I agree with you that AppKernel is part of the app. It is there to configure the app-specific parts of the kernel.

@WouterJ

@WouterJ I hope it is not the case. The classes implementing the business logic should not be extending the framework classes.

I somewhat overdid that statement :)

@gnugat

Since august 2013 (once a month, during SensioLabs's hackdays), a small bulk of colleagues and I have been working on this problematic, so I wanted to give you a heads up on this subject.

We create a composer plugin which:

  • listens to the POST_PACKAGE_INSTALL event (when you run composer require ...)
  • checks if it's a symfony2 bundle (package type symfony-bundle)
  • converts the package name into a fully qualified namespace (using the vendor/composer/autoload_namespaces.php file)
  • inserts a new line in the app/AppKernel.php file

At first we created a bundle with a command allowing you to either:

  • register a bundle, given a fully qualified classname (useful for bundle created manually)
  • register a bundle, given a composer package name (useful when the composer plugin didn't exist)

One does not simply handle the automatic configuration: many of us hit a brick wall when trying to retrieve from the ConfigurationTree the parameters which were required and not null and without a default value. Also, some parameters can only be detected as required when using some services (think about the secret parameter).
In the end this part has been discarded. The best way to solve it would be to have a configuration file in each bundles which describes those parameters, but would every bundle maintainer follow this?

You can have a look at the actual work here: the Symfony2 bundle and the composer plugin.

@jakzal
Collaborator

One does not simply handle the automatic configuration

Although, I'm not sure if that's a good idea in general, you could probably use config:dump-ref, no?

@stof
Collaborator

@jakzal the issue is that you cannot use the command itself, as it would require the bundle to be registered first, which will prevent the booting if you don't set the required parameters.

So you would have to build the DI extension of the bundle without registering it and using the config dumper on its configuration.

@weaverryan
Collaborator

I like this idea! (btw, can someone give this a DX label?).

We should start putting this somewhere in "core" (i.e. something that ships with the SE)? SensioGeneratorBundle, SensioDistributionBundle or FrameworkBundle. I'd really like to see this for 2.6, so that when we're updating bundle docs around the ecosystem, we can talk about this process.

Regarding the configuration, I agree with @gnugat that there should be a way for each bundle to optionally hook into the installation process and craft the questions that should be asked. I don't think trying to use the Configuration will be a good use experience. For example, with FOSUserBundle, you might ask which "db abstraction layer is being used" and then change the questions/processes after that based on the answer.

Thanks @gnat42! I'm really excited about this :)

@gnugat

Well in the end I feel that the whole code should be put in a plugin composer. The bundle is useless: it only provides a Command which is used in rare cases (you created a bundle manually).

I guess we could create a"script handler" like IncenteevParameterHandler, but I found composer plugins to be much easier: to install them you just run composer require, and to remove them you just need to remove one line in your composer.json :) .

I guess we should ask each contributor if they're ok with changing the copyright to extract the code to the core (or maybe we could directly use the repo and rename it?).

@pgodel

Sounds to me that we would need to create some sort of interface between the "bundle installer system" and the bundles, so the system can retrieve information from a bundle before enabling it and based on that generate the required configuration by the bundle.

This would mean that bundles would need to be updated to work with this system, so not all bundles would be fully installable/configurable, but this appears to be the case today anyway.

We need to work towards having a real and modern pluggable bundle system. Installing bundles by hand works, but it is tedious, not user friendly, and in my opinion, for a modern framework, not acceptable.

@nicolas-grekas nicolas-grekas added the DX label
@weaverryan
Collaborator

Let's decide where this should live and make that official so work can continue there. It needs to ship with the SE, but imo it could certainly be its own library (though one of the Sensio bundles seem appropriate too).

My vote would be SensioDistributionBundle: this is an optional thing that helps assemble your project.

And +1 for Pablo about an interface for the bundle to talk to the installer. We can work on that when we know where to work, but it should happen quickly so that we can update the most important bundles to support this (again, ideally for 2.6).

Thanks!

@gnugat

Let's decide what to choose beforehand ;) . Do we really need the Command? Or do we only want the Composer Plugin?
This is important to know because the composer plugin should stay in its own repo (it's package type must be composer-plugin and therefore cannot live in a symfony-bundle package).

I'd vote for the plugin only.

@gnugat gnugat referenced this issue in symfony/symfony-standard
Closed

[WIP][DX] Automatic registration of installed bundles #695

@gnugat

@weaverryan I've created a PR on the SE. Let's move this discussion there :)

@aferrandini

IMO we could dispatch an event like BundleInstalled with properties (InputInterface and OuputInterface) and have some listeners inside the bundles in order to execute some questions to get the bundle configured.
This way, delegates the configuration of the bundle into the bundle.

:+1: using SensioDistributionBundle For this purpose.

@stof
Collaborator

@aferrandini having the bundle register listeners on the event requires that the bundle can be registered without any configuration and booted. But the case being hard for DX is precisely when bundles have some mandatory configuration

@javiereguiluz javiereguiluz referenced this issue in sensiolabs/SensioGeneratorBundle
Closed

add a remove bundle #201

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.