symfony1 icon indicating copy to clipboard operation
symfony1 copied to clipboard

[WIP][RFC] improve plugin loading with `symfony1-plugin` typed composer packages

Open connorhu opened this issue 4 months ago • 2 comments

For Issue #341, I looked into what is needed to put sfDoctrinePlugin in a separate package and not have it shipped with the base framework (e.g. propel orm doesn't need this plugin). I don't really like the solution of the other fos1 plugin packages. What happens there is that composer-installer 1.0 knows about symfony1-plugin and copies the installed package into the appropriate plugins directory. This is not very practical to do in the composer era. Let's take a look at what to do:

  • the sfProjectConfiguration::getAllPluginPaths method should return all packages that are of type symfony1-plugin:
          if (class_exists(Composer\InstalledVersions::class)) {
              $sf1PluginPackageNames = Composer\InstalledVersions::getInstalledPackagesByType('symfony1-plugin');
    
              foreach ($sf1PluginPackageNames as $sf1PluginPackageName) {
                  $path = Composer\InstalledVersions::getInstallPath($sf1PluginPackageName);
    
                  if ($path !== null) {
                      $pluginPaths[basename($path)] = $path;
                  }
              }
          }
    
  • we should not allow composer/installers 1.0 to be in the dependencies, because this will duplicate the plugins

This change would get rid of package codes (which don't work anyway: https://github.com/orgs/FriendsOfSymfony1/discussions/333#discussion-6326793 ), pear codes, and should be default the composer installation way of the framework and the plugins.

What other improvements could be considered?

  • support psr-4 plugins?
  • composer plugin to enable plugins (same as bundle.php).

Files to remove

  • ./package.xml.tmpl
  • ./lib/plugin/*
  • ./test/unit/plugin/*

Deprecated commands/tasks

  • plugin:add-channel
  • plugin:install
  • plugin:uninstall
  • plugin:upgrade

Deprecated methods

  • sfBaseTask::getPluginManager

connorhu avatar Mar 20 '24 15:03 connorhu

Good idea.

Another idea without composer dependency. But using ReflectionClass.

It avoids plugin duplication

we should not allow composer/installers 1.0 to be in the dependencies, because this will duplicate the plugins

If a plugin is installed with composer, it is also autoloaded. Then

class ProjectConfiguration extends sfProjectConfiguration
{
    public function setup()
    {
        $this->enablePlugins([
            \Some\Vendor\SomePluginConfiguration::class,
        ]);
    }
}

where

    public function getPluginPaths()
    {
        if (!isset($this->pluginPaths[''])) {
            $pluginPaths = $this->getAllPluginPaths();

            $this->pluginPaths[''] = [];
            foreach ($this->getPlugins() as $plugin) {
                if (isset($pluginPaths[$plugin])) {
                    $this->pluginPaths[''][] = $pluginPaths[$plugin];
                } else {
                    // START new code
                    try {
                        $pluginConfigurationClass = $plugin;
                        $pluginReflection = new ReflectionClass($pluginConfigurationClass);

                        $classFile = $pluginReflection->getFileName();

                        // to be compatible with loadPlugins()
                        // require_once '${pluginPath}/config/${pluginName}Configuration.class.php'; be noop
                        $pluginName = $this->extractBaseClassWithoutConfigurationSuffix($pluginConfigurationClass);
                        $pluginPath = $this->extractPluginDirectory($classFile);

                        $this->pluginPaths[''][] = $pluginPath;
                        $this->setPluginPath($pluginName, $pluginPath);
                    } catch (ReflectionException) {
                         throw new InvalidArgumentException(sprintf('The plugin "%s" does not exist.', $plugin));
                    }
                    // END new code
                }
            }
        }

        return $this->pluginPaths[''];
    }

alquerci avatar Mar 31 '24 19:03 alquerci

I have nothing against relying heavily on the composer. It has become a quasi-industry standard. Currently, the code relies heavily on an outdated package management system (pear). This code won't live to see composer become obsolete, so integrating it won't be a problem.

connorhu avatar Apr 04 '24 12:04 connorhu