Skip to content
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

CRM-21719 Require Multibyte PHP extension #11599

Merged
merged 1 commit into from
Jan 30, 2018

Conversation

seamuslee001
Copy link
Contributor

@seamuslee001 seamuslee001 commented Jan 28, 2018

Overview

Alters installation to require multibyte PHP functions

Before

mb_ not required

After

mb_ functions required for install

ping @totten @colemanw @mlutfy this i think would be needed to make mb_ functions required on install. I think this makes sense to do given how often we use an mb_ function in the codebase


@colemanw
Copy link
Member

I agree with this change. Note however that this index.php is not always used e.g. when installing via the command line. So the status check is also important to keep around.

@colemanw colemanw merged commit 12b8ed7 into civicrm:master Jan 30, 2018
@totten
Copy link
Member

totten commented Jan 30, 2018

tldr: I'd suggest replicating the test in Civi/Install/Requirements.php

longer: Early in the D8 project, Torrance extracted the list of requirements from the install folder and reproduced them in Civi/Install/Requirements.php, with the intent of making it available for use in hook_requirements. (This is actually used by in 8.x-master's civicrm.install file.)

I also recently used Civi\Install\Requirements as a building-block in the civicrm-setup library. The general idea is described more in this blog.

... So the status check is also important to keep around.

Perhaps CRM/Utils/Check/Env.php should also have a point where it calls Civi/Install/Requirements.php to ensure that the DB/PHP requirements are still met?

@colemanw
Copy link
Member

Perhaps CRM/Utils/Check/Env.php should also have a point where it calls Civi/Install/Requirements.php to ensure that the DB/PHP requirements are still met?

That sounds like a good plan.

@mlutfy mlutfy added this to the 4.7.31 milestone Feb 9, 2018
@seamuslee001 seamuslee001 deleted the CRM-21719 branch March 9, 2018 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants