contao / manager-bundle

[READ-ONLY] Contao Manager Bundle
GNU Lesser General Public License v3.0
17 stars 10 forks source link

system/initialize.php file is wrong #19

Closed qzminski closed 7 years ago

qzminski commented 7 years ago

The system/initialize.php file is wrong which causes failure of legacy extensions that depend on it. The file created by core-bundle triggers the AppKernel which does not exist, it should trigger ContaoKernel isntead.

Related issues: https://github.com/isotope/core/issues/1771 https://github.com/codefog/contao-news_categories/issues/98

aschempp commented 7 years ago

@leofeyer how do we solve this? I can see 2 options:

  1. Overwrite the file generated by contao:install
  2. Improve contao:install command in core so it's extendable, and overwrite it in the manager-bundle to actually setup everything.

I would prefer no. 2 because contao:install is actually reasonable to setup the managed edition anyway...

leofeyer commented 7 years ago

This problem is specific to how the managed edition has been designed. Things would be a lot easier if both editions would use the same kernel:

class AppKernel extends Kernel
{
    public function registerBundles()
    {
        return ManagerBundle::autoloadBundles();
    }

    // …
}

As I have said before, the managed edition and the standard edition should be based on the same structure, except

I don't like any of the current "magic", such as a bundle that builds the application, a hidden app kernel or hidden configuration files, which make it difficult to debug things.

leofeyer commented 7 years ago

I already have a working POC of a managed edition, which is based on the structure of the standard edition. And it is working pretty well.

The POC is based on the convention that the config files, the app kernel and the PHP entry points are generated and can be overwritten at any time. The only exception is a userconfig.yml file.

And the app kernel uses the bundle autoloader in its registerBundles() method.

@contao/developers I would really like to discuss the design of the managed edition before the release of Contao 4.4. I am pretty sure that we could drop the standard edition entirely if we would set up the managed edition differently.

discordier commented 7 years ago

I am open for discussing this. Shall we use a regular mumble call or a dedicated one?

ausi commented 7 years ago

The next Contao call is tomorrow, should we discuss it there?

leofeyer commented 7 years ago

As discussed in Mumble on February 23rd, @aschempp is going to create a PR extending the contao:install command so a matching initialize.php file can be installed.

A decision on restructuring the managed edition has not been made. Since the managed edition requires a different application configuration than the unmanaged edition, the files cannot simply be exchanged. Also, we want to wait for the changes with Symfony Flex and see whether they solve any of our problems.

aschempp commented 7 years ago

see #20

leofeyer commented 7 years ago

Thank you @aschempp.

qzminski commented 7 years ago

I think I have to open this issue. If the initialize.php does not exist and you go to the Install Tool, the file is created with the "default" content which does not work for managed edition. I suspect that's because of the fact that the listener is executed on console.terminate:

https://github.com/contao/manager-bundle/blob/master/src/Resources/config/listener.yml#L8

… which is probably not executed in the install tool:

https://github.com/contao/installation-bundle/blob/master/src/Controller/InstallationController.php#L141

leofeyer commented 7 years ago

I'm not sure how to solve this though. We cannot trigger the console.terminate event in the installation controller without side effects, can we?

leofeyer commented 7 years ago

Fixed in e77b433b3a5eb2e2f12f56a39f895d7be0ca892a.