Problem/Motivation

There now exists an official endpoint for drupal modules and themes. Core can ship with this endpoint specified in it's root composer.json, enabling users to composer require drupal/rules to install the rules module, using this endpoint.

Proposed resolution

Patch the composer.json to add the repository specification

Remaining tasks

The composer endpoints are targeted at being declared beta on July 6th. This can probably be comitted sometime after that

User interface changes

None

API changes

This adds our repository as a package endpoint, and thus adds features that werent there before.

Data model changes

none

Comments

Mixologic created an issue. See original summary.

Mixologic’s picture

Here is the patch, added with:
composer config repositories.drupal composer https://packages.drupal.org/8

Mixologic’s picture

Status: Active » Needs review
Fabianx’s picture

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

Looks good to me.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2758737-2-add-packages-endpoint.patch, failed testing.

Mile23’s picture

Status: Needs work » Reviewed & tested by the community

Patch still applies.

After application I did this:

$ composer require drupal/phpunit_example
Using version 1.x-dev for drupal/phpunit_example
./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
  - Installing drupal/examples (dev-1.x 0001f5b)
    Cloning 0001f5b197bb1932b8239fb618b8858765f9af2a

This added phpunit_example as a dependency and gave me this change to composer.json:

         "composer/installers": "^1.0.21",
-        "wikimedia/composer-merge-plugin": "~1.3"
+        "wikimedia/composer-merge-plugin": "~1.3",
+        "drupal/phpunit_example": "1.x-dev"
     },

...but Composer then ended up satisfying this dependency by adding the whole drupal/examples project. I think this is the right way to do it.

xjm’s picture

That fail looks actually related? Queuing a retest.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2758737-2-add-packages-endpoint.patch, failed testing.

Xano’s picture

I can't do it now, but somebody needs to run composer update --lock after this patch has been applied, and include the result.

amateescu’s picture

Status: Needs work » Needs review
FileSize
1.11 KB
580 bytes

Here you go :)

Mile23’s picture

Status: Needs review » Needs work

+1 on #9.

And RTBC on #10 except for one thing:

Let's add an explanation to extra->_readme in composer.json so it says "This file specifies the packages.drupal.org repository. You can read more about this repository here: https://www.drupal.org/node/2718229"

Mixologic’s picture

Here's a patch with some commenting. I added the word 'composer' in there before one of the repositories lest people think that has something to do with git.

Mixologic’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 12: 2758737-12.patch, failed testing.

Mixologic’s picture

Status: Needs work » Needs review
FileSize
2.33 KB

I forgot to re-run composer update --lock so the hashes didnt match.

When I did re-run it, it changed the references from 1.0.0 to commit hashes for psr-log, which is odd. Odd that they were in the lock file as a tag in the first place, as it is the *only* reference in the lock file like that. Though it looks like it was a throwback all the way back to #1894002: Update vendor libraries and pin them to specific versions in composer.json, so probably okay. The commit hash matches the tag, so we're not actually changing anything there.

Mile23’s picture

Sigh. Patch doesn't apply.

$ git apply 2758737-15.patch 
error: patch failed: composer.lock:4
error: composer.lock: patch does not apply
Mile23’s picture

Status: Needs review » Needs work

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ndf’s picture

You can add the drupal endpoint with $ composer config repositories.drupal composer https://packages.drupal.org/8 as #2 already said.

Using the method from this issue a couple things happen that I am not sure if that is good/recommended:

  1. Core is 'hacked'. Anytime you add a module with $ composer require ... the top composer.json will be changed.
  2. All modules are downloaded to the folder /modules not /modules/contrib . This can be difficult to manage when you add custom modules.
  3. All modules that are managed with composer are *also* included in git. Best practise is that a module is either managed in git (all code of the module) or only by an entry in composer.json (as metadata)

Compared to https://github.com/drupal-composer/drupal-project this method is less sophisticated.
But it does work well!

bojanz’s picture

Core is 'hacked'. Anytime you add a module with $ composer require ... the top composer.json will be changed.

Core doesn't own the root composer.json, the project does. This file is meant to be changed, just like htaccess for example.

All modules are downloaded to the folder /modules not /modules/contrib . This can be difficult to manage when you add custom modules.

Not in 8.3.x:

            "modules/contrib/{$name}": ["type:drupal-module"],
            "profiles/contrib/{$name}": ["type:drupal-profile"],
            "themes/contrib/{$name}": ["type:drupal-theme"],
            "drush/contrib/{$name}": ["type:drupal-drush"]
ndf’s picture

Thanks Bojanz. Missed those changes in 8.3.x !
Regarding to the ownership of the root composer.json > did not know it was meant to be changed.

ndf’s picture

Status: Needs work » Needs review
FileSize
1.64 KB

Rerolled against 8.x-3.x

2 things are different then #15
1. hash + content-hash (sure)
2. no further changes in composer.lock

Mile23’s picture

There's some ambiguity about the composer facade respecting the 'type' property of Drupal projects: #2802009: Provide support for libraries and other non-drupal extensions (modules, themes etc)

Specifically Coder, which is a Drupal project but also a library and not a module. We want to include it as a dev requirement for core: #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core

I just added the facade as per #2 composer config repositories.drupal composer https://packages.drupal.org/8

And then added Coder: composer require --dev drupal/coder 8.2.8

..and everything ended up where it should, but I'm not sure that isn't by accident, since #2802009: Provide support for libraries and other non-drupal extensions (modules, themes etc) is still open.

Mixologic’s picture

Its by accident that it works, because you are requiring drupal/coder 8.2.8 - which happens to be grandfathered in at packagist.org as well.

So basically composer is able to get metadata about coder from both sources, so for now it works. Where we get into trouble is if you were to use the *recommended* versioning of composer require drupal/coder 2.8.0 to install coder. Then it has no idea that its a drupal library because we do not preserve composer.json for anything that doesnt have an .info/info.yml file, and assumes it to be a drupal module.

Since coder is a special case, yet has a workaround, Im inclined to say this wont have a drastic impact on libraries that exist on d.o.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

webflo’s picture

Issue tags: +composer
webflo’s picture

webflo’s picture

I think version number drupal/coder is not a concern anymore, it has been added as dev-dependency in #2744463: Add phpcs, coder 8.2.8 as --dev requirements for drupal/core.

bojanz’s picture

Status: Needs review » Reviewed & tested by the community

Okay, let's go.

We def wants this in 8.3.x as well, should be completely non-disruptive.

tstoeckler’s picture

Awesome, let's get this in.

Edit: Ahh, damn, @bojanz was quicker ;-)

anish.a’s picture

Issue tags: -Needs manual testing
Mile23’s picture

So eventually, as per #24, we'll need to change the requirement to drupal/coder 2.*.0, but not before the issue of d.o projects being libraries is resolved in #2802009: Provide support for libraries and other non-drupal extensions (modules, themes etc) and elsewhere.

alexpott’s picture

Version: 8.4.x-dev » 8.3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed 475e305 and pushed to 8.4.x. Thanks!

We need a new patch for 8.3.x

  • alexpott committed 475e305 on 8.4.x
    Issue #2758737 by Mixologic, amateescu, ndf, webflo, Mile23, bojanz: Add...
cilefen’s picture

Status: Patch (to be ported) » Needs review
FileSize
20.88 KB

The hash key and the date format change in composer.lock because this is composer update --lock with the current version of composer.

Mile23’s picture

cilefen’s picture

Is there an issue open for updating the composer merge plugin config to parse contrib module dependencies?

bojanz’s picture

@cilefen
There is an old issue, but I don't remember if it was still open, cause it doesn't make a lot of sense. If you need to run Composer anyway, then use it to download the whole module.

Mixologic’s picture

ahh.. sweet.
+++

cilefen’s picture

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Patch is green.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 1d1daa8 and pushed to 8.3.x. Thanks!

  • alexpott committed 1d1daa8 on 8.3.x
    Issue #2758737 by Mixologic, amateescu, webflo, ndf, cilefen, Mile23,...
alexpott’s picture

Committed 1d1daa8 and pushed to 8.3.x. Thanks!

cilefen’s picture

Issue tags: +8.3.0 release notes

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.