Superfluous package dependencies.

[generalredneck]

In Issue #4 it was mentioned that you might be willing to take suggestions on what make and info files to ignore. I'm thinking that it might be benificial that only the "Dependencies" of the module that matches the package name be required. However, the dependencies of the other modules that are both not required by the package you are installing and are not named the same as the project should only be marked as recommended.

For instance, lets take ctools for example.

In this case we have the ctools module which is part of the ctools project. In this case I would put drupal/ctools in my composer.json. This should mean that the minimal requirements for me to run this ctools project would be nothing as shown in this info file

name = Chaos tools
description = A library of helpful tools by Merlin of Chaos.
core = 7.x
package = Chaos tool suite
files[] = includes/context.inc
files[] = includes/math-expr.inc
files[] = includes/stylizer.inc

now if I wanted to install some other module such as views_content which is a child module of the ctools module, I should then be required to get the correct dependencies, however, I am forced to download views_content if I even want ctools, therefore I forced to have "views" as a requirement. If I was to specify "drupal/views_content" I think that would be a different story. "Views" would then be a requirement as well as ctools because it's a depency of the views_content module.

I know there are some project names that don't match any name of the modules in the project, but I think I would be ok with recommended being the default.

[kasperg]

:+1: I just had the exact same issue when installing CTools.

[winmillwill]

If I understand correctly, this is just confusing behavior and not actually an issue at all; all the packages that got downloaded also had their dependencies downloaded, and nothing failed to work due to an unfulfilled dependency. This is the contract of every single package manager that resolves dependencies at all. Just because you didn't actually enable any of the example modules or other modules in a project doesn't mean you can expect that your package manager knew you would do that.

Now, all that said, there may be a solution for working around the (archaic) way that Drupal packages modules. If we remove all the 'replaces' links, and instead have metapackages for every module in a project whose name is not the same as the project, then we can support some wacky composer.json files like this:

{
...
require: {
  "drupal/views_content": "7.*",
  "drupal/commerce_checkout_ui": "7.*",
  "drupal/wysiwyg_test": "7.*",
  "drupal/variable_realm": "7.*"
  ...etc
}
...
}

The drawback to this is that for something like commerce, you end up having to specify every single module in the project whose dependencies you want to fulfill, even though they are all on disk, and they will all have to be the same version. So, if you just want drupal/commerce: '~7.2.0' then you need to do more work to actually get addressfield installed. At this point, you probably solve the problem by creating a metapackage of your own to just download all the things, but now you're back to the original behavior, minus the bizarre inclusion of the dependencies of example modules.

Would the metapackage solution result in better behavior from your points of view @generalredneck @kasperg ?

[kasperg]

I am OK with either suggestion. If views_content is crucial to your project I think it is fine to require it explicitly to have it's dependencies fulfilled by the package manager. recommends works for me as well.

I need to use static.drupal-packagist.com more before I see the side effects of either proposal so for now I will leave the decision to however takes a stab at this issue.

I think it is the contract of a package manager to resolve required dependencies for a package. If the developer only declares ctools as a dependency then it is only the the dependencies for ctools that should be resolved. I consider it a package issue that it also contains subpackages that cannot be defined as a dependency explicitly. These should either be split into subpackages with their own dependencies (as you suggested) or the dependencies should be optional (as originally suggested).

Some PHP projects take the latter approach e.g. Assetic, PHP GitHub API and Symfony HttpKernel.

[webflo]

Moved submodule "requirements" into "suggest" section in https://github.com/webflo/drupal-parse-composer/tree/submodule-dependencies

Some examples:

• http://drupal-packagist.webflo.io/packages/drupal/ctools
• http://drupal-packagist.webflo.io/packages/drupal/panels

[winmillwill]

After composer install you should not have anything on disk that does not work. This is exactly what it means to recursively resolve dependencies in software packages.

Suppose you have composer dependencies like this:

{
  ...
  "require": {
    "commerce": "7.1.*"
  }
}

If you then do drush en -y commerce_cart, then what should happen? With the suggested changes, you end up with some unknown version of views downloaded and enabled. Now every developer workstation and every server can have potentially different versions of views.

If you stick to the current behavior, it all works correctly: you get the dependencies of commerce_cart folded into the dependencies of commerce because you can't download commerce without downloading commerce_cart. As it turns out, that is exactly equivalent to commerce requiring commerce_cart!

What this actually means is that the metapackage solution I gave above doesn't work.

In the case of ctools, (and merlin's other modules) the reason the behavior is confusing is that there are example modules with dependencies on other modules. What we could do is agree on a way of identifying modules like these that are not meant to be enabled on a real project. What we would be deciding is that it's ok for those modules to be on disk and for drush en -y and the modules admin page to behave as described above in the case of those modules. We could simply use regex like '^(._)(example|test)(.)_$' and filter out all subdirectories and info files that have the word 'example' or 'test'. This way commerce doesn't have the issues I described above, and merlin's modules don't have the strange behavior.

@webflo @kasperg @generalredneck @dsdobrzynski @craychee what do you think?

[kasperg]

After composer install you should not have anything on disk that does not work

I do not agree with this premise. The examples above show that it does not hold for other non-Drupal packages either.

I am for the solution provided by @webflo. The messages associated with each suggestion specifying exactly which submodule requires each suggestion is a nice addition.

The problem is not only caused by example modules. Panels' submodules i18n_panels require i18n, Ctools' views_content submodule require views etc.

In my opinion your example with drush en -y commerce_cart is problematic because we have currently have separate mechanisms for managing dependencies and drush will happily download modules on its' own. If Drupal adopts composer fully then this should no longer be the case and drush could remind the user to run composer require drupal/views or the like.

[derhasi]

I'm preferring the suggest approach as well. I would want to have the minimal dependencies downloaded so the modules folder would only hold those. Especially when thinking about searching for bugs, I do not want to have code lying there, that isn't used.

I do not think that process will harm any automation, as for the drush en-example --resolve-dependencies should not be used anyway when managing dependencies with composer. You wouldn't use drush dl or drush up(c) in that case either.

We have some conceptional problems with submodules and subthemes in Drupal general. Drush cannot resolve that either and only can try to find a package with the same name. Introducing metapackages could resolve something like that, but currently I think the best way is to maintain relevant dependencies in the root composer.json, as so we really have full control about what we want to build. Otherwise we would end up writing a bunch of replace statements for every dependency that is only necessary for a submodule.

[webflo]

I think we can mark this issue as fixed. Triggered a update on all existing packages an hour ago.