Problem/Motivation
Site Maintainers ought to be able to utilize custom dependencies in their Drupal projects.
These dependencies could be:
- Composer package such as Guzzle OAuth Subscriber
- Drupal module such as Insert Field.
- Drupal theme such as Omega
- 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:
- Essentially revert #1805316: Move composer.json to the project root
- Create a new composer.json file in the project root
- #2372815: [meta] Make the <root>/composer.json file a working example
- 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
- Users can use Composer without modifying a core file.
- Drupal Core is a dependency of the Drupal project
- Drupal's dependencies remain in-tact.
- Composer users can exclude
core
andvendor
directories from their repositories - Allows users to manage contrib modules via Composer (like #1886820: Packagist for Drupal Projects)
- Only requires changes to two
json
files. - Does not require rewritting all current patches
- Does not require any changes to the packaging script
- Allows the
core/vendor
directory to remain in Drupal's repositories - Changes should be transparent to all users, minor changes for core maintainers.
Cons
- 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?
<root>/composer.json
file is a non-working example until #2372815: [meta] Make the <root>/composer.json file a working example is resolved- Executing
composer install
from the root of the project would delete thecore
directory and add it back. Any changes to the folder would be destroyed. - Composer users will need to modify
index.php
in order to use Composer with Drupal.
Remaining tasks
- Test the most recent patch
- #2372815: [meta] Make the <root>/composer.json file a working example
- 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:
- Modify
<root>/composer.json
.- 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.
- 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.
- 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.
- Add this repository:
- 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: Use a single 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
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.
Comment | File | Size | Author |
---|---|---|---|
#228 | custom-composer-1975220-228.patch | 8.9 KB | hussainweb |
Comments
Comment #1
davidwbarratt CreditAttribution: davidwbarratt commentedMarking this issue as a bug report since it's impossible for module developers to use composer.json without breaking Drupal's best practices.
Thanks!
Comment #2
RobLoachWe 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"
Comment #3
davidwbarratt CreditAttribution: davidwbarratt commentedWhy would drupal core be something other than a library? Isn't that what Drupal core is?
Thanks!
Comment #4
Seldaek CreditAttribution: Seldaek commentedThe 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).
Comment #5
davidwbarratt CreditAttribution: davidwbarratt commentedGood 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!
Comment #6
RobLoachSounds 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.
Comment #7
RobLoachComment #8
davidwbarratt CreditAttribution: davidwbarratt commentedI 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!
Comment #9
davidwbarratt CreditAttribution: davidwbarratt commentedI'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!
Comment #10
cosmicdreams CreditAttribution: cosmicdreams commentedWouldn't this become a problem if you ran composer update?
Wouldn't it attempt to install drupal into the core/vendor folder?
Comment #11
catchCan 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?
Comment #12
davidwbarratt CreditAttribution: davidwbarratt commented@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!
Comment #13
davidwbarratt CreditAttribution: davidwbarratt commented@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!
Comment #14
catchOK 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.
Comment #15
davidwbarratt CreditAttribution: davidwbarratt commentedawesome. sounds like a plan. :)
Thanks!
Comment #16
ParisLiakos CreditAttribution: ParisLiakos commentedhaving a vendor directory in each module is really an even worse solution than using libraries module.
correcting status according to #8
Comment #17
ParisLiakos CreditAttribution: ParisLiakos commentedxpost...
Comment #18
davidwbarratt CreditAttribution: davidwbarratt commentedI 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!
Comment #19
patcon CreditAttribution: patcon commenteddavidwbarrett++ well said
Comment #20
davidwbarratt CreditAttribution: davidwbarratt commentedI should add one more:
Ideally the vendor directory should not be included in the repository. This means Drupal Core would be a separate repository.
Comment #21
cosmicdreams CreditAttribution: cosmicdreams commentedWell 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.
Comment #22
davidwbarratt CreditAttribution: davidwbarratt commented@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:
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!
Comment #22.0
davidwbarratt CreditAttribution: davidwbarratt commentedUpdate the recommended core project name, as per Seldaek's suggestion.
Comment #23
ParisLiakos CreditAttribution: ParisLiakos commentedComment #24
Mile23OK, we have two goals:
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.
Comment #25
davidwbarratt CreditAttribution: davidwbarratt commentedMile23,
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!
Comment #26
Mile23I blame blood sugar. :-)
Comment #27
xjmComment #28
alexpottThis 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.
Comment #29
sunFixing issue title accordingly to stop confusion and not presume one of many possible solutions.
Comment #30
sunx-post
Comment #31
jameshalsall CreditAttribution: jameshalsall commentedWhat is the status on this issue? Is it likely to be merged into 8.x before release?
Comment #32
davidwbarratt CreditAttribution: davidwbarratt commentedI'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.
Comment #34
davidwbarratt CreditAttribution: davidwbarratt commentedI 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.
Comment #35
mgiffordComment #36
davidwbarratt CreditAttribution: davidwbarratt commentedUpdate the project description to be better organized and include ideals.
Comment #37
davidwbarratt CreditAttribution: davidwbarratt commentedI 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:
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.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 thecore
directory and manage core with composer (and it's dependencies). At which point the user can simply edit theirindex.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! :)
Comment #38
mradcliffeI'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.
Comment #39
davidwbarratt CreditAttribution: davidwbarratt commentedAlso, 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.
Comment #40
davidwbarratt CreditAttribution: davidwbarratt commentedAttached 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.Comment #41
cosmicdreams CreditAttribution: cosmicdreams commentedOnly 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.
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?
Comment #42
davidwbarratt CreditAttribution: davidwbarratt commentedThe 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
executingcomposer 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 todev
. However, the latter results in thedev
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 modifyindex.php
to point to the root vendor directory rather than the core directory. This would change this line from this:to this:
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:
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.Comment #43
tstoeckler@davidwbarratt you can use "minimum-stability": "dev" and then "prefer-stable": true in order to avoid the problem you mentioned.
Comment #44
davidwbarratt CreditAttribution: davidwbarratt commentedThanks 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 install8.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:Let me know what you all think! :)
Comment #45
davidwbarratt CreditAttribution: davidwbarratt commentedI'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.
Comment #46
tstoecklerWow, 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-specificcomposer.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.
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?
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, rancomposer install
and everything got recreated (but for a few dependencies which are outdated in our repository).Comment #47
nicholasruunu CreditAttribution: nicholasruunu commentedI 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.
Comment #48
davidwbarratt CreditAttribution: davidwbarratt commentedUpdate the description with the real solution.
Comment #49
davidwbarratt CreditAttribution: davidwbarratt commentedtstoeckler,
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 rootcomposer.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.
Comment #50
tstoeckler@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 doingcomposer 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
composer install
must work in the (unsplit) Drupal core repositorycomposer install
must work in the subtree-split of thecore
directoryAt 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):
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 syncThoughts?
Comment #51
tstoecklerSome more concrete responses to #49:
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.
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'm not sure what they would have to understand, specifically. Can you elaborate?
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.Again, I don't see how this can be true, since they are both in the same repo.
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.This would require
composer install
to work in the Drupal root (i.e. not thecore
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.Comment #52
mradcliffeI 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.
Comment #53
bojanz CreditAttribution: bojanz commentedUntrue. 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.
Comment #54
mradcliffeSorry, this was information I learned in Amsterdam and it was true of the plan at the time. I'm glad that plan changed.
Comment #55
Fabianx CreditAttribution: Fabianx commentedHere 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?
Comment #56
davidwbarratt CreditAttribution: davidwbarratt commented#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.
Comment #57
tstoecklerOK, 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. Thuscomposer 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 toexample.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?
Comment #58
Fabianx CreditAttribution: Fabianx commentedIf we use a n example file, why not a YML file that can be converted to JSON? (and can have comments)
Comment #59
davidwbarratt CreditAttribution: davidwbarratt commented#51,
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.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.
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. :(
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.It would require the subtree split (from someone, hopefully d.o.)
Yep, you're right, and I've changed my mind on that to label the current
<root>/composer.json
file a non-working example.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) andcore/composer.json
is working as-is.Comment #60
davidwbarratt CreditAttribution: davidwbarratt commentedComment #61
davidwbarratt CreditAttribution: davidwbarratt commented#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.
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.
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 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.
Comment #62
davidwbarratt CreditAttribution: davidwbarratt commented#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.Comment #63
mradcliffeThe 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.
Comment #64
davidwbarratt CreditAttribution: davidwbarratt commented#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?
Comment #65
davidwbarratt CreditAttribution: davidwbarratt commented#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).
Comment #66
Mile23Just 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 undercore/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
.Comment #67
tstoecklerOK, 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.
Comment #68
tstoecklerOK, I actually forgot -C -M.....
Here we go....
Comment #69
martin107 CreditAttribution: martin107 commented+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.
Comment #70
davidwbarratt CreditAttribution: davidwbarratt commentedtsoeckler,
The patch in #68 doesn't validate. :(
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?
Comment #71
Mile23Still 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 ofindex.php
.Do this instead:
Find that you didn't need the new requirement? Just do this:
The problem here is cultural, not technical: It is not hacking core to use Composer with Drupal. :-)
Comment #72
davidwbarratt CreditAttribution: davidwbarratt commented#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
andindex.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. :)Comment #73
nicholasruunu CreditAttribution: nicholasruunu commented#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
Comment #74
tstoecklerRe @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.
Comment #75
tstoeckler@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.
Comment #76
davidwbarratt CreditAttribution: davidwbarratt commentedAttached is the patch with -C -M as requested. :)
Thanks!
Comment #77
davidwbarratt CreditAttribution: davidwbarratt commented#74,
Also, the reason I'd prefer for it to validate is:
composer.json
file needs to be composer-valid.composer.json
files).Comment #78
mradcliffeShould this be semantic version now?
Comment #79
davidwbarratt CreditAttribution: davidwbarratt commented#78,
Please read #42 which is not only the answer to your question, but also the answer to life, the universe, and everything.
Comment #80
EclipseGc CreditAttribution: EclipseGc commentedHow long you been holding onto that one?
Eclipse
Comment #81
RobLoachIs there anything holding this back from RTBC? Looks pretty good to me.
Comment #82
tstoecklerYeah, 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.
Comment #84
Mile23Making
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 ofcore/
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.
Comment #85
davidwbarratt CreditAttribution: davidwbarratt commentedRe-roll.
Comment #86
davidwbarratt CreditAttribution: davidwbarratt commentedComment #87
bojanz CreditAttribution: bojanz commentedThis 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
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.
Comment #88
davidwbarratt CreditAttribution: davidwbarratt commented#84,
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.
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.
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.
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'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.
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?
Comment #89
donquixote CreditAttribution: donquixote commentedThe following is just a note about index.php.
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:
Or simply
Comment #90
donquixote CreditAttribution: donquixote commentedAbsolutely +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:
Sub-problem: A new module version introduces a new Composer package dependency.
(I think this scenario is far less relevant than the first one)
Besides that we still need the basic Drupal stuff to work:
From the issue description, I think this issue only attempts to solve this one problem:
I think this is a bit weak.
Comment #91
davidwbarratt CreditAttribution: davidwbarratt commentedComment #92
davidwbarratt CreditAttribution: davidwbarratt commented#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.
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: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.
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). :(
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: Use a single vendor directory in the root
Comment #93
Mile23If the point is to have a project-level
composer.json
as well as a core-onlycomposer.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.
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.
Comment #94
davidwbarratt CreditAttribution: davidwbarratt commented#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: Use a single 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 inindex.php
is this line: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.Comment #95
davidwbarratt CreditAttribution: davidwbarratt commentedComment #96
davidwbarratt CreditAttribution: davidwbarratt commented#90,
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.
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. :)
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: [policy, no patch] Switch to Semantic Versioning for Drupal contrib extensions (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. :)
The proposed solution solves this problem by allowing you to modify the
<root>/composer.json
file.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.
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.
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 newcomposer.json
file (use this example if you want to follow along). And executecomposer 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.The proposed solution adds
to the
example.gitignore
file. Simply uncomment those lines and Drupal core will be out of the repository. On deployment, they'll need to executecomposer install
for Drupal core to be downloaded into thecore
directory.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 incore/vendor
.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.
The proposed solution does not change this at all.
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.
Comment #97
daffie CreditAttribution: daffie commentedAs 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:
I would like to mention that I support this issue very much. +1 for me!
Comment #98
donquixote CreditAttribution: donquixote commented@davidwbarratt (#96)
Thanks a lot for these explanations!
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:
I think Composer manager already does this or not?
So it is already one step ahead from a UX perspective compared to this patch.
Comment #99
donquixote CreditAttribution: donquixote commentedAnother question is what file structure will we have after this patch? Will modules go to vendor/Drupal/(modulename) ?
Comment #100
davidwbarratt CreditAttribution: davidwbarratt commented#93,
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.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 thecore/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.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.
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?
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.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
andcore
. 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.Comment #101
davidwbarratt CreditAttribution: davidwbarratt commented#97 & #98,
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.
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 shinnycomposer.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 insites/default/files
and they set thevendor-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:
composer.json
file (it will be overridden).composer require
(it will be removed).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. :)Comment #102
Anonymous (not verified) CreditAttribution: Anonymous commentedIn 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.
Comment #103
davidwbarratt CreditAttribution: davidwbarratt commented#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.
Comment #104
greg.1.anderson CreditAttribution: greg.1.anderson commentedTL/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:
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 useCOMPOSER_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):
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.
Comment #105
davidwbarratt CreditAttribution: davidwbarratt commented#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.
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.
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.
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.
As I discussed in #100 we are not changing the way "Composer usually works" (whatever that even means).
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.
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 runcomposer update
, Composer will destroy everything in thevendor/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 ofdrupal
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.
You can fix this now with my plugin, but you'd still have the problem described above.
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."
This issue does not prevent people from using drush in anyway.
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: Use a single vendor directory in the root so the changes toindex.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 changeindex.php
.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.
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.
Comment #106
greg.1.anderson CreditAttribution: greg.1.anderson commented#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.
Comment #107
davidwbarratt CreditAttribution: davidwbarratt commentedI'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. :)
Comment #108
greg.1.anderson CreditAttribution: greg.1.anderson commentedI 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:
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.
Comment #109
daffie CreditAttribution: daffie commented#2377281: Upgrade to Symfony 2.6 stable has landed, so this needs a reroll.
Comment #110
moshe weitzman CreditAttribution: moshe weitzman commentedPatch looks good to me, and moves us ahead a bit.
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).
Comment #111
daffie CreditAttribution: daffie commented@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.
Comment #112
daffie CreditAttribution: daffie commentedComment #113
davidwbarratt CreditAttribution: davidwbarratt commentedAttached is the Re-Rolled patch.
Comment #115
davidwbarratt CreditAttribution: davidwbarratt commenteddoh! and again. :)
Comment #116
davidwbarratt CreditAttribution: davidwbarratt commentedComment #117
davidwbarratt CreditAttribution: davidwbarratt commented#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:
which should copy the
drupal
project and loadcore
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:
which is the same instructions Symfony uses as it's primary download method.
Comment #118
greg.1.anderson CreditAttribution: greg.1.anderson commentedSlight typo in #117. This worked:
composer create-project --repository-url="http://davidwbarratt.com" drupal/drupal path/ dev-master
Comment #119
daffie CreditAttribution: daffie commentedI installed a new drupal site with the command:
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.
Comment #120
mradcliffeI'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:
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.
Comment #121
davidwbarratt CreditAttribution: davidwbarratt commented#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 requiredrupal/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 movecore
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.Comment #122
daffie CreditAttribution: daffie commentedI 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.
Comment #123
mradcliffe@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.
Comment #124
greg.1.anderson CreditAttribution: greg.1.anderson commented#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.
Comment #125
greg.1.anderson CreditAttribution: greg.1.anderson commentedI 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 getcomposer 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" whencomposer create-project
asks you if you want to remove the vcs files, then you can pull in these updates viagit 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 usegit 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?
Comment #126
davidwbarratt CreditAttribution: davidwbarratt commented#123,
Thanks for your help, let me step through this and see what we can do.
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:You could either execute a command like this:
or you could modify your
composer.json
file and add:and then execute:
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: [policy, no patch] Switch to Semantic Versioning for Drupal contrib extensions (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: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:
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.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: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 runcomposer update
which will update the module itself, plus any dependencies (including ones that have been added/removed).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.composer.json
. This is similar to the way.htaccess
and themodules
folder is handled. However, you can update Drupal core via composer by simply executingcomposer update
which seems like an easier way to update Drupal than to ensure thatcomposer.json
andindex.php
are not being overriden. Actually, if you were to manually update Drupal core, you'd have to runcomposer 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 yourindex.php
file back tocore/vendor/autoload.php
.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.Comment #127
Mile23Again, 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 adrupal.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
Comment #128
davidwbarratt CreditAttribution: davidwbarratt commented#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.
right... but somehow we still have
.htaccess
andexample.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.
The front-controller is
index.php
it's a technical item, not a whole package/project, but sure,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.).
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
toexample.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.Comment #129
davidwbarratt CreditAttribution: davidwbarratt commentedComment #130
daffie CreditAttribution: daffie commentedLet 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:
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. :-(
Comment #131
greg.1.anderson CreditAttribution: greg.1.anderson commented#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.Comment #132
mradcliffe#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.
Comment #133
daffie CreditAttribution: daffie commented@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.
Comment #134
mradcliffe@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.
Comment #135
davidwbarratt CreditAttribution: davidwbarratt commented#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 thecore/
folder or just executecomposer 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?
Comment #136
daffie CreditAttribution: daffie commentedThe 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.
Comment #137
davidwbarratt CreditAttribution: davidwbarratt commented#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?
Comment #138
daffie CreditAttribution: daffie commented@davidwbarrat: Are you interested in my super simple hello world module. Works in combination with https://packagist.org/packages/abbert/helloworld.
Comment #140
mradcliffe#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!
Comment #141
davidwbarratt CreditAttribution: davidwbarratt commented#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.
Comment #142
davidwbarratt CreditAttribution: davidwbarratt commented#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-writescomposer.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.Comment #143
mradcliffe#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.
Comment #144
daffie CreditAttribution: daffie commented@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.
Comment #145
mradcliffe@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.
Comment #146
davidwbarratt CreditAttribution: davidwbarratt commentedIf 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.
Comment #147
mradcliffeNo, 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.
Comment #148
mradcliffeBasically - with this patch we force all users to be dev ops.
Comment #149
daffie CreditAttribution: daffie commented@mradcliffe: Can you post the /composer.json and the /core/composer.json files.
Comment #150
daffie CreditAttribution: daffie commented@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.
Comment #151
daffie CreditAttribution: daffie commentedI 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.
Comment #152
mradcliffeI 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.
Comment #153
mradcliffeUpdated title and issue summary to reflect what the proposed solution allows for so that a reviewer is not confused.
Comment #154
chx CreditAttribution: chx commentedWe ship with
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.
Comment #155
daffie CreditAttribution: daffie commented@chx: For that we have #2372815: [meta] Make the <root>/composer.json file a working example and we also need this #2373203: [no patch] Take over ownership of drupal/drupal and drupal/core on Packagist.
Comment #156
chx CreditAttribution: chx commentedThat 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.
Comment #157
daffie CreditAttribution: daffie commented@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.
Comment #158
davidwbarratt CreditAttribution: davidwbarratt commentedI'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 ofdrupal/drupal
. I've opened up another follow-up to fix that #2388709: Rename example.composer.json to composer.json.Comment #160
daffie CreditAttribution: daffie commentedIs 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.
Comment #163
davidwbarratt CreditAttribution: davidwbarratt commented#160,
I don't really care if it's
example
ordefault
. I only went withexample
because we have anexample.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.orgAny idea why Drupal Installation would fail with that patch?
Comment #164
davidwbarratt CreditAttribution: davidwbarratt commentedI installed Drupal with the patch on simplytest.me without a problem. Is this a test-bot issue? :/
Comment #165
daffie CreditAttribution: daffie commented@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.
Comment #166
chx CreditAttribution: chx commentedit'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.
Comment #167
davidwbarratt CreditAttribution: davidwbarratt commented#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?
Comment #168
daffie CreditAttribution: daffie commentedCan the problem be that there is no /composer.json file any more?
Comment #169
davidwbarratt CreditAttribution: davidwbarratt commentedI 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.Comment #170
adammaloneThe /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.
Comment #171
davidwbarratt CreditAttribution: davidwbarratt commentedDoesn'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
?Comment #172
davidwbarratt CreditAttribution: davidwbarratt commentedGreat... 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?
Comment #173
adammaloneTo start the ball rolling and bring the Drush folk in on this, I've submitted https://github.com/drush-ops/drush/pull/1040.
Comment #174
daffie CreditAttribution: daffie commentedI 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.
Comment #175
davidwbarratt CreditAttribution: davidwbarratt commentedComment #176
alexpottThe 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.
Comment #177
greg.1.anderson CreditAttribution: greg.1.anderson commentedI 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.
Comment #178
davidwbarratt CreditAttribution: davidwbarratt commented#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.
Comment #179
alexpott@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.
Comment #180
davidwbarratt CreditAttribution: davidwbarratt commented#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: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.Comment #181
alexpott@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.
Comment #182
davidwbarratt CreditAttribution: davidwbarratt commented#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!
Comment #183
daffie CreditAttribution: daffie commentedI would like to thank alexpott, being a core maintainer, for stepping in and helping this issue get to RTBC. This issue really needed that.
Comment #184
davidwbarratt CreditAttribution: davidwbarratt commentedComment #185
davidwbarratt CreditAttribution: davidwbarratt commented#147,
I thought I'd take some time and respond to your comment.
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 requiredrupal/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: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 ondrupal/core
. If you're project doesn't depend ondrupal/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 ondrupal/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 executecomposer update
and everything (Drupal core, as well as the package you added) will be updated.Comment #186
davidwbarratt CreditAttribution: davidwbarratt commentedComment #187
mradcliffeI 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.
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.
Comment #188
greg.1.anderson CreditAttribution: greg.1.anderson commentedI 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.
Comment #189
nicholasruunu CreditAttribution: nicholasruunu commented@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.
Comment #190
nicholasruunu CreditAttribution: nicholasruunu commented-- double post --
Comment #191
dawehnerLet'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.
Comment #192
alexpottWhat 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.
Comment #193
alexpottWe need a new patch.
Comment #194
alexpottAlso 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).
from core/UPGRADE.txt - it looks like we should be changing this advice in this patch to include composer.json
Comment #195
mradcliffe@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.
Comment #196
davidwbarratt CreditAttribution: davidwbarratt commentedAttached 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.
Comment #197
webchickI 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:
....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.
Comment #198
greg.1.anderson CreditAttribution: greg.1.anderson commented@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:
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:
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.Comment #199
davidwbarratt CreditAttribution: davidwbarratt commented#197,
You're really close, instead of depending on
drupal/drupal
your project would depend ondrupal/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 blankcomposer.json
file and add this:And you'd have drupal core downloaded and installed into the
core
directory. You can do this withsymfony/symfony
as well aslaravel/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:
Notice that we are not creating
drupal/core
we're starting a newdrupal/drupal
project. Once Composer clonesdrupal/drupal
it will execute composer install. Since thecomposer.json
file in the root ofdrupal/drupal
requiresdrupal/core
just like in our custom example above, you are left with exactly the same thing as if you'd created acomposer.json
file yourself, only this time you also haveindex.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?
Comment #200
Jeff Burnz CreditAttribution: Jeff Burnz commentedHow 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.
Comment #201
davidwbarratt CreditAttribution: davidwbarratt commented#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.
Comment #202
greg.1.anderson CreditAttribution: greg.1.anderson commented@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 ofgit 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.
Comment #203
greg.1.anderson CreditAttribution: greg.1.anderson commentedRelated issue: For those who are interested, there is a discussion on using Composer with Drush extensions in the Drush issue queue.
Comment #204
greg.1.anderson CreditAttribution: greg.1.anderson commentedI 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.
Comment #205
greg.1.anderson CreditAttribution: greg.1.anderson commentedForgot 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.
Comment #206
davidwbarratt CreditAttribution: davidwbarratt commented#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!
Comment #207
greg.1.anderson CreditAttribution: greg.1.anderson commented#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.
Comment #208
tim.plunkettIt 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.
Comment #211
davidwbarratt CreditAttribution: davidwbarratt commented#208,
Per #181 & #182 the name will stay
composer.json
and not be changed.
I've renamed #2372815: [meta] Make the <root>/composer.json file a working example so this is more clear.
Comment #213
alexpottThe last fail appears to be an unrelated random fail - see #2392865: Random fail in \Drupal\system\Tests\Entity\EntityOperationsTest
Comment #215
effulgentsia CreditAttribution: effulgentsia commentedBack to RTBC, as it was prior to the random fail in #209.
Comment #216
Mile23To 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?
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:
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:
Comment #217
davidwbarratt CreditAttribution: davidwbarratt commented#216,
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
andvendor
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 inUPGRADE.txt
like everyone else.My only change to your workflow is in this part:
You wouldn't do:
instead you would do:
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:
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:
Lastly, is there some work that needs to be done on this? Why did you change it to "Needs Work"?
Comment #218
davidwbarratt CreditAttribution: davidwbarratt commentedAlso, as recommended by Composer and Acquia, you're probably not going to commit the
core
andvendor
directories to your version control (hence the update toexample.gitignore
). You will commit yourcomposer.lock
file which is updated oncomposer update
.However, if you'd like to have the directories committed to version control, you certainly can do that as Composer says you can.
Comment #219
daffie CreditAttribution: daffie commentedMile23 removed the RTBC status and I cannot find a reason in his comment why that is needed, so back to RTBC.
Comment #220
bojanz CreditAttribution: bojanz commentedHappy to see the updated issue summary, and RTBC status.
An important step towards solving this problem.
Comment #221
davidwbarratt CreditAttribution: davidwbarratt commentedComment #225
RobLoachUpdated to latest 8.0.x... Tracking on GitHub via https://github.com/RobLoach/drupal/pull/1 .
Comment #226
RobLoachComment #227
daffie CreditAttribution: daffie commentedNeeds reroll from the patch from comment #196.
I am missing this pieces from the previous patch.
I do not think that this change is right.
Comment #228
hussainwebAttempting a reroll...
Comment #229
daffie CreditAttribution: daffie commentedA 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.
Comment #230
alexpottOne 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.
Comment #231
davidwbarratt CreditAttribution: davidwbarratt commentedI've added the Change Record (let me know if it needs something):
https://www.drupal.org/node/2404131
Comment #232
davidwbarratt CreditAttribution: davidwbarratt commentedComment #233
alexpottI'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.
Comment #234
tstoecklerYes, that CR looks great, thanks!
Comment #236
alexpottCommitted da28864 and pushed to 8.0.x. Thanks!
Thanks for adding the beta evaluation.
Comment #237
nicholasruunu CreditAttribution: nicholasruunu commentedWooohoo, thanks everyone!
Comment #238
bojanz CreditAttribution: bojanz commentedOn 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).
Comment #239
chx CreditAttribution: chx commentedCare 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".
Comment #240
alexpottFomr #188
Let's get that followup created. Also the rename is not critical.
Comment #241
plachComment #242
bojanz CreditAttribution: bojanz commentedThe 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.
Comment #243
chx CreditAttribution: chx commentedThis 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.
Comment #244
alexpottHow 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.
Comment #245
chx CreditAttribution: chx commentedComment #246
webchickDid 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.
Comment #247
davidwbarratt CreditAttribution: davidwbarratt commentedComment #248
davidwbarratt CreditAttribution: davidwbarratt commentedI'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!
Comment #251
i.bajrai CreditAttribution: i.bajrai commentedComment #252
dwwI now this has been Closed(fixed) for about 2+ years, so I doubt anyone will notice, but I just posted to the Composer group at g.d.o about a fairly huge pain-point of trying to use composer to manage a site as advocated for here (and in other places):
Why does the "best practice" of using composer to manage a site result in so many composer.lock merge conflicts?
Comments welcome. If I'm doing something stupid, I'd love to know. ;) If I'm basically doing it right, but there's a good way to deal with the problems that doing it right creates, I'd love to document the solution in the appropriate places.
Thanks!
-Derek