| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | base system |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | infrastructure |
Issue Summary
Problem/Motivation
The current use of composer provides a simple way of building out Drupal core's dependencies. However, putting all vendor libs under core/vendor prevents contrib from extending these with additional libraries.
The current suggested method for contrib, is to put a composer.json into each module, and run them separately. However, this does not make use of the dependency resolution of composer. If module x depends on version 1.0 of lib y, and module z depends on 1.1 of lib y, the first one PHP loads wins. This makes it difficult to debug.
Proposed Solution
In order to make use of composers dependency resolution, we should:
- remove all core/vendor from git
- an additional per-project composer.json in the project root, that references Drupal's core composer.json as a dependency
- a unified /vendor directory with all of core's dependencies, plus any others from contrib, generated at packaging time on d.o or re-created locally for site-builders.
- composer install/update can be run when packaging up drupal so the download still has all the vendor libs
Site builders would add their additional library dependencies to the 'project' composer.json and let composer do the work of resolving dependencies.
Currently, site builders who use contrib modules (such as search_api_solr) need to download these external libraries manually, and typically put them in sites/all/libraries. This would create a unified method of getting external libraries that integrates with core.
Steps To Test
- Apply patch
- Remove core/vendor directory - didn't include it in patch as its currently 20MB
- Copy example.composer.json to composer.json
- Tweak (e.g. add in other dependencies)
- Run composer install / update as needed
Remaining tasks/Related Issues
- #1805316: Move composer.json to the project root
- #1867192: Upgrade testbots to PHP 5.4?
- #1683942: File syntax check will choke for modules which require PHP 5.4
- Find out where the D.o project packager code is so we could run a composer install on it when packaging
- Find out where the Testbot code is for downloading and building a project so we could composer install it before running tests
- #1923582: Add ability for testbot to run 'composer install' during installation
Comments
#1
This is a pretty simple patch, but requires some processes outside of the repo in order for this to work.
I've created an example walk-through, which is to use the solarium solr library.
Here's the steps I followed:
Here's what happened:
I ran an install of Drupal 8 and ran a few tests without issue.
I've posted a patch that shouldn't be tested, but give an idea of what I'm talking about.
#2
#3
The last submitted patch, composer-externals-1920666-1-no-test.patch, failed testing.
#4
Should have ended that patch with 'do-not-test'
#5
I like this idea in general. First change I'd make is to convert composer.json to example.composer.json, since that shouldnt be in version control.
#6
I've moved composer.json to example.composer.json as per #5.
#7
One of the guys at packagist.org just removed https://packagist.org/packages/drupal/drupal as he didn't want the drupal namespace to be hijacked.
I've re-uploaded it under kimpepper/drupal
I've also added vendor/* to example.gitignore
Kim
#8
Definitely would love to take this approach as it would mean doing it closer to right. There was a lengthy discussion over at #1475510: Remove symfony components from the core repo and let Composer manage the dependencies instead, but I think the two biggest issues are:
composer installduring compilationcomposer installwhen building Drupal core#9
Thanks for the feedback Rob. I have followed up with some comments and a link back to this issue on #1475510: Remove symfony components from the core repo and let Composer manage the dependencies instead
I am interested to know how making changes in infrastructure come about? This has obviously all been done before with distributions. Who is on the infrastructure team we can ask to chime in with their opinion? (I've tagged the issue with infrastructure already).
#10
Generally, I'm more for focusing building a new system that can handle dependencies like this easily, instead of trying to retrofit and re-invent our existing system to meet new requirements ... but it should be feasible. Given test run times, I'd be wary of anything that increases the amount of external calls and processing, though. We key on /core/vendor to filter out syntax checks, so there will be some additional impacts there too.
A change like this is not trivial from a testbot perspective ... and my available dev time is quite limited. As a result, just a heads up that implementation could potentially take a number of weeks once a firm 'go' was decided.
#11
What's the difference to #1475510: Remove symfony components from the core repo and let Composer manage the dependencies instead ? This issue looks like a 100% duplicate?
#12
@sun I agree they are related. I was approaching this as a way to have contrib modules load external libraries with the same mechanism (e.g. composer). It involves changing the way we currently use composer. The remove core/vendor from git is only a part of this.
#13
Yeah, totally different problem.
#14
Seems like those in this queue might be interested in this, but as a personal project, I'm experimenting with splitting all core modules into separate repos with `git subtree split` on a regular basis, with composer.json files and perhaps running travisci tests on individual repos.
https://github.com/drupal-fracture/split-scripts
https://github.com/drupal-fracture
#15
re: #13: Thanks for not clarifying.
1) Removing external vendor components from the core repo should be exclusively discussed in #1475510: Remove symfony components from the core repo and let Composer manage the dependencies instead
2) Moving /core/vendor into /vendor has consequences: It essentially reverts a major part of the epic #22336: Move all core Drupal files under a /core folder to improve usability and upgrades — How will users without composer-command-line-fu install or update Drupal?
3) Why do we need to touch /core/vendor at all to allow contrib to use composer? Especially since the issue title talks about additional libraries?
#16
1) good point. totally fair.
2 & 3) I think it's that it is misleading to ask people to edit `core/composer.json` and build non-core libs into `core/vendor/`. If composer is going to be able to properly do dependency resolution, `vendor/` needs to be in one place (as far as I can tell), and the repo root seems to be most sensible.
#17
Happy to continue that discussion over there. Although, this discussion assumes that they would be removed from git as well. The vendor directory is a build artifact as a result of running composer install, and not something we modify. I'll leave it at that for this discussion.
I would disagree. These are external libraries after all, and while Drupal depends on them, they are not Drupal code.
It wouldn't have to be a requirement to run composer to install external libs when downloading or upgrading Drupal. If you are downloading from the Drupal project page, the tarball would contain a pre-build vendor dir. This is the major hurdle. Having composer install run when building the drupal package on d.o.
This directory structure is also inline with a lot of other frameworks (CakePHP, Symfony2, even Rails), so it makes it easier for new Drupal developers / site-builders to understand.
The main issue is with duplicate dependencies. The main advantage of using composer is to resolve dependencies from multiple external libraries and find a compatible set that work together. Currently, we assume that is a contrib module requires an external library, it will be bundled with the contrib module, or instructions will be given to download it manually, and put in sites/all/libraries. However, if module x depends on version 1.0 of lib y, and module z depends on 1.1 of lib y, we have to include both, and unfortunately the first one PHP loads wins, and stuff is going to break.
A better example would be the AWS PHP library which depends on Guzzle. Drupal also depends on Guzzle. Which version of Guzzle should we use? Composer should work that out by looking at the version requirements of each and finding a match. We only have one copy of each external library.
This doesn't assume that this solves all our problems. There is still the likelyhood that different modules depend on different versions. However, this gives us a powerful tool for helping solve these dependency issues, that are shared (and will have help to be resolved) by the wider PHP community.
#18
@sun - I was just agreeing with Kim, because he explained it in #12. He also explains the problem quite well in the issue summary.
#19
First off, I want to say that I appreciate this issue. I think figuring out a good way for site/product/distribution builders to manage vendor libraries across core, contrib, and non-Drupal code is an excellent goal, and I appreciate the thinking that's gone into it so far.
Here's just some issues/questions to figure out how to make it work.
Right now, to update a site from 8.0 to 8.1 (once those releases are available) involves just:
- Download the 8.1 tarball.
- Remove your site's core/ directory and replace with the new core/ directory.
- Run update.php.
With this proposal, the new process would be:
- Download the 8.1 tarball.
- Remove your site's
core/directory and replace with the newcore/directory.- Remove your site's
vendor/directory and replace with the newvendor/directory.- Run update.php.
Except the above wouldn't work if the site is running any contrib or non-Drupal code that resulted in the site's vendor/ directory needing to be different than the one in the tarball (e.g., they're using AWS module that requires a newer Guzzle). For these people, is the only solution to figure out how to use Composer? Note that the majority of Drupal sites are operated by people who are not developers or system administrators. In some cases, they may be on a shared hosting environment that either doesn't provide command line access, or the website operator has not yet had to learn how to use the command line.
Well, even before tackling this issue, there's the corollary that I presume we would want core's composer.json to be more relaxed. For example, instead of depending on Symfony's HttpKernel 2.2.0-BETA2, as we do now, we'll want to relax it to something like ">2.2.0-BETA2, <2.4". However, this affects the Drupal core debugging process. For example, suppose I know that my local development site that I use for Drupal core development worked on 8.x commit foo. Then, I update to 8.x commit bar and something's not working. I want to use git bisect to figure out which commit broke it, except it doesn't work, because when I go back to 8.x commit foo, my site is still broken. WTF? Oh, it's because what actually broke my site is Symfony's HttpKernel 2.2.0-RC1, and when I switched back to 8.x commit foo, that didn't simultaneously revert by Symfony code to the same version that that Drupal core commit was originally known to work on. Maybe this is ok, because people will get used to checking for vendor-related bugs separately from Drupal bugs, but it will take some adjustment, because for a lot of Drupal developers, all the Symfony stuff is just under-the-hood magic, not something they're used to having to think about.
#20
While I agree with the rest of your post, I'd like to point out that the number of hosts that do not provide command line access is rapidly approaching zero. Even GoDaddy provides command line access these days.
#21
@effulgentsia thanks for bringing us back to the basics -- we sometimes jump ahead. That's not to say jumping ahead is bad; we just need to go back and think on how to "fix" or accommodate this stuff. I really appreciate your post. Need to think on it a bit before I comment directly
------
A bit of an aside, but I came across this Martin Fowler article recently and found this quote very relevant to all these discussions about Composer and Symfony and the future of Drupal. Thought maybe you guys would find it motivating too :)
I truly feel that to stay competitive and relevant, we have to first imagine our Drupal ecosystem without all these constraints, and then backtrack into covering ground, putting the muscle into solving problems like effulgentsia brings up. Especially now, Drupal needs all of us to start packing intellectual dynamite in our toolbelts :)
#22
It's true that they are making it available, but it is not trivial to actually turn this on in a shared hosting site. My host, for example, requires a copy of my photo ID ... if this had been a requirement before I could first evaluate Drupal a software framework, I would simply have moved on to the next product in my evaluation matrix. The point is that it's an additional barrier to entry for non-developers ... which in turn affects potential overall adoption.
#23
Yes. In that particular use case, if they were a site-builder who is working with AWS, they would need to run composer update. The current situation is they would have needed to manually download the correct version of AWS as specified somewhere in the README of the module, and unzip it into sites/all/libraries. I think this processes actually makes it easier for them. Selecting the correct version and downloading it into the correct location is all automated.
Then they wouldn't be in a position to use external libraries with the current implementation either. :-)
Totally agree. This does raise issues. However, locking down specific versions of core dependencies does not make it any easier for contrib developers to work with external libraries and their dependencies. It merely pushes the problem somewhere else.
#24
I've already had a robust debate with kim.pepper about this already, so he knows my opinions, but for this record:
Composer is awesome, but...
Personally, I feel that any requirement to run anything from command-line for normal operation of Drupal is a step too far. If you've never used it before, its very confusing and difficult. Adding a dependency on it raises the bar for maintaing Drupal sites. Sometimes, as developers, we forget that a great deal of our users not only have no development background, many of them struggle just to maintain Drupal via the UI. But, we accomodate them and make it possible for them to build sites that no other system can build.
Now, for a developer, running composer is easier than dropping a downloaded library in a folder, but for the average non-technical user the latter is something the ACTUALLY know how to do, and the former is gobbledegook.
Our packaging solution must be driven by the same requirements which have defined Drupal to date. That means
- Vendor libraries should have a non-command line method of updating
- Package dependency failures should be elegant and informative, not silent and fatal.
Split core and non: core/vendor & /vendor
Composer normally installs everything in /vendor, however this can be overridden in the config directive...
"config": {"vendor-dir": "custom/path/for/vendor"
}
Using this pattern, core libraries could be easily placed in core/vendor, while non-core would simply be placed in /vendor as usual, without having to tell anyone to add a directive to their composer.json.
Want to update a core library? This is not supported, and shouldn't be, that's why its core. If you really want to do this, you are probably a developer who understands composer and can work this out for yourself.
Run composer from Drupal
Any solution for using Composer for vendors should include a utility to run composer, similar to Drupal update (or part of Drupal update). This could well be contrib, as its only really required if you want to use external libraries which aren't in core. Modules which want to include a library can specify this module as a dependency.
If a module has declared its version to this composer module, then it could responsible for checking versions and elegantly bailing when there's a problem.
Libraries is heading in this direction anyway, however it doesn't seem to have composer support explicitly, so this looks like a new contrib.
#25
Thanks for chiming in @xtfer!
I don't think this is correct. While you can specify where the vendor directory is (e.g. /core/vendor) you can't have 2 vendor directories. The general idea is that you have a single directory for libraries, without duplicates.
Kim
#26
That idea is correct. We cannot require command line operations for normal operation of Drupal under any circumstances.
IMO, this is a feature request and should be moved to D9. We have http://drupal.org/project/composer already. For users on a command line, using drush dl to pull down a new module will also `composer install` the 3rd party dependencies for you. For users not on the command line, they can manually put those files into place.
#27
The composer module does really solve the issue. It will just create a separate vendor directory for each contrib module. In the scenario above, there will be multiple guzzles with different versions, and no clear idea of which is loaded at runtime.
#28
#19 by effulgentsia
By using Composer's
vendor-dirconfiguration ala #1805316: Move composer.json to the project root, the upgrade process stays exactly as is currently documented.#24 by xtfer
Yup! xtfer has it right. Let's move that discussion over to #1805316: Move composer.json to the project root. That patch needs a re-roll.
#10 by jthorson
Understandable... If you show us where the Testbot and project build process take place, we could have a look at what would be required. Both systems must already have Drush available, so leveraging the Composer Drush component might possibly be an option. More investigation would obviously be required.
#10 by jthorson
This is due to Testbot not understanding PHP 5.4 code, and is easily fixed by ignoring those files. We have issues for those too! #1867192: Upgrade testbots to PHP 5.4? and your #1683942: File syntax check will choke for modules which require PHP 5.4 ;-) .
#14 by patcon
Well done, Pat. I believe subtree split was something that sun was having a look at a while ago as well. I could be wrong though. That guy is always up to awesome stuff, as are you apparently.
#15 by sun
It can still live in
core/vendor: #1805316: Move composer.json to the project root#24 by xtfer
cpliakas and I have been talking about something like this. I've seen crazier things than a web interface for a console application for a web interface :-) . We should get together on IRC one day for a brainstorm/idea dump one day. Seems a bit out of scope for this discussion, however.
#24 by xtfer
This is something Libraries could handle, but figuring out what it should handle is the key. So far, I've seen a few modules that assist with the use of Composer in the contrib/module space:
#25 by kim.pepper
Although the best practice is to have one vendor directory to rule them all, it is possible to have multiple vendor directories. PHP's class loader will load the first class it finds when there are multiple installs of the same namespaces. Again, not generally recommended, but it does work. Would be nice to have it fixed though, and this issue does take steps to fix that.
#21 by patcon
+1... We have to step up and push technology forwards. We need to be leaders in the web world. "The drop is always moving"... Right?
Actionable items
This discussion touches on a lot of things. Here are a few issues that I think we should be looking at, along with a couple of things that we should find issues for:
composer installon it when packagingcomposer installit before running tests#29
I think there's too many different issues being circled here.
1) Having each contrib module that uses some composed library pull dependencies down itself is a complete non-starter, as noted above. The use of such libraries, however, I expect to only increase (which is IMO a very good thing). I don't think resolving that is something we should punt to contrib; it's core's job to resolve that.
2) Having one vendor directory for core and another for contrib is also a non-starter. As noted above, that breaks things. We shouldn't try to rely on composer figuring that out, because frankly I don't think it is something that should be figured out.
Which leads me to:
A) Yes, we need to have a core-sponsored way to handle adding composer libraries that does not conflict with core's libraries.
B) No, we don't actually need to be flexible with core's dependencies. If core decides to move from Symfony 2.3.4 to 2.3.5, core will do so and the next core update will include that. Presumably that would likely coincide with a security release in Symfony, and therefore in Drupal, too. If you really want to jump ahead, well, learn how to edit your own composer.json file.
Now here's an important observation:
1) Yes, we want composer to manage dependencies, and handle autoloading of composed libraries.
2) If you use Composer on the command line to bring them down, great, it does that all for you.
3) If your most advanced tool is FTP... How exactly do you setup your manually downloaded file to, you know, actually work and get autoloaded? I don't know. That's a good question we need to figure out.
I would also call out the "must work without CLI" statement. That's been true in the past as a goal, but it's not like we've always met it. A LOT of things in Drupal you can only do if you have shell access. Like, Features, which anyone serious about Drupal is using for at least some things, if not all. (Node types are still problematic.)
But if you just want to test drive Drupal, really, "download and stick on a $5 host" is no longer a useful way to do so. With Acquia, Pantheon, Web Enabled, and similar companies all offering dead-simple often free test drive options, the market has changed. How much, and is it enough that we can expect people not on a Drupal-centric host to have shell access? I don't know, and no one else in this thread has hard data on it either that they've cited. So "the majority of people running Drupal don't have shell access" is not a statement that has any current data to back it up.
#30
This options are known to those already informed about the community ... but the hobbyist does only what they have always known. Download, extract, execute. We've even made it easier for them by auto-creating the database tables, thus eliminating another manual step - let's not counter this improvement by adding another step in it's place unless it's absolutely necessary.
I've opened #1923582: Add ability for testbot to run 'composer install' during installation for conversation on testbot dependencies/requirements this would introduce.
#31
Updated issue summary with list of remaining/related tasks.