DGS-442 Constrain bundled Symfony dependencies to supported PHP versions - #185
Open
TLabutis wants to merge 1 commit into
Open
DGS-442 Constrain bundled Symfony dependencies to supported PHP versions#185TLabutis wants to merge 1 commit into
TLabutis wants to merge 1 commit into
Conversation
invertus/dpdbaltics-api declares symfony/serializer, symfony/property-access and symfony/property-info as unconstrained "*" wildcards. The module's own composer.json constrained only symfony/console, so nothing bounded those three. DGS-430 then added --ignore-platform-reqs to the release build, which discards the config.platform.php = 5.6 pin, and Composer resolved the wildcards to whatever was newest. The published 3.3.1 asset shipped symfony/serializer v8.0.3 (requires PHP >= 8.4), symfony/property-access v6.4.25 and symfony/string v6.4.34 (both PHP >= 8.1), which is why shops on PHP below 8.1 white-screen after installing. The drift is also committed: composer.lock on main already holds those versions, so composer install reproduces it faithfully. Add explicit ^3.4 constraints for symfony/serializer, symfony/property-access, symfony/property-info and symfony/debug, since root constraints override transitive ones, and regenerate composer.lock against the 5.6 platform pin. The shipped set is back to 23 packages, all PHP 5.6 compatible, matching the known good 3.3.0 build. invertus/dpdbaltics-api and invertus/psModuleTabs stay pinned at their existing refs so no API behaviour changes ride along. Drop --ignore-platform-reqs from release.yml and create-zip.yml so Composer's "verifying lock file contents can be installed on current platform" check runs again. Verified: the old lock now fails the build with exit code 4, naming all four packages, while the new lock exits 0. Guard install() behind MINIMUM_PHP_VERSION so an unsupported PHP is refused with a readable message rather than taking the shop offline. Deliberately left out: the module version stays 3.3.1, since bumping it belongs to the release PR, and the constructor's autoLoad/compile path is untouched, so this does not rescue a shop that already installed 3.3.1. Whether 3.3.2 should keep PHP 7.2 support or move to 8.1 is still open in DGS-442; MINIMUM_PHP_VERSION is set to the currently advertised 5.6 and is a one line change either way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Version 3.3.1 takes shops on PHP below 8.1 offline. On PHP 7.2 and 7.3 the whole shop returns a 500 immediately after installing, front office included, so the merchant cannot even reach the Modules page to uninstall.
Root cause
invertus/dpdbaltics-apideclares three Symfony packages as unconstrained wildcards:The module's own
composer.jsonconstrained onlysymfony/console, so nothing bounded those three. DGS-430 then added--ignore-platform-reqsto the release build, which discards theconfig.platform.php = 5.6pin, and Composer resolved the wildcards to whatever was newest at build time.Comparing the published v3.3.1 asset against the known good 3.3.0 build:
symfony/serializersymfony/property-accesssymfony/stringsymfony/deprecation-contractssymfony/debugprestashop/decimal27 packages shipped instead of 23.
symfony/serializer v8.0.3requiring PHP 8.4 is what white-screens the shop.The drift is also committed:
composer.lockonmainalready holds those versions, socomposer installreproduces the broken build faithfully. This is not only a build-time accident.Changes
composer.json- explicit^3.4constraints forsymfony/serializer,symfony/property-access,symfony/property-infoandsymfony/debug. Root constraints override transitive ones, so this bounds the wildcards without needing a change in the API package.composer.lock- regenerated against the 5.6 platform pin. Back to 23 packages, all PHP 5.6 compatible.invertus/dpdbaltics-apiandinvertus/psModuleTabsstay pinned at their existing refs, so no API behaviour changes ride along with this fix.release.yml/create-zip.yml- dropped--ignore-platform-reqsso Composer's platform verification runs again.dpdbaltics.php-install()is guarded behind a newMINIMUM_PHP_VERSIONconstant, so an unsupported PHP is refused with a readable message instead of taking the shop offline. There was no check of any kind before.Verification
Simulating the CI build locally:
composer install --no-devexit codemain)So the restored platform check does catch this class of problem before publishing, which was point 3 of the ticket.
Still open
MINIMUM_PHP_VERSIONis set to the currently advertised5.6. If we go with Option B (require PHP 8.1+), it is a one line change plus the stated requirements, changelog and store listing.3.3.1; bumping to 3.3.2 belongs to the release PR.autoLoad()/compile()path is untouched, so this does not rescue a shop that already installed 3.3.1. The documented workaround stays: install 3.3.0.How to test
Install the built package on PHP 7.2, 7.4, 8.0 and 8.1. In each case the shop should load, and creating a shipment, printing a label and loading pickup points should all work.
Related: DGS-440 (where this was found), DGS-430 and SDESK-232 (release build).