-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Magento 2.4.4 Compilation issues on PHP 8.1 #40
Fix Magento 2.4.4 Compilation issues on PHP 8.1 #40
Conversation
magento/scripts/patch-AC2855.php
Outdated
$is244 = substr($version, 0, 5) == '2.4.4'; | ||
$patchLevel = (int)substr($version, 7); | ||
|
||
if (!$is244 || $patchLevel > 100) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not really following this $patchLevel
comparison. You are trying to cast a string like "2.4.4-p11" to an int. In my tests the $patchLevel
is always 1
. Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, meant to take out the patch level. Since it applies to all versions of the 2.4.4
package.
I'll remove it after the current pipeline finishes running.
But as for the check, we are stripping out everything up to and including the p
character.
So we either end with the numerical value or a empty string (which casts to 0) in the case of no patch.
Hey, thanks for this PR. It's really appreciated! However, the build seems to fail on this error:
Does that ring a bell? |
From what I can tell It seems like 2.4.4 is uninstallable currently 👀. Due to loose constraints within the security & inventory meta packages pulling 2.4.6 code... Couldn't find much on it, although this issue seems like they had a similar thing magento/magento2#38594 I've pushed a potential fix for it by pinning those meta packages to the versions distributed in Waiting for the CI tests to run on the fork, and will check back in afterwards. |
734f6d0
to
77af68c
Compare
All the 2.4 builds within the Fork CI are passing now. |
Thanks @SamJUK |
No Problem, thanks for the repo! Reduces the barrier to entry for automating testing a bunch |
This PR resolves the compilation issues when running 2.4.4 on PHP 8.1.
PHP 8.1 is the only supported version for Magento 2.4.4.
magento/magento2#35292 (comment)