drupal-project icon indicating copy to clipboard operation
drupal-project copied to clipboard

Document how to merge in composer.json of local extensions

Open yched opened this issue 9 years ago • 18 comments

TL;DR : I created a plugin at https://github.com/yched/composer-local-modules, that handles composer.json's from locally committed custom modules and themes. I propose we include it in the composer.json template shipped in the project.

Rationale :

composer_manager.module took care of collecting composer.json's from contrib and custom alike. We intend to make composer_manager obsolete, but currently don't cater for custom code.

One could say that, since those modules are custom to the project and part of the project repo, their dependencies have to be directly added to the project root composer.json.

However, this is not ideal for a couple reasons:

  • you lose track of which module requires what, all dependencies of all custom code are mushed together in the root composer.json. This defeats efforts for structuring custom code into clear units, and after a while, this adds to maintainance burden.
  • there are cases when patching is not enough, and you want to take a contrib module and commit a fork locally (even temporarily). In those cases, you'd need to manually integrate its requires into your root composer.json (and remove them later when you unfork the module and get back to the contrib version)
  • when develpping/maintaining your own contrib module locally, you want to keep its local composer.json as the official one, without manually mirror it into the main composer.json

It is more convenient to allow custom modules to have their own composer.json, and include them as part of dependency solving just like for contrib modules (since a custom module can become contrib, and reversedly).

https://github.com/yched/composer-local-modules uses a custom, local repository type that discovers custom modules/themes with a composer.json and automatically adds them as requires in the root package. This happens at runtime on composer install/update, so it leaves the root composer.json untouched, but tracks the corresponding packages them in composer.lock and installed.json)

yched avatar May 23 '15 22:05 yched

Also, if that is deemed useful, I would happily hand the code over to the drupal-composer vendor so that we can centralize "drupal / composer best practices and tools" to a unified vendor ?

yched avatar May 23 '15 22:05 yched

I'm not really sure if that is a good practice. In terms of contrib module development or extensive contrib patching, I always would recommend to have a separate repository for that.

In case of a custom module, that needs to have its dependencies tracked separately, we should think of the reasons, why we want to do that. I guess in most cases, we it's because we want to encapsulate functionality and maybe make that reusable in a similar project. In that case, I would to tend to put that module in a custom repository.

We already have the "patch-workflow" as an exception on a good composer workflow. I do not think adding "local composer files" will make that a straight-forward easy to use approach. But I'm really not sure.

derhasi avatar May 24 '15 01:05 derhasi

Agreed that my 3rd point about "maintaining your contrib module" is kind of moot, since if it's a published contrib, it has a package on drupal packagist, and thus you bring it on the test site you use for dev/debug as an external dependency, not as locally committed code.

The other two points do stand though.

In terms of contrib module development or extensive contrib patching, I always would recommend to have a separate repository for that.

Can be debated. I'd personnally hate to be forced to use a separate repo on my projects :-)

we should think of the reasons, why we want to do that. I guess in most cases, it's because we want to encapsulate functionality ...

It's mostly about keeping custom modules as separate units, yes, just like contrib modules are. You write separate code units, and figuring out the union of their Why should I have to mush their dependencies myself (and later maintain that mushed, indifferenciated list of dependencies as my code evolves), when figuring out the common deps for of a bunch of code units is precisely the job of composer :-)

... and maybe make that reusable in a similar project. In that case, I would to tend to put that module in a custom repository

Except that you don't always know in advance, you write code for a site, and then later you might decide it contribute it, or reuse it in internal projects. But it often starts as custom code on a specific site. Not treating custom and contrib modules differently is a win, IMO.

We already have the "patch-workflow" as an exception on a good composer workflow

I don't see why either patching or handling local packages would be "exceptions on a 'good' composer workflow". Stuff that vanilla composer doesn't offer, sure, but that's also what plugins are for. Searching for "composer local packages" on google brings a ton of people doing or wanting to do some flavor of this.

Also, this behavior (handle custom code deps like contrib deps) was very precious with composer_manager - but doing it inside composer without manually assembling stuff in composer.json is way cleaner.

yched avatar May 24 '15 21:05 yched

I see your points more clearly now, and the are definitely valid. I will need to try it out your plugin in my next project :)

derhasi avatar May 26 '15 12:05 derhasi

@yched https://github.com/wikimedia/composer-merge-plugin could work as well right?

webflo avatar Jun 27 '15 15:06 webflo

Wasn't aware of that one :-) It indeed seems to provide similar functionnality. From a quick look at the code, it seems the approaches differ a bit.

  • https://github.com/wikimedia/composer-merge-plugin manually merges the content (require, require-dev...) of the locally discovered jsons into the root package. A bit like drupal's composer_manager does (but strictly at runtime, without writing to the root json)
  • my plugin keeps the locally discovered jsons as separate packages provided by a custom runtime repository, and simply runtime-adds those packages to the require section of the root package. No manual merging, I let composer resolve the common dependencies, since it's what it's good at.

My approach feels a bit simpler, but I haven't fully weighted the pros and cons yet.

On Sat, Jun 27, 2015 at 4:25 PM, Florian Weber [email protected] wrote:

@yched https://github.com/yched https://github.com/wikimedia/composer-merge-plugin could work as well right?

— Reply to this email directly or view it on GitHub https://github.com/drupal-composer/drupal-project/issues/25#issuecomment-116089211 .

yched avatar Jun 28 '15 12:06 yched

As a developer who'd create a Drupal site environment, I'd like to have one composer to get all composer added dependencies with hitting one composer update. Module/theme authors need to develop their modules/themes at their respecting repos and just point the dependencies their module has in the composer.json and then let others (or themselves) use the module. Also dealing with DRY, I don't want to mirror my require section from the modules/themes to root composer.json.

Handling modules is always the big hassle and they tend to have new versions. Also when having modules which might use other libraries in the spirit of getting off the island, we need to get those. Also Drupal modules should use more Proudly-Found-Elsewhere code now when it's possible (and there are tons of great stuff to be used instead of writing Drupal versions of them).

I think the point of having the composer is that we can do this. Get all the stuff we need for the application (drupal site). That said, I like to see https://github.com/yched/composer-local-modules merged to drupal-composer. I think this is not anyhow related how these module authors develop their modules.

Also agreeing with the following as it's usually like this:

Except that you don't always know in advance, you write code for a site, and then later you might decide it contribute it, or reuse it in internal projects. But it often starts as custom code on a specific site. Not treating custom and contrib modules differently is a win, IMO.

Getting off the island with the Composer boat. =)

And if you didn't notice, I don't like Composer, I love it!

back-2-95 avatar Jul 29 '15 08:07 back-2-95

As mentioned in https://github.com/wikimedia/composer-merge-plugin/issues/72, looks like the new 'path' repo type in latest Composer (https://getcomposer.org/doc/05-repositories.md#path) could greatly simplify the issue here, since it seems to enable support for local relative folders seen as packages.

Thé doc states "This can be especially useful when dealing with monolith repositories" - a drupal project with local custom modules committed in modules(custom is exactly a "monolith repository" :-)

This still requires explicit definition of each relative path in the 'repositories' section off the root composer.json, so the "automated discovery" part provided by both https://github.com/yched/composer-local-modules and https://github.com/wikimedia/composer-merge-plugin would still be needed, but that's a thin layer.

yched avatar Sep 20 '15 15:09 yched

Hmm, it seems that 'path' repos creates a symlink in /vendor to the local folder (or copies it if the OS doesn't support simlinks)

That's not what we want here, since the local modules are not intended for /vendor, and are already in their correct, final directory...

yched avatar Sep 20 '15 15:09 yched

Looks like we are going to use the composer-merge plugin to deal with /composer.json and /core/composer.json in Drupal core. https://www.drupal.org/node/2380389#comment-10349507 contains the patch

webflo avatar Sep 20 '15 15:09 webflo

Saw that, that's what motivated me to have another look at composer-merge and it's issue queue ;-)

@webflo+++++ !

yched avatar Sep 20 '15 15:09 yched

I think we should use the merge plugin. The solution is good enough and we use it for Drupal core already. How wants to write some docs and add it to our README.md?

webflo avatar Nov 24 '15 09:11 webflo

Is there a reason why this plugin has not been added to the root composer.json file yet?

MaksOta avatar Jul 28 '16 09:07 MaksOta

I managed to get this to work.

First by installing the composer-merge plugin:

composer require wikimedia/composer-merge-plugin

Secondly by adding the following to the "extra" section in the root composer.json:

"extra": {
    ...
    "merge-plugin": {
        "include": [
            "web/modules/custom/*/composer.json",
            "web/profiles/custom/*/composer.json",
            "web/themes/custom/*/composer.json"
        ]
    }
    ....
}

Since I'm not an expert at hand, kindly review if this is the correct way. I'll create a PR afterwards.

rafatwork avatar May 15 '18 07:05 rafatwork

Hello,

Thanks all for the discussion and information

I have made a commit to use this in the Drupal 8 version of the drupal.fr website: https://github.com/Drupal-FR/site-drupalfr/pull/115

And I think to convert my other projects to this system.

Seing the description of https://github.com/wikimedia/composer-merge-plugin, I hope that the capability to merge the "repositories", "require-dev" and "scripts" keys will allow to have this composer plugin also used for contrib modules.

Because modules using JS libraries such as:

  • https://www.drupal.org/project/blazy
  • https://www.drupal.org/project/charts
  • https://www.drupal.org/project/devel (webprofiler)
  • https://www.drupal.org/project/dropzonejs
  • https://www.drupal.org/project/fontawesome
  • https://www.drupal.org/project/fontawesome_menu_icons
  • https://www.drupal.org/project/slick
  • https://www.drupal.org/project/webform: which has a Drush command to update your project composer.json. Which may not be obvious for new user require extra steps which can a add another barrier for Composer usage in the community.

Hope it will become a standard in https://www.drupal.org/project/ideas/issues/2958021.

FlorentTorregrosa avatar May 20 '18 12:05 FlorentTorregrosa

I tried to use this method with a custom profile containing its own custom modules and themes.

https://github.com/Drupal-FR/socle-drupalcampfr/commit/fd0ac15e7a3a428fb98481646513d58611a083a3

In the composer.json of the profile, I had to put the path from the project root:

{
  "name": "drupalfr/drupalcampfr",
  "type": "drupal-custom-profile",
  "extra": {
    "merge-plugin": {
      "include": [
        "www/profiles/drupalcampfr/modules/*/composer.json",
        "www/profiles/drupalcampfr/themes/*/composer.json"
      ],
      "merge-extra": true,
      "merge-extra-deep": true,
      "merge-scripts": true
    }
  }
}

I would have prefer been able to have:

...
      "include": [
        "modules/*/composer.json",
        "themes/*/composer.json"
      ],
...

So I can move my profile currently in profiles to profiles/custom without change of this part.

FlorentTorregrosa avatar May 20 '18 14:05 FlorentTorregrosa

I made a patch for the charts module. https://www.drupal.org/project/charts/issues/2879200#comment-12622071

And applied it on a project: https://github.com/Drupal-FR/socle-drupalcampfr/commit/8f65f6f78b2aa531a2ea387467d44aa19f4c1a69

I will also try for Webform.

FlorentTorregrosa avatar May 20 '18 14:05 FlorentTorregrosa

Patch done for Webform: https://www.drupal.org/project/webform/issues/2974114#comment-12622092

As mentioned in the Webform issue, I have made a separate composer.json file because otherwise for people not havin the composer-merge-plugin, the installation would fail with Composer.

And applied: https://github.com/Drupal-FR/socle-drupalcampfr/commit/a40c406613b510445109e649b0f322ef02244400

FlorentTorregrosa avatar May 20 '18 15:05 FlorentTorregrosa