Removes unnecessary polyfill packages to be installed.#40862
Conversation
|
Hi @hostep. Thank you for your contribution!
Allowed build names are:
You can find more information about the builds here For more details, review the Code Contributions documentation. |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento run all tests |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento run Static Tests |
|
@magento run all tests |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
|
@magento create issue |
|
The security team has been informed about this pull request due to the presence of risky security keywords. For security vulnerability reports, please visit Adobe's vulnerability disclosure program on HackerOne or email psirt@adobe.com. |
Description (*)
A default Magento installation comes with a bunch of composer packages that provide polyfills for PHP functionality that's not always guaranteed to be installed in various ecosystems.
However, after some analysation, it turns out Magento does have a bunch of guarantees of certain software to be available, so we don't need all these polyfills, most of these come from dependencies outside of Magento's ecosystem and needing to support a wide variety of projects.
paragonie/random_compat(brought in viaphpseclib/phpseclib) is not needed as the 2 functions (random_int&random_bytes) were added in PHP 7.0 and higher, Magento requires PHP 8.3 at minimum. Phpseclib still supports PHP 5.6, so that's why it gets pulled in (this will change with phpseclib v4 in the future BTW)ralouphie/getallheaders(brought in viaguzzlehttp/psr7) is not needed as guzzlehttp needs it because it still supports PHP 7.2 and thegetallheadersfunction was only made available in the FPM SAPI since PHP 7.3 and since Magento requires PHP 8.3 at the minimum, it's guaranteed to be availablesymfony/polyfill-ctype(brought in through various symfony packages) is not needed as Magento requiresext-ctypeto be installedsymfony/polyfill-intl-grapheme,symfony/polyfill-intl-idn&symfony/polyfill-intl-normalizer(brought in through various packages) is not needed as Magento requiresext-intlto be installedsymfony/polyfill-mbstring(brought in through various packages) is not needed as Magento requiresext-mbstringto be installedsymfony/polyfill-php73=>symfony/polyfill-php83, Magento requires PHP 8.3 at the minimum at time of writing, so no need to install polyfills for this minimum version and all versions below it. We still want to install polyfill packages for higher versions that Magento also supports, in case a dependency is installable on PHP 8.3 but uses some PHP 8.4/8.5 function that's provided by this polyfillRemoving unnecessary packages leads to:
Related Pull Requests
Fixed Issues (if relevant)
Manual testing scenarios (*)
composer update && composer show | grep polyfilland only expect to see polyfill packages for functionality not guaranteed to be available on a server where you install Magento, likephpseclib/mcrypt_compatorsymfony/polyfill-deepclone, but no othersQuestions or comments
I've added
symfony/polyfill-php74&symfony/polyfill-php83even though Magento's dependencies don't pull them in, but that doesn't mean this will change in the future, by an update of one of the dependencies, or by some 3rd party module or something along those lines. Makes sense? Should I also includesymfony/polyfill-php72and lower, just to be sure?I'm not sure how this change will be guaranteed to affect real composer installations of Magento. Will those
replacelines be part of thecomposer.jsonfrom the meta packagemagento/project-community-edition, or rather frommagento/product-community-edition,magento/magento2-baseormagento/framework, or from something else?I applied the same replacements in
magento/frameworkat the moment, but this may not be the best decision?It would be good if somebody from Adobe with knowledge about how composer packages are build and distributed will pick this up and how to implement this in the best way.
This will also require some maintenance in the future, so if we drop support for PHP 8.3 and make PHP 8.4 the minimum required version, we should also append
symfony/polyfill-php84to the list. So this will need to be documented somewhere that Adobe maintainers that perform PHP upgrades are aware of and should check.Is the approach taken in
Magento/Test/Integrity/ComposerTest.phpthe correct one?Contribution checklist (*)
Resolved issues: