contao / manager-plugin

Contao Manager Plugin
GNU Lesser General Public License v3.0
4 stars 9 forks source link

[RFC] Load plugins on composer install/update #9

Closed aschempp closed 6 years ago

aschempp commented 6 years ago

This implementation stores the current plugins inside the PluginLoader class, similar to ocramius/package-versions.

There are multiple major advantages to that:

  1. The PluginLoader no longer needs to be initialized by "guessing" the position of the installed.json
  2. There will no longer be a bundles.map file in the cache that needs to be rebuild on every container build
  3. The PluginLoader becomes independent of the kernel, which means the contao-api can also use plugins to perform functionality
  4. Making contao/manager-plugin a Composer plugin enables use to add composer-related event handling in the future without needing to add command to the root composer.json (e.g. https://github.com/contao/core-bundle/issues/1223)

There is only one disadvantage I can find: Because the file is overwritten, Composer will complain about git differences on a develop version.

@contao/developers please give your general opinion so I can finish this (unit tests etc.)

aschempp commented 6 years ago

What about dumping it to a file that is .gitignore'd? Would that change anything? I like the concept though :)

That could only work by creating a new class, which does not exist if the Composer plugin never runs. Not a good idea because Composer plugins can be disabled.

ausi commented 6 years ago

There is only one disadvantage I can find: Because the file is overwritten, Composer will complain about git differences on a develop version.

Does ocramius/package-versions have the same problem? And isn’t there a better place to store generated PHP files?

aschempp commented 6 years ago

Does ocramius/package-versions have the same problem?

Absolutely.

And isn’t there a better place to store generated PHP files?

I can't think of any, because you want to have fallback and class autoloading. Apparently ocramius didn't think of any better either 😉

leofeyer commented 6 years ago

Because the file is overwritten, Composer will complain about git differences on a develop version.

I don't think so, because the vendor folder is excluded by default.

ausi commented 6 years ago

And isn’t there a better place to store generated PHP files?

I can't think of any, because you want to have fallback and class autoloading. Apparently ocramius didn't think of any better either 😉

Looks like Composer stores all generated files at the same nesting level as the package folders. E.g. vendor/composer/installed.json or vendor/bin/some-binary. Following this structure the generated class would be saved at vendor/contao/ManagerPluginLoader.php. But this would cause problems with autoloading and the file would not be uninstalled together with the package.

So I think the current location is correct ☺️.

leofeyer commented 6 years ago

So I think the current location is correct

Me too.

aschempp commented 6 years ago

Because the file is overwritten, Composer will complain about git differences on a develop version.

I don't think so, because the vendor folder is excluded by default.

I'm not talking about your local git, but if the plugin is installed from source in the vendor folder.

leofeyer commented 6 years ago

Since the changes did not work (broken unit tests, incomplete PHP objects instead of bundle objects), I have reverted them in the 2.2 branch. The master branch still has them so we can fix the issues in the next version.