PHP extensions and PDO functionality should be checked at installation.

I raise the priority of this issue, because it prevents one from installing Drupal 7.0. Just tried to test 7.0-alpha1 and it failed...

Is there a solution to this?

What version of PHP are you using?

I'm using 5.2.12 in my Gentoo box. I had to enable the json USE flag in order for the json methods to be compiled and enabled in PHP. Afterwards it worked fine. Can we safely assume this is a standard feature enabled in PHPs found in most hosting providers...?

According to http://www.php.net/manual/en/json.requirements.php, the JSON functions are included in PHP by default - I didn't even realize it was possible to compile PHP 5.2 without them, but apparently it is. But yeah, sounds like it's probably a pretty uncommon configuration?

I guess this Drupal 7 requirement should be documented at http://drupal.org/requirements as well as checked during http://api.drupal.org/api/function/system_requirements/7 so that a friendlier error message results?

Yes, Gentoo is a Linux distribution that enables the ability to selectively include or exclude major features from packages, like PHP. Since it's enabled by default and probably used by the majority of providers out there, the best thing to do is to include a friendly message (as it currently stands, no message is displayed at all, just the fatal error from the PHP engine) in case the json methods are not included in the PHP installation.

Despite the PHP manual, there is a --disable-json switch in configure.

I think we should won't fix every issue which relates to a php not installed from php.net. I am sick and tired.

Yup, I got bit by this, too. I'm not sure how to edit the system requirements page, but I'm submitting a patch for system_requirements().

Given that a handful of people ran into this already, we should probably add this check so I committed it to CVS HEAD. If we need to add more, we can streamline the code by wrapping it in a foreach().

PEAR contains a nice tool called PHP_CompatInfo which can scan a directly and list all extension and constants used.

This is the output when running the whole Drupal directory:

+-----------------------------+---------+---+------------+--------------------+
| Files                       | Version | C | Extensions | Constants/Tokens   |
+-----------------------------+---------+---+------------+--------------------+
| ./*                         | 5.2.0   | 7 | bcmath     | CASE_LOWER         |
|                             |         |   | date       | DATE_ISO8601       |
|                             |         |   | filter     | DATE_RFC1123       |
|                             |         |   | ftp        | DATE_RFC2822       |
|                             |         |   | gd         | ...CTORY_SEPARATOR |
|                             |         |   | hash       | ENT_QUOTES         |
|                             |         |   | json       | EXTR_SKIP          |
|                             |         |   | mbstring   | E_ALL              |
|                             |         |   | pcre       | E_COMPILE_ERROR    |
|                             |         |   | session    | E_COMPILE_WARNING  |
|                             |         |   | SimpleXML  | E_CORE_ERROR       |
|                             |         |   | SPL        | E_CORE_WARNING     |
|                             |         |   | xml        | E_ERROR            |
|                             |         |   |            | E_NOTICE           |
|                             |         |   |            | E_PARSE            |
|                             |         |   |            | ...COVERABLE_ERROR |
|                             |         |   |            | E_STRICT           |
|                             |         |   |            | E_USER_ERROR       |
|                             |         |   |            | E_USER_NOTICE      |
|                             |         |   |            | E_USER_WARNING     |
|                             |         |   |            | E_WARNING          |
|                             |         |   |            | FALSE              |
|                             |         |   |            | FILE_APPEND        |
|                             |         |   |            | ...AG_NO_RES_RANGE |
|                             |         |   |            | ..._VALIDATE_EMAIL |
|                             |         |   |            | FILTER_VALIDATE_IP |
|                             |         |   |            | FTP_BINARY         |
|                             |         |   |            | HASH_HMAC          |
|                             |         |   |            | ...ILTER_GRAYSCALE |
|                             |         |   |            | LC_CTYPE           |
|                             |         |   |            | LOCK_EX            |
|                             |         |   |            | LOCK_NB            |
|                             |         |   |            | LOCK_SH            |
|                             |         |   |            | LOCK_UN            |
|                             |         |   |            | NULL               |
|                             |         |   |            | PATHINFO_EXTENSION |
|                             |         |   |            | PATHINFO_FILENAME  |
|                             |         |   |            | PATH_SEPARATOR     |
|                             |         |   |            | PHP_INT_MAX        |
|                             |         |   |            | PHP_OS             |
|                             |         |   |            | PHP_URL_HOST       |
|                             |         |   |            | PHP_VERSION        |
|                             |         |   |            | PREG_SET_ORDER     |
|                             |         |   |            | ...T_DELIM_CAPTURE |
|                             |         |   |            | ..._SPLIT_NO_EMPTY |
|                             |         |   |            | SORT_NUMERIC       |
|                             |         |   |            | ...MKDIR_RECURSIVE |
|                             |         |   |            | ...M_REPORT_ERRORS |
|                             |         |   |            | ..._URL_STAT_QUIET |
|                             |         |   |            | STREAM_USE_PATH    |
|                             |         |   |            | TRUE               |
|                             |         |   |            | T_CLASS            |
|                             |         |   |            | T_INTERFACE        |
|                             |         |   |            | ...D_ERR_FORM_SIZE |
|                             |         |   |            | ...AD_ERR_INI_SIZE |
|                             |         |   |            | UPLOAD_ERR_NO_FILE |
|                             |         |   |            | UPLOAD_ERR_OK      |
|                             |         |   |            | UPLOAD_ERR_PARTIAL |
|                             |         |   |            | ...ON_CASE_FOLDING |
|                             |         |   |            | ...TARGET_ENCODING |
|                             |         |   |            | __CLASS__          |
|                             |         |   |            | __FILE__           |
|                             |         |   |            | __FUNCTION__       |
|                             |         |   |            | abstract           |
|                             |         |   |            | catch              |
|                             |         |   |            | clone              |
|                             |         |   |            | final              |
|                             |         |   |            | implements         |
|                             |         |   |            | instanceof         |
|                             |         |   |            | interface          |
|                             |         |   |            | private            |
|                             |         |   |            | protected          |
|                             |         |   |            | public             |
|                             |         |   |            | throw              |
|                             |         |   |            | try                |
+-----------------------------+---------+---+------------+--------------------+

Obviously, some of these are only used conditionally.

It may be best to just go ahead and add direct install requirements for all of the extensions we require. Even if it's stupid to run a server without some of them, it's possible so we should politely tell those people to get a real server rather than just fataling on them. If they're not cool enough to run Drupal, we should tell them so explicitly. :-)

I don't see PDO on this list. Currently, installs without PDO are not that uncommon (see #698502: Your web server does not appear to support any common database types.) and they fail during the database detection step.

Interesting, I knew I saw that issue with not having any PDO drivers enabled somewhere. How nice it can all fit together :)

IMO, we should not be throwing an exception at #698502: Your web server does not appear to support any common database types. at all; we should have already checked that by system_requirement() time. I have submitted a patch that sort of makes the other issue a duplicate, by solving it in the requirements page... before getting to the install_settings().

So to summarize, this patch checks for all required extensions (as Dries suggested, a foreach loop works), and then if PDO is enabled, does the secondary PDO database extension check.

I'll comment over on #698502: Your web server does not appear to support any common database types. as well.

Erm, somehow I attached the old patch. Let's try again...

So, let's handle the edge case where someone has one or more extensions missing AND no valid PDO driver. :)

Note that you don't need all extensions from the used above, that's just a list of *used* extensions in the code. Especially, we have wrapper functions to use mbstring() if available and plain php functions if not. Also, some of these are only used in specific modules and already handled there, for example bcmath which is needed by openid.module.

Hmmm, but I removed bcmath from the list because I am awar of how it is used and that the OpenID module is checking for it. I didn't include it in the list.

However, you are correct -- mbstring seems like it should be optional. Updated the list.

#17: 641408.patch queued for re-testing.

I would like to see mbstring become a requirements. After all, Drupal is nearly completely unusable without it, and our "emulated" Unicode library is way too limited.

Thanks for the effort everyone! Because you are already working on this, I just simply ask it: Are there any check for filter extension? On Gentoo you can turn it off too. According to my package.use file, I had to enable filter use flag too, to get drupal7 install work properly.

Here is the available use flags on gentoo:

[ebuild   R   ] dev-lang/php-5.2.12  USE="bzip2 calendar cgi cli crypt ctype curl discard-path filter force-cgi-redirect ftp gd hash iconv imap json mysql mysqli nls pcre pdo postgres session simplexml sockets spl suhosin sysvipc threads tokenizer truetype unicode xml xmlreader xmlrpc xmlwriter zip zlib -adabas -apache2 -bcmath -berkdb -birdstep -cdb -cjk -concurrentmodphp -curlwrappers -db2 -dbase -dbmaker -debug -doc -empress -empress-bcs -esoob -exif -fastbuild -fdftk -firebird -flatfile -frontbase -gd-external -gdbm -gmp -inifile -interbase -iodbc -ipv6 (-java-external) -kerberos -kolab -ldap -ldap-sasl -libedit -mcve -mhash -msql -mssql -ncurses -oci8 -oci8-instant-client -odbc -pcntl -pic -posix -qdbm -readline -recode -reflection -sapdb -sharedext -sharedmem -snmp -soap -solid -spell -sqlite -ssl -sybase -sybase-ct -tidy -wddx -xpm -xsl -yaz" 0 kB

Yes. My latest patch checks for filter extension.

Damien, that change is out of the scope of this issue. Other parts of core do conditional checks for mbstring, so if you want to require it then remove those checks in another issue.

I'm unclear on why we're moving the database type checks up here. That part doesn't fatal out now, does it? It didn't used to.

Also, for the insanely-long error message line, can we break the array of replacement values in t() to separate lines, the way we do for database prepared statement values? It makes the code much easier to read, and is more consistent to boot.

Why should we not check databases during the system compatibilty check? I personally think it's a great idea to make a full list of things that need to be updated at that step, so the person can go find a different host or talk to their tech support or whatever. Why wait until a different install step to figure out that they don't have a PDO-compatible database either?

I would like to add to #24 that it is really silly to see a green checked "Verify requirements" and later get basic error information on another screen.

@cha0s: Great! Thank you!

Crell, I didn't understand the complaint with the error lines. Could you clarify?

Seems reasonable to check db types in the system requirements section before we get to db selection.

This line:

$errors[] = $t('Drupal requires you to enable the following PHP extensions: @extensions. See the <a href="@system_requirements">system requirements page</a> for more information.', array('@extensions' => implode(', ', $failures), '@system_requirements' => 'http://drupal.org/requirements'));    

Would be better as:

$errors[] = $t('Drupal requires you to enable the following PHP extensions: @extensions. See the <a href="@system_requirements">system requirements page</a> for more information.', array(
  '@extensions' => implode(', ', $failures), 
  '@system_requirements' => 'http://drupal.org/requirements',
));    

as it is easier to follow, easier to see what the replacements are, and matches the way nearly all DB queries are now structured.

If the list of extensions someone could need to enable is potentially long, would it perhaps be better to format it as a UL list?

Something like this [untested]:

$errors[] = '<p>' . $t('Drupal requires you to enable the PHP extensions in the following list (see the <a href="@system_requirements">system requirements page</a> for more information):', 
array('@system_requirements' => 'http://drupal.org/requirements',)) . 
'</p>' . theme('item_list',  array('type' => 'ul', 'items' => $failures));

Sweet.

@Crell: I already do that in my own personal PHP code but I wasn't sure if I could get away with it under Drupal's standards ;)

@jhodgdon: I like the list, but I'm not sure abou the <p>, it creates a bit too much whitespace IMO. I attached a screenie of how this looks now, but if we need the p for aesthetic or cross-browser reasons, I have no problem to put it back in...

I'm fine with the UL list and no P tag.

I like this patch a lot. Some comments:

1. I don't understand why there is only one requirements check here - shouldn't there be two (one for the required PHP extensions, and a separate one for the database)? With the way it is now, it looks a bit awkward in the UI to have stuff about missing databases under a "PHP extensions" header, and also a bit awkward in the code (e.g., the distinction between $errors[] and $failures[], as well as implode('<br />', $errors))...

   Any reason not to just have two separate sections, "PHP extensions" and "Database"?

2. Further down in system_requirements() I see this existing code:
   

     // Verify if the DOM PHP 5 extension is available.
     $has_dom = class_exists('DOMDocument');
     if (!$has_dom) {
       $requirements['php_dom'] = array(
         'title' => $t('PHP DOM Extension'),
         'value' => $t('Not found'),
         'severity' => REQUIREMENT_ERROR,
         'description' => $t("The DOM extension is part of PHP 5 core, but doesn't seem to be enabled on your system. You need to enable the DOM extension on your PHP installation."),
       );
     }
   

   Shouldn't we include that in this patch as well? (I think just by adding 'dom' to the list of extensions to be checked.)

3. +  foreach (array(
   +    'date', 'filter', 'gd', 'hash', 'json', 'pdo',
   +    'pcre', 'session', 'SimpleXML', 'SPL', 'xml'
   +  ) as $extension) {
   

   The formatting here doesn't seem right. If it's too long for one line, then I believe the correct adherence to coding standards would be something like this:

   $required_extensions = array(
     'date',
     'filter',
     'gd',
     'hash',
     'json',
     'pdo',
     'pcre',
     'session',
     'SimpleXML',
     'SPL',
     'xml',
   );
   foreach ($required_extensions as $extension) {
   ...
   

4. Finally (minor):
   

   +  // Test for at least one matching PDO extension, if PDO's available.
   

   This should probably be "if PDO is available".

Changing title, since we're talking about a lot more than just json_encode(). :-)

Excellent suggestions. I have incorporated them into this latest patch. We also required a reroll since logic seems to have moved from install.php which was there before.

Clearer title again, IMO.

If I could type, that is.

This looks really good and trying it out it seems to work correctly. Very close to ready, in my opinion.

My only main comment is that for PDO, we seem to have enough information here to give them a little more detail about what is wrong - for example, we can easily distinguish between someone who does not have PDO enabled at all (and therefore should have it included in the main list of missing extensions?) vs someone who has it enabled but does not have any supported database drivers. It feels like we could massage that part a little more to make it more helpful.

Otherwise, a couple minor code style things:

+    $description .= theme('item_list',  array(
+      'type' => 'ul', 
+      'items' => $missing_extensions
+    ));

These (and other places in the patch) where array elements are on their own line should always have each line end in a comma, even the last line.

   }
-
+  
   // Test PHP memory_limit
   $memory_limit = ini_get('memory_limit');

This change is adding trailing white space to the patch - blank lines should be blank, so that part should be removed.

It also looks like there is some trailing whitespace further up in the patch (in the $missing_extensions list).

Okay, I fixed the styling issues. I'm not sure about giving the user 2 separate errors for PDO and PDO extensions. Putting that patch here for review.

This looks great. I made a couple tiny changes - which can be seen in the attached interdiff file - they're tiny enough that it was easier to just make them, so I'm setting this straight to RTBC. (The only noticeable one is that I went back to the previous "Database support" wording you had before; I think that's friendlier than "Database extensions".)

Having two separate errors for PDO wasn't what I was thinking - I actually thought we'd use some logic to print the error in one place or the other, depending on which kind of PDO issue they were having. However, I like the way you did it better. My way would have involved hiding some information from people up front, which isn't good. People in this situation are likely to need all the info they can get, and even if they have the PDO extension enabled, the fact that they are seeing this error message likely means their hosting provider still doesn't really "support PDO", so the full text is appropriate. I also think it's OK to show two errors in the case of PDO not enabled at all; both errors are true. They have a missing PHP extension - so we guarantee that that list is correct and complete - but there are additional requirements due to the fact that PDO is the one that is missing, so the second error message explains that.

Looks good! Darn commas! ;)

+++ modules/system/system.install	25 Feb 2010 05:26:27 -0000
@@ -95,30 +95,71 @@ function system_requirements($phase) {
+  foreach (array(
+    'date',
+    'dom',
+    'filter',
+    'gd',
+    'hash',
+    'json',
+    'pcre',
+    'pdo',
+    'session',
+    'SimpleXML',
+    'SPL',
+    'xml',
+  ) as $extension) {
+
+    if (!extension_loaded($extension)) {
+      $missing_extensions[] = $extension;
+    }
+  }

This looks pretty ugly ;)

If we really want the extensions to be on separate lines, then I suggest to do:

$required_extensions = array(
...
);

upfront.

Powered by Dreditor.

Agreed with sun, but otherwise looks RTBC.

OK, I made that change in the attached patch.

Great job, and thanks for the quick re-roll. Committed to CVS HEAD. Thanks!