Problem/Motivation

Site Maintainers ought to be able to utilize custom dependencies in their Drupal projects.
These dependencies could be:

  1. Composer package such as Guzzle OAuth Subscriber
  2. Drupal module such as Insert Field.
  3. Drupal theme such as Omega
  4. Drupal site such as davidwbarratt

All of these things (plus anything else that can be managed with Composer) should be able to be managed withing a Drupal Project, as if it were any other PHP project. Since Composer handles dependencies recursively, any of the above mentioned dependencies could have dependencies of their own and these will be managed within the same instance of Composer.

Since Drupal's composer.json file is in the root directory (which is far better than it being in the core directory), the question has been raised: Is it acceptable to modify Drupal's composer.json file?

This is an important question, however, it appears that the Drupal best practice of never modifying core still remains true.

In naderman & RobLoach's presentation at DrupalCon Munich, it was recommended that Drupal core become a dependency of Drupal. This would allow end-users to modify Drupal's composer.json file, while at the same time maintaining Drupal core's own composer.json file that would not be touched.

Proposed resolution

The patch in #115 does the following:

  1. Essentially revert #1805316: Move composer.json to the project root
  2. Create a new composer.json file in the project root
  3. #2372815: [meta] Make the <root>/composer.json file a working example
  4. Document how to use Composer with Drupal (i.e. modify index.php and change vendor directory)

This allows a power user to manage a complete Drupal installation of core, modules, and PHP libraries with Composer from code distributed outside of drupal.org repositories or drupal.org's packaging system. This solves the use case for dev ops. However it does not attempt to solve the issue of allowing a drupal.org package user to manage custom module or theme dependencies. A drupal.org package user who attempts to mix work flows may end up with a broken site or frustrating user experience.

Resolution Documentation

The <root>/composer.json name will be:
drupal/drupal

The core/composer.json name will be:
drupal/core

The <root>/composer.json file would only have one requirement: drupal/core and the type would change to project

Pros

  1. Users can use Composer without modifying a core file.
  2. Drupal Core is a dependency of the Drupal project
  3. Drupal's dependencies remain in-tact.
  4. Composer users can exclude core and vendor directories from their repositories
  5. Allows users to manage contrib modules via Composer (like #1886820: Packagist for Drupal Projects)
  6. Only requires changes to two json files.
  7. Does not require rewritting all current patches
  8. Does not require any changes to the packaging script
  9. Allows the core/vendor directory to remain in Drupal's repositories
  10. Changes should be transparent to all users, minor changes for core maintainers.

Cons

  1. Disruption: Drupal hosts may want/need to re-work their hosting platforms for Drupal 8 (to take advantage of this). This is also a benefit?
  2. <root>/composer.json file is a non-working example until #2372815: [meta] Make the <root>/composer.json file a working example is resolved
  3. Executing composer install from the root of the project would delete the core directory and add it back. Any changes to the folder would be destoryed.
  4. Composer users will need to modify index.php in order to use Composer with Drupal.

Remaining tasks

  1. Test the most recent patch
  2. #2372815: [meta] Make the <root>/composer.json file a working example
  3. Document how to use Composer with Drupal (i.e. modify index.php and change vendor directory)

Testing

You can test the most recent patch by:

  1. Modify <root>/composer.json.
    1. Add this repository:
      {
        "type": "vcs",
        "url": "git@github.com:davidbarratt/drupal-core.git"
      }

      Not necessary when/if #2372815: [meta] Make the <root>/composer.json file a working example lands.

    2. Require this custom installer
      "davidbarratt/custom-installer": "~1.0"

      Not necessary when/if #2372815: [meta] Make the <root>/composer.json file a working example lands.

    3. Add the necessary config for the custom installer in extra
      "custom-installer": {
        "drupal-core": "core"
      }

      Not necessary when/if #2372815: [meta] Make the <root>/composer.json file a working example lands.

    For your conveince I've compiled all of these changes into this composer.json file.

  2. Modify index.php
    Change this line from:
    $autoloader = require_once __DIR__ . '/core/vendor/autoload.php';

    to:

    $autoloader = require_once __DIR__ . '/vendor/autoload.php';

    Not necessary when/if #2380389: Change index.php to use a vendor directory in the root lands.

You can test composer create-project by executing the following command:

composer create-project --repository-url="http://davidwbarratt.com" drupal/drupal path/ dev-master

repository-url and the version are not necessary when/if #2372815: [meta] Make the <root>/composer.json file a working example lands, in that scenario you'll be able to run:

composer create-project drupal/drupal path/

which is the same instructions Symfony uses as it's primary download method.

API changes

Core contributors will need to change their working directory to /core before executing a composer command. However, this is the same as it was before #1805316: Move composer.json to the project root.

Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Task because it is adding something to drupal.
Issue priority Major because it changes how drupal uses composer.
Prioritized changes Prioritized usability improvement.
Disruption The disruption will be minimal. The only change for users and developers is that they can edit the composer.json file and add extra libraries to their drupal project. There will be no api changes.

Conclusion: This is a prioritized usability improvement with a minimal disruption, so it is allowed in the beta.

Comments

davidwbarratt’s picture

Category:feature» bug

Marking this issue as a bug report since it's impossible for module developers to use composer.json without breaking Drupal's best practices.

Thanks!

RobLoach’s picture

Title:Make Drupal Core a Dependency of Drupal» Introduce a new core/composer.json file

We moved the composer.json to the project root, so this would be a new composer.json file at core/composer.json, it seems.

We'll need to add the "drupal-core" type to Composer Installers at https://github.com/composer/installers/blob/master/src/Composer/Installe... .

So, it might look something like this:

Root Composer.json

name: "drupal/drupal"
type: Nothing

core/composer.json

name: "drupal/drupal-core"
type: "drupal-core"

davidwbarratt’s picture

Why would drupal core be something other than a library? Isn't that what Drupal core is?

Thanks!

Seldaek’s picture

The only reason to have a type:drupal-core is to allow having a custom installer that will install the code in core/ instead of vendor/drupal/drupal-core.

While I can understand why you'd want to do that for familiarity reasons, I still think it's a bad idea. It adds complexity and an unnecessary custom installer to the mix.

If you're gonna have a vendor dir anyway then you might as well tell people in your migration guide that the core dir is now under vendor/drupal/core (I'd also advocate using just drupal/core as package name, no need to repeat yourself IMO).

davidwbarratt’s picture

Good idea!

I updated the recommended core project name.

I think putting Drupal in the vendor directory is a good idea, but I'm not sure if anyone would have any objection to that, but I can't think of why that would cause a problem.

Thanks!

RobLoach’s picture

Status:Active» Needs review
StatusFileSize
new1.76 KB
PASSED: [[SimpleTest]]: [MySQL] 57,119 pass(es).
[ View ]

Sounds like a great idea, there are a lot of things that would need to change in order to have core path-independent. In the mean time, this change takes a step in the right direction. Mimics Symfony's use.

RobLoach’s picture

Issue tags:+composer
davidwbarratt’s picture

I looked at your patch @RobLoach. I like the direction it's going, but I think that could get messy, because what happens if someone modifies the composer.json file in root and then runs `composer update`? As far as I can tell, that's going to override whatever is in the core folder.

I think ideally what would happen is that the packages listed on the root level is just drupal... or maybe (at first) it's nothing at all. Just an empty file ready for users/modules to customize. I guess then we need to decide where we want 3rd party vendor libraries to be installed. We could just create a custom installer to put them in core/vendor I suppose.

This is similar to the way Symfony does it. If you look at the Symfony composer.json file, it lists out completly different packages then the Symfony Standard composer.json file:
https://github.com/symfony/symfony/blob/master/composer.json
https://github.com/symfony/symfony-standard/blob/master/composer.json

So basically everything that Drupal requires should be in core/composer.json. Which would effectively leave composer.json blank.

if we do list out "drupal/core", I think, in our custom installer we could also run or just ignore core's composer.json at first (since it's in the repository already).

I think, we shouldn't really rely on using Composer Installers to install Drupal core (since there is only a single project that will take that type). I think it would be better to add a custom installer that will put it in the right folder.

so then it would look like this:

root composer.json
name: "drupal/drupal"
type: "drupal"

core/composer.json
name: "drupal/core"
type: "library"

However, if we can't override what happens to "drupal/core" on install (with a custom installer). I'd be in favor of changing the type, but not just to use composer installers (for a single project). If there was going to be more than one project with that type then I'd be in favor of it, but that doesn't seem to be the case.

For reference, I discussed this here:
https://github.com/composer/installers/pull/38

Thanks!

davidwbarratt’s picture

Priority:Normal» Critical

I'm going to make this critical, because currently in Drupal 8 there is no standard way for contrib developers to work with dependencies other than the ones Drupal has provided in core. This is a major issue if contrib developers are to mimic the way core modules are created. Without this being resolved some how contrib authors are forced to develop modules in the exact same "Drupal-way" as Drupal 7.

Thanks!

cosmicdreams’s picture

+++ b/composer.jsonundefined
@@ -29,6 +29,9 @@
     "vendor-dir": "core/vendor",

Wouldn't this become a problem if you ran composer update?

Wouldn't it attempt to install drupal into the core/vendor folder?

catch’s picture

Status:Needs review» Postponed (maintainer needs more info)

Can someone explain why this is:

1. A release blocker?

2. A bug report rather than a task?

It's not clear from the issue summary exactly what use-case currently isn't possible, looks to me like it's the ability to put arbitrary dependencies in core/vendor but wouldn't it be possible to ship a composer.json file with a contrib/custom module and have a vendor directory there instead?

davidwbarratt’s picture

@cosmicdreams,

I'm not sure I quit understand what you are saying. Are you asking about the proposed solution or what is currently in Drupal?

Thanks!

davidwbarratt’s picture

Status:Postponed (maintainer needs more info)» Active

@catch,

Let me see if I can clear some things up a bit.

The use-case is a developer who wants to utilize 3rd-party libraries, much in the same way Drupal core has done. There are several current solutions in Drupal 7 to do this, Libraries API is by far the most popular. However, it doesn't really manage dependencies all that well, or rather, not as well as Composer, which I assume is why Drupal core uses Composer.

Drupal 8 utilizes Composer to manage it's dependencies. This is an amazing step forward. Using a dependency manager not only makes it easy to update 3rd-party libraries, but it also prevents version conflicts between dependencies.

If you are familiar with composer, you'll know that a project has a "root" composer.json file, and every dependency has it's own composer.json file. You are correct, that a module should have it's own composer.json file with it's own dependencies. However, according to the way Composer works, I would need to list that module in the root composer.json file. Once I listed it in the root composer file, I could run `composer install` and all of the module's dependencies would be installed into the root vendor directory.

Let's say though, that we don't want site builders to modify composer.json, then, in your scenario, the site builder would have to `cd` into every module directory and run `composer install`. However, this presents a huge problem because the dependencies are no longer being managed across projects. This makes version conflicts a real possibility. Also, most site builders are not going to go through the hassle of doing that to update the dependencies of each and every module in a project, which introduces security concerns.

Ideally, Drupal Core should be a dependency of Drupal. We would be saying "Drupal requires Drupal Core". That way there is only 1 item in the root composer.json file which would be "drupal/core". Just like .htaccess and .gitignore, the composer.json file should be able to be modified by site builders and modules.

There is a stop gap in contrib called Composer Manager. This module looks through all of the modules for a composer.json file and generates a "root" composer.json file. I'm not sure what the plan is for this module though and how it will work with Drupal 8's composer.json file without causing versioning conflicts. For instance, what if someone requires a different version of guzzle? etc. Composer is already made to handle these kinds of conflicts, but right now, it doesn't look like that is the strategy that Drupal is taking.

So I think that we need to 1) Determine the best way for developer's and site builders to utilize composer.json. I believe this includes some fairly major reorganizing of the project, however, maybe it does not. and 2) Communicate that preferred method to developers and site builders.

I made this a release blocking issue, because depending on the solution to the problem, it ought to be figured out before telling developers how to use 3rd-party libraries.

I made this a bug rather than a task, because right now, there is NO way to properly manage 3rd-party libraries in Drupal contrib. Without a fix of some kind (which may just be documentation), then contrib developers are blocked from doing this properly.

I hope this answers most of your questions.

Thanks!

catch’s picture

Category:bug» task

OK that's a task - it needs to be figured out but there's no functional bug. But I'm OK leaving this at critical to attract some more discussion - it does need to be sorted out one way or the other even if the answer isn't to change anything.

davidwbarratt’s picture

awesome. sounds like a plan. :)

Thanks!

ParisLiakos’s picture

Category:task» bug
Status:Active» Needs work

having a vendor directory in each module is really an even worse solution than using libraries module.
correcting status according to #8

ParisLiakos’s picture

Category:bug» task

xpost...

davidwbarratt’s picture

I agree.

Ideally there needs to be a single vendor directory.
Ideally that vendor directory would be in the root of Drupal.
Ideally Core would be inside of that vendor directory (vendor/drupal/core).

Thanks!

patcon’s picture

davidwbarrett++ well said

davidwbarratt’s picture

I should add one more:

Ideally the vendor directory should not be included in the repository. This means Drupal Core would be a separate repository.

cosmicdreams’s picture

Well this DEFINITELY sounds like something that needs to be talked through.

Is it necessary that drupal/core be placed into the vendor directory? Is this how other projects that use composer work?

I suppose we have to consider two use cases:

Drupal projects

Projects where a Drupal site needs to be built out. From this perspective, we currently expect to find a core directory that contains all of drupal including all of it's dependencies. What you seem to be saying is that we could allow composer to install drupal into /vendor/drupal/core and have all of it's dependencies install in /vendor.

In my opinion that would be weird for a while but I'd ultimately get used to it.

Projects that include Drupal through composer

Making these changes for Drupal projects would seem to make it easier to include Drupal into the mix for projects that want to interact with it. Drupal would be installed by composer into the vendor folder for this other project and it's OO code would autoload.

I can see why it's important not to include drupal's dependencies in the same folder of Drupal Core now. PSR-0 is supposed to allow more freedom in where the code is placed within a file system. I suspect the main motivation for including the vendor folder in /core is to ensure all of drupal is the core folder so we can say "Don't touch that" to site administratrators/builders.

davidwbarratt’s picture

@cosmicdreams,

correct. What I'm thinking is that it should work something like the way Symfony Standard and Symfony work.

Syfony Standard is to Drupal, what Symfony is to Drupal Core.

Also notice that when you Download Symfony. By default, you can download a package. The package is Syfony Standard with all of the dependencies (including Symfony itself) already loaded for you (no composer needed).

Doing it this way also lets you install Syfony Standard with the following command:

php composer.phar create-project symfony/framework-standard-edition path/ 2.3.5

Ideally, I think this is how it should work. It would provide the simplist and cleanest solution for Drupal developers and site builders. However, it would take some work to modify how Drupal Core developers work with Drupal. It would also take writing a script that would run `composer install` whenever a release is created.

Thanks!

davidwbarratt’s picture

Issue summary:View changes

Update the recommended core project name, as per Seldaek's suggestion.

ParisLiakos’s picture

Mile23’s picture

Status:Needs work» Needs review
StatusFileSize
new395 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,463 pass(es).
[ View ]

OK, we have two goals:

  1. Let people download Drupal and then start using it without learning any Composer at all.
  2. Let Composer-literate people do installations through Composer.

As it turns out, this is very easy to accomplish.

First we apply attached patch.

Second, we make a listing on packagist.org for drupal/drupal.

Third, we type composer create-project drupal/drupal [some path]

There is no fourth step.

The non-Composer users can just download the package.

The Composer users get Composer to do it for them, and also get version bumps from dependencies if they're available. Because Drupal's composer.json uses vendor-dir in it's config section, Composer will update that directory.

That's all there is to it.

davidwbarratt’s picture

Status:Needs review» Active

Mile23,

I'm not sure where you got those goals from. Yes, I think it's important to be able to install Drupal through composer, however, your patch does not answer the question raised in the initial post:

Is it acceptable to modify Drupal's composer.json file?

The patch you've submitted does not address the purpose of this issue, which is to allow a user-editable composer.json file in the root of Drupal.

Thanks!

Mile23’s picture

I blame blood sugar. :-)

xjm’s picture

Component:drupal.module» base system
alexpott’s picture

Priority:Critical» Major
Related issues:+#2002304: [META] Improve Drupal's use of Composer

This should only be at most a major. #2128353: D8 strategy for Composer Manager has found a solution to use composer manager for d8 so we have a way forward if you use composer manager. Yes it is not as nice as using composer update from the command line but as moshe said in the same issue.

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!

sun’s picture

The patch you've submitted does not address the purpose of this issue, which is to allow a user-editable composer.json file in the root of Drupal.

Fixing issue title accordingly to stop confusion and not presume one of many possible solutions.

sun’s picture

Title:Introduce a new core/composer.json file» Allow a user-editable composer.json file in Drupal's root directory to manage (custom) dependencies

x-post

jameshalsall’s picture

What is the status on this issue? Is it likely to be merged into 8.x before release?

davidwbarratt’s picture

Status:Active» Needs review
StatusFileSize
new94.07 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 47,097 pass(es), 9,153 fail(s), and 7,245 exception(s).
[ View ]

I've updated everything needed to make the split (excluding all of the tests and removing the "vendor" directory in core).

I ended up giving up on putting drupal inside the vendor directory. There are just too many hard-coded references to "/core/". I wish that Drupal would just make it's paths relative so you can put Drupal in any folder, but perhaps that's out of scope for this issue. If we'd like to go down that path, then I can work on making that happen.

As a result, I had to use a Composer Installer plugin to get Drupal to be in the "core" directory. I ended up using my own Composer Custom Type Installer, but any installer could be substituted (it just needs to get Drupal in the core directory). If we'd like to make our own Drupal Core Installer, that might be the best approach and would require less config in composer.json. The alternative to doing this would be to make drupal capable of being in any directory.

If you'd like to see how all of this would work, I made a repository here:
https://github.com/davidbarratt/drupal8
just clone it and run `composer install` and you should be good to go.

That repo includes a slightly modified version of the composer.json in the patch. the only difference is the reliance on my Drupal core repository which is just a subtree split of the core folder. I didn't include this in the patch, because if the repository is split, then this will not be necessary.

Also, I had to include the non-stable dependencies of Drupal core in the root composer.json file. It was either do that or set the minimum-stability flag which seemed like a worse idea. Hopefully these dependencies are stable by the time Drupal 8 is released and they can be removed from the root composer.json file.

Status:Needs review» Needs work

The last submitted patch, 32: split-1975220-32.patch, failed testing.

davidwbarratt’s picture

StatusFileSize
new93 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,918 pass(es).
[ View ]

I removed the references to the new, non-existent autoload.php file so hopefully this will no longer break every test known to man.

We'll need to add the `composer install` command to the test runner so it gets the needed dependencies before executing all of the tests, at that point #32 should work.

mgifford’s picture

Status:Needs work» Needs review
davidwbarratt’s picture

Issue summary:View changes

Update the project description to be better organized and include ideals.

davidwbarratt’s picture

StatusFileSize
new179.89 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,025 pass(es).
[ View ]

I realized that splitting the repository could be extremely disruptive for the current Drupal 8 development. It would involve changes to D.o and would involve rewriting all of the currently submitted patches to be compatible with the new structure.

While I still think this would be the "Ideal", I recognize that that is a huge amount of work.

I'd love to write a patch that would make Drupal core "folder agnostic" (i.e. you can put Drupal core in any folder), but I feel like that's probably not going to happen in the 8.0.x cycle. Since that's not going to happen, my next choice would be what I put in #32 and split the repositories.

However, because of the amount of work that would take, I'm submitting a third solution to the problem. I think this will be the least disruptive, it satisfies the issue completely and only requires a slight modification to the way things are done.

The patch I've included doesn't require any changes to D.o, or the test script, or any of the current patches. The things that would need to change are:

  1. cd into the core directory before core dependencies are updated with composer, but this was already done this way before #1805316: Move composer.json to the project root was committed, so this should be a minimal system change.
  2. Maintain a subtree split of the core directory. I think this essential to the other "ideals" being a reality in the future and it's really essential for anyone who wants to manage Drupal core with Composer. I'm using the Drupal core subtree split, but it is not maintained well and is certainly not official. Ideally this split would be submitted to Packagist so there is no need to define a repository in the composer.json. This subtree split ideally should exist somewhere on Drupal's servers and be split at least every release.

Anyone who wants to manage their dependencies with Composer (including drupal itself) can simply edit the root composer.json file, and run composer update which will destroy what is in the core directory and manage core with composer (and it's dependencies). At which point the user can simply edit their index.php file to point to the vendor directory in the root (which was just created by composer.json), rather than the one in the core directory. I can write up instructions on this after these things are done.

As I mentioned in #32 I'm currently using my own installer to get Drupal core into the core directory. Ideally this should be a drupal project of some sort. I'm more than happy to write something that is Drupal-specific and can be maintained by the Drupal community (it would be a very little amount of code I would think). Also, their are a few dependencies in the root composer.json file that can be removed after they are stable.

Let me know what you all think! :)

mradcliffe’s picture

I'm really excited about this. IT would be nice to hear from hosting providers on the impact to their 8.0.x developments so far i.e. Pantheon, Acquia, Bluehost, Platform.sh, Hotdrupal, etc...

I think there still would be a role for composer manager because there needs to be automation for testing infrastructure at least.

davidwbarratt’s picture

Also, I think the subtree split of "Drupal Core" should live here:
https://www.drupal.org/project/core

:)

This would mean that drupal/drupal would refer to https://www.drupal.org/project/drupal and drupal/core would refer to https://www.drupal.org/project/core. This would also be consistent with the work they are doing in #1886820: Packagist for Drupal Projects.

davidwbarratt’s picture

StatusFileSize
new179.79 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,031 pass(es).
[ View ]

Attached is yet another patch (sorry for so many).

I did some more thinking and realized that, if we are not splitting the repository #37, then there really isn't a point in forcing a custom installer. Since the composer.json file can be modified, someone can simply add whatever installer they want to use (and this should be documented). If they don't use an installer, Drupal will go in vendor/drupal/core which as I've pointed out, will not work.

I still think Drupal should make an installer or rely on Composer Installers. RobLoach has made a pull request for this to work. Either way, the dependency for the custom installer should really be in core/composer.json rather than the root as described by Composer.

Although, as I demonstrated, it does work if you put it in the root, but there is no guarantee that it will be loaded before Drupal core. With this patch committed, then my issues from RobLoach's pull request are resolved and that can be merged into Composer Installers (or Drupal can create their own installer). After that an issue can be created on D.o for adding the dependency to the core/composer.json file.

cosmicdreams’s picture

  1. +++ b/composer.json
    @@ -1,47 +1,15 @@
    +    "drupal/core": "~8.0",

    Only saying 8.0 is misleading and inconsitent with semantic versioning.

    Should be 8.0.* or 8.0.0 if you want to be specific about the version.

  2. +++ b/composer.json
    @@ -1,47 +1,15 @@
    +    "phpunit/phpunit-mock-objects": "dev-master#e60bb929c50ae4237aaf680a4f6773f4ee17f0a2"

    +++ b/core/composer.json
    @@ -0,0 +1,47 @@
    +    "phpunit/phpunit-mock-objects": "dev-master#e60bb929c50ae4237aaf680a4f6773f4ee17f0a2",

    Why is this here twice? Does that mean we need to install two compices of phpunit-mock-objects.

Does this work? If so what do I need to do to test?

davidwbarratt’s picture

The tilde operator is the next significant release and is what is recommended by Composer for dependencies that use Semantic Versioning. However this can be changed if you think it should be restricted to 8.0.*.

Since the default value for minimum stability is stable executing composer update would result in an error since three of Drupal's dependencies are not yet stable. to get around this, there are two choices, either copy those dependencies into the root composer.json file (until they are stable) or change the minimum stability to dev. However, the latter results in the dev version of ALL dependencies being installed, rather than just those three. Since this probably isn't desirable, I went with the former for now.

Technically, you get a copy of everything, which is installed in the root vendor directory, you'd then just need to modify index.php to point to the root vendor directory rather than the core directory. This would change this line from this:

$autoloader = require_once __DIR__ . '/core/vendor/autoload.php';

to this:

$autoloader = require_once __DIR__ . '/vendor/autoload.php';

Then you'll have all of Drupal's dependencies, plus all of your custom dependencies and they will be auto-loaded for you.

To test this, you need several things, a subtree split of Drupal with a composer.json file in it. You'll also need to modify the root composer.json file to point to this repository/branch (if you'd prefer, you can apply the patch and create a subtree split of the core folder). Lastly, you'll need a custom installer to put this in the core directory. As I mentioned in #32, I've put all of this in a repository here (changes to composer.json and index.php):
https://github.com/davidbarratt/drupal8

For your connivence, here is the test composer.json file:

{
  "name": "drupal/drupal",
  "description": "Drupal is an open source content management platform powering millions of websites and applications.",
  "type": "project",
  "license": "GPL-2.0+",
  "repositories": [
    {
      "type": "vcs",
      "url": "git@github.com:davidbarratt/drupal-core.git"
    }
  ],
  "require": {
    "davidbarratt/custom-installer": "1.0.*@alpha",
    "drupal/core": "dev-composer_split",
    "doctrine/common": "dev-master#a45d110f71c323e29f41eb0696fa230e3fa1b1b5",
    "kriswallsmith/assetic": "1.1.*@alpha",
    "phpunit/phpunit-mock-objects": "dev-master#e60bb929c50ae4237aaf680a4f6773f4ee17f0a2"
  },
  "config": {
    "preferred-install": "dist",
    "autoloader-suffix": "Drupal8"
  },
  "extra": {
    "custom-installer": {
      "drupal-core": "core"
    }
  }
}

NOTE If a subtree split of core is created and a custom installer is created/used as I've recommended in #37, you wont need to make any modifications to the root composer.json to test (other than changing the version number to 8.0.*@beta) and of course changing your index.php which should be documented for everyone who wants to use Composer.

tstoeckler’s picture

@davidwbarratt you can use "minimum-stability": "dev" and then "prefer-stable": true in order to avoid the problem you mentioned.

davidwbarratt’s picture

StatusFileSize
new179.97 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,018 pass(es).
[ View ]

Thanks for the tip tstoeckler! You're exactly right.

I'm glad you pointed that out too, because symfony/yaml was moved to a dev version while I've been working on this. Your change means that the core composer.json file will not have to have any copied dependencies in it. As an added benefit, the end-user will not have to change the Drupal version (once the subtree split is maintained) since ~8.0 will install 8.0.0-beta1. I've attached a patch with the changes and I've also updated my test composer.json file. The test file now looks like this:

{
  "name": "drupal/drupal",
  "description": "Drupal is an open source content management platform powering millions of websites and applications.",
  "type": "project",
  "license": "GPL-2.0+",
  "repositories": [
    {
      "type": "vcs",
      "url": "git@github.com:davidbarratt/drupal-core.git"
    }
  ],
  "require": {
    "davidbarratt/custom-installer": "~1.0",
    "drupal/core": "dev-composer_split"
  },
  "minimum-stability": "dev",
  "prefer-stable": true,
  "config": {
    "preferred-install": "dist",
    "autoloader-suffix": "Drupal8"
  },
  "extra": {
    "custom-installer": {
      "drupal-core": "core"
    }
  }
}

Let me know what you all think! :)

davidwbarratt’s picture

I've opened up #2352091: Create (and maintain) a subtree split of Drupal core to discuss having a subtree split of Drupal core on Drupal.org.

tstoeckler’s picture

Wow, this issue is incredibly awesome!

This would make the maintenance of https://github.com/tstoeckler/drupal-core a lot less hacky. As documented there one needs to basically copy almost the entire core composer.json into the project-specific composer.json in order to use the subtree.

In the same light, this issue is a prerequisite for #2352091: Create (and maintain) a subtree split of Drupal core and in general Composer-based Drupal 8 site development.

  • +++ b/composer.json
    @@ -1,47 +1,14 @@
    -    "php": ">=5.4.2",
    -    "sdboyer/gliph": "0.1.*",
    -    "symfony/class-loader": "2.5.*",
    -    "symfony/css-selector": "2.5.*",
    ...
    -    "mikey179/vfsStream": "1.*",
    -    "stack/builder": "1.0.*",
    -    "egulias/email-validator": "1.2.*"
    +    "drupal/core": "~8.0"

    I'm not sure about removing this. In theory it's bad practice to duplicate code, but with this change composer install no longer works in the root of the repository (without using a custom subtree split).
    Thoughts?

  • +++ b/core/composer.json
    @@ -0,0 +1,47 @@
    +      "Drupal\\Driver": "../drivers/lib/Drupal/Driver"

    For anyone stumbling upon this weirdness: see #2209307: Convert database drivers into a regular extension type for resolving this properly.

  • The actual core/composer.json file works beautifully. I deleted by vendor directory, ran composer install and everything got recreated (but for a few dependencies which are outdated in our repository).

    nicholasruunu’s picture

    I think it's very important this ends up in D8.

    One of the biggest issues with D7 is that everything must be in a module, it shouldn't.
    If you want your domain to be separate from Drupal we need to be able to make our own namespaces and have our own dependencies.

    I hope this is seriously considered, if any help is needed ping me.

    davidwbarratt’s picture

    Issue summary:View changes

    Update the description with the real solution.

    davidwbarratt’s picture

    tstoeckler,

    I don't see any other way to make Drupal Core a dependency of Drupal without having the subtree split. Without the subtree split this issue is pointless.

    Yes, I suppose we could copy all of the dependencies from core/composer.json to the root composer.json, but that really just sets up two different projects with identical dependencies, we are not saying that one depends on the other.

    The problem with doing it that way is that all of the Drupal maintainers would need to understand the setup, which is a-typical. And any changes to one composer.json would need to be copied to the other. I think this is too much of a burden since it doesn't seem like most Drupal developers are excited about composer, let alone understand it's purpose. I think what we'd end up with is two composer.json files that do not have identical dependencies (i.e. the root one will be out of date).

    Also, any time there is an update to one of the dependencies in a point release of Drupal 8, we'd have to document to all users who update that they need to update the dependencies in their composer.json file (that they have customized).

    Lastly, adding all of the dependencies in both places would mean that Drupal core itself would not be updated (without the subtree split).

    It should be as easy as running composer update to update Drupal core and all of it's dependencies.

    For this reason, I think the only way to really do this, is to maintain a subtree split of Drupal core so that core can be a dependency of Drupal.

    I think what would happen after this issue is committed, is that https://github.com/composer/installers/pull/38 would be pulled into installers. Then Drupal core would require composer/installers, but it doesn't need to be included in the main repository, so it should have no effect on non-composer users, but allow composer users to install Drupal core in the right place automatically.

    tstoeckler’s picture

    @davidwbarratt I totally get your point of view but I think we have a chicken-and-egg problem here, and I think - at least temporarily - duplicating the composer.json will allow us to resolve that. Let me explain what I mean by that:

    Even though there is a subtree split out there, it is not official or endorsed in any way. Anyone is of course free to use it, but in terms of the Drupal core source code and its maintenance it might as well not exist. This is unfortunate, but that is how Drupal core development has always been handled. Just like the Drupal core source code is completely oblivious to Drush - even though (more or less) everyone uses it. We have never added a single Drush command to the Drupal core source code. And in the same light we cannot depend on the existance of an external GitHub repository in order to use Drupal via Composer.

    To be more precise: With the existance of the composer.json we have committed that Drupal working with Composer is a requirement. I personally find the current interaction very broken - and we are fixing a big chunk of that in this issue - but one needs to acknowledge that currently doing composer require drupal/drupal works (given the repository is known to the composer.json, and with a very forgiving definition of "works") and with your patch it doesn't. And I'm not sure that is acceptable. Note that this is not about my personal opinion of what "works" and "acceptable" means; if it were this issue would have been committed long ago. We need to convince the core committers that this is something that needs to happen, and I think we have better chances, if we don't break anyone's workflow.

    On the other hand, in order to make a subtree in some way official or endorsed, we need this issue to go in first, as the usability of the subtree is very much impeded without it as noted above.

    Putting those two constraints together means that for a patch in this issue to be considered acceptable

    1. composer install must work in the (unsplit) Drupal core repository
    2. composer install must work in the subtree-split of the core directory

    At least that is my stance.

    And given that there is simply no way around two, largely duplicated composer.json files because Composer does not support any import-mechanism or something similar.

    Some further notes on why I think this duplication would be acceptable (even though it's of course far from ideal):

    • #2352091: Create (and maintain) a subtree split of Drupal core would allow to remove the duplication
    • Because we have the vendor directory committed to Git anyway, the contents of the composer.json file(s) are only of secondary importance, and can be updated at any time if they are incorrect or if they get out of sync

    Thoughts?

    tstoeckler’s picture

    Some more concrete responses to #49:

    Without the subtree split this issue is pointless.

    Yes, but for the core issue queue there is a difference between an "official" subtree and just some repo on GitHub. For me personally, I don't care, I just want this to go in and I can use my subtree-split properly. That distinction is important here, IMO.

    Yes, I suppose we could copy all of the dependencies from core/composer.json to the root composer.json, but that really just sets up two different projects with identical dependencies, we are not saying that one depends on the other.

    Well, as long as they are both inside of the same Git repository it is not exactly clear what "one depends on the other" really means, because 1. they are basically the same thing (but for packaging) and 2. you can never get one without the other anyway. So while in theory this is a totally valid point, I'm not sure I agree given the current situation of one Git repository.

    The problem with doing it that way is that all of the Drupal maintainers would need to understand the setup, which is a-typical.

    I'm not sure what they would have to understand, specifically. Can you elaborate?

    Also, any time there is an update to one of the dependencies in a point release of Drupal 8, we'd have to document to all users who update that they need to update the dependencies in their composer.json file (that they have customized).

    This is true with or without the patch and with or without my proposal. It is unfortunate and of course why a subtree split makes sense in the first place, but even in 8.0.x you will have people modifying their composer.json and they will run into these problems.

    Lastly, adding all of the dependencies in both places would mean that Drupal core itself would not be updated (without the subtree split).

    Again, I don't see how this can be true, since they are both in the same repo.

    It should be as easy as running composer update to update Drupal core and all of it's dependencies.

    This stays possible with my proposal for both use-cases (i.e. running composer update in /path/to/drupal and in /path/to/drupal/core) whereas with your proposal only the latter is possible.

    I think what would happen after this issue is committed, is that https://github.com/composer/installers/pull/38 would be pulled into installers. Then Drupal core would require composer/installers, but it doesn't need to be included in the main repository, so it should have no effect on non-composer users, but allow composer users to install Drupal core in the right place automatically.

    This would require composer install to work in the Drupal root (i.e. not the core directory) in the first place, which - with your proposal - is only the fact with the subtree-split and as explained above IMO that means "it doesn't work" in terms of the core issue queue, at least for now.

    mradcliffe’s picture

    I talked with several people in Amsterdam and I heard the following:

    - Most people are hacking core's composer.json and not using Composer Manager or don't know it exists. I explained Composer Manager a couple of times and did not really get a favorable reaction from most of the developers I spoke with.
    - Commerce plans on pretty much ignoring composer and doing what core does: duplicating third party code and adding it to their repository.

    It's pretty much a step backwards from best practice established over the last several years.

    I think we're at the point that anything major to the repository is too disruptive. It's unfortunate because Drupal 8 really does need the right solution. The issue summary could use some organization with regard to explaining the solutions and their benefits/risks so that we can spam this issue constantly in IRC to actually get a decision made.

    bojanz’s picture

    - Commerce plans on pretty much ignoring composer and doing what core does: duplicating third party code and adding it to their repository.

    Untrue. Commerce currently uses composer_manager. Unfortunately, composer_manager needs a full rewrite, which we're planning to tackle: #2350639: Rewrite 8.x-1.x.
    I also opened a matching core issue at #2350647: Allow contrib to use libraries via Composer, listed under "related" for this issue.

    Haven't had a chance to review this whole issue yet (though I agree with the OP), just wanted to respond to the comment first.

    mradcliffe’s picture

    Untrue. Commerce currently uses composer_manager.

    Sorry, this was information I learned in Amsterdam and it was true of the plan at the time. I'm glad that plan changed.

    Fabianx’s picture

    Here is another idea, which is probably not right, but just posting it.

    There is the possibility to create the .json file from a .yml file.

    The advantage of the .yml files is that they can be cat'ted together and have comments.

    So could have a pre-setup step to create the .json file from a list of .yml files, which would solve the problem for the .json.

    HOWEVER

    All this issue talks about is the .json file. IIRC we use a .lock file in core, in which case the .json file is 100% ignored unless using 'update' instead of 'install' and in case of 'update' it would again dirty your .lock file. Has that been discussed, yet?

    davidwbarratt’s picture

    #50,

    I don't think it's a chicken-and-egg problem at all.

    Although the sole purpose of this issue, is to allow users to edit the <root>/composer.json file without asking the user to modify core. Users are allowed to modify things like .htaccess .gitignore and index.php to their likeiing, and as suggested in #14, perhaps the answer is to "do nothing".

    The point of this issue is not to make a subtree split of drupal "work". For this reason, the subtree split should exist regardless of this issue, but if it exists, it's somewhat pointless if It's "not recommended" to edit the <root>/composer.json file. However, if I can edit the root composer.json file, I can at least use someone else's split of Drupal core.

    In your drush example, it's true that Drupal does not have any drush commands, but at the same time, Drupal does not do things purposely to prevent Drush commands from working. In the current state of things, we have a scenario where Drupal is preventing users from properly using Composer.

    As I mentioned in #22, the proper command to install Drupal would be:
    composer create-project drupal/drupal
    which would download drupal into your current directory.
    alternatively you could execute:
    composer require drupal/core
    assuming a d.o. subtree split exists, but would just give you what's in the core directory.

    You're correct. the <root>/composer.json file that I've proposed in #44 does not allow composer install to work without someones (hopefully d.o.'s) sub-tree split. However, as I mentioned before, the purpose of this issue is to allow users to modify the <root>/composer.json file to their liking. I think it's ok that it doesn't work without some work on someone's part or on the part on d.o.

    As I've proposed it in #44 composer install will work from the core directory (or a subtree split of core) as-is. However, the only people that should be doing that, are core maintainers who want to update the dependencies that are in versioned in the repository.

    I think in the ideal world, d.o. will maintain a subtree split of Drupal core. I don't think this is any different than what Symfony does. Drupal even relies on these subtree splits in it's own composer.json file! To take it a step further, I think d.o. should maintain subtree splits of any package that could be used outside of Drupal (i.e. in another PHP project).

    However, if d.o. does not decide to maintain a subtree split. I still want to be able to manage drupal core using a different split.

    If you, as a user, want to copy all of the dependencies from Drupal core to the root composer.json file (I'm not sure why you would do this, since they'd also all be in the split you'd have to specify), then by all means! this issue does not prevent you from doing so, but in fact does the opposite, it enables you to do so.

    Please think of the <root>/composer.json file I've included as just an example. I'd like it to be a "working" example (like .htaccess), but it might end up being like example.gitignore where it's there, but it doesn't do anything. If that ends up being the case, it might be worth just removing it before release.

    tstoeckler’s picture

    OK, so now we're approaching a consensus :-)

    So we are now both on the same page that the the root composer.json in the current patch does not work. Thus composer create-project drupal/drupal does not actually work with the core repository with this patch applied. Therefore I would suggest we implement your idea of renaming this file to example.composer.json to allow people to get started with a Composer-based workflow more easily. Because JSON does not allow comments in the file, we cannot acutally document this example file, but it's still better than nothing IMO.

    Thoughts?

    Fabianx’s picture

    If we use a n example file, why not a YML file that can be converted to JSON? (and can have comments)

    davidwbarratt’s picture

    #51,

    Yes, but for the core issue queue there is a difference between an "official" subtree and just some repo on GitHub. For me personally, I don't care, I just want this to go in and I can use my subtree-split properly. That distinction is important here, IMO.

    You're right. I've changed my mind, it does have a purpose with or without the subtree split. However, ideally the <root>/composer.json would be a working example.

    Well, as long as they are both inside of the same Git repository it is not exactly clear what "one depends on the other" really means, because 1. they are basically the same thing (but for packaging) and 2. you can never get one without the other anyway. So while in theory this is a totally valid point, I'm not sure I agree given the current situation of one Git repository.

    I don't agree with the 1 git repository at all. :) However, I acknowledge that it would be an immense amount of work for us to split Drupal into two distinct, permanent repositories. I think it's unrealistic for this cycle. Also, I don't completely agree with your point that becuase there are two things in the same repository, it means that it's not clear that one depends on the other. If you look at the Symfony repository , all of their "components" are in the same repository, but they maintain subtree splits of their components. Symfony still depends on that component (which is in the same repository). Drupal requires Symfony's Routing Component which is just a read-only subtree split of Symfony. While I think it would probably be "better" to have two distinct repository, there is an outside precedent to it being ok to not do it that way. Honestly, I wish Drupal would maintain subtree splits of all kinds of components that could be used outside of Drupal.

    I'm not sure what they would have to understand, specifically. Can you elaborate?

    They'd have to understand that it's two separate projects, it isn't one project depending on another project in the same repository (see Symfony example above). I actually can't think of an example that is two distinct projects in the same repository with composer, so this might end up being another drupal-ism. :(

    This is true with or without the patch and with or without my proposal. It is unfortunate and of course why a subtree split makes sense in the first place, but even in 8.0.x you will have people modifying their composer.json and they will run into these problems.

    Correct, without the subtree split, they'd have to manage Drupal's dependencies themselves, which would be a pain and open people up to all kinds of inconsistencies and security risks. Having a clean composer.json file with a single (working) dependency is the best solution in my mind and I think it's what we should be working towards.

    Again, I don't see how this can be true, since they are both in the same repo.

    It would require the subtree split (from someone, hopefully d.o.)

    This stays possible with my proposal for both use-cases (i.e. running composer update in /path/to/drupal and in /path/to/drupal/core) whereas with your proposal only the latter is possible.

    Yep, you're right, and I've changed my mind on that to label the current <root>/composer.json file a non-working example.

    This would require composer install to work in the Drupal root (i.e. not the core directory) in the first place, which - with your proposal - is only the fact with the subtree-split and as explained above IMO that means "it doesn't work" in terms of the core issue queue, at least for now.

    You've convinced me. <root>/composer.json is non-working (however, I think we should make every attempt to make it working before 8.0.x is released) and core/composer.json is working as-is.

    davidwbarratt’s picture

    Issue summary:View changes
    davidwbarratt’s picture

    #52,

    I've updated the issue summary. I hope it's clearer and easier to read. This whole issue has been a fight (even with myself) of ideals vs. realities. I think the most recent patch from #44 gets us closest to the ideals without disregarding any of the realities.

    Most people are hacking core's composer.json

    I mean I don't know what else you would do if you want to use Composer (on a project level) with Drupal 8, hence the reason of this issue, and why I think it's really important.

    and not using Composer Manager or don't know it exists. I explained Composer Manager a couple of times and did not really get a favorable reaction from most of the developers I spoke with.

    I've used Composer Manager on a few projects, and I think it's somewhat of an antipattern. If you want to modify the generated composer.json file, the only way you can do that is with a hook. You're not "allowed" to change the generated file what-so-ever (even to add a custom dependency of your own). Also, the only way to add dependencies is by adding them in a module. I think this sets up another drupal-ism where Drupal does things differently than the rest of the world.

    Lastly, you aren't able to use Composer manager to specify the sites/modules/themes you'd like in your project (unless you specify them in a module). It makes things like #1886820: Packagist for Drupal Projects impossible. For an example of what a project-based composer file would look like, please see:
    https://github.com/davidbarratt/drupal7/blob/master/composer.json
    where I'm requiring a Drupal site, and here:
    https://github.com/davidbarratt/davidwbarratt/blob/develop/composer.json
    where that site is requiring specific Drupal modules, etc.
    I believe (correct me if I'm wrong) that kind of a setup would be impossible (or rather difficult) with Composer Manager.

    I think Composer Manager solves a lot of problems for a lot of people, but for me it creates more problems than it solves. I think the resolution of this issue might make it better for the Drupal 8 Composer Manager since it wont have to worry about combining dependencies with Drupal 8, it can simply require drupal/core and be done with it. :)

    I think we're at the point that anything major to the repository is too disruptive.

    I agree. This is why the "Real Solution" (#44), does not change anything major in the repository. I also don't think this is even a minor API change so in my mind there isn't any reason this can't go in before 8.0.0 is released.

    I'll try to be available more via IRC.

    davidwbarratt’s picture

    #55 & #58,

    I'm not sure how familiar you are with Composer, but it's a 3rd-party application that has nothing to do with Drupal. Drupal (and many other projects) use Composer to manage 3rd-party dependencies.

    The issue of using YAML files with composer has been discussed at great length here:
    https://github.com/composer/composer/issues/440

    While I agree that YAML would be "better" it's simply not what Composer uses. If you'd like to fork composer and get it to use YAML instead of JSON, then be my guest. However that is way way outside of the scope of this issue and it's even outside of the scope of Drupal itself. Please go discuss your proposition with Composer on that GitHub thread.

    For Drupal module dependencies, YAML was actually chosen:
    https://www.drupal.org/node/1935708
    so if you need to manage dependencies between two modules, you can do that with YAML and Drupal (not Composer).

    On your mention of the composer.lock file. The file is actually in json format, it just has a different extension.

    mradcliffe’s picture

    Issue summary:View changes

    The only disruption I can think of is for Drupal hosts to have to re-work infrastructure if they've already started down one path because Drupal 8 is in beta.

    davidwbarratt’s picture

    #57,

    Awesome! I think we are too.

    I've opened a follow-up issue to deal with the working / non-working state of the <root>/composer.json file:
    #2372815: [meta] Make the <root>/composer.json file a working example

    I think we should just leave the <root>/composer.json file the way it is right now since ideally it would become a working example. If that doesn't happen we'll change the name (or make the documentation clear somehow, with a text file perhaps?).

    Sound like a plan?

    davidwbarratt’s picture

    #63

    I mean I suppose. #44 doesn't really change anything in regards to 8 (as in they'd manage it the same way Drupal has been manged for years). But if they would like to take advantage of it, then sure. In that regard it's just a feature.

    That's the problem with this issue, on one hand it's a bug (i.e. I can't change composer.json) on the other hand it's a feature (manage Drupal modules/sites/themes via Composer).

    Mile23’s picture

    Just want to point out a proof of concept I did.

    Blog post here: http://mile23.com/content/install-drupal-8-module-composer

    Repo here: https://github.com/paul-m/composer-drupal-module

    Open a terminal, navigate to DRUPAL_ROOT, and type composer require mile23/page-example dev-master.

    This will install the module under module/ and an external dependency under core/vendor/.

    That's because Composer already knows how to do everything. :-)

    The problem is that it's a huge cultural shift to have people installing modules through Composer, though it's not really different from drush dl.

    tstoeckler’s picture

    StatusFileSize
    new772 bytes
    new188.69 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,122 pass(es).
    [ View ]

    OK, so let's do this.

    I have one small suggestion for this patch which I have implemented in the attached patch. Even though JSON does not support any native commenting, the auto-generated composer.lock contains a "_readme" key explaining the contents of the file. So I think it makes sense to add a similar note to our "example" composer.json.

    I used -C -M on the patch to prove that nothing in the composer.lock and none of the dependencies actually changed.

    If people agree with the small addition here, then this is RTBC, I just don't want to RTBC my own patch, otherwise I would mark it that directly.

    tstoeckler’s picture

    StatusFileSize
    new7.95 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,110 pass(es).
    [ View ]

    OK, I actually forgot -C -M.....

    Here we go....

    martin107’s picture

    +1

    This new approach makes sense to me... It is a standard composer configuration.

    For me having a core/composer.json file is a better separation of concerns.

    davidwbarratt’s picture

    StatusFileSize
    new96.39 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,118 pass(es).
    [ View ]

    tsoeckler,

    The patch in #68 doesn't validate. :(

    $ composer validate
    ./composer.json is valid for simple usage with composer but has
    strict errors that make it unable to be published as a package:
    See http://getcomposer.org/doc/04-schema.md for details on the schema
    The property _readme is not defined and the definition does not allow additional properties

    However, you could move it into extra and then it will validate. I've attached a patch that moves it there, fixes a grammatical error in your text and changes it from "below" to "above".

    Lastly, I was thinking the changes to /core/vendor shouldn't be included in the patch, but since they patch passes testing... why not?

    Mile23’s picture

    Still minus a zillion from me.

    Just use the existing composer.json like the rest of the world.

    There aren't really any concerns to separate. You're only making core/ a dependency of index.php.

    Do this instead:

    composer create-project drupal/drupal drupal_root ~8@dev
    cd drupal_root
    composer require something/else ~1

    Find that you didn't need the new requirement? Just do this:

    git checkout -- composer.*
    composer install

    The problem here is cultural, not technical: It is not hacking core to use Composer with Drupal. :-)

    davidwbarratt’s picture

    #71,

    You're right, I've stated from the beginning that the problem is a policy, not a strictly a technical problem.

    However, the problem people will have, is after they've changed their <root>/composer.json file as you've suggested, they will then have to merge any dependencies that have been added/changed when they update Drupal.

    Of course, none of us know how often it will change. It could almost never change like .htaccess and index.php or it could change rather often. My money is on it changing often (for bug fixes, new features, etc.)

    Because of this, we can make it 1000X easier on the admin by making core a dependency of your drupal project. Also, in doing this, core folder does not have to be in your repository. :)

    nicholasruunu’s picture

    #71,

    Also just to bring home #72 point more, this minimizes the littering of "your own" composer.json file.
    You're not requiring those vendor files for you app, drupal is, all we should require from the start is drupal itself.

    Also, look at how Laravel (Most starred PHP project on GH) does it: https://github.com/laravel/laravel/blob/master/composer.json

    tstoeckler’s picture

    Re @Mile23: Well, you are right that this is not a problem if you allow yourself to hack core. Not hacking core, though, is not a "cultural" rule, it is completely technical because it not only makes upstream contributing harder, but also upgrades a pain as mentioned by @davidwbarratt.

    @davidwbaratt: The changes in vendor are internal to composer and they are necessary. Or to be more precise they follow directly from using composer install from the core direcory. I am fine with moving the "_readme" part to "extra" although the file not validating is not a concern IMHO as - even it is valid syntax-wise - it is not valid conceptually anyway because "drupal/core" will never be found (as explained in the _readme).

    Please roll your patch with -C -M and then I will RTBC.

    tstoeckler’s picture

    @nicholasruunu: How Laravel does it is exactly where we want to get with Drupal as well, but we're still a couple steps away from it. This patch would get us a big step closer but not all the way there yet.

    davidwbarratt’s picture

    StatusFileSize
    new7.98 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch custom-composer-1975220-76.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    Attached is the patch with -C -M as requested. :)

    Thanks!

    davidwbarratt’s picture

    #74,

    Also, the reason I'd prefer for it to validate is:

    1. It sets up a precident that the composer.json file needs to be composer-valid.
    2. It doesn't block #2373203: [no patch] Take over ownership of drupal/drupal and drupal/core on Packagist (As the error message says, packagist only accepts valid composer.json files).
    mradcliffe’s picture

    +++ b/composer.json
    @@ -1,47 +1,25 @@
    +    "drupal/core": "~8.0"

    Should this be semantic version now?

    davidwbarratt’s picture

    #78,

    Please read #42 which is not only the answer to your question, but also the answer to life, the universe, and everything.

    EclipseGc’s picture

    How long you been holding onto that one?

    Eclipse

    RobLoach’s picture

    Is there anything holding this back from RTBC? Looks pretty good to me.

    tstoeckler’s picture

    Status:Needs review» Reviewed & tested by the community

    Yeah, I think 8.0.* is a safer choice in the Drupal ecosystem, but until that makes a difference we still have a year ahead of us - even in the most optimistic of scenarios.

    Let's do this.

    Status:Reviewed & tested by the community» Needs work

    The last submitted patch, 76: custom-composer-1975220-76.patch, failed testing.

    Mile23’s picture

    Making core/ into a dependency of DRUPAL_ROOT only kicks the ball down the road, and doesn't solve the problem of updates, because plenty of stuff outside of core/ could be updated in order to improve Drupal, including such trivialities as the front controller. :-)

    Also, #76 reorganizes so that all dependencies are within ./vendor/, which is a change that I kind of agree with but the patch doesn't remove ./core/vendor, so is that supposed to just be there? Maybe add ./core/vendor in the root composer.json file, so that we don't have to re-write all the documentation and test scripts for phpunit?

    I think this is a wrong-headed way to go about solving the problem of updating Drupal, unless a *lot* more work is done to make core into something that can actually be this modular. Also, it's far too disruptive for 8.0.x beta.

    davidwbarratt’s picture

    StatusFileSize
    new6.95 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,261 pass(es).
    [ View ]

    Re-roll.

    davidwbarratt’s picture

    Status:Needs work» Needs review
    bojanz’s picture

    Status:Needs review» Needs work

    This patch is almost RTBC, and yet nobody has provided an updated summary on what the workflow is supposed to be after this patch lands, and how this can be tested.

    The "example" composer.json is incomplete and just adding

      "repositories": [
        {
          "type": "vcs",
          "url": "git@github.com:tstoeckler/drupal-core.git",
          "reference": "8.0.x"
        }
      ],

    to it gave me nothing (composer update failed).

    The implied requirement of having to modify index.php also feels wrong. index.php should be smart enough to pick up the other vendor/ directory, or there shouldn't be another vendor/ directory.

    davidwbarratt’s picture

    #84,

    Making core/ into a dependency of DRUPAL_ROOT only kicks the ball down the road

    What do you mean? If by this you mean "defer conclusive action with a short-term solution" (source), then yes, I suppose we are. However, as you've pointed out, the conclusive action would be way way too much work for the 8.0.x cycle and be way too disruptive.

    doesn't solve the problem of updates, because plenty of stuff outside of core/ could be updated in order to improve Drupal, including such trivialities as the front controller. :-)

    I mean on a certain level, you're right. However, the only "code" not in the core folder (that I'm aware of) is the front controller... a whole whopping 34 lines of code. I think the odds of this changing is minimal. In the 7.x cycle, the only real change to index.php was on October 15th 2009, well before the January 2011 release and obviously hasn't changed since the release unless you count the SVN migration, which isn't a real change.

    Having said all that, I'm in full support of having a single vendor directory in the root and that's why it's Ideal #1:
    "There needs to be a single vendor directory."
    Having that would prevent having to modify index.php at all. Also, if #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead the reality of having a single vendor directory is much higher, but this has been discussed at great length in that issue.

    Also, #76 reorganizes so that all dependencies are within ./vendor/, which is a change that I kind of agree with but the patch doesn't remove ./core/vendor, so is that supposed to just be there?

    The patch leaves /core/vendor because right now, that is necessary for the packaging script. This is also one of the ideals listed in the Issue Summary.

    Maybe add ./core/vendor in the root composer.json file, so that we don't have to re-write all the documentation and test scripts for phpunit?

    I've tested this several times, and it's a bad idea to put the vendor directory inside of a dependency's folder. If drupal core is upgraded, Composer will see if there have been any "modifications" to the package, because there have been, it will refuse to update, you can force it to continue, but doing that will destroy the core folder and add the updated version (which will then destroy your vendor directory). To get around this, you'd have to then go into /core and delete the vendor directory and run composer install to get it back. It's a huge hassle and one of the limitations of Composer.

    As for test scripts and PHPunit, I'm not sure how this is a problem. The test script bootstrap loads the core/vendor directory not the <root>/vendor directory. Obviously the patch completed all of the test successfully, so this shouldn't be an issue.

    I think this is a wrong-headed way to go about solving the problem of updating Drupal, unless a *lot* more work is done to make core into something that can actually be this modular.

    I'm all ears to a solution that gets as close to the Ideals as possible while also acknowledging the Realities. I think the solution that we have in front of us does this; however, if you have a better idea please don't hesitate in telling us.

    Also, it's far too disruptive for 8.0.x beta.

    What about the current patch is too disruptive? It doesn't require rewriting anything. All we are doing is reverting #1805316: Move composer.json to the project root and creating an example composer.json file in the root. What about this is so disruptive?

    donquixote’s picture

    The following is just a note about index.php.

    doesn't solve the problem of updates, because plenty of stuff outside of core/ could be updated in order to improve Drupal, including such trivialities as the front controller. :-)

    I mean on a certain level, you're right. However, the only "code" not in the core folder (that I'm aware of) is the front controller... a whole whopping 34 lines of code. I think the odds of this changing is minimal. In the 7.x cycle, the only real change to index.php was on October 15th 2009, well before the January 2011 release and obviously hasn't changed since the release unless you count the SVN migration, which isn't a real change.

    There is at least one issue where I want to change the front controller (index.php). (unfortunately I did not have much time for this recently)
    The goal of which is to refactor the bootstrap and the DrupalKernel.
    I think the main reason why it has not changed much is that the bootstrap and kernel are scary systems that people don't want to touch.

    Even with 34 lines of code, the index.php currently has a lot of assumptions and specifity.

    A solution could be to move all this into a separate place, so the index.php would become:

    <?php
    require_once __DIR__ . '/core/vendor/autoload.php';
    Drupal::indexPhp();
    ?>

    Or simply

    <?php
    require 'core/real.index.php';
    ?>
    donquixote’s picture

    This patch is almost RTBC, and yet nobody has provided an updated summary on what the workflow is supposed to be after this patch lands, and how this can be tested.

    Absolutely +100 (units of approval).
    We need to identify the need and the typical use cases and scenarios, and then for each solution we would have to describe a work flow for each of these scenarios. Just throwing patches at the problem won't help.

    As I see it, these are the main things we need to solve:

    • A module depends on a Composer package.
      Sub-problem: A new module version introduces a new Composer package dependency.
    • A site maintainer wants to add arbitrary Composer packages, independent of module dependencies.
      (I think this scenario is far less relevant than the first one)
    • Module or custom Composer dependencies conflict or overlap with the core Composer dependencies.
    • Multisite: Different sites within the multisite depend on different Composer packages.
    • Sonia Superuser wants to download core and contrib with Composer.
    • Sonia Superuser wants to get Drupal core out of her site repository, into a separate package.
    • Sonia Superuser wants to get Symfony and friends out of the Drupal core repository, so she can have them in separate version control and use her own fork of Symfony etc.
    • Joe Blogger does not like the commandline, and wants to operate with zip files and FTP.

    Besides that we still need the basic Drupal stuff to work:

    • Drupal download + site install.
    • Module download + module install + module update.

    From the issue description, I think this issue only attempts to solve this one problem:

    A site maintainer wants to add arbitrary Composer packages, independent of module dependencies.
    (I think this scenario is far less relevant than the first one)

    I think this is a bit weak.

    davidwbarratt’s picture

    Issue summary:View changes
    davidwbarratt’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new7.66 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,423 pass(es).
    [ View ]

    #87,

    I've updated the Issue Summary to include instructions on testing, or for your convenience, you can simply clone my repo and execute composer install.

    I also found a typo in the last patch (object instead of array for repositories) so I'm attaching a new patch.

    The implied requirement of having to modify index.php also feels wrong.

    I agree. Ideally :
    "There needs to be a single vendor directory."
    and
    "That vendor directory would be in the root of Drupal."

    However, because of the realities of how much work this would take, I don't think that's possible for this release cycle (unless I'm wrong).

    Having said that, I did some more research into setting core/composer.json vendor directory to one level higher like so:

    "vendor-dir": "../vendor"

    This actually works beautifully and I would go ahead and make a patch to do this; however, the problem is, is that we are currently ignoring only specific files in the vendor directory that are not technically needed by Drupal core (it's fine if they are there, but Drupal isn't using them). I'm assuming this is done specifically for the packaging script(s). I would prefer to #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead which would solve the issue.

    index.php should be smart enough to pick up the other vendor/ directory

    That would be nice, but it would be kind of silly to check for a directory on every single page request (whether you use Composer or not). :(

    or there shouldn't be another vendor/ directory.

    I agree, but since this is a bigger issue that can't be easily solved and kind-of side tracks this one a bit, I created a new issue to discuss it #2380389: Change index.php to use a vendor directory in the root

    Mile23’s picture

    Having said that, I did some more research into setting core/composer.json vendor directory to one level higher like so:

    "vendor-dir": "../vendor"

    If the point is to have a project-level composer.json as well as a core-only composer.json, then the project-level settings will always take precedence. That's how Composer works.

    Thus, regardless of what ./core/composer.json says, the vendor directory will be where ./composer.json says it should be.

    This leads to the issue I brought up in #84: Composer will download drupal core for you and the repo will include a pre-populated core/vendor. Your root project file will also specify its own vendor directory, so now you have two copies.

    This isn't such a big deal because your root's vendor/autoload.php will determine all the classloading, but it's a big deal in the sense that you have extra code laying around in your project directory.

    This is, as I say, a cultural problem rather than a technical one. Just use Composer the way it's intended.

    @donquixote

    // index.php
    require 'core/real.index.php';

    Just kick the dependencies down the road. :-) It's not really a solution. At some point you have to decide "This is the top of Drupal." Currently, the top of Drupal is DRUPAL_ROOT.

    Once core becomes a loosely-coupled collection of packages, I will delight in RTBCing this issue. That moment will be at least a year from now, optimistically.

    davidwbarratt’s picture

    #89

    I agree that ideally it would be best to not modify the index.php file.

    I've created a followup issue to see if we can figure out how to get around doing this
    #2380389: Change index.php to use a vendor directory in the root

    I like your idea of moving the code into a seperate file (in the core direcotry). The only thing then that needs to be left in index.php is this line:

    $autoloader = require_once __DIR__ . '/core/vendor/autoload.php';

    However, it would be better if we could just get the vendor directory out of the repository completely
    #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead
    Then index.php could point the the <root>/vendor directory.

    davidwbarratt’s picture

    Issue summary:View changes
    davidwbarratt’s picture

    #90,

    We need to identify the need and the typical use cases and scenarios, and then for each solution we would have to describe a work flow for each of these scenarios.

    I've updated the Issue Summary to include a "Purpose" section to help explain why this is important. I'll also respond to each of your scenarios. I'm not sure how they should be worked into the Summary, but hopefully it will help.

    Just throwing patches at the problem won't help.

    Of course not, but that's the chicken and egg problem of Drupal development... with a patch, it's not helping, without a patch, nobody cares. :)

    A module depends on a Composer package.
    Sub-problem: A new module version introduces a new Composer package dependency.

    With the proposed solution you'd have two choices on how to install a module. You can either do it the traditional way, or you can install it via Composer (I'm assuming #1612910: Switch to Semantic Versioning for Drupal contrib extenstions (modules, themes, etc) lands or #1886820: Packagist for Drupal Projects is created or you the module maintainer has put it on packagist). If you install it the traditional way, you'd be responsible for updating your composer file yourself, but this isn't any different then how it would be done if you were to manual install any PHP project. I suppose in the Drupal world you could use Composer Manager for that. I think it's fairly safe to assume that if you are a Composer user (like me), you're going to install/manage the module with Composer. If this is the case, then Composer will update the module with the new dependency and everything is happy. :)

    A site maintainer wants to add arbitrary Composer packages, independent of module dependencies.
    (I think this scenario is far less relevant than the first one)

    The proposed solution solves this problem by allowing you to modify the <root>/composer.json file.

    Module or custom Composer dependencies conflict or overlap with the core Composer dependencies.

    The proposed solution makes Drupal core a dependency of Drupal. Composer will manage the dependencies between your dependencies, so it will figure out the best version of a dependency to use (that fits everyone's requirements) or it will throw an error. Basically the proposed solution let's composer do it's thing, rather than relying on the user to figure out.

    Multisite: Different sites within the multisite depend on different Composer packages.

    That's fine. You can either manage those dependencies yourself (if you install the site manually), or make the site a dependency. I've demonstrated that in my site. Please take a look at the composer.json file. Basically the Drupal site is just another dependency, it's a dependency of your project.

    Sonia Superuser wants to download core and contrib with Composer.

    The proposed solution allows her to do this. If #2372815: [meta] Make the <root>/composer.json file a working example lands you'll simple be able to execute composer create-project drupal/drupal and boom! Without that you can still simply create an empty folder, in that folder create a new composer.json file (use this example if you want to follow along). And execute composer install. You'll still need to create some other folders and copy the index.php file, so not completely ideal, but it will technically work. If you'd like to manage contrib with Composer, you'd have to use something like #1886820: Packagist for Drupal Projects to make all of the contrib modules available to Composer, or you could do them one by one but that would be annoying.

    Sonia Superuser wants to get Drupal core out of her site repository, into a separate package.

    The proposed solution adds

    #core
    #vendor

    to the example.gitignore file. Simply uncomment those lines and Drupal core will be out of the repository. On deployment, they'll need to execute composer install for Drupal core to be downloaded into the core directory.

    Sonia Superuser wants to get Symfony and friends out of the Drupal core repository, so she can have them in separate version control and use her own fork of Symfony etc.

    Right now, there is no way to get them out of the Drupal core repository until #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead lands. However, you can remove them from your project's repositories using the steps from the last use-case. Pointing your index.php file to the <root>/vendor directory will use the dependencies you specify rather than the ones in core/vendor.

    Joe Blogger does not like the commandline, and wants to operate with zip files and FTP.

    The proposed solution changes absolutely nothing for him. The package of Drupal is still fully-loaded and ready to go, no command-line needed. The only scenario I can see this becoming a problem, is if Joe wants to use a module that depends on a Composer package, however, this is no different than the current scenario with or without the patch. It is also no different than any other PHP project. Ideally the module maintainer should provide an alternative method of integrating the dependency without the command line.

    Drupal download + site install

    The proposed solution does not change this at all.

    Module download + module install + module update.

    The proposed solution does not change this at all. The traditional methods will still function normally.

    I hope that I've provided a better use case for this patch and why it's important.

    daffie’s picture

    As I have started as a "Joe Blogger" and looking at this issue from his eyes, I see a problem. Joe Blogger usually does not have a very expensive hosting. And if Joe Blogger want to install modules that require the updating of the composer.json file, he has two options:

    1. The first is he has a testing environment in which he installs composer and runs the composer update process. For the expert that maybe simple, but for Joe Blogger that will be very difficult. After running composer update he has to copy all files to his hosting provider. With zip files and ftp that is a lot of work.
    2. The second method is that he installs the module and his hosting provider supports the running of the "composer update" process. This will be for Joe blogger the preferred method. The problem in my eyes is that the hosting provider now must support this. Will this be a new requirement for drupal 8. If not, we will in my eyes, have to communicate to Joe Blogger and his friends that a module needs to change the composer.json file and run "composer update". And with this warning we get a sort of second class users who cannot use a number of modules. Is this something that we as a community want.

    I would like to mention that I support this issue very much. +1 for me!

    donquixote’s picture

    @davidwbarratt (#96)
    Thanks a lot for these explanations!

    The only scenario I can see this becoming a problem, is if Joe wants to use a module that depends on a Composer package, however, this is no different than the current scenario with or without the patch. It is also no different than any other PHP project. Ideally the module maintainer should provide an alternative method of integrating the dependency without the command line.

    And with this warning we get a sort of second class users who cannot use a number of modules. Is this something that we as a community want.

    I am seeing this from the perspective of a module maintainer. Currently (D7) I will depend on 3rd party libraries only if I really have to. Because I know it will be a pain for the average user.
    I am not a Joe Blogger myself (though I've been one at some point I guess). But I have to care about this type of user because I want to produce contrib modules that are useful to a wide audience.
    I would like a world where it is easy to add new dependencies in new module versions. And consider that these can have dependencies themselves.
    So, I think that "changes absolutely nothing for him" is insufficient.

    I think what we really need is a way for such users to run composer from the web UI.
    This could involve:

    • Making the composer.json and vendor/ directory web-writable (not a big friend of this)
    • Generating this stuff in the files/vendor directory, and then asking the user to copy it to vendor/ manually.
    • Providing a web service to build the vendor/ directory, and asking the user to download it.

    I think Composer manager already does this or not?
    So it is already one step ahead from a UX perspective compared to this patch.

    donquixote’s picture

    Another question is what file structure will we have after this patch? Will modules go to vendor/Drupal/(modulename) ?

    davidwbarratt’s picture

    #93,

    If the point is to have a project-level composer.json as well as a core-only composer.json, then the project-level settings will always take precedence. That's how Composer works.

    Thus, regardless of what ./core/composer.json says, the vendor directory will be where ./composer.json says it should be.

    I do not believe that is accurate. In my testing Composer uses the current working directory to pull the config from, or you can specify the working directory. If the working directory is the core folder, then that is the config that is used. Not only does it not use the config from the directory above, I'm not so sure it's even aware of it.

    This leads to the issue I brought up in #84: Composer will download drupal core for you and the repo will include a pre-populated core/vendor. Your root project file will also specify its own vendor directory, so now you have two copies.

    Correct, unless #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead lands. However, since you are modifying index.php to include the auto-loader from the <root>/vendor directory, rather than the core/vendor directory, none of the existing dependencies are actually being included into your project. I'm all for removing the vendor directory from the core folder, however, this is currently required by the packaging script, and unless that is changed in #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead there's really no point in discussing it here.

    This isn't such a big deal because your root's vendor/autoload.php will determine all the classloading, but it's a big deal in the sense that you have extra code laying around in your project directory.

    I agree 100% and I'm in full support of removing the dependencies from the repository in #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead, however that is outside of the scope of this issue, and it's really not a dependency of this issue either.

    This is, as I say, a cultural problem rather than a technical one. Just use Composer the way it's intended.

    What is "The way it's intended"? Do you have a source for this? I believe the way it's intended to be used, is that all third-party code is a dependency of your project. Isn't this exactly what we are doing? We're making Drupal a third-party dependency of our make-believe project. I believe that's exactly the way it was intended to be used. So, what are you talking about?

    Just kick the dependencies down the road. :-) It's not really a solution. At some point you have to decide "This is the top of Drupal." Currently, the top of Drupal is DRUPAL_ROOT.

    It helps if you separate "Drupal" from "Drupal core". If you don't, then yes, this whole issue makes 0 sense. I think the "root" of Drupal will always be wherever your index.php file is. However, you have to decide: What is the root of Drupal? Is it the root of Drupal core or the root of my project? I think since all of the code was moved out of the "root" of Drupal and into the core folder, this has already been decided. Also... just so you know, DRUPAL_ROOT was removed in #2328111: Replace most instances of the DRUPAL_ROOT constant with the app.root container parameter.

    Once core becomes a loosely-coupled collection of packages, I will delight in RTBCing this issue. That moment will be at least a year from now, optimistically.

    I believe this patch is a baby-step towards that reality. It's not moving us away from that, but rather towards that. I mean we are basically acknowledging the first decoupled piece, which is drupal and core. This is the same thing for Symfony and Laravel and many other PHP projects. I think after this is a reality and #2372815: [meta] Make the <root>/composer.json file a working example lands, we can then talk about making other subtree-splits of smaller components that could be used outside of Drupal. We need to establish the precedent that your project is the root (like any other PHP project) and that core is a dependency of that project.

    davidwbarratt’s picture

    #97 & #98,

    But I have to care about this type of user because I want to produce contrib modules that are useful to a wide audience.

    Then don't require Composer. :) I mean in my mind it's completely up to the module maintainer. If they want to support an alternative method they can, or they cannot (and limit their audience). We can't force anyone to provide an alternative. However, I think Composer is becoming the defacto standard in the PHP world, so it's worth talking about. The proposed solution does not encourage nor discourage module maintainers into using or not using Composer. It simply makes it a possibility.

    I think what we really need is a way for such users to run composer from the web UI.
    This could involve:
    Making the composer.json and vendor/ directory web-writable (not a big friend of this)
    Generating this stuff in the files/vendor directory, and then asking the user to copy it to vendor/ manually.
    Providing a web service to build the vendor/ directory, and asking the user to download it.
    I think Composer manager already does this or not?
    So it is already one step ahead from a UX perspective compared to this patch.

    Let's talk about Composer Manager a bit. I've already mentioned that I believe it's an antipattern.

    Let me clear up some things that might help. Composer Manager works by taking all of the composer.json files in all of your installed modules, and generating a new shinny composer.json for you to execute on the command line. It does this by shoving all of the requirements of each modules together into a single file. I'm assuming it does some sort of black magic (like composer) to resolve conflicts, but I'm not sure how complex it is. For Composer Manager to generate this file via the UI and not via drush, the web directory (or wherever you want to store your composer.json file) must be web writable. I think by default they put it in sites/default/files and they set the vendor-dir to some other place. Regardless, last time I checked, there was no way to execute composer commands from the UI. I haven't seen a UI for executing composer commands. I think this would be an incredible module and it would help a lot of people out (for the situations both of you described).

    Composer Manager does provide some challenges if you want to:

    1. Manually modify composer.json file (it will be overridden).
    2. Execute composer require (it will be removed).
    3. Add a module, theme, profile, site, etc. as a dependency of your project.
    4. Use a custom dependency for your project not attached to a module.

    For something to be in the end composer.json file, it has to be in a module. I think this sets up a Drupal-ism that doesn't need to exist.

    At any rate, the proposed solution does not nullify Composer Manager (for those that want to use it), nor does it make it more difficult. If anything, it actually makes it easier. Composer will no longer have to merge Drupal's dependencies with a module's dependencies. It can simply just modify the example composer.json file provided in this patch. :)

    Brian Altenhofel’s picture

    In an ideal world, core modules would be under /vendor/Drupal/. Contrib should be able to be under their own vendor if they opt to do so.

    davidwbarratt’s picture

    #99,

    Well that would certainly be nice. :) But Drupal does not support this, so the good folks over at Composer Installers have provided a Drupal installer that will install the dependencies in the proper location.

    Part of #2372815: [meta] Make the <root>/composer.json file a working example is to get (Add ability to install "drupal-core" projects to the project root) into Composer Installers.

    greg.1.anderson’s picture

    TL/DNR: There are errors in the suggestion below; might as well skip to #107.

    I would like to call out some of the awesome work that has been going on in another issue, #1886820: Packagist for Drupal Projects. Some fine folks have made Packagist projects for all of the repositories on drupal.org, and they are kept up to date. These resources can already be used to manage Drupal 7 builds with Composer, and similar techniques are being used in the WordPress community. Of course, Drupal 7 and WordPress do not ship with their own composer.json, so Drupal 8, which does, is a bit more complicated to manage. However, I think that it is possible to use these techniques for D8 without restructuring Drupal (which would require re-rolling all of the patches on d.o -- no thank you!), and without changing the way that Composer usually works.

    Consider the following prototypical composer.json file I threw together:

    {
        "name": "greg-1-anderson/composer-demo",
        "description": "A demo Composer project, modified from example at https://www.drupal.org/node/1886820#comment-9043671",
        "minimum-stability": "dev",
        "repositories": [
            {
                "type": "composer",
                "url": "http://drupal-packagist.webflo.io/"
            }
        ],

        "require": {
            "composer/installers": "~1.0",
            "drupal/drupal": "~8.0.0-beta3",
            "drupal/globalredirect": "8.1.x-dev",
            "guzzle/guzzle": "3.*",
            "phansys/getid3": "2.1.*@dev"
        },

        "extra": {
            "installer-paths": {
                "drupal": ["type:drupal-core"],
                "vendor/drupal/drupal/modules/{$name}/": ["type:drupal-module"],
                "vendor/drupal/drupal/themes/{$name}/": ["type:drupal-theme"]
            }
        }
    }

    You can put this in your own composer.json file today, and composer install will load its dependencies without throwing an error. (n.b. you will need a longer timeout to bring in Drupal 8 -- I use COMPOSER_PROCESS_TIMEOUT=3000 composer install)

    Unfortunately, we are not done yet, as the resulting project will not run. There are a couple of problems. First, the "installer-paths" directive that instructs Composer to put drupal-core into a "drupal" directory is ignored, and Drupal goes in vendor/drupal/drupal as usual (per existing Composer conventions). The installer paths directives for Drupal themes and modules do work, though, which is why for now I have them set to vendor/drupal/drupal/modules and themes--this puts the referenced modules in the right place, as mentioned in #103, above. If we fixed the installer-paths situation for Drupal core, then we would change the module directive to simply be "drupal/modules/{$name}/": ["type:drupal-module"],.

    We would then have the following project layout (for Drupal 8):

    myproject
    ├── composer.json
    ├── drupal
    │   ├── composer.json
    │   ├── core
    │   ├── modules
    │   ├── themes
    │   └── index.php
    └── vendor

    The web root does not move relative to where it is today -- it stays in the Drupal root folder, which is 'drupal', above. Contrib modules and themes go in modules and themes, as usual. Now, Drupal works the same way as any other Composer dependency -- you "require" it from your composer.json, and Composer loads it into its own directory. Folks can continue to use Drupal + drush dl or drush make, as they do today, or they can use Composer, and the end result will be very similar (except that using Composer will avoid problems w/ dependencies as intended, whereas Drupal + drush dl require Composer Manager, or some other non-ideal workaround.)

    The only remaining problem we have with this idea is that Drupal currently expects that the vendor directory should be in the Drupal root, whereas in this example it would be somewhere else (in the Drupal root's parent folder). It seems to me that it would be much easier to solve this problem than to do all of the things that #2372815: [meta] Make the <root>/composer.json file a working example would require.

    I am going to continue to investigate along these two lines -- fix the install path directive for Drupal Core, and allow Drupal to find the vendor directory when it is loaded as a dependency of some other project. I'll cross-post references to any issues I open here. If anyone here has any ideas why this might not work, or wants to help push things along in this direction, please let me know.

    davidwbarratt’s picture

    Issue summary:View changes

    #104,

    Your comment is so ignorant that it doesn't really dignify a response, but since you seem very passionate that I (and many other people on this thread) are completely stupid... here goes.

    #1886820: Packagist for Drupal Projects

    Many of the same people on this thread are also on that one. I've read the whole thing, I've commented on both issues, and I think they both have their merits for different reasons. I (and many others) have put a lot of work into both issues so I'd appreciate at least a little respect by perhaps taking the time to fully understanding the differences between the two issues.

    However, I think that it is possible to use these techniques for D8 without restructuring Drupal

    There are some radical "Ideals" that are talked about in this issue (even by myself) of restructuring Drupal, but the real and proposed solutions, do not in any way mean restructuring anything. I'd love to take the ideals out of the Issue Summary, but it seems like every other comment someone is bringing up an Ideal that's already been discussed at great length. In your case, you didn't seem to read the "Real Solution", but perhaps this needs to be renamed to "Proposed Solution" so I'll do that.

    which would require re-rolling all of the patches on d.o -- no thank you!

    That is not accurate. The proposed solution does not mean re-rolling all of the patches on d.o. It is not that radical of a change.

    without changing the way that Composer usually works.

    As I discussed in #100 we are not changing the way "Composer usually works" (whatever that even means).

    Unfortunately, we are not done yet, as the resulting project will not run. There are a couple of problems. First, the "installer-paths" directive that instructs Composer to put drupal-core into a "drupal" directory is ignored, and Drupal goes in vendor/drupal/drupal as usual (per existing Composer conventions).

    I wrote a Composer plugin that solves this problem, but it does not solve the root of the problem that I'll get to in a moment.

    The installer paths directives for Drupal themes and modules do work, though, which is why for now I have them set to vendor/drupal/drupal/modules and themes--this puts the referenced modules in the right place, as mentioned in #103, above

    This, is the major problem. And it's not easily solvable. Actually, this is not at all what I described in #103. What's the difference between what I said and what you just described? The difference is now you are putting dependencies inside another dependency. This isn't a problem when you first run composer install but if Drupal core is upgraded and you run composer update, Composer will destroy everything in the vendor/drupal/drupal and put Drupal back. It does this, and it will not put your modules/themes or any custom code you've written back into it. It will be gone. If you are wise and you don't put your vendor directory in version control (because you're not supposed to), then anything you've done inside of Drupal is instantly destroyed. :(

    This issue, makes core a dependency of drupal that way you can put third-party code where it's supposed to go (or your own custom code) and upgrading Drupal will not destory your code).

    There is a work-around to this that I've figured out for Drupal 7, but the solution is pretty complicated and involves adding symlinks everywhere and basically gives Drukpal 7 the Drupal 8 structure anyways. Of course these steps have to be completed each time Drupal is upgraded, which is why it is scripted.

    If we fixed the installer-paths situation for Drupal core, then we would change the module directive to simply be "drupal/modules/{$name}/": ["type:drupal-module"],.

    You can fix this now with my plugin, but you'd still have the problem described above.

    The web root does not move relative to where it is today -- it stays in the Drupal root folder, which is 'drupal', above. Contrib modules and themes go in modules and themes, as usual. Now, Drupal works the same way as any other Composer dependency -- you "require" it from your composer.json, and Composer loads it into its own directory.

    That is not accurate either. It's not the way any other Composer dependency works because you've just put dependencies inside of a dependency which is the antithesis of the way Composer works.

    This issue, however, allows people to use Drupal core "the same way as any other Composer dependency -- you "require" it from your composer.json, and Composer loads it into its own directory."

    Folks can continue to use Drupal + drush dl or drush make, as they do today, or they can use Composer, and the end result will be very similar (except that using Composer will avoid problems w/ dependencies as intended, whereas Drupal + drush dl require Composer Manager, or some other non-ideal workaround.)

    This issue does not prevent people from using drush in anyway.

    The only remaining problem we have with this idea is that Drupal currently expects that the vendor directory should be in the Drupal root, whereas in this example it would be somewhere else (in the Drupal root's parent folder).

    Yes, that is a problem with your solution or with this issue. However, with your solution, you've introduced another issue. Now you have to go into a dependency and edit a file in the dependency itself. This will cause Composer to warn you that the dependency has been modified before it allows you to continue with an update, if you do update Drupal it will destroy your changes to index.php.

    This issue on the other hand will not override your changes to index.php whenever you upgrade Drupal core. I'd like to see #2380389: Change index.php to use a vendor directory in the root so the changes to index.php are as minimal as possible, but if #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead makes it, it probably wouldn't even be necessary to change index.php.

    It seems to me that it would be much easier to solve this problem than to do all of the things that #2372815: [meta] Make the <root>/composer.json file a working example would require.

    Well I'm all ears to hearing a solution, in #2372815: [meta] Make the <root>/composer.json file a working example I've provided a clear outline of how all of this is going to work and as far as I know, there are no problems to be solved, so if you do see a problem, please comment on one of the related issues.

    I am going to continue to investigate along these two lines -- fix the install path directive for Drupal Core, and allow Drupal to find the vendor directory when it is loaded as a dependency of some other project. I'll cross-post references to any issues I open here. If anyone here has any ideas why this might not work, or wants to help push things along in this direction, please let me know.

    Here is how you can help: Test the patch, if the patch works for you, mark as RTBC. This issue solves all of your problems, you don't need to look for solutions anymore unless you can point out legitimate problems with the most recent patch.

    I'm sorry if I'm being a little too sassy, I just feel like you provided an inferior solution without even attempting to understand the currently proposed solution.

    greg.1.anderson’s picture

    #105, thank you for taking the time to respond to my comment. First off, let me apologize profusely on your first point -- I clearly do not think that you or anyone else on this thread are stupid. The reason I posted here was to get your feedback, because you have clearly done a lot of valuable work here. To that end, I also apologize for posting prematurely. I should not have brought up any new ideas at this stage without completing the thought and producing code to go with it. Mea culpa for bringing an IRC or idea-thrown-across-the-table-at-a-sprint to a more formal setting.

    My final apology is for comments that I made on rejected solutions that appear in the issue summary. I did not intend to slight the current patch with those remarks, and should have deleted them entirely. Thank you also for pointing out that putting modules and themes inside of a composer-managed directory is a bad idea. I realized that after I walked away from the computer. I needed to leave, and should have left my thoughts as a draft rather than pressing "save".

    In any event, I will return here iff I have actual improvements or contributions to the ideas here.

    davidwbarratt’s picture

    I've updated the Issue Summary to make it cleaner and fit within the template.

    Since the "Ideals" are not possible in Drupal 8.0.x, I've moved them into two Drupal 9.x issues:
    #2385387: Permanently split Drupal and Drupal core into seperate repositories
    and
    #2385395: Make Drupal core folder agnostic and allow it to be placed in vendor/drupal/core

    We can talk about what Drupal should Ideally be there since Drupal 9.0.x is probably a long ways away and few patches have been written against it, disruptive ideas like the ones above should be left until then.

    As far as this issue, I think the changes are only minimally disruptive (if at all) and I really hope that someone can test the most recent patch and mark it as RTBC so we can get it in Drupal 8.0.x before it's release and start using Drupal with Composer. :)

    greg.1.anderson’s picture

    I tested this -- at least, I tested the prepared repository, I did not apply the patch to Drupal 8 and make my own split core.

    This is what I did:

    • Copied the prepared composer.json from the summary to an empty folder /path/to/d8-test
    • cd /path/to/d8-test && composer install
    • cd /path/to/existing-d8 && cp .csslintrc .e* .htaccess index.php README.txt robots.txt web.config /path/to/d8-test
    • Edited index.php to autoload the vendor at the Drupal root rather than the one in core
    • drush qd --use-existing --root=/path/to/d8-test

    Drupal came up just fine. I also ran 'composer update' in the 'core' directory, to update the unused vendor directory. A couple of components were updated; seemed like all was well here.

    I think that the split of composer.json into the core directory is fine, and will cause no problems. The one area that I think might need more testing is the non-working/example composer.json at the root of the project. I don't know if there are any circumstances where it might cause problems for this to be a non-working file. Are there any scripts or tools that will need to be updated, for example, to point to core/composer.json? Since there was a move in the other direction in the past, it is probably not intractable to go back; however, there are folks on this thread better suited than I to determine what testing is needed here.

    daffie’s picture

    Status:Needs review» Needs work
    Issue tags:+Needs reroll

    #2377281: Upgrade to Symfony 2.6 stable has landed, so this needs a reroll.

    moshe weitzman’s picture

    Status:Needs work» Reviewed & tested by the community
    Issue tags:-Needs reroll

    Patch looks good to me, and moves us ahead a bit.

    +++ b/composer.json
    @@ -1,48 +1,24 @@
    +  "extra": {
    +    "_readme": [
    +      "This is an example file to show how a Drupal website can be managed via",
    +      "Composer. It does not work out of the box but requires a Git subtree",
    +      "split of the core directory to be added to the repositories",
    +      "section above."
    +    ]
       }

    I'd love to get more detail here or a link to a docs page. We can do that in #2372815: [meta] Make the <root>/composer.json file a working example (a followup).

    daffie’s picture

    Status:Reviewed & tested by the community» Needs work

    @moshe weitzman: #2377281: Upgrade to Symfony 2.6 stable updates the composer.json file. In the current patch for this issue still uses symfony 2.6 beta1. So it needs a reroll. The basis of this patch also looks good to me.

    daffie’s picture

    Issue tags:+Needs reroll
    davidwbarratt’s picture

    Status:Needs work» Needs review
    Issue tags:-Needs reroll
    StatusFileSize
    new7.54 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch custom-composer-1975220-113.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    Attached is the Re-Rolled patch.

    Status:Needs review» Needs work

    The last submitted patch, 113: custom-composer-1975220-113.patch, failed testing.

    davidwbarratt’s picture

    StatusFileSize
    new7.47 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,079 pass(es).
    [ View ]

    doh! and again. :)

    davidwbarratt’s picture

    Status:Needs work» Needs review
    davidwbarratt’s picture

    Issue summary:View changes

    #108,

    There isn't anything that I'm aware of that will need to be changed other than what's in the issue summary.

    Also, you might try doing something like this:

    composer create-project --repository-url="http://davidwbarratt.com/packages.json" drupal/drupal path/ dev-master

    which should copy the drupal project and load core and all it's dependencies.

    After #2372815: [meta] Make the <root>/composer.json file a working example is complete, you'll be able to run:

    composer create-project drupal/drupal path/

    which is the same instructions Symfony uses as it's primary download method.

    greg.1.anderson’s picture

    Slight typo in #117. This worked:

    composer create-project --repository-url="http://davidwbarratt.com" drupal/drupal path/ dev-master

    daffie’s picture

    Issue summary:View changes
    Status:Needs review» Reviewed & tested by the community

    I installed a new drupal site with the command:

    composer create-project --repository-url="http://davidwbarratt.com" drupal/drupal path/ dev-master.

    This resulted in a working drupal site that is the same as drupal head with the patch from this issue installed. He added something extra in the required part of the /composer.json file to show that it works.
    The code in the patch looks good.
    The testbot gives it green.
    All looks good to me, so RTBC from me.
    Moshe weitzman finds the patch also RTBC.

    mradcliffe’s picture

    Status:Reviewed & tested by the community» Needs review
    Issue tags:+Needs manual testing

    I'm not sure if there is a clear enough test plan to consider this RTBC. The test in the issue summary only covers installing core and doesn't go over updating core.

    Manual testing for allowing "manage (custom) dependencies" should also cover:

    1. Install a module that has a custom PHP dependency.
    2. Update core and confirm that site is not broken with installed module with custom PHP dependency.
    1. Install a module that has a custom PHP dependency.
    2. Install a second module that has a different PHP dependency that may share dependencies with the first PHP dependency.
    3. Update core and confirm that site is not broken with installed modules and PHP dependencies.

    I'm not sure how to do the last step in that test flow - how do we test updating core if we're testing the latest HEAD?

    What I would like to see documented by testing is what the user experience is going to be in those scenarios for dev ops (manual or automated) or site builder.

    davidwbarratt’s picture

    #120,

    I don't think all of that is necessary because you're basically asking to test composer itself, but I'll play along...

    Do you know of any 8.x contrib modules that have a composer.json file and require a php dependency (with composer)? Do you know of more than one? if you do I can make an example for you.

    If not I can probably create an example with #1886820: Packagist for Drupal Projects, but most require drupal/drupal which obviously conflicts with this patch, since it would technically require drupal/core once it's commited. If we really need to get around this, pick a 8.x module and I'll make a test composer.json file for it.

    As for doing a composer update of Drupal core.. the easiest way to do that would be to fork my repo:
    https://github.com/davidbarratt/drupal-core

    set your fork as the repository url:
    https://github.com/davidbarratt/drupal8/blob/master/composer.json#L15

    then run composer update

    now go make a change against your fork and commit it to the 8.0.x branch. then go and execute composer update and it should move core to your commit. If you were to tag your commit, it will checkout the tag rather than the commit (just like any other dependency).

    However, like I've said, you're really asking for tests of Composer, not of Drupal or of this patch. This patches aim is not to solve all the problems with composer, but rather make it possible for people to modify the composer.json included with Drupal. With the most recent patch this is a non-working example and their is a follow up to #2372815: [meta] Make the <root>/composer.json file a working example where all of these issue should be worked out.

    daffie’s picture

    I have to agree with davidwbarratt that adding PHPUnit tests are not necessary for this issue, because you are basically asking to test composer itself.

    But maybe we can convince mradcliffe that the patch is working as we want with two examples.

    1. The first example in my eyes would be to create a packages.json file. When used with the command "composer create-project" will create a new drupal site that is identical to drupal head with the patch from this issue applied.

    2. The second example would be to create a very basic package with a class with one function in it. The function would only return a "hello world" string. Or something in that order. Then using the basic drupal site created in example one, edit de composer.json file in the root directory and add the created package in the right place. Be very specific. Then run the command "composer update". Also be very specific what that command must be and in which directory and so. Then create a new module that requires then new created package. The module must be very simple and only get the "hello world" string from the package and put it on the screen. After this patch has landed I think that it would be a good idea to add this example to the https://www.drupal.org/project/examples project.

    mradcliffe’s picture

    StatusFileSize
    new762 bytes
    new758 bytes

    @davidwbarratt, I do think it's necessary to run through some manual tests to make sure that the patch works for devops, developers, and site builders and not just an automated flow. I'm not sure if unit tests are necessary either and I wasn't trying to suggest them, @daffie.

    @davidwbarratt, I know a couple of existing modules for d8, but they've been changed to depend on Composer Manager now: Omnipay and Xero API. I also was working on a couple of test modules after my last comment. I've attached them now, but I haven't tested them. They simply define a service provided by a third party dependency.

    @daffie, your example in #2 is pretty much what I'm looking for as a site builder user. In other words, an example work flow for a site builder who's downloading Drupal updates from the site or with drush would be the following:

    1. Download Drupal (extract tarball if not using drush).
    2. Install Drupal.
    3. Download module (extract tarball if not using drush).
    4. Modify composer.json per module's instructions.
    5. Run composer update.
    6. Install module.
    7. Download Drupal (extract tarball if not using drush).

    My expectation as a user is that my site should not be broken and I should not have to redo changes to composer.json that I made in #4. Anything less is going to aggravate me as an user or developer who's providing module support.

    If that work flow is working right now with whatever testing has been done, then put it back to RTBC.

    greg.1.anderson’s picture

    #123,

    Isn't the proposed workflow more like this:

    1. composer create-project --repository-url="http://davidwbarratt.com" drupal/drupal path/ dev-master
    2. drush si ...
    3. composer require drupal/somecontribmodule
    4. drush en

    I can get #1 and #2 to work today, but I have trouble with step #3. I believe that this should work in theory, but it currently doesn't; although I'm not certain why, I think it is because the type of drupal/drupal was changed from "drupal-core" to "project" (with the "core" directory taking the existing "drupal-core" type), which makes drupal/drupal incompatible with the metadata currently in the projects on http://drupal-packagist.webflo.io/. At least, that's what I suspect is the case, if I understand #117 and the referenced Symfony documentation correctly.

    So, what I think would be really helpful here would be one or two example Drupal modules published the same way that drupal/drupal and drupal/core are today on http://davidwbarratt.com, with an appropriate composer.json in drupal/drupal that routes Drupal extensions brought in via composer require to the modules or themes directory, as appropriate.

    This suggestion would change the scope of the issues slightly, with this issue becoming a "working example with 3rd-party infrastructure", and #2372815: [meta] Make the <root>/composer.json file a working example becoming a "working example with drupal.org infrastructure". Just my opinion, mind you; a compromise between testing Composer functionality, and showing that the split works to bring in modules via Composer. I think that the current patch would be pretty easy to test like this; I'll try it myself when I have a chance, but I think it would be even faster for someone with the infrastructure already set up to try.

    I make this suggestion in the spirit that it seems even easier than #123. The steps in #123 also seem to be an adequate test to me, if they are in fact easier to perform, so don't mind me if no one wants to set up the test infrastructure.

    greg.1.anderson’s picture

    Status:Needs review» Reviewed & tested by the community

    I thought that extending http://davidwbarratt.com/packages.json to include a reference to a Drupal module would be sufficient to get Composer to pick up the package, but it seems not. I'm just going to leave this link to Satis here for later reference.

    Anyway, I tried to test per #123, but modified slightly based on my understanding.

    1. Download Drupal -- composer create-project --repository-url="http://greenknowe.org" drupal/drupal web dev-master
    2. Install Drupal -- drush qd ...
    3. Download module -- drush dl globalredirect (since I can't get composer require to work yet)
    4. Modify composer.json per module's instructions -- skipped, but made a local modification for testing
    5. Run composer update
    6. Install module -- drush en globalredirect
    7. Download Drupal -- ???

    Not sure what testing steps were intended by 7. To update drupal/core, (the "core" directory in your Drupal root), use composer update. This does not modify your composer.json or other files not managed by composer (e.g. modules directory), so there is no danger of aggravation here. What if index.php or some other file at the Drupal root changes upstream? If, in step #1 you answer "no" when composer create-project asks you if you want to remove the vcs files, then you can pull in these updates via git pull. This is subject to the normal rules of git; if you have modified composer.json, you can check your modification in on a branch, and use git merge to combine with changes from upstream. Under the proposal in this issue, the drupal/drupal composer.json has only a reference to drupal/core, and therefore should never change -- so you shouldn't ever get merge conflicts when you do this.

    Now, if you decided to remove the vcs files in step #1, then that is another matter. In that case, you must track upstream files via your own methods, in which case you are responsible for managing your own aggravation (i.e. do not overwrite your own local modifications if using tar to unpack a bundle that might contain an unmodified copy of composer.json).

    Perhaps this is the level of testing that #123 was looking for?

    davidwbarratt’s picture

    #123,

    Thanks for your help, let me step through this and see what we can do.

    1. You can do this normally. This patch adds the ability to also do this by executing:
      composer create-project --repository-url="http://davidwbarratt.com" drupal/drupal path/ dev-master

      repository-url and the version are not necessary when/if #2372815: [meta] Make the <root>/composer.json file a working example lands, in that scenario you'll be able to run:

      composer create-project drupal/drupal path/
    2. This will be done normally and this patch doesn't change that.
    3. This will be done normally, however, we've also added another way of doing this.
      You could either execute a command like this:
      composer require drupal/xero ~8.0

      or you could modify your composer.json file and add:

      "drupal/xero": "~8.0"

      and then execute:

      composer update

      However, this doesn't currently work, and that doesn't have anything to do with this patch, it doesn't work becuase the current version numbers in contrib are not compatible with Composer. There is an effort to fix this in #1612910: Switch to Semantic Versioning for Drupal contrib extenstions (modules, themes, etc), however, the version numbers can be translated through a custom version of packagist which is the effort that is being applied at #1886820: Packagist for Drupal Projects.

      Because Composer will first read the data from a "packagist" we can trick it to perform tests for us.

      to get these modules installed, I added fictional "8.0.0" versions of them to my packages.json file.

      Now you can add the following to your composer.json file:

      {
        "type": "composer",
        "url": "http://davidwbarratt.com"
      }

      and you'll have access to these modules in the fake versions.

      (Xero however requires another repository to be added to the root file).

      I've compiled all of this into a kitchen sink for your testing.

      You can get the kitchen sink by executing this command:

      composer create-project --repository-url="http://davidwbarratt.com" drupal/drupal path/ dev-kitchen-sink

      That will download/clone the kitchen sink repo and execute composer install.

      In the kitchen sink I have all 4 modules that you mentioned being loaded via Composer. because they are loaded via composer, there is no need to add their dependencies to your root composer.json file, Composer will go grab those dependencies and add them to the root of your project for you.

    4. You can certainly go do that, just modify the composer.json file as your instructed to do. I've done you one better though and allowed the entire module to be loaded via Composer. However, let's say you didn't want to do that and you wanted to manually install the module and manually handle the dependencies. Sure, so if this were test1, you'd be instructed to add this requirement:
      "knplabs/github-api": "~1.2"

      You can do this and it will install it like any other dependency, if you've modified your index.php to point to the correct autoloader, then you are good to go. Although a module might depend on a lot of different dependencies, and those dependencies might get updated in the future, so ideally you'd just install the entire module with Composer which will allow the module maintainer to handle the dependencies. This way the only thing the admin has to do to update the module is run composer update which will update the module itself, plus any dependencies (including ones that have been added/removed).

    5. That should be fine to execuate at any time. Right now composer.json is a non-working example, but there is an effort underway to change that #2372815: [meta] Make the <root>/composer.json file a working example.
    6. Nothing in this patch should change that.
    7. If you were to update Drupal manually, you'd need to not override composer.json. This is similar to the way .htaccess and the modules folder is handled. However, you can update Drupal core via composer by simply executing composer update which seems like an easier way to update Drupal than to ensure that composer.json and index.php are not being overriden. Actually, if you were to manually update Drupal core, you'd have to run composer install anyways (which would delete the core folder and add it back) so that the dependencies would be updated. Basically if you are going to use Composer, you need stick with it. But it's easy to get out of it if you want, just point your index.php file back to core/vendor/autoload.php.

    My expectation as a user is that my site should not be broken and I should not have to redo changes to composer.json that I made in #4. Anything less is going to aggravate me as an user or developer who's providing module support.

    I agree. I think the only way you'd lose changes to composer.json is if you manually overrode them, but this is the same thing as modifing .htaccess. However, as I stated above, you shouldn't be copying files over at all, you should just let composer update everything.

    Mile23’s picture

    Status:Reviewed & tested by the community» Needs work

    @davidwbarrat #122: This patches aim is not to solve all the problems with composer, but rather make it possible for people to modify the composer.json included with Drupal.

    Again, the concept is 'hacking Drupal.'

    I asked about it, and @webchick mentioned in IRC that 'hacking drupal' is basically equivalent to requiring a merge to pull Drupal from the repo.

    So if you just make part of Drupal core a dependency of Drupal... uh.. front controller let's call it, and then change the front controller's composer requirements in any way, then your git pull will require a merge. So it's the same problem, only now you have two composer.json files.

    I've been hacking around on embedded composer a bit, and while I haven't been able to make a proof of concept (due to time constraints), it does look like a reasonable alternative to what you're doing here.

    Embedded composer lets you define your own parallel alternative to composer.json files, and these can then be used for dependency management by the app.

    This way, rather than modifying the root composer.json, you'd make a module with a drupal.json (or whatever name) file, and the install process would pull down the dependency through composer's solver. You can then put these dependencies in a separate vendor directory, with whatever name you want.

    Embedded Composer here: https://github.com/dflydev/dflydev-embedded-composer

    davidwbarratt’s picture

    #127,

    I hope you know you are driving me crazy. I'm not sure what your issue is, but you still haven't provided a legitimate alternative. I'm not really sure what your real issue is with this solution or with me personally, but switching this back to "Needs Work" because you don't like it is annoying at best.

    I asked about it, and @webchick mentioned in IRC that 'hacking drupal' is basically equivalent to requiring a merge to pull Drupal from the repo.

    right... but somehow we still have .htaccess and example.gitignore in the root. The former is intended to be modified, the latter is intended to be copied and modified. However, these files, are probably not going to change during the life cycle for Drupal 8, so even if you did pull the Drupal repo, there wouldn't be a merge conflict.

    However, Composer is different, because I think it's extremely likely that the composer.json file will be changed many many times over the course of Drupal 8's life cycle. I mean I've already re-rolled this patch like 5 times already just for changes to composer.json. If there is any reason what-so-ever to update Drupal's dependencies that has to be done via composer.json. Also by allowing people to modify composer.json we're saying that it's ok to mess with Drupal's dependencies, which could create major incompatibilities.

    This issue creates a space for people to modify composer.json with a very very minimal chance that it is updated at all and if it is, that it wont conflict anyways. It also prevents people from messing with Drupal's dependencies which, by making core a dependency of drupal, are enforced by Composer.

    So if you just make part of Drupal core a dependency of Drupal... uh.. front controller let's call it

    The front-controller is index.php it's a technical item, not a whole package/project, but sure,

    and then change the front controller's composer requirements in any way, then your git pull will require a merge. So it's the same problem, only now you have two composer.json files.

    I don't think that's correct, because the "front controller's" composer.json file is not going to change (or if it does, it will be minimal. We don't need to change this file to update Drupal's dependencies (which might need to be updated to fix bugs or security issues). So you can pull and you wont have a merge conflict since it contains only your changes. :)

    Also, as I said before, having drupal core as a dependency of drupal means that composer will enforce Drupal's dependencies. There isn't a chance for a mix-match (where someone update Drupal, but didn't update the dependencies, etc.).

    I've been hacking around on embedded composer a bit, and while I haven't been able to make a proof of concept (due to time constraints), it does look like a reasonable alternative to what you're doing here.

    Embedded composer lets you define your own parallel alternative to composer.json files, and these can then be used for dependency management by the app.

    This way, rather than modifying the root composer.json, you'd make a module with a drupal.json (or whatever name) file, and the install process would pull down the dependency through composer's solver. You can then put these dependencies in a separate vendor directory, with whatever name you want.

    So basically you want to setup another drupal-ism? I mean you might as well say "hell with it" and just document that you're not allowed to use Composer with Drupal 8. That seems rather foolish. The industry is moving towards using composer, Drupal 8 uses composer, we need a way for site admins to be able to use Composer too.

    However, if you can come up with a solution that is not a Drupal-ism, then I'm all ears. Otherwise, please stop changing the issue status. Just create a followup issue to this one if it's really a problem, but reverting #1805316: Move composer.json to the project root doesn't seem like it would prevent what your proposing anyways.

    If it would make everyone happy we can rename composer.json to example.composer.json thereby rendering your argument NULL. We've already talked about this on this thread, but we decided the follow up would be better #2372815: [meta] Make the <root>/composer.json file a working example.

    davidwbarratt’s picture

    Status:Needs work» Needs review
    daffie’s picture

    Let me start by saying that the world does not revolve around Drupal. If it revolves around something then it is your (clients) project. And if you want to use Drupal in that project, that is wonderful. But that does not mean that the inclusion of Drupal means the exclusion of everything else. If I want to include Drupal and one or more other packages I should be able to do so.

    Having said that, and looking at the current state of Drupal 8.0.x-dev, I can see that Drupal has chosen the Composer project to manage its dependecies. I am glad that Drupal has chosen Composer as it is in the PHP-world the standard for managing dependencies. That all combined is it logical to use Composer also to add my other dependencies to my project. The problem is that Drupal 8.0.x-dev has only composer.json file and in that file are all the dependencies of Drupal. And with it the problem that file get regular updates from the Drupal project. Updates are essentialy good, but if I add my extra dependencies for my project to the file, then I have to carefully check after every update of Drupal if the update overwrites the composer.json file. I have do enough support work in my life to know that this will result in a support nightmare.

    The only viable solution in my eyes is to have at least two composer.json files. One for the Drupal package with all its dependencies and an other one for all the other dependencies I want to add to my project. The best solution for this is in my eyes is the one that is proposed in this issue. One composer.json file in the /core directory for the Drupal package and the other one in the directory itself for my other dependencies. With the added benefit that we stick to the mantra "never hack core".

    We are using Symfony as a dependency of our Drupal project, just as somebody else will use Drupal as a dependency of their project. Drupal is not the top project, just somewhere in the middle. That does not mean that I do not think that Drupal is not important or that I do not love Drupal. I most certainly do.
    Composer is not made for just adding dependencies to the Drupal project. It can and is made to add dependencies to every project and Drupal is just one of all the possible dependencies to add.

    @davidwbarrat: In my comment #122 I propose two examples to test if it works:

    1. The first example in my eyes would be to create a packages.json file. When used with the command "composer create-project" will create a new drupal site that is identical to drupal head with the patch from this issue applied. If this works then the command "composer update" will also work. And dependencies to the Drupal project can be added, updated and deleted. Very important!
    2. The second example would be to create a very basic package with a class with one function in it. The function would only return a "hello world" string. Or something in that order. Then using the basic drupal site created in example one, edit de composer.json file in the root directory and add the created package in the right place. Be very specific. Then run the command "composer update". Also be very specific what that command must be and in which directory and so. Then create a new module that requires then new created package. The module must be very simple and only get the "hello world" string from the package and put it on the screen. After this patch has landed I think that it would be a good idea to add this example to the https://www.drupal.org/project/examples project. This is to show that adding extra dependencies works. Nothing fancy, it is just to show that is works.

    If you can do this I will give this patch a RTBC.
    This is in my eyes a very important issue.
    And if for some reason the committers do not like this issue/patch, let them say it themselves.

    Oh and #2386585: Upgrade to Symfony 2.6.1 has landed so you need another reroll. :-(

    greg.1.anderson’s picture

    #130,

    Your test #1 is possible today; I did it in #125, and it works. #126 enhances the package to include a reference to some example Drupal modules. I think that this is a more important test than what you suggest in #2; general Composer dependencies are just going to work. Drupal modules are a special kind of Drupal dependencies that should be shown to work, I think. I couldn't get this to work, but I'm sure it was a user error. I won't have time to test #126 this weekend, but it looks to me like it should work.

    The remaining issue is whether it is okay to make the front-end controller user-modifiable or not. Some seem to think this is okay; we'll see what the core committers think. I understand from Moshe that there is an issue to move logic from index.php into core, which will help here a lot. The largest remaining issue then would be the possibility that some folks might miss updates to .htaccess if they only do composer update. Perhaps this is tractable.

    mradcliffe’s picture

    #124,

    I've followed the composer issues as a contrib developer who wants to be able to provide clear instructions for all types of users - not just composer power users. The issue seems to have morphed into providing a solution only for those power users and not just for modules that want to include dependencies in way that can be used by all users. My test examples are to make sure that the proposed solution works for all users who want to install a module with a third party PHP dependency via composer.

    daffie’s picture

    @mradcliffe: The solution for non composer power users will come from contrib. That will be fixed. But for that to be fixed in contrib we need this core patch to make it all possible. So please do not worry about non composer power users.

    mradcliffe’s picture

    @daffie, I disagree. We should not shun non-power users. I think that Drupal should remain accessible to all users, and that contrib maintainers should strive to provide the best user experience for all users and not a subset of users. The initial point of these composer issues were to solve this in core as it's clunky and even more convoluted to try to solve this in contrib.

    Additionally, with #115, we need to document that a user cannot simply just extract Drupal to update their site, but that they need to diff between /composer.json and /composer.lock or else end up with a broken site after composer update and upload.

    davidwbarratt’s picture

    #134,

    Why would they need to diff? There wont be any (or very few) updates to <root>/composer.json (with this patch) so they can just update everything in the core/ folder or just execute composer update and it will update the core folder for you.

    All of the dependency updates will be made in core/composer.json so no need to diff. :)

    Also.. what is a "composer power-user"? If you as a module maintainer want to ask your user to specify a needed dependency, this solution allows you to do that. Without the patch, you're modifying Drupal core's composer.json file, which no module should ask someone to do.

    Do you have a better solution?

    daffie’s picture

    Status:Needs review» Needs work
    Issue tags:+Needs reroll

    The patch needs a reroll, because #2386585: Upgrade to Symfony 2.6.1 landed.

    I have made my own "hello world" module and used https://packagist.org/packages/abbert/helloworld as a source. It works fine for me. If it still works after the reroll it will get a RTBC from me.

    davidwbarratt’s picture

    #130,

    1) This is already done, checkout:
    https://github.com/davidbarratt/davidwbarratt/blob/master/web/packages.json
    and I'm hosting that here:
    http://davidwbarratt.com/packages.json
    However, there are more comprehensive ones in #1886820: Packagist for Drupal Projects, but none of them account for the changes of this patch.

    2) I can do this if you'd like, I haven't made a Drupal 8 module yet, so I'll have to learn that. :) However, I think the examples in #128 are sufficient, but it's probably important to have an example for other developers to follow, so I'll work on that.

    I'll re-roll the patch next. Anything else?

    daffie’s picture

    @davidwbarrat: Are you interested in my super simple hello world module. Works in combination with https://packagist.org/packages/abbert/helloworld.

    mradcliffe’s picture

    #135,

    Because composer.json will be overwritten each time a new version is extracted when a user updates Drupal and the user will lose their dependencies in composer.lock after composer update!

    davidwbarratt’s picture

    Status:Needs work» Needs review
    Issue tags:-Needs reroll

    #138,

    Nice! I think our messages crossed. I'll take a look at that.

    I was able to apply the patch cleanly to to the current HEAD (with the Symfony upgrade). I believe since there were not changes to composer.json and composer.lock is only renamed by this issue, nothing was an issue. I checked core/composer.lock and I saw Symfony 2.6.1 so I think we're good on that.

    Just in case, I queued #115 for testing again.

    davidwbarratt’s picture

    #140,

    That already happens with .htaccess. Plus if you just extract the whole Drupal archive you're going to override your modules, themes, and sites as well. You've always had to extract without overriding those folders / files. This one just prevents you from having to diff against whatever is changed in Drupal.

    Everytime you run composer update Composer re-writes composer.lock. This is a feature, not a bug and there's nothing we can do to change that. If you use Composer, it's better to update Drupal core via Composer (and this should be documented). If you are going to do it manually, composer.json needs to be handled with care, just like .htaccess, just like modules, sites, themes; etc.

    mradcliffe’s picture

    #142,

    And that's what this issue should be solving! Allowing custom dependencies in composer.json/lock so that they're not overridden when you update Drupal!

    Does Composer have a way to compare the differences between a composer.json file and composer.lock file? It really should so that one doesn't blow away dependencies in composer.lock that aren't in stored composer.json file. I think that's the only solution for Drupal 8 as-is that works for all users.

    daffie’s picture

    @mradcliffe: All of your own projet dependencies will be in /composer.json. All of the drupal/core dependencies will be in /core/composer.json. If this is what you are asking.

    mradcliffe’s picture

    +++ b/composer.json
    @@ -1,47 +1,24 @@
       "require": {

    @daffie, The contents of this file will always be overridden when you extract Drupal and thus you will lose your changes to it.

    Edit: and by default, Composer will then blow away composer.lock when doing an update.

    It would be much preferable if there was a Composer command to merge or resolve changes between json file and lock file.

    davidwbarratt’s picture

    If you need to update Drupal simply execute
    composer update
    ta-da! Drupal is updated and all of it's dependencies as well as your custom dependencies, without overriding your composer.json file.

    That ^ is impossible without this patch.

    Also, not having this patch, doesn't solve your "problem" so we have a solution and it's that ^. Do you have a better solution? If we do nothing, that still has the same problem you've mentioned, except now you can't update Drupal with composer and avoid overriding your composer.json file.

    Alternatively, you can just copy the "core" directory from the archive and override your "core" directory, but I'm not sure why you would do that, because you'd also need to copy the dependencies from core/vendor into <root>/vendor so it's not a smart idea either way, so you should just update Drupal with composer (if you use Composer).

    I'm so tired of people screaming at this solution without providing an alternative.

    mradcliffe’s picture

    If you need to update Drupal simply execute
    composer update
    ta-da! Drupal is updated and all of it's dependencies as well as your custom dependencies, without overriding your composer.json file.

    No, that's completely incorrect. I extract Drupal, composer.json now no longer has my modifications. I run composer update, now composer.lock no longer has my modifications AND vendor directory is gone. Instead of "ta-da!" I get "OH NO MY SITE IS BROKEN!!" because guess what... the dependencies are no longer there!

    Let's run through that again...

    1. My composer.json has require: { "mradcliffe/xerobundle": "dev-no-bundle-support as master" }.
    2. I download Drupal and extract it.
    3. My composer.json file NO LONGER HAS #1.
    4. I run composer.update.
    5. Now my composer.lock file NO LONGER HAS #1 and vendor/BlackOptic/XeroBundle no longer exist.
    6. I upload the files to web server.
    7. Now my site is broken because Symfony's service container cannot locate the class and cannot load it.

    So again, not "ta-da!"

    I think I'm completely against trying to work this patch in without #2385387: Permanently split Drupal and Drupal core into seperate repositories and all the other issues that would fix core - like a better packaging system on drupal.org.

    mradcliffe’s picture

    Basically - with this patch we force all users to be dev ops.

    daffie’s picture

    @mradcliffe: Can you post the /composer.json and the /core/composer.json files.

    daffie’s picture

    @mradcliffe: This is the basis for getting it to work. When this lands there will be a contrib module with a wonderful GUI that will make it easy to add packages. Do not worry about it.

    daffie’s picture

    Status:Needs review» Reviewed & tested by the community

    I looked at the code of this patch and it all looks good.
    After installing this patch I did a clean install and that works as normal.
    After making an extra requirement to the /composer.json file + the edits from the summery issue, I ran "composer update". My extra package got downloaded. The testsite was still oke.
    After installing my helloworld module with a dependency on the helloworld package from https://packagist.org/packages/abbert/helloworld the testpage showed the "Hello World!" string from the package.
    All that combined is for me enough to give this patch a RTBC.

    mradcliffe’s picture

    I won't change the status, but...

    I fundamentally disagree with this patch because it increases the barrier of entry for using Drupal. The patch will lead to broken sites because it does not allow "a user-editable composer.json file in Drupal's root directory to manage (custom) dependencies" for all use cases.

    mradcliffe’s picture

    Title:Allow a user-editable composer.json file in Drupal's root directory to manage (custom) dependencies» Allow a Composer power user to manage Drupal, modules, and PHP dependencies with a custom root composer.json
    Issue summary:View changes

    Updated title and issue summary to reflect what the proposed solution allows for so that a reviewer is not confused.

    chx’s picture

    Status:Reviewed & tested by the community» Needs work

    We ship with

    1. example.settings.local.php
    2. example.sites.php
    3. example.gitignore

    The file you plan to add says "This is an example file"... Please rename to example.composer.json which a) matches a pattern b) fixes the problems of @mradcliffe if I get it correctly. This was raised by @tstoeckler and agreed by @davidwbarrat in #128 and then it disappeared.

    daffie’s picture

    Status:Needs work» Reviewed & tested by the community
    chx’s picture

    Status:Reviewed & tested by the community» Needs work

    That issue has four children including a hard policy decision. There is no guarantee all those will happen. Renaming to composer.json once it works and everyone is happy is a trivial exercise but to commit a non-working example as composer.json is not a good idea.

    daffie’s picture

    @chx: Yes, and we would really like to talk about this hard policy decision. The problem is how to get that started. Can you help with that. Maybe IRC.

    davidwbarratt’s picture

    Issue summary:View changes
    Status:Needs work» Needs review
    Related issues:+#2388709: Rename example.composer.json to composer.json
    StatusFileSize
    new5.64 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch custom-composer-1975220-158.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    I'm not totally convinced it's a good idea to rename it to example.composer.json per what I said in #64. However, if this makes everyone happy, let's do it. Attached is a patch with the change.

    As brought up in #155, #2373203: [no patch] Take over ownership of drupal/drupal and drupal/core on Packagist requires a composer.json file in the root of drupal/drupal. I've opened up another follow-up to fix that #2388709: Rename example.composer.json to composer.json.

    Status:Needs review» Needs work

    The last submitted patch, 158: custom-composer-1975220-158.patch, failed testing.

    daffie’s picture

    Is it an idea to add a default composer.json in the root directory. So if you f*ck it up you always have the default.composer.json file. Just like default.settings.php and default.services.yml.

    Status:Needs work» Needs review

    Status:Needs review» Needs work

    The last submitted patch, 158: custom-composer-1975220-158.patch, failed testing.

    davidwbarratt’s picture

    #160,

    I don't really care if it's example or default. I only went with example because we have an example.gitignore anyways. I think it's ok if people rename it and get rid of the original one because it's non-working until #2372815: [meta] Make the <root>/composer.json file a working example anyways. and shocker! they can always go download the original, unmodified file from drupal.org

    Any idea why Drupal Installation would fail with that patch?

    davidwbarratt’s picture

    I installed Drupal with the patch on simplytest.me without a problem. Is this a test-bot issue? :/

    daffie’s picture

    @davidwbarratt: Can you post an interdiff.txt file so we can see what kind of changes you made. It is also possible that there is a problem with the testbot.

    chx’s picture

    it's not testbot ; both drush and the interactive installer are broken.

    Edit: but I don't have the time to debug it further.

    Edit2: yes, it's example, it's not a default.

    davidwbarratt’s picture

    StatusFileSize
    new1.71 KB
    new131 bytes

    #165,

    Attached is the inter-diff, literally just renamed the file. To prove this I've attached a -C -M interdiff as well.

    I don't see how this isn't a testbot issue. Perhaps we should requeue #115 for testing?

    daffie’s picture

    Can the problem be that there is no /composer.json file any more?

    davidwbarratt’s picture

    I searched the code and I saw two references to composer.json I'm not sure if this is the issue though...

    This one (are we even running this?)
    https://github.com/drupal/drupal/blob/8.0.x/core/vendor/easyrdf/easyrdf/...
    which is a really bad practice anyways. I think at that point you have access to the Composer object which will give you the whole json file if you want it. Also, it shouldn't be requiring vendor/autoload.php (i.e. it shouldn't be aware of the autoloader).

    https://github.com/drupal/drupal/blob/8.0.x/core/vendor/phpunit/phpunit/...
    Not sure if this is coping the one from the root or the core directory.

    typhonius’s picture

    The /composer.json file has just been moved to /core/composer.json.

    I just manually applied the patch and could install Drupal with the GUI. The reason this fails for testbot is because drush_valid_drupal_root() does a check for the a composer.json in the docroot.

    We'd need to work with the Drush team to get a new version of environment.inc out with a check for another file (perhaps core/core.services.yml) and work with the testbot team to get that new version of Drush onto the testbot servers.

    davidwbarratt’s picture

    Doesn't look like doap.php is included or autoloaded anywhere and appears to be a script used by the library. I think I can say the same for the build.xml file... maybe drush requires the <root>/composer.json?

    davidwbarratt’s picture

    Great... so the issue is the testbot (or drush to be more specific).

    I've opened an issue for the Drush team:
    https://github.com/drush-ops/drush/issues/1039

    Do we want to wait until this is resolved, or commit #115?

    typhonius’s picture

    To start the ball rolling and bring the Drush folk in on this, I've submitted https://github.com/drush-ops/drush/pull/1040.

    daffie’s picture

    I have thought some more over the problem that @mradcliffe has with this issue. To edit the /composer.json file and to update with the command "composer update" you have to make some changes to the /composer.json file. To really get this functionality to work we need #2352091: Create (and maintain) a subtree split of Drupal core. What we can do this get the functionality that we want is to change this issue to just one that is a preliminary one. Leave the part that we can add our own dependencies out. After this issue lands we go and work on #2352091: Create (and maintain) a subtree split of Drupal core and after that one #2372815: [meta] Make the <root>/composer.json file a working example.
    After #2352091: Create (and maintain) a subtree split of Drupal core we do not need all the artificial stuff we now are putting in /composer.json for testing.

    davidwbarratt’s picture

    Status:Needs work» Needs review
    alexpott’s picture

    Status:Needs review» Needs work

    The reason .htaccess is not example.htaccess (or web.config for that matter) is because without them Drupal does not work. The same can not be said for composer.json. I think we should be renaming it to example.composer.json in this issue. Other than that the approach in the issue makes sense to me.

    greg.1.anderson’s picture

    I pushed a change to drush master.

    I made a fresh clone of drupal 8, and applied the patch from #158. Without the change to Drush, drush status did not recognize the patched directory as a Drupal root. WIth the latest drush master, drush status did recognize the directory as a Drupal root.

    I have not done any more than this minimal level of testing, but seems like it should work now, once the testbot is updated to use the latest Drush.

    davidwbarratt’s picture

    #176,

    While composer.json is not required for Drupal, it is required for certain Composer commands, the most notable of which is composer create-project which is the primary download method for both Symfony and Laravel.

    But if that's the cost of getting this committed, then so be it. I hope we can fix it later #2388709: Rename example.composer.json to composer.json.

    alexpott’s picture

    @davidwbarratt yes but having the additional step of copying the example.composer.json to composer.json makes it 100% that this is the project's file and not Drupals.

    davidwbarratt’s picture

    #179,

    It's both... drupal's file (not core's file) is used by packagist so you can execute things like composer create-project.

    However it is also your project's file because you can customize it to your liking.

    This is defined by the type paramater in the composer.json file.
    For the one in root, we've set it to project which:

    This denotes a project rather than a library. For example application shells like the Symfony standard edition, CMSs like the SilverStripe installer or full fledged applications distributed as packages. This can for example be used by IDEs to provide listings of projects to initialize when creating a new workspace.

    By changing it to example.composer.json we're removing the composer create-project feature as well as saying that our site admins are not as intelligent(?) as other site admins and do not understand what "type": "project" means.

    alexpott’s picture

    Status:Needs work» Reviewed & tested by the community

    @davidwbarratt good points. I guess the question here is how do other projects manage updating their composer.json and keeping a project in git. btw I'm not saying anyone is not intelligent I'm just trying to get the best solution and leave Drupal in the most releasable state.

    I concede that I was wrong in #175. Having composer create-project work is important. Please disregard my request to change the name to example.composer.json.

    With this change we get the ability for projects to have custom composer.json and core to continue to add new dependencies in core/composer.json without breaking sites. This change looks like good progress to me. Would love to see this done.

    davidwbarratt’s picture

    #181,

    After you start your drupal project (with create-project or however), that then becomes your project so you can manage it in Git however you'd like. For me I never use the Drupal/Symfony Standard git repo because I want my history to be nice and clean, but I know webchick is different. Whenever you run composer create-project you will be asked if you want to keep the Git repo or not, I think that's an important choice and that's why they left it up to the user running the command. If you keep the git repo, it's understood that you'll have to merge any of your changes against the existing repo (if you even update it). Though, like I said, I always tell composer to not keep the repo so I can setup my own in the root.

    In practice, I typically see any required changes in the "project" documented, but (at least from what I've seen) those have been minimal (if ever), of course you could do it the other way and merge the changes to, it's really up to you and both ways are acceptable.

    #115 is ready to go then. :)

    Thanks for your help!

    daffie’s picture

    I would like to thank alexpott, being a core maintainer, for stepping in and helping this issue get to RTBC. This issue really needed that.

    davidwbarratt’s picture

    Title:Allow a Composer power user to manage Drupal, modules, and PHP dependencies with a custom root composer.json» Allow a Composer users to manage Drupal, modules, and PHP dependencies with a custom root composer.json
    Issue summary:View changes
    davidwbarratt’s picture

    #147,

    I thought I'd take some time and respond to your comment.

    No, that's completely incorrect. I extract Drupal

    I think you may have misunderstood my instructions in #146. I didn't say anything about extracting Drupal. I simply said: "If you need to update Drupal simply execute composer update" and "Drupal is updated and all of it's dependencies as well as your custom dependencies, without overriding your composer.json file."

    You don't need to extract anything.

    The problem is, is that your composer.json file, for your project, does not require drupal/core. This is what's a little confusing, the <root>/composer.json is not an example, it's a boilerplate. It's something that needs to be added to/extended, not overridden.

    Your composer require should have looked like this:

    {
      "require": {
        "drupal/core": "~8.0",
        "mradcliffe/xerobundle": "dev-no-bundle-support as master"
      }
    }

    In this way, you can modify your composer.json file to be whatever you'd like it to be, but you ought to start with what's in the existing one first and simply add your package to it. The problem is, is that you overrode the new <root>/composer.json with your own that did not depend on drupal/core. If you're project doesn't depend on drupal/core then yes, you'd run into exactly the problem you described.

    I removed what you put in the Issue Summary, simply becuase it's not true. If you as a package maintainer want to ask your users to add a dependency to their composer.json file, you certainly can do that, but they ought to do it alongside their dependency on drupal/core as well as any other dependencies they've added.

    I hope this helps. With a proper composer.json file in place, there is no need to download and extract drupal (potentially overriding your file). You can simply execute composer update and everything (Drupal core, as well as the package you added) will be updated.

    davidwbarratt’s picture

    Title:Allow a Composer users to manage Drupal, modules, and PHP dependencies with a custom root composer.json» Allow a Composer user to manage Drupal, modules, and PHP dependencies with a custom root composer.json
    mradcliffe’s picture

    Title:Allow a Composer user to manage Drupal, modules, and PHP dependencies with a custom root composer.json» Allow a dev ops user to manage Drupal, modules, and PHP dependencies with a custom root composer.json
    Issue summary:View changes

    I think you may have misunderstood my instructions in #146. I didn't say anything about extracting Drupal. I simply said: "If you need to update Drupal simply execute composer update" and "Drupal is updated and all of it's dependencies as well as your custom dependencies, without overriding your composer.json file."

    You don't need to extract anything.

    The problem is, is that your composer.json file, for your project, does not require drupal/core. This is what's a little confusing, the /composer.json is not an example, it's a boilerplate. It's something that needs to be added to/extended, not overridden.

    I am a bit offended by these comments because of the stress on the word you.

    On the contrary, I am not doing anything.

    What I have, is an opinion and a point - that the patch has the goal of solving one thing for one user group and reduces usability for current users. And the following comment in #185 proves this point.

    If you as a package maintainer want to ask your users to add a dependency to their composer.json file, you certainly can do that, but they ought to do it alongside their dependency on drupal/core as well as any other dependencies they've added.

    I would love it if all users could embrace one methodology of installing Drupal (using a sub-tree split and running Composer update), but I don't see it happening. As a module maintainer, I don't see myself stipulating one way of installing Drupal over another. I can offer suggestions and be as open as possible instead of restricting to one use case or another.

    Clearer documentation about what the patch provides for some users can only help, not hurt. I feel that the revert was destructive instead of constructive so I am adding back the comments and trying to be even clearer.

    greg.1.anderson’s picture

    I would like to suggest that we consider naming our example composer file "example.composer.json" rather than "composer.json". I think this will benefit some folks, and we can still do this and maintain the behavior of composer create-project, as described in #180.

    Allow me to explain my thinking. Currently, we have three repositories:

    (1) http://git.drupal.org/project/drupal.git
    (2) https://github.com/davidbarratt/drupal-core.git
    (3) https://github.com/davidbarratt/drupal8.git

    Repository (1) is the current and ongoing master repository for Drupal 8. It is where core committers will make new commits, and it is where download tarballs will be created from. It is what folks will get if the use 'git' to pull Drupal directory.

    (2) & (3) are currently third-party repositories; I am not sure these are going to be maintained at these URLs for now, but ultimately they will be read-only repositories automatically maintained by scripts on drupal.org, once the infrastructure is updated to support this. I name them as I have for identification purposes only, since I do not know what the final URL is going to be.

    If someone creates a Drupal site via composer create-project, they will reference some packages.json file (currently http://davidwbarratt.com/packages.json) which will in turn have references to repositories (2) and (3), but not to (1).

    My idea is that when the infrastructure script creates repositories (2) and (3) from repository (1), it could rename example.composer.json in repository (1) to composer.json before committing it to repository (3). That way, composer create-project would still work correctly, and, at the same time, if someone were to unpack a Drupal tarball on top of their existing site (due to misunderstanding or for whatever reason), then example.composer.json would not overwrite their customized composer.json file.

    The downside to "example.composer.json" over "composer.json" is that the former cannot be committed here until the drupal.org infrastructure is updated to start using the latest Drush master; otherwise, the testbot would fail for all Drupal 8 patches, and that wouldn't be any good. So, it might be worthwhile to go ahead and commit this as "composer.json", and make the switch to "example.composer.json" later, in a follow-up issue, once Drush is updated on drupal.org.

    nicholasruunu’s picture

    @mradcliffe,
    It's getting a bit ridiculous now.
    Your changes are are not clearer, they are plain wrong.
    This is not for composer "power users" or Devops.
    This is for regular PHP guys who wants to utilize composer and will help get us even further "off the island".

    Composer is PHP's de facto package manager, and not knowing this simple tool is damaging to you as a developer.

    And I don't understand why you are trying to sabotage.
    This will not change a thing for people who don't want to use composer.

    It's absolutely crucial for Drupal 8 that composer will work as expected for anyone serious to give it a second glance.

    nicholasruunu’s picture

    -- double post --

    dawehner’s picture

    Title:Allow a dev ops user to manage Drupal, modules, and PHP dependencies with a custom root composer.json» Allow a Composer user to manage Drupal, modules, and PHP dependencies with a custom root composer.json

    Let's revert at least the title, because composer is basically everything beside a tool for devops.

    @mradcliffe
    A hell big size of the PHP community out there won't even touch Drupal because you can't use it with the usual composer flow.
    For this itself it would help to make this change.

    alexpott’s picture

    What is missing from this issue is proper outlines of people's expectations of how things are supposed to work. What we are witnessing is a clash of these expectations. I think it is reasonable to prioritise the composer workflow over a custom workflow that involves creating your own repository that contains Drupal. Why? Because the moment you do the second you are moving away from Drupal's core repo anyway. Once you create your own repository you are free to change any file it contains and you will have to manage upstream changes. composer.json is not the only file where this is the case. Changing .htaccess is much more common and tricky since it involves security. Also once a user creates there own repo or checkouts Drupal they are free to do what they want - it is open source after all.

    The talk of power user vs some other user type is also misleading on this issue. We can be sure that people using Drupal packages provided by cpanel do not care about this. The moment you are checking out Drupal via git you are a power user.

    Reading #147 the problem is step 2. Once a user is managing their project through composer - downloading Drupal and extracting it is wrong and of course this is going to break stuff - the same would occur if the .htaccess file was modified. And just as with changing .htaccess I don't think the root composer.json should change once D8 is released unless we have extremely good cause.

    alexpott’s picture

    Status:Reviewed & tested by the community» Needs work

    We need a new patch.

    alexpott’s picture

    Also I think the instructions in #147 step 2 could work with the following modification - extract the latest Drupal and copy the core directory over the existing directory. A related issue is #2389811: Move all the logic out of index.php (again).

    3. Remove all old core files and directories, except for the 'sites' directory
       and any custom files you added elsewhere.

       If you made modifications to files like .htaccess or robots.txt, you will
       need to re-apply them from your backup, after the new files are in place.

       Sometimes an update includes changes to default.settings.php (this will be
       noted in the release notes). If that's the case, follow these steps:

       - Make a backup copy of your settings.php file, with a different file name.

       - Make a copy of the new default.settings.php file, and name the copy
         settings.php (overwriting your previous settings.php file).

       - Copy the custom and site-specific entries from the backup you made into the
         new settings.php file. You will definitely need the lines giving the
         database information, and you will also want to copy in any other
         customizations you have added.

    from core/UPGRADE.txt - it looks like we should be changing this advice in this patch to include composer.json

    mradcliffe’s picture

    @nicholasruunu, I apologize for referring to "regular PHP users" as a power user or a dev ops user if that contains any negative connotation. However I don't appreciate using phrases such as "ridiculous" or "sabotaging" when I have made it clear that I am doing neither. I attempt to distinguish between users so that we do not marginalize people similar to #192.

    I agree with @dawehner and @alexpott's comments in #191-#194.

    - Removed tag that no longer applies, added tag that applies.

    davidwbarratt’s picture

    Status:Needs work» Needs review
    StatusFileSize
    new8.9 KB
    FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch custom-composer-1975220-196.patch. Unable to apply patch. See the log in the details link for more information.
    [ View ]

    Attached is a re-rolled patch based on #115. I've also updated the documentation in UPGRADE.txt

    @mradcliffe I sincerely apologize if I've offended you. I thought perhaps you were misunderstanding what I was saying and I thought you deserved a response / explanation. I did not intend to insult you in any way. I hope you understand that I genuinely wanted to help.

    @greg.1.anderson I'd also like to thank you for apologizing in #106. It really means a lot to me and it's actions like that that make me truly appreciate this community.

    webchick’s picture

    I really don't want to derail this discussion, but I do want to capture what I said in IRC a few days back (sorry for not doing it at the time), because I'm still scratching my head a bit. This comment might be safe to ignore... I'm definitely no Composer expert.

    The Composer docs state https://getcomposer.org/doc/01-basic-usage.md#composer-json-project-setup that if you want to use Composer to manage dependencies in your project, you create a blank composer.json file in your custom repo and start specifying dependencies. Packagist https://packagist.org/ says the same thing. So, per those docs, if I were building a Drupal site and wanted my dependencies managed by Composer, I would start my repo with a blank composer.json file and add to it:

    {
        "require": {
            "drupal/drupal": "8.*",
        }
    }

    ....or similar. And add another similar line for whatever other dependencies my custom, Drupal-based project has.

    So I'm a bit confused why this issue seems to be advocating a flow of instead directly modifying the composer.json of one of my custom project's dependent libraries. I have never done that in any of the code I've written that has Symfony dependencies, for example.

    There seems to be consensus growing on the solution in the patch, so this might just be a request for an updated issue summary, because I don't grok "All of these things (plus anything else that can be managed with Composer) should be able to be managed withing[sic] a Drupal Project, as if it were any other PHP project." because at least in my (limited) Composer experience, no other PHP project has you ever editing their composer.json files.

    greg.1.anderson’s picture

    @davidwbarratt, thank you for #196. I appreciate the help you have given me here; it has been very helpful to my understanding of composer.

    If you and the others on this thread could bear with me for a moment, I'd like to share an experiment I did. I forked your custom composer installer, which was very helpful, thank you, and added a little more code that allows composer install (and by extension, composer create-project) to move code into a user-specified location (the feature provided by your installer), while excluding certain directories (e.g. core/vendor, modules, and so on).

    The composer.json "extra" content looks like this, at the moment:

        "extra": {
            "custom-installer": {
                "drupal-core": "drupal/",
                "drupal-module": "drupal/modules/{$name}/",
                "drupal-theme": "drupal/themes/{$name}/"
            },
            "merge-exclusions": {
                "drupal-core": [
                    "modules",
                    "themes",
                    "profiles",
                    "core/vendor"
                ]
            }
        },

    The directories listed in the exclusions are not touched when the composer project is installed, updated or removed. This means that there are now directories that are physically inside the project's install directory that are no longer logically part of the project.

    With this, to install an example Drupal project:

    $ composer create-project --repository-url="http://greenknowe.org/drupal8" drupal/drupal-project myproject/ dev-master
    $ cd myproject

    The drupal8 project contains just a composer.json and a placeholder README. It installs from the current Drupal repository on drupal.org.

    To download a module:

    $ composer require drupal/feeds '8.3.x-dev'

    To download a theme:

    $ composer require drupal/zen '8.7.x-dev'

    To update an external library, needed by a module, or to update the module itself, or to update the Drupal 'core' directory, or to update the .htaccess file:

    $ composer update

    You can see that there is a lot in common in this experiment with the tact taken in #196 and earlier as far as usage and capabilities. The main advantages are that with this custom installer, a single composer update will update all of the Drupal files, including .htaccess, and the drupal.org infrastructure changes to support the core split are not necessary. The main disadvantage is that there is an extra directory level created here that you can't get rid of -- but this also serves as a place to put the user's copy of composer.json. This might be more comfortable and for those who do not want to manage the files at the Drupal root separately from the core directory. The custom installer might be viewed as a bit of a Drupalism as well -- but Composer does provide for the concept of custom installers that modify the way code is placed in ways that are specific to a certain project, so perhaps it is not so far out of line. Composer novices probably won't notice that the installer is odd; composer experts might not like the way it changes the package model.

    Also, this technique is compatible with #196. Even if a "non-working" core split is committed, it will still be possible to install with this custom installer using 3rd-party repositories, which I imagine will be maintained somewhere until drupal.org is doing it automatically.

    This is just proof-of-concept code right now; the implementation of remove is not complete, and refinements to the "extra" metadata would probably be useful. The installation operation works right now, though -- if you run the commands in the example above, it will produce a working (installable) Drupal 8 site that you can add modules to via composer require. This might just be an oddity to examine and pass by -- there is no core patch here -- but I wanted to show what might be done, and hope it will be at least interesting if not useful. Ultimately, we're all looking forward to the day that modules and themes just live in the vendor directory, and I certainly appreciate all of the work that people are doing to get us closer to that point, in one way or another.

    davidwbarratt’s picture

    #197,

    You're really close, instead of depending on drupal/drupal your project would depend on drupal/core. Assuming #2372815: [meta] Make the <root>/composer.json file a working example (I need to rename/update that issue again) is committed, then you could certainly start from a blank composer.json file and add this:

    {
        "require": {
            "drupal/core": "8.*",
        }
    }

    And you'd have drupal core downloaded and installed into the core directory. You can do this with symfony/symfony as well as laravel/framework

    However, this doesn't really set you up for success because you are missing an index.php, .htaccess and all of the other files in the root of Drupal. To get around this, both Symfony and Laravel have created "distributions" of sorts, that are boilerplates for your customization.

    You can get this boilerplate by executing:

    composer create-project drupal/drupal path/ 8.*

    Notice that we are not creating drupal/core we're starting a new drupal/drupal project. Once Composer clones drupal/drupal it will execute composer install. Since the composer.json file in the root of drupal/drupal requires drupal/core just like in our custom example above, you are left with exactly the same thing as if you'd created a composer.json file yourself, only this time you also have index.php, .htaccess etc.

    when you execute composer create-project composer will ask you if you'd like to keep the drupal/drupal repository, or if you'd like to ditch it so you can setup your own.

    Does that help clarify things?

    Jeff Burnz’s picture

    How will these changes affect me as a contrib project developer who wants to use composer to install dependancies?

    I'm not really clear on that from reading this issue, which is btw very hard to follow. I see a lot of talk about Drupal core, but I think the bigger use case for composer is contrib and module/theme dependancies.

    davidwbarratt’s picture

    #200,

    The patch does not attempt to solve the problem(s) in contrib, but It enables contrib to modify/extend the root composer.json file without hacking core and without having to merge dependencies with core. I believe Composer Manager attempts to do this with mixed results.

    I'm a module maintainer as well (as are most of the people on this thread). I do not believe we are making contrib worse in anyway, in fact I think the opposite is true, I think we're enabling several different ways of working with Composer in Drupal.

    For me, I plan on managing all of my modules with #1886820: Packagist for Drupal Projects. I know not everyone will want to do that so I'm sure there will be other solutions.

    greg.1.anderson’s picture

    @webchick: For comparison, you might want to try to run the compose commands in #198, just to see how it works. With this composer installer, the user gets a separate composer.json to modify and maintain; Drupal's composer.json remains unchanged.

    A couple of things of note:

    #198 is not necessary. The process described in #199 works perfectly well, and is the standard for how symfony projects handle this sort of thing.

    #196, on the other hand, is necessary. If we are going to make progress towards the ultimate goal of keeping Drupal components (modules, themes and core) in the 'vendor' directory, then we will need to split 'core' out from the root composer.json file.

    Why do #198, then? I thought some might be more comfortable treating Drupal like a composer library rather than a project, because the library model is more familiar. When you install Drupal like this, the .htaccess file and other files at the Drupal root are managed by composer, and can be updated along with everything else when you do a composer update. When you set up Drupal per #199, you need to do the additional step of git pull to update these files -- presuming that you kept your repository when you created the project. This is easy, of course, and these files will rarely change. Those who do not remember to do the second step will miss updates, though, and will thereby end up with partially-updated sites.

    The point was previously made that folks should just learn the standard ways of doing things with composer and symfony, and there is a lot of wisdom in that. #198 can be updated to work after this issue lands, though, so it remains an option that some might consider using for a time.

    greg.1.anderson’s picture

    Related issue: For those who are interested, there is a discussion on using Composer with Drush extensions in the Drush issue queue.

    greg.1.anderson’s picture

    I tested #196 in the following ways:

    - Applied the patch to Drupal 8.0.x.
    - Reviewed the changes to upgrade.txt -- looks good.
    - Installed Drupal via `drush qd` -- worked fine.
    - Ran 'composer update' in the core directory -- packages with updates were updated.

    Marking this RTBC, since it accomplishes what it planned to do -- commit a non-working composer.json at the root of the project, and a working composer.json in the core folder that will be the basis for a split core repository. If this is committed, then a working composer workflow for Drupal can be sustained via third-party repositories until the infrastructure on drupal.org is ready.

    Not sure if #197 & c. are satisfied; feel free to change the status if more discussion on strategic goals are needed.

    greg.1.anderson’s picture

    Status:Needs review» Reviewed & tested by the community

    Forgot to set status. Also, I note this issue is still tagged "Needs Documentation". I presume that the changes to upgrade.txt in #196 satisfy this, and further documentation (along the lines of what was requested in #200) will be part of the follow-on issue -- but leaving the tag for now.

    davidwbarratt’s picture

    #198,

    Well isn't that clever. Probably something I could use for Drupal 7 since it'd be better than my Drupal Structure script. I think what you have is a good idea, but I'm still for native support of Composer in Drupal 8 (as I'm sure you are too). Did you test with a tagged release like "8.0.0-beta3"? I think tagged releases are handled differently in composer (i.e. they are copied, not cloned maybe?) but maybe I'm wrong. I like that it can work even after this patch is committed for people who want to use Composer in an alternative way, but I think we should reduce the cleverness as much as possible and be in-line with the broader PHP community as much as we can be.

    Thanks for all your hard work!

    greg.1.anderson’s picture

    #206,

    Yes, I agree, the best situation is when the target fully supports Composer, and does not require custom loaders. It is important to work toward that goal; custom installers can help drop things into place in the interim, before such support is possible. To that end, my idea would have been much more useful if I had implemented it quite a while ago, but as it was, I only thought of it recently.

    My test was for "~8.0.0-beta3". I just call through to the existing Composer download manager and downloader, so anything that works with the standard installer should also work with mine. I tried removing the ~, to see if I could in fact install exactly "8.0.0-beta3", and it failed. I tried again with a fresh composer.json that contained only drupal/drupal 8.0.0-beta3, and no custom installers, and that also failed with the same error message. 8.0.0-beta3 requires phpunit/phpunit 4.1.*, which requires symfony/yaml ~2.0, but Drupal 8.0.0-beta3 explicitly requires symfony/yaml dev-master#499f7d7. I am not sure why that used to work, but does not work now. Seems like it should never have worked. Shrug.

    Anyway, the implementation of the custom installer does have some holes. If you install using a Composer git downloader, then `git status` on the result will show all of the excluded directories as local modifications. It would be better to remove the excluded folders from git as well, and add them to .gitignore, but that would cause havoc with the upstream if you ever tried to pull. Maybe the vcs directories must be removed with this installer.

    The composer update and remove functions do not have ideal implementations at the moment either. Composer remove does not have any of the protections offered by the vcs downloaders, which will tell you if you modified files before going ahead with the removal. Composer update always removes the component, and then installs it again, even if there was no update available. These limitations could be fixed with more work, but honestly, I will probably just use the techniques in #199 once this issue is committed. It might still be worth maintaining for other use cases; I'll send you a PR if you want the code back in your original repository.

    tim.plunkett’s picture

    +++ b/core/UPGRADE.txt
    @@ -58,8 +58,9 @@ following the instructions in the INTRODUCTION section at the top of this file:
    -   If you made modifications to files like .htaccess or robots.txt, you will
    -   need to re-apply them from your backup, after the new files are in place.
    +   If you made modifications to files like .htaccess, composer.json, or
    +   robots.txt you will need to re-apply them from your backup, after the new
    +   files are in place.

    @@ -93,7 +94,8 @@ following the instructions in the INTRODUCTION section at the top of this file:
    -5. Re-apply any modifications to files such as .htaccess or robots.txt.
    +5. Re-apply any modifications to files such as .htaccess, composer.json, or
    +   robots.txt.

    It seems odd to make these changes, if the plan is to move it back to example.composer.json? Also, those are only examples, it's not an exhaustive list, so it doesn't *need* to be updated.

    Status:Reviewed & tested by the community» Needs work

    The last submitted patch, 196: custom-composer-1975220-196.patch, failed testing.

    Status:Needs work» Needs review
    davidwbarratt’s picture

    #208,

    Per #181 & #182 the name will stay composer.json and not be changed.

    I've renamed [#2372815] so this is more clear.

    Status:Needs review» Needs work

    The last submitted patch, 196: custom-composer-1975220-196.patch, failed testing.

    alexpott’s picture

    The last fail appears to be an unrelated random fail - see #2392865: Random fail in \Drupal\system\Tests\Entity\EntityOperationsTest

    Status:Needs work» Needs review
    effulgentsia’s picture

    Status:Needs review» Reviewed & tested by the community

    Back to RTBC, as it was prior to the random fail in #209.

    Mile23’s picture

    Status:Reviewed & tested by the community» Needs work

    To start off with: I can't find a way to test whether this works. Is it someone's repo? Are there clear instructions for testing?

    @webchick in #197: There seems to be consensus growing on the solution in the patch, so this might just be a request for an updated issue summary, because I don't grok "All of these things (plus anything else that can be managed with Composer) should be able to be managed withing[sic] a Drupal Project, as if it were any other PHP project." because at least in my (limited) Composer experience, no other PHP project has you ever editing their composer.json files.

    Symfony, for instance, does this when you say composer create-project symfony/framework-standard-edition. It gives you a composer.json file at the root directory, with dependencies on all the components of Symfony. It's expected that you'll modify this, because you've told Composer that you're creating your project using Symfony's project framework.

    Drupal could do the same thing, and this issue is working towards that direction. The problem is that Drupal has rules about 'hacking core,' which this issue skirts by changing the rules. :-) Like this:

    +++ b/core/UPGRADE.txt
    @@ -58,8 +58,9 @@ following the instructions in the INTRODUCTION section at the top of this file:
    -   If you made modifications to files like .htaccess or robots.txt, you will
    -   need to re-apply them from your backup, after the new files are in place.
    +   If you made modifications to files like .htaccess, composer.json, or
    +   robots.txt you will need to re-apply them from your backup, after the new
    +   files are in place.

    @alexpott #192: What is missing from this issue is proper outlines of people's expectations of how things are supposed to work.

    Absolutely.

    The most important concept is the separation of the project from Drupal. You're not making a Drupal site, you're making a project that needs Drupal in it.

    In Symfony, all the components are loosely-coupled and can therefore be requirements of each other, and of a central project. The project is different from the web site or the framework or anything else. This is a model to emulate in Drupal. In this issue's current solution, Drupal core becomes the single component which is required.

    So, in terms of workflows, we want to be able to use Composer to make a Drupal-based project:

    $ composer create-project drupal/drupal myProject/ 8.*
    $ cd myProject
    $ composer require drupal/console *
    $ composer require myVendor/myMissionCriticalTool *
    $ composer require drupal-contrib/some-contrib-module 8.*
    $ git add -A
    $ git commit -m 'Initial commit of my Drupal-based project.'
    $ git remote add origin ...
    $ git push origin master

    # later....

    $ git pull origin master
    $ composer require myVendor/myMissionCriticalTool 2.*
    $ git add -A
    $ git commit -m 'Upstream bugfix'
    $ git push origin master

    # six months later....

    $ git pull origin master
    $ composer require drupal/drupal 8.1.*
    ## some update procedures....
    $ sudo core/scripts/run-tests.sh --all
    ## fix some stuff....
    $ git add -A
    $ git commit -m 'Drupal is kewl.'
    $ git push origin master

    davidwbarratt’s picture

    Status:Needs work» Needs review

    #216,

    To start off with: I can't find a way to test whether this works. Is it someone's repo? Are there clear instructions for testing?

    The method for testing is in the issue summary under "Testing". Since <root>/composer.json is a non-working example until #2372815: [meta] Make the <root>/composer.json file a working example. I've also given a lot more examples in #126.

    You can see a working example composer.json here:
    https://github.com/davidbarratt/drupal8/blob/master/composer.json
    or a rather complex example here:
    https://github.com/davidbarratt/drupal8/blob/kitchen-sink/composer.json

    I don't think this issue is changing the rules at all, it changes the workflow that some users will use, but most probably wont ever touch. If you do use Composer, the UPGRADE.txt doesn't even apply since upgrading your project with composer, will only result in the core and vendor directories changing, everything else in the root of drupal will stay exactly as it is. If you upgrade some other way, then you'll need to follow the directions in UPGRADE.txt like everyone else.

    My only change to your workflow is in this part:

    # six months later....

    $ git pull origin master
    $ composer require drupal/drupal 8.1.*

    You wouldn't do:

    $ composer require drupal/drupal 8.1.*

    instead you would do:

    $ composer require drupal/core 8.1.*

    However, you shouldn't even need to do that, since the version constraint is ~8.0 the tilde operator will update you to 8.1.0 since ~8.0 is the same as >=8.0 <9.0.

    so you could simply do:

    $ composer update

    to update drupal core and all of it's dependencies.

    What we are doing in this patch is identical to what Symfony does, as in their Symfony Standard composer.json they have:

    {
      "require": {
        "symfony/symfony": "2.7.x-dev"
      }
    }

    Lastly, is there some work that needs to be done on this? Why did you change it to "Needs Work"?

    davidwbarratt’s picture

    Also, as recommended by Composer and Acquia, you're probably not going to commit the core and vendor directories to your version control (hence the update to example.gitignore). You will commit your composer.lock file which is updated on composer update.

    However, if you'd like to have the directories committed to version control, you certainly can do that as Composer says you can.

    daffie’s picture

    Status:Needs review» Reviewed & tested by the community

    Mile23 removed the RTBC status and I cannot find a reason in his comment why that is needed, so back to RTBC.

    bojanz’s picture

    Happy to see the updated issue summary, and RTBC status.
    An important step towards solving this problem.

    davidwbarratt’s picture

    Status:Reviewed & tested by the community» Needs work

    The last submitted patch, 196: custom-composer-1975220-196.patch, failed testing.

    Status:Needs work» Needs review

    Status:Needs review» Needs work

    The last submitted patch, 196: custom-composer-1975220-196.patch, failed testing.

    RobLoach’s picture

    Status:Needs work» Needs review

    Updated to latest 8.0.x... Tracking on GitHub via https://github.com/RobLoach/drupal/pull/1 .

    RobLoach’s picture

    StatusFileSize
    new8.59 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,368 pass(es).
    [ View ]
    daffie’s picture

    Status:Needs review» Needs work
    Issue tags:+Needs reroll

    Needs reroll from the patch from comment #196.

    +++ b/core/composer.json
    @@ -0,0 +1,47 @@
    diff --git a/composer.lock b/core/composer.lock

    diff --git a/composer.lock b/core/composer.lock
    similarity index 100%

    similarity index 100%
    rename from composer.lock

    rename from composer.lock
    rename to core/composer.lock

    I am missing this pieces from the previous patch.

    +++ b/core/composer.json
    @@ -22,26 +22,25 @@
    -    "easyrdf/easyrdf": "0.9.*",
    +    "easyrdf/easyrdf": "0.8.*",
    ...
    -    "zendframework/zend-feed": "2.3.*",
    +    "zendframework/zend-feed": "2.2.*",

    I do not think that this change is right.

    hussainweb’s picture

    Status:Needs work» Needs review
    Issue tags:-Needs reroll
    StatusFileSize
    new8.9 KB
    PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 83,410 pass(es).
    [ View ]

    Attempting a reroll...

    daffie’s picture

    Status:Needs review» Reviewed & tested by the community

    A simple reroll of the patch from comment #196. That one was RTBC, so back to RTBC.
    I was unable to create an interdiff.txt file for #196 to #228.

    alexpott’s picture

    Status:Reviewed & tested by the community» Needs work
    Issue tags:+Needs change record

    One last thing we need a change record so that people with existing projects or who are working on patches that change d8's dependencies or upgrade them know what to do.

    Once that is done I think we're clear to proceed and commit the patch. Also somewhere in our documentation (https://www.drupal.org/documentation/develop) we need a page on Drupal 8 and composer.

    davidwbarratt’s picture

    Status:Needs work» Needs review
    Issue tags:-Needs change record

    I've added the Change Record (let me know if it needs something):
    https://www.drupal.org/node/2404131

    davidwbarratt’s picture

    Status:Needs review» Needs work
    alexpott’s picture

    Status:Needs work» Needs review

    I've swapped some things around in the CR so the important information comes first and fleshed out a couple of things. Also I've added the CR to the followup to make composer.json working so we can update that when and if that lands.

    tstoeckler’s picture

    Status:Needs review» Reviewed & tested by the community

    Yes, that CR looks great, thanks!

    • alexpott committed da28864 on 8.0.x
      Issue #1975220 by davidwbarratt, tstoeckler, mradcliffe, RobLoach,...
    alexpott’s picture

    Status:Reviewed & tested by the community» Fixed
    Issue tags:-Needs Documentation

    Committed da28864 and pushed to 8.0.x. Thanks!

    Thanks for adding the beta evaluation.

    nicholasruunu’s picture

    Wooohoo, thanks everyone!

    bojanz’s picture

    On a related note, I just rewrote composer_manager: https://www.drupal.org/node/2350639#comment-9494077

    This issue opens the door for simplifying the code further, so composer_manager can be used to automatically rebuild the root composer.json when people can't use Composer as a Drush Make replacement (due to the still-unfinished nature of the followup issues, etc).

    chx’s picture

    Category:Task» Bug report
    Priority:Major» Critical
    Status:Fixed» Active
    Issue tags:+Needs issue summary update

    Care someone explain where and why we lost the example.composer.json name and ended up with a ridiculous nonworking composer.json with an explanation that sounds like git gibberish?

    Edit: The issue summary still says "Rename the example /example.composer.json to /composer.json".

    alexpott’s picture

    Priority:Critical» Major
    Status:Active» Fixed

    Fomr #188

    The downside to "example.composer.json" over "composer.json" is that the former cannot be committed here until the drupal.org infrastructure is updated to start using the latest Drush master; otherwise, the testbot would fail for all Drupal 8 patches, and that wouldn't be any good. So, it might be worthwhile to go ahead and commit this as "composer.json", and make the switch to "example.composer.json" later, in a follow-up issue, once Drush is updated on drupal.org.

    Let's get that followup created. Also the rename is not critical.

    plach’s picture

    Category:Bug report» Task
    bojanz’s picture

    The problem with not naming it example.composer.json is that 'composer update' will happily run, then things will break.
    We shouldn't allow it to be run until it's functional for UX reasons.

    chx’s picture

    Category:Task» Bug report
    Priority:Major» Critical
    Status:Fixed» Active

    This should be rolled back until the infra is ready to commit the example file. Committing broken stuff is not a good idea. By and large, HEAD can be considered broken at this point. And the issue summary still needs an upgrade.

    alexpott’s picture

    How can HEAD be considered broken? At the moment Drupal's dependency on composer working is that core can update its dependencies. This is possible through editing ./core/composer.json.

    I've run composer update in the root directory - it results in no changes to anything and composer correctly reporting something is broken. There has been no commitment to have composer update work form root in the previous to this patch landing. In fact the way in which we were using composer meant that it you did manage you dependencies through composer and used git to checkout drupal then doing a git pull would probably hose your site.

    This patch is one step in the right direction - rolling back would hinder progress. We have two routes now to make things work - either fix infra to make an example file work or fix infra to have a sub tree split etc...

    I'm not going to play status wars but in no way is this critical or needing revert.

    chx’s picture

    Category:Bug report» Task
    Priority:Critical» Major
    Status:Active» Fixed
    Issue tags:+sad chx
    webchick’s picture

    Issue tags:-sad chx

    Did my best to create the follow-up that Alex asked for in #2404919: Rename composer.json -> example.composer.json.

    Also, the issue queue is not a forum with which to express passive-aggressive commentary, so removing that tag. I refer you to https://www.drupal.org/conflict-resolution if you feel there's something here that needs sorting out here.

    davidwbarratt’s picture

    Issue summary:View changes
    Issue tags:-Needs issue summary update
    davidwbarratt’s picture

    I've added some documentation for those who have been asking for it.

    https://www.drupal.org/node/2404989

    Please feel free to improve it. Thanks!

    Status:Fixed» Closed (fixed)

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

    The last submitted patch, 158: custom-composer-1975220-158.patch, failed testing.