Based on #3272110-20: Drupal 9 and 10's Drupal\Component composer.json files are totally out of date
@andypost noticed that the change for that issue used symfony/finder, but also noted that it's a dev requirement, so not quite so concerning from the perspective of #1451320: Evaluate Symfony2's Finder Component to simplify file handling
I was curious if this would break the change, when we don't install dev.
It did, but it also broke core's ability to generate meta packages.
This happens because drupal/drupal says this:
"autoload": {
"psr-4": {
"Drupal\\Core\\Composer\\": "core/lib/Drupal/Core/Composer"
}
},
"autoload-dev": {
"psr-4": {
"Drupal\\Composer\\": "composer"
}
},
rm -rf vendor/
composer install
COMPOSER_ROOT_VERSION=9.5.x-dev composer update --no-dev 'drupal/core*'
Note the --no-dev.
During the no-dev update, we see errors like this:
Class Drupal\Composer\Composer is not autoloadable, can not call post-update-cmd script
This comes from #3076234: Relocate Scaffold plugin outside of 'core' directory to try and isolate the scaffold plugin.
Merge drupal/drupal’s autoload-dev into autoload, and the whole process works.
Since drupal/drupal is supposed to be a project only for core development, merge its autoload-dev into autoload.
This will allow Composer to generate the autoload for either dev or no-dev.
Decide if this matters.
Mitigate based on needs of #1451320: Evaluate Symfony2's Finder Component to simplify file handling and other similar dependency questions.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
mile23Merging autoload-dev to autoload.
Discuss. :-)
Comment #4
neclimdulhmm.... hmmm... many thoughts but not sure I have an opinion. This does seem to work though.
1) Does this actually work? It looks like it requires "composer/composer" which is a dev requirement. TIL the answer is yes it works, the post script is run in the composer process so it has access to the interfaces of the composer binary but your IDE is going to complain.
2) Echoing the note in the tasks, does it matter? Specifically:
- What's the process that would lead to someone running that command on core? Seems more like something you'd run on a site, not a project like Drupal.
- Is it ok to continue warning in those cases?
3) If I understand all the pieces at play here, they are kinda complicated, it seems like this is going to be registering a namespace on websites that then can't be resolved. Probably not a huge deal but that doesn't seem ideal either.
Comment #5
mile23There are more than a few moving parts here, and they're all named Composer even though they're unrelated, which kind of sucks.
#4.1: We're not moving the Composer package out of dev. We're only registering the Drupal\Composer\ namespace (
autoload, notrequire). This is where the metapackage generator lives as well as the plugins such as scaffold.#4.2: If you're doing core development, and you say
composer update --no-devthen it's actually quite important that the post-update scripts run, even if you're not installing dev stuff for whatever reason. They generate the metapackages, and after #3272110: Drupal 9 and 10's Drupal\Component composer.json files are totally out of date will also do some validation on the components. I don't think it matters that much how likely someone is to run it. It's a bug right now, and it's the kind of bug it's better to fix now than to find later after we break a release. It's possible to do a composer update with no-dev for some dev reason and then commit the change without updating the metapackages, and then we're out of sync.#4.3: drupal/drupal is for core development, and not for being deployed to production. The worst-case outcome for someone who (erroneously) uses drupal/drupal for production after this change is that they have an extra psr-4 namespace declared for the composer/ directory, which will be present, because they're using the repo. Whether that's a problem is something to discuss, but I don't think it is within drupal/drupal.
As always: Other suggestions welcome.
Comment #7
smustgrave commentedCan the MR be updated for 10.1 please.
Don't have much to chim in for #5 sorry!
Comment #8
mile23Updated MR against 10.1.x.
Comment #9
smustgrave commentedMarking because this doesn't seem to break anything. And the great explanation in #5 seems to show this is a bug for only drupal/drupal.
This is my only concern but if they are using drupa/drupal that seems like user error?
Comment #10
catchGiven drupal/core is dev only, why do we need to support running this with --no-dev?
Comment #11
mile23Rebased and pushed again for another test.
@catch: The downside of leaving this bug in core is that a developer might composer install without dev for some reason and make a patch that breaks the metapackages. Then a maintainer might end up not noticing that. It seems vaguely unlikely, but it also seems like it would be a real pain to figure out if it broke something else.
Comment #12
smustgrave commentedDoesn't seem like it could break anything right?
So it could help those who do what @Mile23 said in #11
Least that's how I'm reading it.
Comment #13
alexpottI think approach is fine and it don't think ot matters that this is only supposed to be used for development. Only putting in 10.1.x because not sure this is important enough to backport.
Committed 6a9c88c and pushed to 10.1.x. Thanks!