Tighten ComposerPluginsValidator: support only specified version constraint

100% of failures are variations of:

Undefined array key "drupal/core-project-message"

/var/www/html/modules/contrib/automatic_updates/package_manager/src/Validator/ComposerPluginsValidator.php:219
…

and

Drupal\package_manager\Exception\StageException: Undefined array key "drupal/core-vendor-hardening"

/var/www/html/modules/contrib/automatic_updates/package_manager/src/Stage.php:635
/var/www/html/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php:450
/var/www/html/modules/contrib/automatic_updates/package_manager/src/Stage.php:490
/var/www/html/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php:164
/var/www/html/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/ComposerPluginsValidatorTest.php:101
/var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

Caused by
PHPUnit\Framework\Error\Warning: Undefined array key "drupal/core-vendor-hardening"

/var/www/html/modules/contrib/automatic_updates/package_manager/src/Validator/ComposerPluginsValidator.php:219
…

This is the first time that we're needing to retrieve the versions of installed composer plugins, and FixtureManipulator + \Drupal\package_manager\ComposerUtility::getInstalledPackages() both contain flaws that are very far out of scope for this issue to tackle.

Per #3334994-10: Add new InstalledPackagesList which does not rely on Composer API to get package info → , this is now hard-blocked on #3334994 — that will resolve these test failures 👍

📌 Add new InstalledPackagesList which does not rely on Composer API to get package info Fixed landed 👍

Wim Leers → closed merge request !693

Closed old MR, new MR for 3.0.x. Transferred the pointers too.

Now working to adopt 📌 Add new InstalledPackagesList which does not rely on Composer API to get package info Fixed 's additions here.

📌 Add new InstalledPackagesList which does not rely on Composer API to get package info Fixed did not unblock this:

1) Drupal\Tests\package_manager\Kernel\ComposerPluginsValidatorTest::testValidationDuringPreCreate with data set "another supported composer plugin" (array(array(true)), array(array('drupal/core-vendor-hardening', '9.8.0', 'composer-plugin')), array())
PhpTuf\ComposerStager\Domain\Exception\RuntimeException: The command "'/opt/homebrew/bin/composer' 'show' '--format=json' '--working-dir=/private/tmp/package_manager_testing_roottest40722248/active'" failed.

Exit Code: 1(General error)

Working directory: /Users/wim.leers/core

Output:
================


Error Output:
================

In PluginManager.php line 169:
                                                                               
  Plugin drupal/core-vendor-hardening is missing a require statement for a ve  
  rsion of the composer-plugin-api package.                                    
                                                                               

show [--all] [--locked] [-i|--installed] [-p|--platform] [-a|--available] [-s|--self] [-N|--name-only] [-P|--path] [-t|--tree] [-l|--latest] [-o|--outdated] [--ignore IGNORE] [-M|--major-only] [-m|--minor-only] [--patch-only] [-D|--direct] [--strict] [-f|--format FORMAT] [--no-dev] [--ignore-platform-req IGNORE-PLATFORM-REQ] [--ignore-platform-reqs] [--] [<package> [<version>]]



/Users/wim.leers/core/vendor/php-tuf/composer-stager/src/Infrastructure/Service/ProcessRunner/AbstractRunner.php:58
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/src/ComposerInspector.php:202
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/src/ComposerInspector.php:159
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/src/Validator/ComposerPluginsValidator.php:203
/Users/wim.leers/core/core/lib/Drupal/Component/EventDispatcher/ContainerAwareEventDispatcher.php:111
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/src/StatusCheckTrait.php:45
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php:190
/Users/wim.leers/core/modules/contrib/automatic_updates/package_manager/tests/src/Kernel/ComposerPluginsValidatorTest.php:74
/Users/wim.leers/core/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

Caused by
Symfony\Component\Process\Exception\ProcessFailedException: The command "'/opt/homebrew/bin/composer' 'show' '--format=json' '--working-dir=/private/tmp/package_manager_testing_roottest40722248/active'" failed.

… and that's triggered just by calling $this->inspector->getInstalledPackagesList($this->pathLocator->getProjectRoot());!

AFAICT that's being worked on in 📌 Update FixtureManipulator to work with InstalledPackagesList, real composer show command Fixed . So postponing on that now …

📌 Update FixtureManipulator to work with InstalledPackagesList, real composer show command Fixed landed, this is unblocked!

All tests pass locally, I self-reviewed and didn't find anything left. Ready for final review by @phenaproxima or @tedbow.

"1 failure", but really 13:

Testing Drupal\Tests\package_manager\Kernel\ComposerPluginsValidatorTest
..........FF.......FF.FF.FFFFFFF                                  32 / 32 (100%)

Time: 04:14.627, Memory: 6.00 MB

There were 13 failures:

Just pushed a commit that should decrease that number, but won't yet bring it to 0.

"2 failures", but really still the same sole test class failing, but with fewer failures now:

Testing Drupal\Tests\package_manager\Kernel\ComposerPluginsValidatorTest
...................FFEFF..FFFFFF                                  32 / 32 (100%)

Time: 04:18.265, Memory: 6.00 MB

That's 10 failures and 1 error instead of 13 failures.

Will continue this tomorrow.

This will be green now! 😊

🏓 @phenaproxima

👀

I found some stuff...

Stuff I did here:

• Changed all composer in comments to Composer. A pet peeve I will never let go of.
• Naming is really, really hard, especially with a lot of functional mutation like this plugin (which, by the way, is a pattern I like and appreciate for its lack of side effects). I renamed some variables to hopefully give a better sense of what everything means, because words like "supported" and "allowed" easily get jumbled together. To help keep things as clear as possible, I also moved certain lines around to hopefully group related logic together.
• Removed unnecessary use of TranslatableMarkup and FormattableMarkup.
• Removed some unnecessary nesting.
• Added some comments, again in the name of clarity.

Reassigning to Wim for final review.

On second thought, I'm hoping @tedbow can give this a review so we can land it.

Changed all composer in comments to Composer. A pet peeve I will never let go of.

❤️ This reminds me of MANY people typing "CK Editor", "Ckeditor", "Ckeditor", "CkEditor" and however many other permutations 🤣 In that case, it's even worse though, because "CKEditor" is the only acceptable spelling. For Composer it's at least "Composer" when referring to the project, and composer when referring to the command line tool.

(Which is exactly why I keep mixing the two up — I basically always refer to composer, which trips you up, which I'm sorry for 😅)

I renamed some variables to hopefully give a better sense of what everything means

👏 I like all of your changes!

Removed unnecessary use of TranslatableMarkup and FormattableMarkup.

This was not unnecessary. It just was ambiguity in Package Manager infrastructure that allowed your changes to pass tests 😅

Okay, looks like there's many more violations … fixing those is out of scope here. Created 🐛 Many callers of ValidationResult::createError() violate its interface due to lack of strict typing Fixed for fixing that.

Okay, that makes sense to me. Agreed that the violation of the interface -- due, IMHO, to PHP's lack of generics -- is a pervasive problem we should correct in another scope.

Since you like my changes, and I like my changes, I feel okay about kicking this over to RTBC. I made one small change, which is that I don't think @raw_allowed_plugin_name is a clear placeholder. The value going in is raw, but it will be escaped so that it's non-raw in the actual string.

I don't see any real reason to hold off on merging this beta blocker.

🥳

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