Skip to content

Fix/php backwards compatibility#127

Merged
jooname merged 2 commits intomasterfrom
fix/php-backwards-compatibility
Jun 3, 2025
Merged

Fix/php backwards compatibility#127
jooname merged 2 commits intomasterfrom
fix/php-backwards-compatibility

Conversation

@jooname
Copy link
Contributor

@jooname jooname commented Jun 3, 2025

Should solve #115, except for PHP 7.0.

Back in #99 references to loophp/phposinfo package were updated due to the namespace change in version 1.7 of that package, but the package requirement was not updated in composer.json of vaimo/composer-patches. This meant that anyone with PHP <7.4 would be able to install vaimo/composer-patches, but they wouldn't be able to use it, if loophp/phposinfo came with version 1.6. This was technically implied as breaking change in 5.0.0 release, but in retrospective the composer.json would've been polite to also adjust accordingly.

Although #125 will be dropping support for PHP <7.4, those changes will be added in next major release, leaving version 5 still in awkward situation where it can be installed in older versions but with inability to use it. Following changes update loophp/phposinfo namespace references conditionally through class_alias function, which should support both 1.6 and 1.7 versions of loophp/phposinfo. This will not be carried over to #125 in favor of not needing this workaround and to be able to at least partially leave the more severely deprecated PHP versions behind.

Additionally there is one adjustment to the changes from #86, where use of spread operators locked the implementation to PHP versions 7.4 and above. This was missed by accident and was not correctly reflected in the release version 5.3.0, so the logic was now updated to be backwards compatible.

Why I also updated the minimum PHP constraint to 7.1 in composer.json is because of #121. The solution there is needed for PHP 8.4, but is not compatible with 7.0 and older. It also does seem like most 1.6 versions of loophp/phposinfo also do not support 7.0, except for some subversions of 1.6.1.

jooname added 2 commits June 3, 2025 11:07
…sinfo versions 1.6 and 1.7, previously we were locked to 1.7 despite composer.json allowing installing 1.6. Updated also src/Utils/FileSystemUtils.php to use array_merge instead of merging arrays through spread operators, since latter is only supported in PHP 7.4.
…ecame the case already earlier when PHP 8.4 compatibility changes were implemented.
@jooname jooname merged commit 55eb5a5 into master Jun 3, 2025
@jooname jooname deleted the fix/php-backwards-compatibility branch June 3, 2025 09:46
@allanpaiste
Copy link
Contributor

allanpaiste commented Jun 3, 2025

Great work @jooname !!

I'm curious actually: what is the state of keeping the patch package backwards compatible with 5.x?

I used to consider this essential as patching is very beneficial for extremely old code-bases more than it is for the new ones.

@jooname
Copy link
Contributor Author

jooname commented Jun 3, 2025

@allanpaiste I've been on the fence with that. In this case I mainly felt that it would make sense for 5.x to actually work in versions it can be installed to, since it did end up in a bit messy state. But aside from that I wouldn't really be afraid of dropping support for older versions, especially if the workarounds for backwards compatibility start getting messy.

It is a good point about the old code-bases though, we definitely should avoid dropping the support unless it really becomes necessary. I was already wondering is 7.4 too excessive to support in this day but it definitely doesn't cause issues so why not. Might also reconsider couple of things with #125

@tenzap
Copy link

tenzap commented Jun 3, 2025

What about, when v6 is ready with php 8.4 support, reverting the changes made for php 8.4 in v5, to restore proper functionality with php 7.0 in version > 5.3.1?

Edit: IIRC it is possible to avoid the php 8.4 deprecation warning by doing something like that

-public function __construct($failedPatchPath, $message = '', ?\Exception $previous = null)
+public function __construct($failedPatchPath, $message = '', $previous)

And it would then work with both php 7.0 & 8.4

jooname added a commit that referenced this pull request Jun 4, 2025
…from #127 are pretty simple to include. Included also PHP 7.0 support back, since the PHP 8.4 compatibility changes from #121 were possible to achieve another way. Updated Github workflow to include more PHP versions. Adjusted loophp/phposinfo package compatibility adjustment from #127 to be autoloader based, since the change from pull request didn't work with the testsuite. Managed to also adjust the testsuite to take into account the autoloader customisation. Fixed also issues in the testsuite where all package names were prefixed with __ even though only a handful of packages were supposed to be affected. Adjusted the testsuite to understand Composer 1 and 2 outputs again, but flipped the intention of .output to be based on latest Composer rather than 1 like earlier. Updated dependency whitelists based on the older versions, looks like symfony/console has lots of incompatibilities that have been suppressed with function_exists and autoloader workarounds.
@jooname
Copy link
Contributor Author

jooname commented Jun 4, 2025

@tenzap Very good point! I ended up taking a different approach with #125 and decided to restore PHP 7.0 support as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants