I am trying to figure out if I should use a libraries or composer dependency management from contrib, or if I should just fallback to the easier and simpler DX of doing outdated require_once myself.

I'd like to require a component that requires symfony and guzzle (more components than Drupal 8), but this pulls those as duplicates into sites/all/vendors. I think this is undesirable to have duplicates around.

  • Will duplicate code cause performance issues?
  • Will duplicate code cause Drupal core to behave differently since it could possibly load up a different version of Symfony?
  • On the other hand, if I need guzzle/service, then is it better to have all this duplicate code spewed in sites/all/vendor?
CommentFileSizeAuthor
#39 replace-strategy-2128353-39.patch10.96 KBcpliakas
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mradcliffe’s picture

cpliakas’s picture

Composer Manager is designed to eliminate the duplicates, but with D8 this isn't flushed out yet. If the packages installed are the same version then there should be no issue, however it is pretty confusing and having duplicate code just doesn't feel right (not to mention add risk if one of the vendor directories change).

So with that being said, I think we have to brainstorm ways to make this work with Drupal in a way that manages a single vendor directory. Maybe that is overriding Drupal's core composer.json file and installed dependencies (technically hacking core), or some other solution that isn't coming to mind. Anyways, I'm all ears for ideas to attack this issue.

dmouse’s picture

A few days ago talking to some friends about this, we thought about making a plugin for composer to intercept commands.

this plugin would make a merge between composer.lock of Drupal and composer.lock of composer_manager to avoid duplicates.

http://getcomposer.org/doc/articles/plugins.md
[sample plugin to intercept command] https://github.com/dmouse/composer-plugin

to probe this plugin i am use this composer.json

{
  "name": "hechoendrupal/composer-plugin-sample",
  "description": "merge composers",
  "license": "MIT",
  "repositories": [
    {
      "type": "vcs",
      "url": "https://github.com/dmouse/composer-plugin.git"
    }
  ],
  "require": {
    "hechoendrupal/composer-drupal": "dev-master"
  }
}

composer_manager can be have to requirement this plugin.

clemens.tolboom’s picture

Interesting idea ... a drupal composer plugin. Can it hook into composer update | install adding packages?

For graphapi I'm testing locally to have at least a composer.json file placing the required libraries into graphapi/lib (so there are 'local' libs). Next manually move the files to sites all/ ... it's completely bad practice ... no dependency management :-/

My loader does

function graphapi_loader() {
  $loader = drupal_classloader();
  $loader->add('Fhaculty', DRUPAL_ROOT . '/sites/all/libraries/graph/lib');
  $loader->add('Fhaculty', DRUPAL_ROOT . '/sites/all/libraries/graph-uml/src', TRUE);

Ugly but for now workable.

cpliakas’s picture

Would / could the plugin make this module obsolete? The main goal of this module is to provide an easy bridge to packages managed by Composer, and in an ideal world it is not necessary. Curious to hear thoughts on that.

dmouse’s picture

The wonderful case in Drupal I think is what you can added requirements in the root composer.json but this file now in Drupal is it in the core and your modification is "Hack the core".
http://getcomposer.org/doc/00-intro.md#introduction
https://github.com/hechoendrupal/DrupalAppConsole/issues/27

Composer now can be install Drupal modules
https://github.com/composer/installers
https://github.com/hechoendrupal/DrupalAppConsole/blob/master/composer.j...

This composer plugin is necesary add to composer.json file like a requirement, but if not is posible change the root composer.json. composer_manager can live to create a new composer.json file and add this requirements.

-- My two cents

moshe weitzman’s picture

Feel free to dream here. Drupal 8 can still change, and especially lose the assumption that hacking composer.json is hacking core. We can treat it as editable like .htaccess and robots.txt. Hope that helps!

mradcliffe’s picture

My current solution is to suggest to users of my module

  • a) Hack core composer.json, which requires
    • Use diff or compare new core composer.json and redo dependencies by hand (multiply this by number of modules that do this).
    • Re-running composer every core update.
    • Hope that someone's not using an automated drush (re-)make build script that would choke on the above steps without a bunch of hacking
  • b) Provide a composer.json in my module anyway so that a user could use Composer Manager, which requires:
    • Run drush & composer
    • Hope that any Symfony or Guzzle updates provided by Composer Manager take effect over Core after any updates.
  • c) Don't know how to use composer or drush? Get out.
donquixote’s picture

#7 Dreaming is fun :)

What about:

An Option to let Composer generate a sites/sitename/vendor/ directory, that *replaces* core/vendor/ completely, with the core libraries + additional libraries from contrib.

The original core/vendor will still be there, but it will be ignored if there is a sites/sitename/vendor directory.

This option could be triggered by something like composer_manager, or it could become native core behavior, as soon as core detects a composer.json (or something else to define composer dependencies) in a contrib module.

This could be implemented with Composer scripts that hook into the generation process.

The nice thing: sites/sitename/vendor could be made web-writable at least for the time of the generation and during module updates. So users could run this stuff from the web UI.

mradcliffe’s picture

I think that requires hacking index.php. :-)

donquixote’s picture

Not just that. It also means we need to find out which sites/ subdirectory before we have autoload.php available.

OR we could split the autoload.php in two parts:
1. Instantiate a ClassLoader and register the core namespaces without modules and libraries.
2. Find out which sites/ subdirectory.
3. Register namespaces and stuff from sites/sitename/vendor/*. Or, if there is nothing there, fall back to core/vendor.

dmouse’s picture

The wheel has already been invented.

Composer was created to resolve dependencies and its dependencies, in the new world PHP each package has its own composer.json, if self autoload do something like this:

https://github.com/swiftmailer/swiftmailer/blob/master/composer.json

Dreaming:
Make Drupal like a library, this library can be installed in any project via composer.

https://github.com/dmouse/drupal/tree/decoupling

Create a base structure for any Drupal project this base must be provided an installer and the basic structure folder, so you only need run composer install and you are done!

https://github.com/dmouse/drupal-skelleton/blob/master/composer.json
https://github.com/dmouse/drupal-skelleton
https://drupal.org/node/2115533

For users who don't want to install it using composer could have a tar.gz with everything ready, as Symfony does

http://symfony.com/download?v=Symfony_Standard_Vendors_2.4.1.tgz

-- D

donquixote’s picture

@dmouse:
This sounds all nice, but how do you deal with multisite?

cpliakas’s picture

@dmouse, thanks for sharing.

The wheel has already been invented.

Composer was created to resolve dependencies and its dependencies, in the new world PHP each package has its own composer.json, if self autoload do something like this:

As stated in the "Philosophy" section of the project page I agree. The ideal is that Composer just works and this module isn't needed. However, that isn't the world we live in today due to Drupal's structure and current architecture.

We need to come up with specific ideas as to how module maintainers can make use of Composer in D8. That is the call to action here.

dmouse’s picture

@donquixote multisites maybe working like now, maybe you can put your multisites projects in skeleton folder.

@cpliakas so, my proposed solution using composer_manager is #3 create a plugin like https://github.com/dmouse/composer-plugin and this plugin delete the repeated packages. Composer_manager is the bridge for the root composer.json and all composer.json in each module.

donquixote’s picture

Awesome, I just found the solution!

We can use the "replace" setting in composer.json, so that packages that are already in core are not downloaded in sites/all/vendor.

We could then implement a custom AutoloaderInit class that will register the new autoload stuff to the existing core autoloader, instead of instantiating a new one.

The result is a two-step autoloader init (or 2+):

  1. Core autoload.php for core + core/vendor, at a time when Drupal does not know yet which multisite we are on, which modules are enabled, and which further composer packages beyond core/vendor are needed.
  2. Core decides which multisite we are on, and which modules to load.
  3. Optionally, we could now run something equivalent to composer install/update, if dependencies have changed since last time we checked. Of course this only works if we have a web-writable vendor directory.
  4. Composer manager can register the additional packages.
  5. Request goes on, page is served.

Imo, this stuff belongs into core.

donquixote’s picture

What we need to test is how this works if core/vendor package versions are different from versions required by modules.
If modules require a higher version than is present in core/vendor, then you have a problem - but hopefully Drupal will update its core packages from time to time.
If the core/vendor version of a package is higher than the minimum required, everything should be fine - but we still want to test it.

Once this works, core might actually want to remove some libraries out of core/vendor that are not needed by core itself (but only by core modules), to flex up package versions.
Or the other way round, core modules could then depend on composer packages which are not already present in core/vendor.

cpliakas’s picture

@dmouse, really cool idea. Starred your GitHub repository, and will likely interact with you there since I am a GitHub fanboi and find it easier to communicate there ;-)

@donquixote, still digesting your idea, and thanks for sharing the details. I want to make sure I fully understand before responding in more detail.

cpliakas’s picture

Title: Is it okay to have duplicates of symfony, guzzle, etc... in sites/all/vendors? » D8 strategy for Composer Manager

Changing title since this thread has evolved into a discussion on how this module will work with D8.

cpliakas’s picture

Experimented with a couple of things here:

@donquixote, the more I digest your idea the more I like it. I manually tampered with the composer.json file to try out the "replace" technique, and it works well! As I see it, we can simply source Drupal core's composer.lock file to build the replace property which seems pretty easy to do. Summing up my view of this technique:

Pros

  • Doesn't touch any core files
  • Still integrates in semi-frictionless manner with existing Drupal workflows
  • Facilitates a multisite architecture where different sites have different dependencies

Cons

  • Splits up the vendor directories which might be confusing (still on the fence as to whether this is a con)
  • Challenges when people require the base component, etc. "guzzle/guzzle", replace doesn't handle this and there are multiple version of the code with potentially different versions.

Anything else I missed? Overall I think this could work, and there are potentially ways we could get around bullet #2.

cpliakas’s picture

Re bullet #2 in the comment above, if any of the packages below are required

  • docrtine/doctrine
  • guzzle/guzzle
  • symfony/symfony
  • symfony-cmf/symfony-cmf
  • zendframework/zendframework

I would venture to say that guzzle/guzzle is the most probable package to be required by a module's composer.json file or one of the requirement's dependencies.

donquixote’s picture

Challenges when people require the base component, etc. "guzzle/guzzle", replace doesn't handle this and there are multiple version of the code with potentially different versions.

I don't quite understand the problem here. How can I reproduce this?
(Maybe I'm just slow)

donquixote’s picture

If this is a limitation of Composer itself, we could talk with Jordi and have it fixed upstream.

mradcliffe’s picture

The library that I depend on requires guzzle/guzzle AND symfony/symfony in its composer.json (and packagegist) so the trade-off in #20/#21 is spot-on.

cpliakas’s picture

@mradcliffe, thanks for pointing that out. I maintain the https://github.com/acquia/acquia-sdk-php stuff, so I am likely going to require a package that has guzzle/guzzle as a dependency as well.

@donquixote, I don't think this is a limitation of Composer because it is acting logically IMHO. Let me try to describe the scenario in more detail using Guzzle as a use case. First, Guzzle, like the other packages mentioned in #21, can be downloaded as one massive package (guzzle/guzzle) or you can take-only-what-you-need by requiring it's individual components. Drupal core uses the take-only-what-you-need strategy and requires guzzle/http.

Your replace idea works perfectly if a module requires the guzzle/http component, however if a module requires guzzle/guzzle then it will download the entire package as a single atomic unit which naturally includes the guzzle/http code as part of it's codebase. Since guzzle/guzzle is a monolithic package, Composer cannot break it up into it's components which results in some duplicate code.

This is a problem because if you are using \Guzzle\Service\Client, that class could be the 3.8 version whereas the class it is extending (\Guzzle\Http\Client) would be the 3.7 version shipped with core since Drupal's autoloader would be sourced first. The inter-package inconsistency would likely cause issues at some point.

donquixote’s picture

@cpliakas (#25):
https://github.com/guzzle/guzzle/blob/master/composer.json
Guzzle has a pretty long "replace" list. So the problem could be boiled down to different "replace" lists being in conflict with each other.
This can happen in a pure-Composer scenario: If you have two different packages that both replace the same thing, and you can't omit one of them, you get a conflict.

A solution on Composer level is to provide a "replaced-by" setting in addition to the "replace" setting. E.g. Guzzle can be replaced by downloading all of the sub-packages.
Or, for Guzzle, Symfony etc, the "replaced-by" + "replace" could be unified into "combines", to indicate that it replaces all its sub-packages, but does not add anything on its own.

This would allow us or Composer to split those packages up and only download the parts that are not already replaced by something else.

If we don't want to wait, and since we already know which packages we are dealing with, we could maybe implement this logic in composer manager, by preprocessing the require list.

donquixote’s picture

A quick note to throw in, remotely related:
https://github.com/composer/composer/issues/2690

HINT: DO NOT RUN COMPOSER UPDATE IN ANY AUTOMATED FASHION

This could be a problem..

donquixote’s picture

Here is a potential solution:
Look for a package "guzzle-require-all", or create it ourselves (and upload to packagist).
This package will "replace" guzzle/guzzle, but none of its sub-packages. Instead, it will have all of the sub-packages under "require".

With the old misbehavior of composer (which is now fixed afaik) of downloading packages you did not really ask for, Composer might even have decided by itself to download this custom guzzle replacement.

Now that this misbehavior is fixed (or that's what I understand), we need to explicitly allow Composer to download the guzzle-require-all, instead of guzzle/guzzle.
We don't want to add it under "require" unless we really know it is going to be needed. But we could add it under "suggest". I hope that "suggest" will be considered for "allowed" packages.

donquixote’s picture

stof in https://github.com/composer/composer/issues/2690#issuecomment-36338845 mentioned
https://github.com/dflydev/dflydev-embedded-composer which is supposed to cover exactly this use case.
I have not studied it in detail yet, but it is definitely something to look at.

donquixote’s picture

Ouch.. there may be another problem: Drush :)
If Drush decides to have its own Composer dependencies, e.g. by using a 3rd party commandline helper library, then we suddenly have even more different situations to handle:
- Drupal core standalone (on early bootstrap and web-based site installation)
- Drupal core + modules
- Drush standalone (e.g. when you "drush dl drupal").
- Drush + Drupal core (on drush-based early bootstrap and drush-based site installation)
- Drush + Drupal core + modules (in module-specific drush requests)

Nasty. isn't it?
If we follow this through, then every site in the multisite would need two vendor directories: One for modules minus core, and another for modules minus core minus drush.
And even core would need to have two vendor directories. One for core standalone, and one for core minus drush.

cpliakas’s picture

I hear your concern about Drush, and the points you made are valid IMO. In the spirit of moving forward, thought, I propose we ignore the Drush problem for now and focus on the Drupal use cases. I sit a couple of seats away from Moshe and will ping him on this thread to weigh in on his thoughts.

Regarding your idea on a "guzzle-require-all" package, that is definitely a solution however I am very reluctant to maintain additional packages even if the level-of-effort of doing so is low. As an alternative, maybe Composer Manager ships with modules, e.g. "Guzzle Integration / Helper / Extension / Some other cool name", that simply contains an empty .module file and composer.json file with the following data:

{
    "replace": {
        "guzzle/guzzle": "v3.7.1",
        "guzzle/http": "v3.7.1"
    },
    "require": {
        "guzzle/plugin": "v3.7.1",
        ...
    }
}

As a module developer, if one of your dependencies (or dependency's dependencies) is "guzzle/guzzle", then you have to add the Guzzle module as a dependency to your module's .info.yml file. I do understand this adds a little friction, but it doesn't seem like a huge barrier and we eliminate the need for maintaining separate packages while sticking with standard Composer and Drupal workflows which is the primary goal of this project. We would have to very clearly document the solution and throw some big red dsm()'s on the packages and requirements pages if the guzzle/guzzle is required and the integration module isn't enabled, but again that seems doable.

I did think about the scenario where Composer Manager is smart about it and detects when a package or one of it's dependencies requires a package like guzzle/guzzle, but the are technical hurdles in that we would have to mess with Composer's workflows and potentially implement a plugin in a separate codebase.

cpliakas’s picture

I ended up committing an experiment to the https://drupal.org/node/1929436/git-instructions/2128353-replace branch which more-or-less implements the solution above for Guzzle. If it is acceptable we can add support the other packages mentioned in #2128353-21: D8 strategy for Composer Manager.

To recap, contributed modules that have a dependency on guzzle/guzzle must depend on the guzzle_dependency module. The guzzle_dependency module implements hook_composer_file_alter() to replace the monolithic guzzle/guzzle package and instead installs all Guzzle components that make up the full guzzle/guzzle package minus the ones included in Drupal core.

The result is that we get all the classes required by the module in versions consistent with the packages in core without any duplicate code between Drupal core's vendor directory and the contributed vendor directory. Personally, I think this is a viable, sustainable option.

cpliakas’s picture

Category: Support request » Task
donquixote’s picture

Given that the list of packages in core is known and not going to change a lot, I think we can do this without the need of submodules.

cpliakas’s picture

I agree, and the 2128353-replace branch of Composer Manager actually does handle the replacement of core packages without enabling any of the submodules just like you want. For example, if in a custom module's composer.json file you require cpliakas/psolr which has a dependency on guzzle/service, then everything works as expected in that all necessary Guzzle components are downloaded minus what is included in core using versions that are compatible with core. So in case I haven't said it before, I think your "replace" idea is genius and puts us in a much better place. So thank you.

Where we get into trouble is if our module requires cpliakas/magento-client-php which has a dependency on guzzle/guzzle. We could replace guzzle/guzzle in the core Composer Manager module and add the components explicitly, however there is no to know whether a requirement has a guzzle/guzzle dependency until after composer update is run so we would have to install all of Guzzle's additional components no matter what. Therefore just by enabling composer manager you would have to download all of Guzzle, Doctrine, Symfony, Symfony CMF, and Zend Framework. That is a ton of unnecessary code if non of your modules needs these projects. I don;t want to explore a 2 step Composer process because then we are messing with how Composer works natively which is not in the spirit of this project.

The submodule approach allows modules to opt in when they require the full packages.

The advantage over maintaining wrapper packages on packagist is that the submodules are able to automatically detect which version of the packages core has installed, whereas the composer.json file of the wrapper packages would have to manually track core and have different versions depending of which version of core you were using. Therefore it does not sound reasonable or sustainable to pursue that approach.

donquixote’s picture

I was thinking of Composer script hooks.
But now that I look, I don't see any to alter the package information while it is being processed.

So maybe the module idea is not so bad :)

Alternatively, did you look at the https://github.com/dflydev/dflydev-embedded-composer dflydev stuff?

cpliakas’s picture

No, I haven't looked at it. Seem very relevant, and thanks for sharing!

As a side note I use the git-subsplit stuff from those guys on a daily basis, so I am definitely a fan.

cpliakas’s picture

After some more research, looks like doctrine/doctrine doesn't exist and symfony-cmf/symfony-cmf is simply a top-level composer.json that requires the components instead of replacing them, therefore these do not require submodules.

So it looks like we only need three submodules which isn't that bad, "Guzzle Dependency", "Symfony Dependency", and "Zend Framework Dependency".

cpliakas’s picture

Status: Active » Fixed
FileSize
10.96 KB

In the spirit of moving forward I am going to move forward with this strategy for now, definitely open to changing before Drupal 8.0 if it doesn't work out.

Posting 2 follow-up issues to address the unresolved issues in this thread:

dmouse’s picture

Great job @cpliakas!

cpliakas’s picture

Thanks @dmouse!

With that being said this is more @donquixote's idea, I was more of the code monkey that worked out some of the details.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.