Add composer_dependency module, have it help users figure out how to install Composer dependencies [#2536576]

Problem/Motivation

There are a lot of different approaches to Composer in Drupal 8.

We need a single way to provide info to the less-sophisticated user that they need to learn some Composer in order to proceed.

We should support this in core, because if we don't, we run the risk of having overlapping or mutually-exclusive processes imposed by contrib developers.

Note that this proposal does *not* manage any dependencies through Drupal. It *only* tells the user whether Composer-based dependencies are present or not.

Proposed resolution

A core module that works like this:

For The Site-Builder:

Contrib modules which need to use Composer-based dependencies should declare their dependency on composer_dependency module.

When a user tries to install the contrib module, composer_dependency will also be installed.

During its installation phase, composer_dependency will implement hook_requirements() to check and see if the Composer package is installed as a dependency of Drupal. This is because we'll use various Composer classes to perform other dependency checking.

If Composer is not installed as a dependency, composer_dependency will not install, forcing the user to install it. Like this:

cd core
composer require composer/composer

Obviously this would change if we change where Drupal's vendor/ directory is, and the current dual-composer.json system might need to be revisited, or supported differently here.

Now composer_dependency will allow itself to be installed.

The user can then go to the status report page of their site and composer_dependency will tell them which modules have unmet dependencies.

For Contrib Developers:

This module will handle telling the user that dependencies are unmet for your module.

If you need to check, you'd get the Drupal dependency service and ask it:

$drupal_requirements = \Drupal::service('composer_dependency.drupal_requirements');
$has_dependencies = $drupal_requirements->checkForModule('mymodule');

The service is based on a factory, which will throw a RuntimeException if it's unable to create the service because Composer isn't installed as a dependency. Generally, you don't want to catch this; you want the user to run into this problem.

Remaining tasks

Discuss.

Refactor.

Add tests.

User interface changes

API changes

Data model changes

Comment	File	Size	Author
#8	interdiff.txt	6.07 KB	Mile23
#8	2536576_8.patch	11.82 KB	Mile23
#8	8.0.x: PHP 5.5 & MySQL 5.5 12,956 pass, 2 fail
#2	2536576_2.patch	9.99 KB	Mile23
#2	8.0.x: PHP 5.5 & MySQL 5.5 12,942 pass, 1 fail
#1	2536576_1.patch	33.81 KB	Mile23
#1	8.0.x: PHP 5.5 & MySQL 5.5 12,942 pass, 1 fail

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 20 July 2015 at 19:19

Status:

Active

» Needs review

File	Size
2536576_1.patch	33.81 KB
8.0.x: PHP 5.5 & MySQL 5.5 12,942 pass, 1 fail

Here's a patch. It's a proof-of-concept.

It creates a module.

Go to the modules page and install it, and it will fail with an error message saying you need to install Composer as a dependency.

So do this at the command line:

cd core
composer require composer/composer

Now you can install composer_dependency.

Go to the status report page.

You'll now see a warning saying that composer_dependency has unmet dependencies.

That's because composer_dependency arbitrarily depends on crell/api-problem in its composer.json file, for demo purposes.

So again go to the command line:
composer require crell/api-problem

Reload the status page and you'll see a message saying all Composer-based dependencies are met.

@todo:

Tests.
Refactor so it's all way better and more testable.
There's a problem in that I'm checking the version constraints incorrectly, so we can't use versions like ~1.7, and must say 1.7. If there's motion on this, I'll do more research and get it figured.

Comment #2

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 20 July 2015 at 19:05

File	Size
2536576_2.patch	9.99 KB
8.0.x: PHP 5.5 & MySQL 5.5 12,942 pass, 1 fail

Ugh. Bad patch.

Here's a better one.

Comment #3

20 July 2015 at 19:28

The last submitted patch, 1: 2536576_1.patch, failed testing.

Comment #4

20 July 2015 at 19:31

Status:

Needs review

» Needs work

The last submitted patch, 2: 2536576_2.patch, failed testing.

Comment #5

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 21 July 2015 at 01:17

Parent issue:

» #2002304: [META] Improve Drupal's use of Composer

Comment #6

twistor CreditAttribution: twistor as a volunteer commented 21 July 2015 at 04:51

The user can then go to the status report page of their site and composer_dependency will tell them which modules have unmet dependencies.

This seems like a broken design. If we go this route, modules shouldn't be installable if they're missing dependencies. If I use a dependency in hoot_init(), then the whole site is broken while dependency is missing. To combat that, I have to create my own hook_requirements(). So what's the point :)

Thinking a bit more about it, it seems like we don't even need a module, but a helper method for modules to check their composer deps would be very useful. Then, module authors can decide if they're blocking or not.

Comment #7

timmillwood

English

🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK

CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented 21 July 2015 at 07:30

Issue tags:

+Needs beta evaluation, +Needs product manager review

Everyone seems to be running around like headless chickens due to composer dependencies in D8. I think you're forgetting we had composer (and other) dependencies in D7 too.

In D7 there were a handful of modules that loaded external dependencies via composer, and these all required the composer manager module. I think this could also be a good solution for site builders in D8 too, however I would hate to see modules set composer_manager in the dependencies of their info.yml, because it's possible to install the composer dependencies without composer_manager.

Another thing in D7 (and D6 and D5) that was done is depending on libraries like for third party APIs or Javascript frameworks or WYSIWYG editors. All that was done to tell the developers / site builder what to do was a INSTALL, README or project page guide.

So I think at the moment I am against composer_dependency in core, not sure why it can't go in contrib to be honest.

Comment #8

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 21 July 2015 at 15:46

Status:

Needs work

» Needs review

File	Size
2536576_8.patch	11.82 KB
8.0.x: PHP 5.5 & MySQL 5.5 12,956 pass, 2 fail
interdiff.txt	6.07 KB

If I use a dependency in hoot_init(), then the whole site is broken while dependency is missing. To combat that, I have to create my own hook_requirements(). So what's the point :)

There's no more hook_init(). https://www.drupal.org/node/2013014

But yah, the point is that your subscriber/listener can use the service instead of writing all that code yourself. Also, everyone can use it the same code the same way. Also, we're doing the user a favor here by having one way to show that dependencies are unmet.

Attached is a patch that adds a checkForModule() method to the service. So now your subscriber/listener just says something like this:

if (!$repository_service->checkForModule('myModule')) {
  drupal_set_message("Nope. Need to install stuff in Composer.");
}

Thinking a bit more about it, it seems like we don't even need a module, but a helper method for modules to check their composer deps would be very useful. Then, module authors can decide if they're blocking or not.

That's what the service is. We make it a dependency module rather than an always-on service for the cases where there are no Composer dependencies and the user doesn't need to care.

One of the big design problems we've encountered is that, as a site-builder, once you go down the Composer path, you can't go back. This module makes that explicit.

Everyone seems to be running around like headless chickens due to composer dependencies in D8. I think you're forgetting we had composer (and other) dependencies in D7 too. [..] So I think at the moment I am against composer_dependency in core, not sure why it can't go in contrib to be honest.

It would be in core because then we all standardize on at least *some* behavior. Requiring a contrib module makes it one step more complicated for the user who hasn't yet learned much about Composer or CLI or whatever.

composer_dependency just adds a minimal behavior so the user can check what's wrong.

Comment #9

21 July 2015 at 16:12

Status:

Needs review

» Needs work

The last submitted patch, 8: 2536576_8.patch, failed testing.

Comment #10

twistor CreditAttribution: twistor as a volunteer commented 21 July 2015 at 16:26

That's what the service is. We make it a dependency module rather than an always-on service for the cases where there are no Composer dependencies and the user doesn't need to care.

Ahh, I didn't look at the code, was just going off the description and comments.

Everyone seems to be running around like headless chickens due to composer dependencies in D8. I think you're forgetting we had composer (and other) dependencies in D7 too.

I don't think anyone is forgetting that. There are much larger hurdles to using Composer in 8.x since Drupal core is also using it. Core's dependencies have to be resolved with contrib's.

I think this is an important step.

Tangent: I would say something ala composer_manager's drupal-rebuild should be in core as well. Perhaps in the scripts directory. Something that will aggregate module's composer deps. Trying to manage module's composer dependencies manually is tedious, error prone, and defeats the whole point of composer.

Holding off on an actual code review until this gets a bit more traction.

Comment #11

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 21 July 2015 at 17:01

Tangent: I would say something ala composer_manager's drupal-rebuild should be in core as well.

I'm intentionally trying to keep this as a status-only module for now. We can iterate later.

I just think its important for core to have some way for contrib developers to tell the site-builder what's going on with dependencies.

Holding off on an actual code review until this gets a bit more traction.

Yah, it's still not ready for prime time. Waiting for the same thing before proceeding too far. :-)

Comment #12

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 22 July 2015 at 16:51

Issue summary:

View changes

Edited to make it clearer what contrib developers should do.

Comment #13

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 22 July 2015 at 16:55

Issue summary:

View changes

Comment #14

Fabianx CreditAttribution: Fabianx as a volunteer commented 23 July 2015 at 07:25

Category:	Feature request	» Task
Priority:	Normal	» Major

Comment #15

timmillwood

English

🏴󠁧󠁢󠁷󠁬󠁳󠁿 Wales, UK

CreditAttribution: timmillwood at Appnovation for Pfizer, Inc. commented 23 July 2015 at 08:01

+++ b/core/modules/composer_dependency/composer.json
@@ -0,0 +1,16 @@
+        {
+            "name": "Paul Mitchum",
+            "email": "paul@mile23.com"
+        }
+    ],

I don't think we need to state the author.

+++ b/core/modules/composer_dependency/composer.json
@@ -0,0 +1,16 @@
+        "crell/api-problem": "1.7"

You're requiring this package but you've not added it as part of the patch.

+++ b/core/modules/composer_dependency/composer_dependency.info.yml
@@ -0,0 +1,6 @@
+name: Composer Dependency
+type: module

I think this would be better as a core component that gets called by system or update module.

Comment #16

Fabianx CreditAttribution: Fabianx as a volunteer commented 23 July 2015 at 08:11

I agree, making it a module is not necessary, we already require composer in core, so it is a dependency already.

Thanks for the initiative here!

Comment #17

Mile23

English

Seattle, WA

CreditAttribution: Mile23 as a volunteer commented 23 July 2015 at 18:02

#15 @timmillwood: This is a proof of concept. It has a composer.json file to show the behavior. That would be removed before RTBC.

#16 @fabianx: We don't require composer in core. (Search core/composer.json and you'll see.) We have enough composer to support autoloading and stuff, but not the package itself.

If we did require it, this would be easier, but I don't see a reason to, other than to support this module's behavior.

Also, it's a module so that we can limit its effects to people who find themselves needing a Composer-based dependency. This way it never complicates life for anyone else.