dev/translation#48 Add check for PHP-Intl Extension #17668
Conversation
|
(Standard links)
|
CRM/Utils/Check/Component/Env.php
Outdated
| public function checkPHPIntlExists() { | ||
| $messages = []; | ||
| if (!extension_loaded('intl')) { | ||
| $messages[] = CRM_Utils_Check_Message( |
There was a problem hiding this comment.
Should be $messages[] = new CRM_Utils_Check_Message(. It gives an error 500 currently.
|
By the way php-intl is disabled by default in windows, but I guess depends on the hosting. It's easy enough to enable. |
|
@demeritcowboy fixed now |
|
Great. It's working now. |
|
@seamuslee001 I've merged this based on @demeritcowboy's review - but I think we should probably re-add handling for when the extension is not present where we have started to use this library - as a precaution (it can be as hacky as it always was...) |
|
A lot of folks have a deep hatred for the system check because they feel it's always playing gotcha with them. I'm putting this at the top of the features list in the release notes so people are aware of this, but I'm worried that unless someone reads the release notes, they'll upgrade, see no mention of this, and then have this warning appear to users. At a minimum, a system check that declares something to be a problem when it wasn't an actual problem before should be accompanied by an upgrade message. I also agree with @eileenmcnaughton that we'll need to take precautions for people who go into 5.28 without php-intl. It's unreasonable to expect everyone to have updated to 5.27 and seen the warning. |
|
@agh1 @seamuslee001 I just looked and it doesn't seem like the installer is checking for the presence of php-intl. Though I'm also not sure if the |
|
@MegaphoneJon has a good point about the installer. I just opened #17728 to offer a pre-upgrade message explaining what's going on. |
|
Thanks @agh1! I don't think this will prevent an upgrade to 5.28 - nor will it prevent a fresh install of 5.28 where this PHP extension doesn't exist. It seems like we need both. I'm willing to do the latter if someone else can do the former? |
|
You're right. I have been more focused on 5.27. It looks from @eileenmcnaughton's comment above that she wanted to be sure that someone on 5.28 without php-intl at least won't have things exploding. That's probably the best course, since any code to prevent an upgrade to 5.28 would be shipped in 5.28, and that means you've already swapped out your old version. We always say to back up before upgrading, but it's not great to rely upon rolling back the code as someone's only recourse. I don't know the first thing about what dev/translation#28 is going to be used for, so this should probably be left to someone involved with that. I'll comment on that ticket to cross-reference. |
|
@agh1 @seamuslee001 @MegaphoneJon at the moment there will be a fatal error in 5.28 subtractCurrencies function without php-intl enabled. I think we actually need to detect the presence of the extension in that function & fall back to the old handling. Ditto when we switch over the more prevalent money functions. We don't want to support both forever but I foresee a minimum of 6 months where we do. That patch isn't done yet but assuming the rc is cut before it is it should target the rc. |
|
See #17759 |
Overview
This adds in a system check for the PHP-Intl extension that will be needed from 5.28 onwards
Before
No System check
After
System Check
ping @eileenmcnaughton @kcristiano