Add composer/composer as a dev dependency of core so that we can test Composer plugins

We need to remember to remove the test folders from the new dependencies - ie change \Drupal\Core\Composer\Composer::vendorTestCodeCleanup()

Hmmm I would have expected core/composer.json to get the dependency not the root composer.json

I dunno it feels odd to add a dependency for testing here. I think core/composer.json's dev section is best because that's where all the dev dependencies for the entirety of core testing live at the moment. Yes this isn't the best way to do it but make this the first exception without doing the rest feels odd.

+1 for this as an interim measure. As mentioned above, this only lasts until #2943856 lands. Sounds like adding to drupal/core does not work.

I understand the wish to appear to do things in the way that is considered "correct" but halfway houses with things spread all over the place makes things more difficult and not less. This would be the first instance of a dev dependency in the root composer.json. Let's not do that.

Adding it to drupal/core does work because we merge dependencies. As this issue is not about #2912387: Stop using wikimedia/composer-merge-plugin let's not preempt the order that everything happens.

Thanks @Mile23 for the dependency evaluation info. Tagging.

We'll probably want to look at those two seld packages and add the evaluation for them as well. Not that I expect there to be a problem with the maintainership of something Composer requires. ;) But hey, left-pad. And it would be good to have the other info on things like e.g. security contacts (since Composer itself includes no concept of security releases) and their release cycles.

Thanks!

As a result of the last composer initiative meeting, some of the following determinations were proposed:

1. Since drupal/drupal needs to have some sort of vendor cleanup, *but only for the short time while its still being used to create -dev tarballs*, we'll not make drupal's root composer.json rely on the new vendor test cleanup plugin. This makes us not need to concern ourselves with drupal/drupal.

2. As such, composer/composer is therefore *only* required as a dev dependency in order to test the plugin. We should put that dependency into drupal/core/composer.json's `require-dev` section.

While it might potentially 'save us time down the road' to have that dev dependency specified at the component level and allow it to be merged back up in order to test it, we dont actually know what down the road is going to look like, and dev dependencies in components add additional maintenance challenges. Which is why we came to the consensus to manage it in drupal/core/composer.json vs the require-dev section of the vendor test code cleanup plugin.

Version of symfony/finder changed in composer.lock, which I believe is undesirable, right?

Re #16: it only changed from the last patch, but its something being added currently, so may as well add the most recent version.

Im unfamiliar with the policies surrounding adding a dev requirement to core.

Is it our responsibility to ensure that, if end users do the very wrong thing and install dev dependencies on a production site, *and* those dependencies live under docroot, that the dev dependencies existence does not introduce a security issue? Because following those steps will almost certainly introduce a security issue.

These libraries should only be there if you are developing or testing drupal core itself, and should not show up for any user that is requiring drupal/core in their project. (unless somebody manually duplicates them and puts them into their project as an actual requirement that then gets added to their projects for testing purposes e.g. https://github.com/webflo/drupal-core-require-dev) but that should never be used in production and should only be used for testing a site.

Seems like the best we can do when it comes to the security of something like this is #2830880: Warn site admins when composer dev dependencies are installed inside of docroot in conjunction with #3057094: Add Composer vendor/ hardening plugin to core, and that there isn't much more process in place for coordinating security issues with the upstream dependencies themselves.

RE #12 The seld/* libraries are seldaek, aka Jordi Boggiano, the lead developer of Composer, so essentially the same degree of confidence as the rest of the composer/* libraries.

Assuming all the above is true, believe we should move forward with adding these as dev requirements.

This looks good to me, needs a re-roll though. Removing the FM tag, so we just need an RM to sign off too. I'll ping them

Back to RTBC as the reroll was succesful and the tests are green again. There is a .05kb size difference in the patch but that is not a lot and I am assuming it is because of different context-lines included.

Patch still looks good; back to RTBC after re-roll & passing tests.

This patch is a blocker for progress on the rest of the composer initiative, and considering that it is not a code patch, and is a policy/dependency adjustment, is there anything we can do to accelerate this to a consensus state?

Would it help to have a short meeting with the release managers to address any outstanding concerns?

Thanks @Mile23 - green again.

Committed a933b3a and pushed to 8.8.x. Thanks!

This seems like it should probably at least have a CR. Also it should be added to the dev dependencies section here: https://www.drupal.org/core/dependencies#dev

Re: #18, two words: dev tarballs. Right?

I debated whether this merited a release notes mention, but I think probably not since it's a dev dependency and won't be included in the tagged releases, thus not affecting the thing that has the release notes. (A new production dependency should probably be mentioned alongside the dependency version updates in case some site or project required an unresolvable conflicting version of the same dependency or such.)

re #18 So how do we deprecate dev tarballs? We've went to great lengths to ensure we can still make them in d8.8/8.9 for bc sake, but it would be nice to remove that requirement entirely, unless we have a really valid use case for them. (we'd still have tarballs of -dev _versions_, just not ones that contain dev dependencies themselves).

durned drupal forms and all that caching jazz.

The most common place users are going to encounter dev tarballs is when they use drush dl to download a dev version of Drupal. I'm not sure if we need to support running the tests in this configuration. If I had to guess, I might say that the main use-case for running tests after drush dl of a dev version would be in folks' CI scripts.

I think it would be reasonable to remove dev dependencies from dev tarballs - opened a follow-up at #3128715: [policy, no patch] Remove dev dependencies from dev tarballs.

Moving this back to fixed.

publish the cr