Start using $root/vendor for non-core dependencies only

Keep $root/core/vendor, and ensure that $root/vendor only contains non-core dependencies.

What happens if a contrib/custom module requires one of the core dependencies, but with a higher version requirement ?

If I get things right on the approach:
- on composer install, the package code remains in $root/core/vendor but gets bumped to a higher version,
- and then on next "tarball download" of core, it gets overwritten by the lower version again, and the module is broken until you re-run composer install ?

I.e same problem than mentioned at the biginning of the IS ?

Sorry if this is re-hashing discussions that already took place, but it does seem like allowing tarball downloads on an install where you have already leveraged composer is a can of worms, not sure why we want to support it.

If you have used composer already, it means you can, which means you should, and then possibly it means we should prevent you from doing otherwise ?

What happens if a contrib/custom module requires one of the core dependencies, but with a higher version requirement ?

I think it would be easily possible if we keep /autoload.php. That way people could opt out of the "dual composer approach" completely, use the subtree split and manage everything in one composer file.

OK, so I applied the patch and did this:

cd core
./vendor/bin/phpunit

...and everything works as expected.

Then I did this:

cd DRUPAL_ROOT
composer require crell/api-problem

Unfortunately this leads to reinstalling all of core in vendor/ and ignoring core/vendor.

Also autoload.php is still broken which leads to an error when trying to run phpunit, but that's mentioned above and a little out of scope here.

Since Composer bends over backwards to be rigid in its uses, we'll have to write about as much code as embedded composer. In particular:

+++ b/core/lib/Drupal/Core/Composer/Composer.php
@@ -36,4 +46,78 @@ public static function preAutoloadDump(Event $event) {
+  public static function createInternalRepository() {
+    $package_repository = new InstalledArrayRepository([new CompletePackage('drupal/core', '8.0.0.0', '8.0.0')]);
+    $vendor_repository = new InstalledFilesystemRepository(new JsonFile('core/vendor/composer/installed.json'));
+    $repository = new CompositeRepository([$package_repository, $vendor_repository]);
+
+    return $repository;
+  }

We can't just fake out composer with a version number of 8.0.0.0, because installed.json tells another story. We have to find drupal/core's composer.json on the file system, load it, attach it to the installed repo, etc.

So basically that code has to be replaced with an overloading of various factory methods like those in https://github.com/dflydev/dflydev-embedded-composer/blob/master/src/Dfl... We can probably just subclass that and get some benefit.

Once we do that, we might as well allow for the same sort of flexibility that's present in embedded composer, such that we could dynamically find and get the status of dependencies, eg, for the Drupal status page.

Use autoload.php to register both autoloaders.

I think that's unnecessary. We can just require both generated autoload.php files (core/vendor/autoload.php and vendor/autoload.php) at the top of index.php and any other front controllers. One line fixes everything. :-) In fact, that would potentialy resolve the version question yched mentioned; if we register the vendor/autoload.php file first (ie, the contrib one), and a class appears in both places, the contrib one wins. That is, probably, what we want since we can assume that's the newer version. If that breaks due to incompatible versions, then it would break no matter what we do and that's an unresolvable problem.

If that breaks due to incompatible versions, then it would break no matter what we do and that's an unresolvable problem.

The question is whether we want to tell people their chosen combination of modules will not work during installation, or whether we want to let things break in such a way that people have to debug PHP errors. By not resolving dependencies and collisions beforehand there is also the chance that people will run into problems long after they have first built their site, because after a year they start using previously unused features that happen to conflict with the other code they have. We don't want to go to great lengths to make things easy for people who aren't tech-savvy and then end up showing cryptic technical errors.

Note that rerunning "composer update' would redownload the missing dependencies into $root/vendor, but that is still potentially too fragile.

It's fragile indeed. However, if you've already installed a module using Composer, you're technically able to run composer update afterwards (you already have the tools and the skills). On top of that, as mentioned before, there is always the chance that core's dependencies are not compatible with those of contrib (core is compatible with two major versions of a dependency and ships with the lower one, while contrib is only compatible with the higher version) and not running composer update then after a core update will break your site.
TLDR; If you've done composer require once, AFAIK you're stuck with composer update forever.

I think some of the discussion here might be overthinking things. Surely if core is incompatible with a Drupal 8 module's dependencies then that module is incompatible with core. If contrib uses a later version than core but core is compatible then everything is good - right?

The point is that no matter people say as long as we provide a tarball people are going to do crazy things. Saying

TLDR; If you've done composer require once, AFAIK you're stuck with composer update forever.

Is all well and good but we have absolutely no way to enforce this.

alexpott: I think more to the point, it's saying "if you try to mix and match workflows you're on your own and we don't support it, if you break something well we told you so."

Is all well and good but we have absolutely no way to enforce this.

alexpott: I think more to the point, it's saying "if you try to mix and match workflows you're on your own and we don't support it, if you break something well we told you so."

What Crell said. If you adopt a certain workflow not used by core by default, you'll have to remember to use (or deprecate) that workflow yourself. There are many other such situations where manual intervention is needed and that's worked fine so far.

What Crell said. If you adopt a certain workflow not used by core by default

This begs the central question which remains unanswered:

What *is* core's default workflow?

The patch here doesn't work (not that it never would of course), but I think we need to define the behavior before anything productive can happen.

Some behaviors are defined and tested for here: #2002304: [META] Improve Drupal's use of Composer

Unfortunately no one seems to have held anyone's feet to the fire on the composer bof at DrupalCon LA, and so we don't have much documentation on what was decided there, or why.

Given that, I'm back to the spirit of #2432893: Make Compser DrupalWTF Go Away and think we should have one vendor directory and one composer.json file at the root level. If we can't get it together enough to decide on a behavior, we won't ever get it together to create and maintain a subtree split, much less adopt the complexity of a module-by-module embedded composer solution. Thus we shouldn't try. Too many person-hours (thinking especially of mine, but others as well) have been wasted on this, so let's punt.

For 8.1.x we can then iterate on what people are clamoring for.

If you can't do this:

$ cd DRUPAL_ROOT
$ composer require crell/api-problem
$ ./vendor/bin/phpunit
# No errors or fails.

...then things are fundamentally broken.

The patch from @Bojanz works really well, been looking at how we can add two autoloaders, my only issue is it's causing conflicts because there are two vendor/composer/autoload_real.php files

Fatal error: Cannot redeclare composerRequireDrupal8() (previously declared in /home/vagrant/Code/d8/core/vendor/composer/autoload_real.php:56) in /home/vagrant/Code/d8/vendor/composer/autoload_real.php on line 50

The issue I was having related to both composer.json files having the same autoloader-suffix, I have updated this in a new patch and auto updated autoloader.php

So the final issue now is to get both autoload.php's added, just putting:

require_once 'core/vendor/autoload.php';
require_once 'vendor/autoload.php';

Would do the trick, but as it get's passed into DrupalKernel() I've no idea how it'd work.

@crell is pointing me towards #composer

This issue seems incomplete or point me to another issue if there is one, but we need to figure out how to avoid putting vendor into the docroot at all, it would seem?

Ok, so to restate the problem again:

Composer wants us to have one vendor directory.

Drupal core's needs tell us to package Drupal as a tarball with it's own private vendor directory managed by core developers only. core/vendor/

Some contrib modules need external libraries.

Contrib wants us to require the use of composer to manage these external libraries (which is wholly reasonable, btw).

This issue attempts to leverage embedded composer techniques to have two vendor directories, so that core is satisfied by its own core/vendor/ directory, and contrib modules can then store their dependencies in a root-level vendor/ directory.

This is so that beginner-level people's Drupal sites won't stop working when they remove the top-level vendor/ directory during a tarball update or whatever.

Unfortunately, this issue suffers from lack of a defined behavior and a way to demonstrate that this behavior is implemented.

Also, the patch we have adds is_readable() to every request, which was rejected here for performance reasons: #2406681: Add an autoload.php in the repo root to control the autoloader of front controllers

And in a larger scope, we're basically at a stalemate of needs because we don't have a defined behavior that we want.

I say break some of these expectations, reduce complexity, use a single top-level vendor directory, and educate people about how composer works. #2380389: Use a single vendor directory in the root

Then, after release, we can iterate against *actual* needs. (Prediction: Once people learn how to use composer create-project to install Drupal it will become the de facto standard way of installing Drupal, and composer update will be the preferred way to update Drupal core.)

personally I'm in favor of closing this issue. I'm not sure how it will effectively work having two vendor directories and goes against composer and all other php projects.

I think the d.o packager and testbot should put the vendor directory in the core folder. It makes things neat and allows those to host on ftp to just overwrite the core folder when upgrading.

We should then allow for the vendor directory to be in the root or outside the docroot. This could be done by updating the autoload.php in the root directory or maybe we could introduce a variable that stores the vendor directory path.

Would love to hear some thoughts or feedback.

Here are some related issues
#2380389: Use a single vendor directory in the root
#2002304: [META] Improve Drupal's use of Composer
#1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead

See related issue - we should move vendor out of core and out of the webroot. #2508591: vendor/ is web accessible