I am not sure if there are any plans regarding php 5.3 at this time.

I am currently testing php5.3.0alpha3 and found several issues with current HEAD. Most of them are because PHP 5.3 seems to create a warning message when a function with by-reference parameters is called by value. For example, this does happen when a hook defines his parameters as by-reference but is called with module_invoke(_all).

These errors are pretty easy to fix, however, there are some others where I'm not sure.

- PHP 5.3 changed the wording of the warning message "The second argument should be either an array or an object" to "array_key_exists() expects parameter 2 to be array, null given ". Because this warning mesage is used by simpletest.test to ensure that php warnings are correctly reportet, the test does now fail. I don't know how this should be resolved so that it does work with php 5.3 and older versions too.

- Some of the upload user image tests are broken. I haven't looked at those in detail yet. This is maybe another issue or a misconfiguration/bug of gd because there are no php warnings reportet, in contrast to all other fails.

- Some aggregator tests are broken, because of a by reference warning, but I'm unable to figure out where the function is called. For some reason, $form_state is given by value instead of by reference and only for one single form of the whole test suite

I'm attaching a first patch which does fix most of the by reference warnings I've found, a (maybe incomplete) list:
- multiple hook_user_cancel
- multiple hook_user_categories
- multiple hook_user_logout
- theme_upload_form_current
- theme_status_report
- multiple file_validate_* functions

No Idea which category this belongs, because multiple modules are affected.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chx’s picture

I do not readily understand "warning message when a function with by-reference parameters is called by value". You have function x(&$foo) and then you call it with x($bar) and that $foo in the scope of x is a reference is bar is determined by the fact that x(&$foo) is defined as a reference. Now, if you say that call_user_func_array('x', array($bar)) throws a warning then PHP 5.3 is utterly stupid and I would consider it a bug and file a bug on php.net.

chx’s picture

Oh and thanks a ton for working on this!

chx’s picture

Thanks for working on the issue. The PHP team confirms this and considers it a good thing 'cos earlier it failed silently so yeah this is a valid bug report. sigh.

Dave Reid’s picture

Issue tags: +PHP, +PHP 5.3

Yay references! Following to watch progress.

catch’s picture

Priority: Normal » Critical

Bumping to critical - while 5.3 might not get much use before Drupal 7 is released, it's going to be out for some time before Drupal 9 is released and we drop support for D7. Would be better to have this as a release blocker to avoid dealing with it in a point release at some point.

Berdir’s picture

Priority: Critical » Normal
FileSize
10.72 KB

Some Updates:

- For the record, bug report about the warning message, was closed as bogus.
http://bugs.php.net/bug.php?id=43484

- aggregation fails were actually a drupal_execute "bug", because drupal_execute didn't consider $form_state as by-reference. It seems that the aggregator tests are the only place in the test suite where drupal_execute is used.

- upload profile image failed because I compiled gd without jpeg support

- another by-reference fix in comment_user_cancel

Still open:

- PHP 5.3 changed the wording of the warning message "The second argument should be either an array or an object" to "array_key_exists() expects parameter 2 to be array, null given ". Because this warning mesage is used by simpletest.test to ensure that php warnings are correctly reportet, the test does now fail. I don't know how this should be resolved so that it does work with php 5.3 and older versions too.

I tried to find a function with an unchanged warning but had no luck so far.

Berdir’s picture

Priority: Normal » Critical

Re-setting priority to critical.

Dave Reid’s picture

Marked #253023: status report, etc. broken under php 5.3 as a duplicate of this issue.

Berdir’s picture

Status: Needs work » Needs review

Ok, instead of tricking PHP to issue a warning message, I'm currently using trigger_error. This is not exactly the same as what has been tested before, but it has the advance that it does test (only) the parsing/testing of error messages and not the wording of them.

With this change, all tests pass now, so I'm setting this to CNR.

The tests finished in 13 min 37 sec.
8924 passes, 0 fails, and 0 exceptions

1337... this *has* to be good :)

Damien Tournoud’s picture

Status: Needs review » Active

Would be better with a patch :)

Berdir’s picture

Status: Active » Needs review
FileSize
11.76 KB

Forgot the patch...

Dries’s picture

This patch looks good to me. Will commit after another review.

Damien Tournoud’s picture

Two nitpicks:

     // Generates a warning inside a PHP function.
-    array_key_exists(NULL, NULL);
+    trigger_error('Simpletest test warning message', E_USER_WARNING);
   }

This is not exactly the same. The idea here was to generate an error *inside* a PHP internal function. This is in fact a regression test: our backtracing code wasn't able to deal with this situation at some point. Could we find an error message that haven't changed between the two versions?

+  
+  // Use by-reference parameter of $form_state
+  $args[1] = &$form_state;
+

This sentence should end with a dot.

Berdir’s picture

Status: Needs review » Needs work

> Could we find an error message that haven't changed between the two versions?
I tried :) The issue is that every single simple "wrong input"-warning seems to have been changed. Tried string, array, file.

For example, the changes of array_key_exists:

$ php -r'array_key_exists(NULL, NULL);'
Warning: array_key_exists(): The second argument should be either an array or an object in Command line code on line 1

$ php53 -r'array_key_exists(NULL, NULL);'
Warning: array_key_exists() expects parameter 2 to be array, null given in Command line code on line 1

What we could do is not test the whole string but only the function name "array_key_exists". This should be enough to be sure that the warning is there and the location is correct.

Berdir’s picture

Status: Needs work » Needs review
FileSize
11.62 KB

Ok, new try, changes:

- Fixed comment in drupal_execute
- Use only 'array_key_exists' to test the warning message as described above and added a comment to explain why, similiar to common.test

Dries’s picture

Status: Needs review » Fixed

Works! Committed to CVS HEAD.

chx’s picture

Version: 7.x-dev » 6.x-dev
Status: Fixed » Needs review
Berdir’s picture

Making Drupal 6 php 5.3 compatible is more complicated, mostly because of the following reason:

hook_user (maybe others too) is sometimes called with module_invoke (=by value) and sometimes with user_module_invoke (= by reference). This means that I can't simply remove the & from the hook definitions, as it is possible in Drupal 7 thanks to the removal of the $op switch.

These are the module_invoke(_all) calls in the user module:

modules/user/user.module:  module_invoke_all('user', 'delete', $edit, $account);
modules/user/user.module:    if ($data = module_invoke($module, 'user', 'categories', NULL, $account, '')) {
modules/user/user.module:    if ($data = module_invoke($module, 'user', $hook, $edit, $account, $category)) {
modules/user/user.pages.inc:  module_invoke_all('user', 'logout', NULL, $user);

( The third is in the function _user_forms and $hook defaults to form)

Discussed with chx in irc, there are 2 options:

1) Change atleast (there may be other hooks with the same issue, I've not yet tested everything) all calls of hook_user to user_module_invoke. I haven't tested this yet but it should work. But it is a fairly big change and does actually "break" the api. In the case of hook_user, $account will always be by-reference in contrast to the current version. This is what you would expect anyway but it could still change/break something if a contrib module changes $account for some reason.

2) This is simple, don't support 5.3 with drupal 6. While the actual error is "only" a warning, it does break things because all by reference parameters are set to NULL.

----------------

Another issue is E_DEPRECATED. However, there are simply two ways to deal with it and it's not about doing it or not. E_DEPRECATED has been added to PHP 5.3 and in contrast to E_STRICT, it is included in E_ALL and is enabled by default (atleast in the current alpha3 version). Deprecated is, beneath others especially ereg. See http://wiki.php.net/doc/scratchpad/upgrade/53 for a full list.

ereg functions are still used in Drupal 6 in multiple places:

grep -R ereg includes/ modules/ | grep -v CVS
includes/unicode.inc:  if (!$bom && ereg('^<\?xml[^>]+encoding="([^"]+)"', $data, $match)) {
includes/unicode.inc:      $data = ereg_replace('^(<\?xml[^>]+encoding)="([^"]+)"', '\\1="utf-8"', $out);
includes/file.inc:    $regex = '/\.('. ereg_replace(' +', '|', preg_quote($extensions)) .')$/i';
includes/file.inc:        elseif ($depth >= $min_depth && ereg($mask, $file)) {
modules/user/user.module:  if (ereg("[^\x80-\xF7 [:alnum:]@_.-]", $name)) return t('The username contains an illegal character.');
modules/user/user.module:  if (strpos($name, '@') !== FALSE && !eregi('@([0-9a-z](-?[0-9a-z])*.)+[a-z]{2}([zmuvtg]|fo|me)?$', $name)) return t('The username is not a valid authentication ID.');
modules/blogapi/blogapi.module:  if (eregi('<title>([^<]*)</title>', $contents, $title)) {
modules/blogapi/blogapi.module:    $contents = ereg_replace('<title>[^<]*</title>', '', $contents);

1. Easy way: exclude E_DEPRECATED from being being displayed/logged. I'm quite sure this is the way to go (ereg has been replaced in drupal 7 already, so...) but I don't wan't to make that decision :)

2. "Fix" these "errors" be replacing the ereg functions with preg_*

---

Note: Other issues might come up, these are the two that I found so far.

Berdir’s picture

Status: Needs review » Needs work

Forgot to set the right status

catch’s picture

It seems quite possible that we'll require PHP 5.3 in Drupal 8. IMO it'd be a pretty bad situation to not support it for Drupal 6, but require it for Drupal 8 - it'd prevent an upgrade from Drupal 6 to Drupal 8 on the same server - and many sites upgrade every other Drupal version. Passing $user by reference in a few places doesn't seem like too drastic of an API change to put this off, and the ereg to preg fixes could equally backported quite safely.

Berdir’s picture

PHP 5.3 RC1 was released today, and there are some test failures in D7 in our exception handling and e-mail validation tests. It however seems that both of them are bugs in PHP:

See:
filter_var & e-mail validation: http://bugs.php.net/bug.php?id=47772
autoload inside error handler: http://bugs.php.net/bug.php?id=47714

Feel free to vote and/or contribute on these issues, if you can :)

I'm trying to create a patch for D6 now.

Berdir’s picture

Status: Needs work » Needs review
FileSize
18.44 KB

Ok, first attempt of a D6 patch. I tried to backport from D7 the ereg* related stuff and some things of my patch which was commited against D7. This, especially the ereg* changes, needs a very thorough review, as there are almost no tests to validate that it works correctly.

Changes:
- Fix drupal_execute, backport of my patch
- fix theme calls which use by reference params, backport of my patch
- Change module_invoke_all() calls to user_module_invoke()
- Change module_invoke calls to $function (only possible way, because we need by reference and return value, no existing function does offer that)
- Replace all ereg* calls with preg_* (in one case, it seems that ereg_replace() can be replaced by just str_replace), backported from D7
- Fix all calls to file_scan_directoy() and drupal_system_listing() to use preg-style regex, backported from D7
- Replaced user_validate_name with the D7 version. The whole function in D6 is very hard to understand and doesn't follow the coding style. However, one check seems to miss in the D7 version, and I don't know why:
- if (strpos($name, '@') !== FALSE && !eregi('@([0-9a-z](-?[0-9a-z])*.)+[a-z]{2}([zmuvtg]|fo|me)?$', $name)) return t('The username is not a valid authentication ID.');
If that is still needed, someone needs to convert that to preg_match, that's too much regex for me :)

moshe weitzman’s picture

Typo in a few places: 'refernce'.

I do think some sort of validation on authentication id is needed in d6. that regex is too much for me too.

Berdir’s picture

I just noticed that we have a problem here.

drupal_system_listing() and file_scan_directory() expose their ereg regex, this means that it will break every module which uses this function, for example SimpleTest.

I think we can't do that with D6, so we probably have to go with option 1) as described in #18.

Berdir’s picture

- Changed back to ereg() in file_scan_directory() and changed back all calls to it
- Fixed "refernce" typos
- If the authentication ID is really needed, someone else needs to write that, I can't do it on my own, I don't even know for what that is used :)

penguins’s picture

Is it possible to apply the patch to my current Drupal 6.10. At the moment I'm having trouble editing user profiles.

Berdir’s picture

@26
Copy paste error, try the updated patch.

Someone up to the conversion of the regex in #22? If we really need it and all else fails, I can re-add the ereg() call.. we can't remove all of them anyway.

Mike Wacker’s picture

So when I tried to load the status report on a D6 website w/ PHP 5.3, I got a fatal error. Checking the logs I found a message (in addition to a lot of ereg warnings) similar to this:

Location http://www.example.com/admin/reports/status
Message Parameter 1 to theme_status_report() expected to be a reference, value given in /var/www/d6upgrade/includes/theme.inc on line 617

As of PHP 5.3, PHP will replace an argument mistakenly passed by value instead of reference with a NULL argument, whereas in PHP 5.2 or earlier it would just continue as if nothing happened. Presumably passing a NULL argument caused some sort of fatal error down the line.

Granted, this was on an experimental website where we were working on an upgrade from D5 to D6 (and it looks like we're on 6.9 on that website), but can anyone replicate this, or is it just something specific on my end?

Update: Never mind, it's fixed as of Drupal 6.10

penguins’s picture

FileSize
116.71 KB

Thank your for the patch, Berdir.

I've applied the patch. User edit page is now showing the account informations form, however, it doesn't show user current's username and email address. Please check out the attached picture.

I have successfully changed username and password, the patch also fix status report issue.

Berdir’s picture

@28
This is a known issue and should be fixed with my patch.

@29
Another copy paste error it seems. Try the updated patch, it should be fixed now. Thanks for testing this, it seems I'm not able to think myself :) No idea why I didn't noticed the empty form.

quikone’s picture

I am sure this is a dumb question but I won't know unless I ask it. How does one apply this patch?

Berdir’s picture

pwolanin’s picture

patch need to be unified diff format

Berdir’s picture

Oh, updated patch...

pwolanin’s picture

Status: Needs review » Needs work

why did you change the regex? It's not neccesary to do so for a 6.x fix, and woudl seem to risk changes in behavior.

for example:

-  if (!$bom && ereg('^<\?xml[^>]+encoding="([^"]+)"', $data, $match)) {
+  if (!$bom && preg_match('/^<\?xml[^>]+encoding="(.+?)"/', $data, $match)) {

and

-  if (eregi('<title>([^<]*)</title>', $contents, $title)) {
+  if (preg_match('/<title>(.*?)<\/title>/i', $contents, $title)) {
     $title = strip_tags($title[0]);
-    $contents = ereg_replace('<title>[^<]*</title>', '', $contents);
+    $contents = preg_replace('/<title>.*?<\/title>/i', '', $contents);

Also, typically if the regex contains '/' we use a different delimiter such as '@':

 $contents = preg_replace('@<title>[^<]*</title>@i', '', $contents);
Berdir’s picture

The plan was to remove ereg() because that is marked as deprecated on PHP 5.3. However, it is not possible to do that, because some ereg() calls expose their regex (see #24) so I reverted these calls and excluded E_DEPRECATED errors from being displayed.

I agree that it does not make sense to partially remove ereg() then, so I reverted all ereg related changes back.

The updated patch is now pretty simple and only does the following things

- If necessary, defines a E_DEPRECATED constant
- excludes all E_DEPRECATED errors from getting displayed
- removes & from theme functions (because of a change with by reference handling in PHP 5.3, already in D7)
- The fix for drupal_execute (because of the same change, already in D7)
- and some user hook invocation changes (because of the same change, solved slightly different in D7 because of the renamed hooks)

Also improved coding style of the comments a bit, this should look a lot better now..

Berdir’s picture

Status: Needs work » Needs review

Setting to needs review again..

pwolanin’s picture

Status: Needs review » Needs work

looks better, but this comment needs some work:

// We can't use neither module_invoke nor user_module_invoke because we need the return value and by reference.
killes@www.drop.org’s picture

Parameter 1 to profile_load_profile() expected to be a reference, value given in /includes/module.inc on line 450

This is not addressed by this patch I believe.

Berdir’s picture

@killes

Can you tell me how to reproduce that error and have you tested it with the patch? I created some profile fields, and everything seems to work fine.

Gerhard Killesreiter’s picture

yes, I had applied this patch. I currently cannot reproduce it since I've gone back to 5.2 for now.

I think all you need to do is to remove the & in the function definition of profile_load_profile.

penguins’s picture

Does Drupal 6.x support PHP5.3?

I have downloaded yesterday's snapshot of Drupal6-x-dev, found out that non of the patches have been added to it. So I'm wondering whether Drupal 6 is going to support PHP 5.3.

gr00vus’s picture

Hi,

Installing the patch I get the following:


patch -b -p0 < drupal.php53_compat_6-D6.patch
patching file includes/common.inc
Hunk #2 FAILED at 584.
1 out of 2 hunks FAILED -- saving rejects to file includes/common.inc.rej
patching file includes/form.inc
patching file modules/system/system.admin.inc
patching file modules/upload/upload.module
patching file modules/user/user.module
patching file modules/user/user.pages.inc

Here are the contents of common.inc.rej:

***************
*** 577,583 ****
      return;
    }
  
-   if ($errno & (E_ALL)) {
      $types = array(1 => 'error', 2 => 'warning', 4 => 'parse error', 8 => 'notice', 16 => 'core error', 32 => 'core warning', 64 => 'compile error', 128 => 'compile warning', 256 => 'user error', 512 => 'user warning', 1024 => 'user notice', 2048 => 'strict warning', 4096 => 'recoverable fatal error');
  
      // For database errors, we want the line number/file name of the place that
--- 584,590 ----
      return;
    }
  
+   if ($errno & (E_ALL ^ E_DEPRECATED)) {
      $types = array(1 => 'error', 2 => 'warning', 4 => 'parse error', 8 => 'notice', 16 => 'core error', 32 => 'core warning', 64 => 'compile error', 128 => 'compile warning', 256 => 'user error', 512 => 'user warning', 1024 => 'user notice', 2048 => 'strict warning', 4096 => 'recoverable fatal error');
  
      // For database errors, we want the line number/file name of the place that

Is the portion that's failing the part that addresses the ereg() problem?

The ereg() thing really makes drupal nearly unusable with errors being displayed. Is there going to be an actual fix for this at some point?

Thanks.

penguins’s picture

Try using drupal.php53_compat_5-D6.patch. You can set the errors to be written to a log file in the administrer -> Site Configuration -> Error reporting, that way the pages will not be cluttered with the error messages.

Berdir’s picture

The patch is against the -dev version, because that does not contain the additional E_NOTICE check. However, you can manually overwrite the failing line.

The patch is not yet commited. killes reported a bug with profiles that I can't reproduce. If you try out the patch, please report any error messages or broken things that you can find.

gr00vus’s picture

Changing the error reporting helps. I'm a newbie - where are the error logs located in a default configuration? Is there a way to control where the logs are written to?

I still get this error applying the version 5 patch:

"
Hunk #2 FAILED at 584.
1 out of 2 hunks FAILED -- saving rejects to file includes/common.inc.rej
"

Searching for 584 in the patch file I find:

@@ -577,7 +584,7 @@ function drupal_error_handler($errno, $m

What would I need to do to fix this "hunk?"

I don't know anything about patch or how it works really, but it looks like this line is incomplete (no terminating ")" at the very least)?

This time the common.inc.rej contents are:

***************
*** 577,583 ****
      return;
    }
  
-   if ($errno & (E_ALL)) {
      $types = array(1 => 'error', 2 => 'warning', 4 => 'parse error', 8 => 'notice', 16 => 'core error', 32 => 'core warning', 64 => 'compile error', 128 => 'compile warning', 256 => 'user error', 512 => 'user warning', 1024 => 'user notice', 2048 => 'strict warning', 4096 => 'recoverable fatal error');
  
      // For database errors, we want the line number/file name of the place that
--- 584,590 ----
      return;
    }
  
+   if ($errno & (E_ALL ^ E_DEPRECATED)) {
      $types = array(1 => 'error', 2 => 'warning', 4 => 'parse error', 8 => 'notice', 16 => 'core error', 32 => 'core warning', 64 => 'compile error', 128 => 'compile warning', 256 => 'user error', 512 => 'user warning', 1024 => 'user notice', 2048 => 'strict warning', 4096 => 'recoverable fatal error');
  
      // For database errors, we want the line number/file name of the place that

Thanks again.

penguins’s picture

gr00vus,

which version of Drupal are you patching?

One way to apply the patches is to delete the lines with "-" sign in front (from the common.inc.rej) and replace it with the lines with "+" sign in front.

penguins’s picture

Berdir,

I'm testing using Windows Server 2008 and PHP 5.3 RC3-dev and I found the following errors (after applying drupal.php53_compat_5-D6.patch)

1. Unable to change Garland theme's colour scheme (Go to Administer -> site building -> themes -> click on "configure" on the right hand site and Garland theme -> change colour scheme to anything other than Blue Lagoon (default)).
Found the following error msg:

The selected file could not be copied, because no file by that name exists. Please check that you supplied the correct filename.

2. Unable to change website logo (Go to Administer -> site building -> themes -> click on "configure" on top of the page -> upload an image on Logo Image Settings).
Found the following error msgs:
warning: Parameter 1 to file_validate_is_image() expected to be a reference, value given in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #2 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #1 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
The selected file could not be copied, because no file by that name exists. Please check that you supplied the correct filename.

3. Unable to change website shortcut icon (Go to Administer -> site building -> themes -> click on "configure" on top of the page -> upload an image on Shortcut Item Settings).
Fount the following error msg:
The selected file could not be copied, because no file by that name exists. Please check that you supplied the correct filename.

4. I am able to attach a file to a content, but it finishes with several warnings:
warning: Parameter 1 to file_validate_image_resolution() expected to be a reference, value given in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #2 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #1 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #1 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.

Berdir’s picture

Status: Needs work » Needs review
FileSize
6.49 KB

Thanks for your tests..

1. Unable to change Garland theme's colour scheme (Go to Administer -> site building -> themes -> click on "configure" on the right hand site and Garland theme -> change colour scheme to anything other than Blue Lagoon (default)).
Found the following error msg:

The selected file could not be copied, because no file by that name exists. Please check that you supplied the correct filename.

I can't reproduce this. Maybe something is wrong with your write permissions?

2. Unable to change website logo (Go to Administer -> site building -> themes -> click on "configure" on top of the page -> upload an image on Logo Image Settings).
Found the following error msgs:
warning: Parameter 1 to file_validate_is_image() expected to be a reference, value given in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #2 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #1 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
The selected file could not be copied, because no file by that name exists. Please check that you supplied the correct filename.

Confirmed and fixed.

3. Unable to change website shortcut icon (Go to Administer -> site building -> themes -> click on "configure" on top of the page -> upload an image on Shortcut Item Settings).
Fount the following error msg:
The selected file could not be copied, because no file by that name exists. Please check that you supplied the correct filename.

Again, I can't reproduce this.

4. I am able to attach a file to a content, but it finishes with several warnings:
warning: Parameter 1 to file_validate_image_resolution() expected to be a reference, value given in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #2 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #1 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.
warning: array_merge(): Argument #1 is not an array in C:\inetpub\wwwroot\drupal6-patched\includes\file.inc on line 553.

This was the same bug as above.

@killes
I found your bug I think.

sites/all/modules/realname/realname.module:    module_invoke($module, 'load_profile', $account, $type);

Or maybe another module... This is not a bug in core. That should be something like

$function = $module . '_load_profile';
$function($account, $type);
ludovicofischer’s picture

Part of this patch has to be applied manually to a drupal 6.12 installation:
In common.inc, line 577, drupal 6.x CVS has
if ($errno & (E_ALL))
where drupal 6.12 has
if ($errno & (E_ALL ^ E_NOTICE))

This change appeared in rev 1.756.2.53, without being mentioned in the logs.

Berdir’s picture

That is the difference between a stable and a -dev release, dev releases display E_NOTICE messages.

gr00vus’s picture

@penguins

I'm using version 6.12. I realize the patch is targeted at the development version, which probably explains the patch errors - should I switch over to using the development version instead? I'm not making a production site (yet), it's mainly personal use right now, though it may end up with a wider audience in the near future. I figure I'd start with the stable version.

Thanks.

penguins’s picture

Berdir,

Thank you for the patch, it stopped the warning. You are right about the file permission, the PHP upload_tmp_dir dir is not readable by user. I can now change theme's colour and change the website logo.

I found another warning message when trying to change content's status. To replicate, go to Administer -> Content Management -> tick several titles and change the update options (from publish to unpublish or the other way).

Warnings found:

notice: Undefined property: stdClass::$new in C:\inetpub\wwwroot\drupal6-patched\modules\upload\upload.module on line 410.

penguins’s picture

gr00vus,

If you are going to turn the site into a production site, then it is better for you to use the stable version of PHP and Drupal (in this case PHP 5.2.9 and Drupal 6.12).

Berdir’s picture

Just as a note, PHP 5.3 will become stable soon, probably in this week: http://news.php.net/php.internals/44397

So, please report back if it the patch works or if you have other issues.

@53
that is a E_NOTICE, that is now displayed because you use the -dev version (or changed the relevant check in the error callback function). This *is* a bug, but not related to this issue nor PHP 5.3. This notice won't be displayed with a stable version.

Edit: Also, to get this commited, we need confirmation that it still works on PHP4 with my patch. It *should*, but I haven't tested it. If anyone does still have a PHP4 install, please test it with this patch applied. Especially theme settings, profile stuff, and file/image uploads with automatic size conversion.

chx’s picture

I think that a contrib project that adds 5.3 compatibility is better. No big ISP or distro will feature PHP 5.3 soon (heck, we needed to fight a war for PHP 5.2 for D7 and RHEL still do not even have that!) so if you run 5.3 you are running your own PHP. Then run your own Drupal too.

moshe weitzman’s picture

@chx - are you suggesting that the currently shipping drupal should not run on the currently shipping PHP? i can't imagine thats a good idea. i agree that this is critical. people setup their servers all the time and many of them run php 5.3.

Mike Wacker’s picture

On the same page, how exactly would a contrib module solve this? I can't think of a good way to do what this patch does via a contributed project without hacking core. Plus, looking at the patch, it has a very low risk of breaking anything, so our existing user base who's not on PHP 5.3 should not be affected.

Also, many devs use both a live and development server, with the dev server running the edgier version of PHP. And it would be hard to do development there if issues like these break Drupal.

chx’s picture

people setup their servers all the time and many of them run php 5.3. <= wow. who ??? I would not touch PHP 5.3 with a ten feet barge pole for another 4-5 minor releases.

The contrib project should contain this patch. As said, if you run your own server, you can run your own Drupal.

shrop’s picture

subscribe

clintthayer’s picture

Anyone have any thoughts on Drupal 5 and php 5.3?

Any thoughts would be helpful.

Mike Wacker’s picture

I think the points chx makes are valid for Drupal 5. By the time 7 is released and 5 is no longer supported, no major ISP or distro will have PHP 5.3. However, since 6 will be supported until Drupal 8 comes out (quite a long time), it probably has to be patched for PHP 5.3 at some point in time.

chx’s picture

Status: Needs review » Reviewed & tested by the community

Berdir says the latest patch does away with the hook_user API break. Aye. So let's go then.

Garrett Albright’s picture

PHP 5.3 deprecates the ereg library, which appears to be in use in a disturbing number of places in core:

$ grep -n 'ereg' -r ./
./includes/file.inc:626:    $regex = '/\.('. ereg_replace(' +', '|', preg_quote($extensions)) .')$/i';
./includes/file.inc:895:        elseif ($depth >= $min_depth && ereg($mask, $file)) {
./includes/unicode.inc:138:  if (!$bom && ereg('^<\?xml[^>]+encoding="([^"]+)"', $data, $match)) {
./includes/unicode.inc:148:      $data = ereg_replace('^(<\?xml[^>]+encoding)="([^"]+)"', '\\1="utf-8"', $out);
./modules/blogapi/blogapi.module:692:  if (eregi('<title>([^<]*)</title>', $contents, $title)) {
./modules/blogapi/blogapi.module:694:    $contents = ereg_replace('<title>[^<]*</title>', '', $contents);
./modules/user/user.module:385:  if (ereg("[^\x80-\xF7 [:alnum:]@_.-]", $name)) return t('The username contains an illegal character.');
./modules/user/user.module:398:  if (strpos($name, '@') !== FALSE && !eregi('@([0-9a-z](-?[0-9a-z])*.)+[a-z]{2}([zmuvtg]|fo|me)?$', $name)) return t('The username is not a valid authentication ID.');
./sites/all/modules/logintoboggan/logintoboggan.module:855:  if (ereg("[^\x80-\xF7[:graph:] ]", $pass)) return t('The password contains an illegal character.');
./sites/all/modules/pathauto/pathauto.inc:183:  if (function_exists('mb_eregi_replace')) {
./sites/all/modules/pathauto/pathauto.inc:184:    $output = mb_eregi_replace($ignore_re, '', $output);

I'm crushing, so I'll just exclude E_DEPRECATED from error_reporting for now, but it would be good to see these swapped out with their preg equivalents.

EDIT: Apparently you can't exclude E_DEPRECATED? Rrgh, I wish this friggin' client wouldn't insist on using their own in-house Windows server.

Dave Reid’s picture

Changing from ereg to preg is going to be impossible without changing the APIs in Drupal 6. I don't see that being likely.

Berdir’s picture

My patch does exclude E_DEPRECATED errors, that works just fine.

Garrett Albright’s picture

Changing from ereg to preg is going to be impossible without changing the APIs in Drupal 6. I don't see that being likely.

I'm not doubting you, but could you give an example why the API would have to change? Why can't the calls just be rewritten to use preg equivalents?

Berdir’s picture

http://api.drupal.org/api/function/drupal_system_listing/6

$mask is a ereg-style regex, that is a function parameter. Changing that to preg would break any module that would use that function. If we can't remove *all* calls to ereg_*, we can just leave all of them there, we need to hide E_DEPRECATED anyway.

Mike Wacker’s picture

Yeah, the ereg to preg changes cause a lot more code churn and entails a lot more risk. This patch does just enought to ensure PHP 5.3 compatibility and no more.

peterx’s picture

Function _user_categories($account) is passed $account. The following line uses $user. I changed $user to use $account and some error messages disappeared from the themes list.
if (function_exists($function) && ($data = $function('categories', $null, $user, ''))) {

christefano’s picture

I wish I'd seen this before I upgraded a site to a 5.3 RC a few months ago. Looking forward to mysqlnd. Subscribing.

Garrett Albright’s picture

So where does that put D7? Can ereg (and other issues which may cause E_DEPRECATED) be removed from D7? There doesn't seem to be an analogue for this issue for the Drupal 7 branch. Given that one of the big pushes of D7 is its correctness, I would assume that this would be the case.

Dave Reid’s picture

Garrett Albright’s picture

-1 for three-year-old issue threads with misleading/inconspicuous titles. It looks like #64967: Replace ereg with preg is actually the relevant issue as far as ereg/preg is concerned. But I guess what I'm asking is, are there other E_DEPRECATED issues that have been spotted besides the ereg one?

Berdir’s picture

Status: Reviewed & tested by the community » Needs work

As usual, the fixes have been commited to D7 first (in this very issue..), D7 is fully PHP 5.3 compatible.

#70 pointed out a error in this patch, I will re-roll this asap.

Berdir’s picture

Status: Needs work » Needs review
FileSize
6.49 KB

Renamed wrong $user to $account.

MisterSpeed’s picture

5.3 is out and is the recommended standard right now; let's presume that the folks at PHP are as careful with what they do as we are and make sure that we deliver a version of Drupal which works on the currently shipping version of PHP. I have tested Drupal on 5.3 and while a few things break the solutions are not complicated.

On the ereg<->preg issue the translation of an ereg pattern into a preg one is rather straightforward, so instead of changing every upstream call to reformulate patterns as preg let's just transcode the pattern at execution time. If nobody else wants to tackel that one we'll assign someone from the team to do that. PHP 5.3.0 has performance advantages, and our sites will need those soon enough.

mike booth’s picture

PHP 5.3 is now installed by default when you install the Macports PHP5 package on the Mac. It's causing some consternation among my newer coworkers.

Further evidence that we need to figure out how to get Drupal running cleanly on PHP5, whether or not said procedure is actually a good idea in production. Now that the PHP project has declared that PHP 5.3 is "released", it will continue to turn up all over the place and trip up many, many users.

Offtopic but handy: Those who need to install an older version of a Macports PHP5 package should consult this annoying procedure:
http://www.jesuscarrera.info/2009/02/28/downgrade-ruby-version-using-mac...

Damien Tournoud’s picture

Priority: Critical » Normal
Status: Needs review » Needs work
+    $function = $module .'_user';
+    // $null and $user need to be passed by reference.
+    if (function_exists($function) && ($data = $function('categories', $null, $account, ''))) {

^ The comment says $user, the function says $account

+    $function = $module .'_user';
+    // We can't use neither module_invoke nor user_module_invoke because we need the return value and by reference.
+    if (function_exists($function) && ($data = $function($hook, $edit, $account, $category))) {
+      $groups = array_merge($data, $groups);

^ Parse error near "by reference"? ;)

+  //// Only variables can be passed by reference workaround.
+  $null = NULL;
+  user_module_invoke('logout', $null, $user);

^ Four slashes? ;)

Let's wait for a few minor releases of PHP 5.3 to call that "critical", can we?

Damien Tournoud’s picture

Also: this chunk will make the "stable to dev and back" patch fail to apply:

-  if ($errno & (E_ALL)) {
+  if ($errno & (E_ALL ^ E_DEPRECATED)) {

Hopefully, that will make Gabor remember that we want this in stable versions:

-  if ($errno & (E_ALL ^ E_DEPRECATED)) {
+  if ($errno & (E_ALL ^ E_NOTICE ^ E_DEPRECATED)) {
peterx’s picture

#76 is working on PHP 5.3 D6.13 and using book, comment, database logging, help, menu, path, PHP filter, statistics, taxonomy, and update. I switched on aggregator, blog, and blog api and they worked. Can we at least put #76 through so that we have something working for commonly used core modules when the next security release of D6 goes out. We can then test the optional core modules from a stable base.

Berdir’s picture

- I will fix the things pointed out by DamZ soon.. Seems I am unable to produce a proper patch :-/

- I *really* doubt that ereg/preg conversion will happen in D6, atleast not in this patch. The errors are hidden and ereg will not be removed before PHP6, so it should be fairly save for now.

- Core patches are only commited when they are working correctly. There are only some minor improvements left to do I think, it should work fine with all core modules.

Damien Tournoud’s picture

I forgot to say that I'm -1 on any kind of hacks to remove ereg and friends. Those are still perfectly supported (even if deprecated) by PHP 5.3. There is really no point in spending any energy on that for Drupal 6. As noted already, Drupal 7 is already 100% ereg free (in compliance with PHP rules and regulations).

Berdir’s picture

Status: Needs work » Needs review
FileSize
6.43 KB

Re-roll with some comment improvements...

ngr’s picture

I recently got drupal running on my computer, but in Administer, the status report link takes me to a blank page.

When I go to "Reports" I get an error message:

"warning: Parameter 1 to theme_status_report() expected to be a reference, value given in C:\wamp\www\Drupal\includes\theme.inc on line 617."

My question is if with the first path that you attached I can fix it. And the second question is if I can fix the warning with your path, where I have to put it.

thank you

Berdir’s picture

This is fixed with my latest patch. See http://drupal.org/patch/apply on how to apply patches.

johnthomas00’s picture

Subscribe +1
Will the next Drupal-6.x release include this fix?

febbraro’s picture

Subscribe.

FYI the Remi repo for CentOS put PHP 5.3 live over the weekend so now all servers that autoupdate packages will roll to 5.3 automatically and be greeted with a whole slew of "notices", great way to start a week.

eric02138’s picture

Yup - Remi got me, too.

peterx’s picture

One of my test VPSs had 5.3.0 appear in WHM for a couple of days then the ISP removed the option because of the megazillion questions from people who chose to upgrade productions sites to 5.3.0 without testing. Not just Drupal sites. If Drupal 6 was the first major CMS compliant with 5.3.0 then there would be a lot of ISPs recommending an upgrade from X to Drupal.

chx’s picture

Status: Needs review » Reviewed & tested by the community

I think we are good to go although I still think it's absolutely unfair that one hand we need to patch up old Drupals to support bleeding edge (are you nuts to use a .0 PHP release?) but even the newest Drupal can't require the same. My heart yearns for closures.

donquixote’s picture

On my machine I need PHP 5.3, otherwise I get apache crashes caused by php_mysqli.dll

roball’s picture

I also tried

yum --enablerepo remi update php

on a CentOS 5.3 server, but had to revert back to PHP 5.2.10, since Drupal 6.13 didn't work properly on PHP 5.3.0. Looots of problems...

donquixote’s picture

Issue tags: +call by reference

adding "call by reference" tag. We'll get a lot more of this for contributed modules.

chx’s picture

yeah regardless of this getting committed , D6 contrib will be broken on 5.3...

Aleksic’s picture

Hi good people,
I was apply patch but still I`m unable to view edit/account...
Can someone help me please?!
Thanks

donquixote’s picture

@Slobodan (#99):
Noone is saying that Drupal will be 5.3-ready after this patch. It's just one step.

If you really want to go 5.3 and need help, please have a look into your apache error.log (if the error messages are not shown in your browser window) and report as much information as you can get.

A lot of the 5.3 errors in contributed modules can be fixed by patching.

agerson’s picture

Subscribing... Getting the function ereg() is deprecated in drupal \includes\file.inc on line 895 error.

Ammermag’s picture

FYI: Macport does not have a PHP 5.2.x version available. After looking at many blogs and tips, the best way to downgrade my Macbook pro/Mac OS X Leopard back to a PHP 5.2.x was to get version 5.2.9 from http://www.entropy.ch/software/macosx/php/. Installation is easy as it uses a standard Mac package to install. No compile is required. This is good if you are only worried about getting source for the latest Mac OS x.

Garrett Albright’s picture

Supposedly there's a way to install older versions of MacPorts ports using the techniques on this page, and they even use PHP 5.2 in an example at the very bottom of the page. I couldn't get it to work, though; I'm trying to upgrade from PHP 5.2.8 to 5.2.10, but it keeps trying to download the PHP 5.3.0 tarball, then bailing after the hashes mismatch - I guess it's comparing the hashes of the 5.3.0 tarball against 5.2.10's known hashes.

However, so long as you haven't deleted deactivated ports on your machine, it should be fairly easy to downgrade to a previously-installed version of a port. Run port installed php5 and you'll see all the versions of the PHP 5 port you have installed, with one of them marked as active. If you see a previous 5.2 version there you'd like to downgrade to, try something like:

port deactivate php5 @5.3.0_0
port activate php5 @5.2.10_0

You'll want to run it as root, obviously, and the version numbers may differ depending on what you have installed. If all else fails, copy and paste the port and version from the output of port installed.

MacPorts could definitely stand to improve in the realms of ignoring particular port versions and easily installing older versions of ports.

pescetti’s picture

subscribing

Cyberwolf’s picture

subscribing

donquixote’s picture

Funky hack for call_user_func_array (here in theme.inc, line 615)

<?php
  if (isset($info['function'])) {
    // The theme call is a function.
    if (class_exists('ReflectionFunction') && is_string($info['function']) && function_exists($info['function'])) {
      $reflect_function = new ReflectionFunction($info['function']);
      foreach ($reflect_function->getParameters() as $i_param => $param) {
        if ($param->isPassedByReference()) {
          $args[$i_param] = &$args[$i_param];
        }
      }
    }
    $output = call_user_func_array($info['function'], $args);
  }
?>

Anyway, I think the PHP implementation of call_user_func_array stinks.

AkkuDreamz’s picture

any update on this from the drupal team?
when are we going to get a PHP 5.3 compatible drupal.

AdrianB’s picture

Subscribing

XAMPP now ships with 5.3.0 and it's quite popular as a local development platform. Even if you don't use 5.3 on production server you may want to run the same code local using XAMPP.

The problem with D6 and 5.3 has led to issues like this: http://www.apachefriends.org/f/viewtopic.php?f=29&t=36766

BMDan’s picture

To resolve the macports issue, I found a good reference at http://bangpound.org/node/7548

To downgrade a box with RPM as a package manager, download all relevant files into a new directory, cd into it, and run:
rpm -Uvh $(rpm -qa --qf '%{NAME} %{VERSION} %{ARCH}\n' | awk '/^php.* 5\.3/ {print $1"*"$3".rpm"}')

Once you see no errors except for version-downgrade ones, just change the line to read:
rpm -Uvh $(rpm -qa --qf '%{NAME} %{VERSION} %{ARCH}\n' | awk '/^php.* 5\.3/ {print $1"*"$3".rpm"}') --oldpackage

Standard disclaimers: Run this code at your own risk, YMMV, not responsible for missing cats, etc.

Garrett Albright’s picture

Hmm, those steps at the bangpound site seemed to work. Sweet, thanks! I had heard about the local repository solution, but figured it was going to be more difficult than that.

However, the steps about creating a local repository on the Macports site seem to be a little out of date. I added the local repository above the rsync one in the sources.conf file as per the instructions, but it could never find my PHP package when I ran portindex. It seems you also need to move that little [default] tag from after the rsync one to after the local one. So my sources.conf looks like this:

file:///Users/Albright/Documents/ports/ [default]
rsync://rsync.macports.org/release/ports/

After doing that (and downloading and downgrading the PHP package and touching the Portfile), MacPorts was finally able to see the PHP 5.2.10 package after running portindex and I was finally able to sudo port upgrade outdated with impunity. Ah, it's been a while… looks like I'll be compiling all morning!

roball’s picture

If you are on RHEL or CentOS, reverting from 5.3.0 to 5.2.10 worked for me as follows:

Ensure that '/etc/yum.conf' contains the line

showdupesfromrepos=1

Then run

rpm --nodeps -e php php-cli php-common php-gd php-ldap php-mbstring php-mysql php-pdo php-pear php-pecl-pdflib php-xmlrpc
yum --enablerepo remi install php-5.2.10 php-cli-5.2.10 php-common-5.2.10 php-gd-5.2.10 php-ldap-5.2.10 php-mbstring-5.2.10 php-mysql-5.2.10 php-pdo-5.2.10 php-xmlrpc-5.2.10
yum --enablerepo remi install php-pear php-pecl-pdflib-2.1.6
donquixote’s picture

Guys, I would like to propose a more radical and complete solution to the call by reference problem. The currently proposed patches only solve some symptoms.

A typical problem is that different hook implementations can have different by-value / by-reference signatures. This especially applies to the more generic functions like module_invoke_all().

The idea is to introduce a function call_user_func_array_ref, which automatically checks which arguments need to be passed by reference. This function can then be used as a replacement for call_user_func_array.

Here's the code (maybe include it in common.inc, I don't care):

<?php
function call_user_func_array_ref($callback, &$args) {
  if (class_exists('ReflectionFunction')) {
    if (is_string($callback)) {
      $reflect = new ReflectionFunction($callback);
    } else if (is_array($callback)) {
      if (is_object($callback[0])) {
        $reflect = new ReflectionMethod(get_class($callback[0]), $callback[1]);
      } else if (is_string($callback[0]) && class_exists($callback[0])) {
        $reflect = new ReflectionMethod($callback[0], $callback[1]);
      }
    }
    if (!is_object($reflect)) {
      return null;
    }
    $args_new = array();
    $params = $reflect->getParameters();
    // the keys don't need to be sorted..
    foreach (array_keys($args) as $i => $key) {
      if (isset($params[$i]) && $params[$i]->isPassedByReference()) {
        $args_new[$key] = &$args[$key];
      } else {
        $args_new[$key] = $args[$key];
      }
    }
    return call_user_func_array($callback, $args_new);
  } else {
    return call_user_func_array($callback, $args);
  }
}
?>

I tested it with the following little script:

<?php
function foo($x, &$y) {
  $x .= '*';
  $y .= '*';
}

// cufa = call_user_func_array

echo '<pre>                       | cufa_ref   | cufa';

echo "<br/>array('four', 'five')  | ";

$args = array('four', 'five');
call_user_func_array_ref('foo', $args);
echo implode(' ', $args);

echo ' | ';

$args = array('four', 'five');
call_user_func_array('foo', $args);
echo implode(' ', $args);


echo '<br/>array(&$a, &$b)        | ';

$a = 'six';
$b = 'seven';
$args = array(&$a, &$b);
call_user_func_array_ref('foo', $args);
echo "$a $b";

echo ' | ';

$a = 'six';
$b = 'seven';
$args = array(&$a, &$b);
call_user_func_array('foo', $args);
echo "$a $b";

echo '</pre>';
?>

Result (on my system):

                       | cufa_ref   | cufa
array('four', 'five')  | four five* | four five
array(&$a, &$b)        | six seven* | six* seven*

As you can see, call_user_func_array_ref always changes the second argument, not the first one, as implied by the function signature. The native call_user_func_array, on the other hand, does not care at all about the function signature.

Boobaa’s picture

subscribing

joep.hendrix’s picture

subscribing

pwolanin’s picture

@donquixote - this patch is supposed to be for 6.x, so it must to every extent possible maintain the current API. Also - I'm guessing the reflection calls in that code sample would be rather slow? Also, please attach large code samples rather than pasting inline.

lucifurious’s picture

Yeah really. Why not just override __call() and do it in there?

donquixote’s picture

__call is for methods, not functions.

@pwolanin:
Introducing a new function does not change the API, except that it can theoretically cause name clashes with custom or contrib code. But usually that has not been a reason for not introducing a new function.

Now obviously we need to be careful where it is really safe to replace the native call_user_func_array with call_user_func_array_ref(). In some cases, I would say, call_user_func_array_ref is more robust than a custom-made patch, in others it is not. So, we would only use this function in those places where it makes sense: Mostly these will be the places where we don't know the function signature.

Performance: I don't know how costly the reflection stuff is. But if we have many calls to the same function, some caching (remember the signature info in a static array) could help.

Another problem is that the solution has no effect in PHP 4. This doesn't have to be a problem per se, but it means we need to test things in PHP 4 and PHP 5.2 and PHP 5.3.

frankcarey’s picture

#84 working for me as well (can finally access certain pages without white-screens), but still have ereg errors showing up, will try to set error_reporting elsewhere. Thanks Berdir, you're everywhere!

AkkuDreamz’s picture

subscribing

thinkyhead’s picture

PHP 5.3.0 is part of the standard install with Mac OS X 10.6 Snow Leopard, which lots of us are certain to be using for our Drupal development. It would be really great if all the deprecated functions and other errors could be patched in the next release of Drupal 5 and 6. Of course, there are issues with 3rd party modules and themes, including Zen, but most of the errors by far are in Drupal core.

chicagomom’s picture

FYI, @windows.php.net/download/: "If you are using PHP with IIS you should use the VC9 versions of PHP". 5.3 is the first VC9 version.

stewart.adam’s picture

subscribing

GrimSage’s picture

subscribing

jrabeemer’s picture

For those who upgraded to Snow Leopard MacOS 10.6, it's now bundled, by default, with PHP 5.3.

If you plan to upgrade your Mac, you may want to hold off. I get tons of ereg error messages with the latest Acquia Drupal.

Garrett Albright’s picture

Priority: Normal » Critical

Okay, this is getting pathetic. "Wait for D7" isn't going to cut it at this point; we need PHP 5.3 support now. Who knows how many potential new Drupal users are going to unknowingly install the stable version of Drupal on the stable version of PHP, see all the errors, and then start posting on their blogs and mailing lists about what a piece of unusable crap Drupal is? I'm sure it's happened already.

I'll help. What needs to be done to get a D6.14 out the door with full PHP 5.3 compatibility? Code? Money? Sex? Drugs? I'm game. Let's do it.

Berdir’s picture

Test this patch, especially on older versions so that we can be sure that it still works on old, but still supported PHP versions (PHP 4!).

Garrett Albright’s picture

Do you mean the patch in #84? I'm getting a failing hunk in includes/common.inc on line 590. I'm not going to attempt to fix it because, to tell the truth, I don't really get how bitwise operators work well enough to do so in this case (shame, shame). But I'll see what an installation with the other hunks applied can do.

Our company still has a few accounts with our crappy old host who still hasn't upgraded from PHP 4.3 or thereabouts. I'll give it a try there once I get it uploaded (they don't support SSH either - really classy outfit, these guys).

Berdir’s picture

The patch is against the -dev version....

jrabeemer’s picture

/off topic

FYI, if you're on Snow Leopard and need to get some work done in the meantime, you can install the latest MAMP (w/PHP 5.2.6), set your webroot and port 80, copy your databases over and it will work.

I shut down my built-in Apache in the meantime until this patch gets committed.

donquixote’s picture

I don't want to be spamming, but:

For those who need an immediate solution for D6+contrib in PHP 5.3, this is what I did:
- put the reflection-based call_user_func_array_ref (see above) into my bootstrap.inc
- I think you still need to apply the above patch (I did it manually), especially the error reporting flags.
- Every time I see some message complaining about by-reference args in call_user_func_array, I simply change the call_user_func_array call to call_user_func_array_ref. (unless I can find a very obvious alternative solution).

So far this strategy works totally ok. No more by-ref errors appearing. Maybe it has a little cost in performance, but it can't be that much, compared to other bottlenecks.

I'll have to do the patching again when I want to upgrade the respective modules or core. That's the price. But, changing some call_user_func_array to call_user_func_array_ref is manageable.

Berdir’s picture

call_user_func_array is one of the most called functions, especially if you have many modules. And Reflection is really slow, so this probably not just a little performance bottleneck. Just saying....

donquixote’s picture

I wonder why it is slow? The information is all in memory...

http://stackoverflow.com/questions/294582/php-5-reflection-api-performance
sjon_hortensius says he gets +500% when replacing an empty static function call. Which is really not much, if you take into account that an empty function call should be awesome fast, and the reflection call naturally requires more than one function call, with more than one parameter.

But maybe this one would be faster, and more likely to produce the same result in 5.3 (and lower) as call_user_func_array did before 5.3:

<?php
function call_user_func_array_ref($callback, [&]$args) {
  $args_new = array();
  foreach (array_keys($args) as $i => $key) {
    $args_new[$key] = &$args[$key];
  }
  return call_user_func_array($callback, $args_new);
}
?>

EDIT:

I'm not sure if we should have $args to be passed by-ref. It would be faster in many cases, but can have side effects. Passing it by-value would mean one more array copy (arrays can be big in Drupal), but would be more reliable.

I think there should be both versions available.
Or, why not go all the way and provide all three versions? If benchmarks prove the reflection beast to be slow, we could try caching the signature (or the entire $reflect object) in a static array. Cache could be enabled / disabled with a "remember" argument. (just thinking)

The point is:
The native implementation of call_user_func_array sucks. It totally ignores the signature, and allows call-time pass-by-reference even if that is clearly disabled in PHP settings. Having an alternative could be a good thing also for future code.

Garrett Albright’s picture

I'm still working at testing PHP 4 compatibility with the patch. The problem is that a lot of these old crappy hosting accounts have MySQL 4.0.something (yeah, seriously), and I keep getting the error about MySQL 4.1.1 being required when I'm trying to install. I know I've gotten D6 to work with MySQL 4.0 before somehow, but I can't remember how and such hackery would defeat the purpose.

Some of these accounts are supposed to have MySQL 5.something on them, but I'm not sure which ones. I'll keep trying.

Garrett Albright’s picture

Okay, I finally got a patched dev version on to a server with PHP 4.4.4 and MySQL 5.0.26. I enabled all core modules and posted a node with a menu item and a custom path without running into a single error. That's not a very extensive test, I know, but everything seems solid.

Berdir’s picture

If you can, please test image upload with automatic image resizing, as there is a change that needs to be tested. Thanks.

Garrett Albright’s picture

I gave my account a user picture, intentionally selecting an image larger than the maximum allowed resolution. It downscaled it with no problems. I'm not sure where else core resizes images, but if there's another place you want me to check, let me know.

elgreg’s picture

For those also having snow leopard difficulties - the quickest solution for me was to just grab the mac port of PHP 5.2.10 and then change the apache config to point to the old one - that way I can always switch to php 5.3 if I need it. To wit:
In terminal (and with macports already installed - which you probably have to redo if you switched to Snow Leopard and haven't already):
shut down apache

sudo apachectl stop

Then install php5.2.10 with the apache and mysql libs

sudo port install php52 +apache2 +mysql5

Then update my Apple shipped apache httpd.conf file to switch the location of the loaded php module

sudo nano /private/etc/apache2/httpd.conf
# change the LoadModule php5_module        libexec/apache2/libphp5.so
# to
LoadModule php5_module        /opt/local/apache2/modules/libphp5.so

Then restart apache

sudo apachectl start

Hope someone besides me finds this useful. :)

Garrett Albright’s picture

Sweet, it looks like that php52 package is new as of just a few days ago. Thanks for pointing it out, elgreg.

sender’s picture

subscribing

jpdaley’s picture

Thanks elgreg. Your instructions worked perfectly.

I just commented out the original LoadModule line, and added the new one. Now by simply switching which line is commented out I can go back and forth between php5.3 to php5.2. Works like a charm!

One note: I already had a MySql installation going, so I had to reconnect the socket.
My socket was running in /tmp/mysql.sock
I made a new link in the /opt/local/var/run/mysql5 directory to point to my original socket and all worked well.
In terminal make the appropriate directories for /opt/local/var/run/mysql5 using sudo mkdir.
then:
cd /opt/local/var/run/mysql5
sudo ln -s /tmp/mysql.sock mysqld.sock

replace /tmp/mysql.sock with whatever your original socket was.

Thanks again elgreg!

Garrett Albright’s picture

Linking isn't necessary - you should be able to change the location of where the MySQL and MySQLi modules looks for your socket in the php.ini file. MacPorts puts it at /opt/local/etc.php.ini. Look for mysql.default_socket and mysqli.default_socket.

bjcool’s picture

subscribing

elgreg’s picture

Awesome - glad you found it handy. I may just need to figure out why I _didn't_ need to change my socket. :)

Anonymous’s picture

tracking

drubage’s picture

Subscribing... PLEASE a patch for this that actually fixes all problems would be great, PHP 5.3 seems fast so we'd like to take advantage!

toddgeist’s picture

subscribing

Aleksic’s picture

subscribing

XerraX’s picture

need a patch for latest dev

XerraX’s picture

Status: Reviewed & tested by the community » Active
Dave Reid’s picture

Status: Active » Needs work

@XerraX: Please read the handbook page on Issues status values: http://drupal.org/node/156119

Gábor Hojtsy’s picture

I'd really love to get the next Drupal 6 release out with PHP 5.3 support. While you argue about how to build an all-encompassing solution which would solve all of contribs at the same time, we might slip the next Drupal 6 release again not to say the Drupal 5 branch, which would also need these changes. I'd say we should solve the Drupal core issues in core and fix the contrib issues in the respective modules unless we find a core soution which people agree on. The reflection based solution did not seem to catch that many hearts.

Also, I'd kindly ask people to follow up on this issue if they have feedback on the proposed approaches or patches. Needing to cherry-pick the few relevant comments from all the chatter about workarounds and Mac setups does not help the cause too much.

Berdir’s picture

Status: Needs work » Reviewed & tested by the community

The patch in 87 still applies to latest -dev for me? There are some offsets, but it works just fine.

donquixote’s picture

After some time of playing around, I can confirm that the reflection thing is not a desirable solution, even though it does suppress the error messages. Most of the complaints by PHP 5.3 are justified, and should be fixed instead of hiding them away.

Hook implementations should respect the hook signature, and hooks with by-ref parameters should not be called with module_invoke or module_invoke_all.

Another problem in some contributed modules is object-valued by-reference parameters which are called by-value. In PHP 5.x there is no need to have these parameters being by-ref, but in PHP 4.x it is necessary, unfortunately. One example is views, see http://drupal.org/node/569724.

Quite frustrating that we still need to support PHP 4.x.

That said, I agree with #151.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs review

Reviewing the patch in #84 by looking at the code:

1. On the file_save_upload change: not all file validation functions seem to take $file as a reference. Looks like of the ones I found in core, only the resolution validator modifies the data actually (resizes the image), so we do need to support this pass by reference. Although I don't know how it worked in PHP 4 at all, given that it was passed by copy before this patch. I've seen http://api.drupal.org/api/function/file_validate_extensions/6 and http://api.drupal.org/api/function/file_validate_size/6 do not take $file by reference, so would result in a deprecated call time pass by reference error. Maybe that is all cleared off by our error settings, and it's not that big of a deal.

2. The only other change I have review issues with is why array_merge_recursive() becomes array_merge() in user_register_validate(). That seems to be totally unrelated and not reasoned in the patch either.

Berdir’s picture

1. "$array = array(&$param); call_user_func_array($function, $array)" and $function(&$param). The first one does *not* result in a call time pass by reference error, see http://php.net/call_user_func_array.

"Note: Referenced variables in param_arr are passed to the function by a reference, others are passed by a value. In other words, it does not depend on the function signature whether the parameter is passed by a value or by a reference. "

And yes, this is bad....

2. That is probably wrong...

Gábor Hojtsy’s picture

Status: Needs review » Needs work

OK, let's get 2. fixed then.

Berdir’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
6.83 KB

Re-rolled, only change is array_merge to array_merge_recursive.

Gábor Hojtsy’s picture

Version: 6.x-dev » 5.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks, committed to Drupal 6, moving down to Drupal 5.

Berdir’s picture

Great, finally! :)

Uhm, is there really anyone crazy enough to use D5 together with PHP 5.3 ? :)

donquixote’s picture

I guess you don't have much choice if you have different projects running on the same server, some of them with D5, some D6, some wordpress, Zend, whatever, and one of them wants the latest PHP version. And your budget does not allow upgrading to D6 or moving to a different server.

I wonder how many people still use PHP 4.x with D6.. thinking backwards feels odd. It would be so nice to have a Drupal that ditches PHP 4 support, but still works with all the contributed modules for D6.

catch’s picture

@donquixote - that's called Pressflow - it's a not-really-a-fork which has MySQL and PHP5-specific optimizations for D6.

hass’s picture

+

Garrett Albright’s picture

I wonder how many people still use PHP 4.x with D6.

We have a few sites doing this, unfortunately, as our old shared hosting company has never upgraded past 4.4.3 or so. And some of their servers are still running MySQL 5.0, too… It took a while, but I finally convinced my boss that we needed to start ditching those losers.

Anyway, glad to see the patch that this thread produced was accepted in D6.14. Good work, everyone.

andrewfn’s picture

Status: Patch (to be ported) » Needs review
FileSize
8.08 KB

Here is my patch for Drupal 5, rolled against today's dev.
I started with the D6 patch, #157 and manually applied all the patches that were relevant to D5. Then I fixed problems until I got no errors on any of my seven Drupal 5 sites. I might have missed something, but it works pretty well for me.

Dave Reid’s picture

Status: Needs review » Needs work

Contains testing code and unnecessary changes to .info files on initial review.

andrewfn’s picture

Status: Needs work » Needs review
FileSize
5.62 KB

Removed testing code and changes to .info files.

andrewfn’s picture

removed unnecessary extra empty line

thekevinday’s picture

A Drupal 6.14 php 5.3 issue popped up for me with: drupal_retrieve_form

Here is a proposed patch;

diff -Nurp ../drupal-6.14.orig/includes/form.inc ./includes/form.inc
--- ../drupal-6.14.orig/includes/form.inc       2009-12-03 10:11:43.000000000 -0600
+++ ./includes/form.inc 2009-12-03 10:12:16.000000000 -0600
@@ -322,7 +322,7 @@ function drupal_execute($form_id, &$form
  *   these additional arguments and conditionally return different
  *   builder functions as well.
  */
-function drupal_retrieve_form($form_id, &$form_state) {
+function drupal_retrieve_form($form_id, $form_state) {
   static $forms;

   // We save two copies of the incoming arguments: one for modules to use

Please make sure that it is safe for me to remove the & at this location.

Berdir’s picture

No, it's not. You don't need to change the function, you need to fix who is calling the function in a wrong way. See http://api.drupal.org/api/function/drupal_execute/6 for a correct example (The "$args[1] = &$form_state;" part is important).

You can use the following snippet to find out by whom the function is called:

if (is_null($form_state)) {
  debug_print_backtrace();
  die();
}

Place that directly at the top of drupal_retrieve_form and it will show the full backtrace once called incorrectly.

Hint: I've just checked again that all calls to that function are correctly implemented in core so it is either a contrib module or even custum code. If it is a contrib module, open a bug report if there isn't already one.

thekevinday’s picture

thanks, this helps out a lot I will look into finding the correct troublesome module.

thekevinday’s picture

Okay, so after further studying of drupal and the numerous modules I have had to make PHP 5.3 band-aids for, I have identified the following.

The problem seems to happen in the following case:
When call_user_func_array(..) is called, the $args array must get created.
Apparently, with php 5.3 when an $args gets created, one must explicitly set the reference inside of the $args array.

For example:

  $args = array(
    $node_form_id,
    $node_form_state,
    $node
  );

Is passed as the arguments to:

function some_function($id, &$state, $node){
 ..

Because parameter 2 is set as reference by the some_function(..), then the $args must have the second parameter passed as a reference:

  $args = array(
    $node_form_id,
    &$node_form_state,
    $node
  );

Does this sound correct?

If so, then the proper solution is indeed not removing the reference parameter with some_function(..) but instead by finding every instance of call_user_func_array('some_function', $args) and correct the $args array contents.

If some_function(..) never actually needs $state to be a reference, then the band-aid patches are probably simplest fix and are valid, otherwise the band-aid patches are dangerous and should be reverted.
(I am going to point any PHP 5.3 reference threads I participated in to this post.)

gpk’s picture

Have updated the http://drupal.org/requirements page.

VM’s picture

The update on the requirements page may have been made in haste.

PHP 5.3 and Drupal 6.15 seem to continue to throw:
Deprecated: Function ereg() is deprecated in C:\wamp\www\mysite\includes\file.inc on line 902

http://drupal.org/node/689570#comment-2496144

I've confirmed this by trying to install 6.15 on WAMP with PHP 5.3

Berdir’s picture

Unless you've overriden the default drupal error handler, those E_DEPRECATED errors shouldn't be displayed, no idea why they would show up for you.

VM’s picture

I didn't override anything. It's a fresh install of both WAMP and Drupal 6.15.

The comment I liked to is also dated today. where the user installed WAMP then installed Drupal 6.15 and recieved the same using PHP 5.3 that ships with WAMP. I suppose it could be WAMP specific. Off to Ubuntu to test there.

Re-test of php53_warnings.patch from comment @comment was requested by matt.nz.

jrwjrw’s picture

I'm seeing the same problem with a fresh 6.15 install on Solaris AMP using PHP 5.3.1. This is annoying since the Drupal Installation Requirements states that Drupal 6.14 and higher support PHP 5.3.

Berdir’s picture

Ah I see the issue. It seems that the function that does use ereg() is called before the error handler from Drupal is set. You can "fix" it by disabling E_DEPRECATED errors in your php.ini or in a htaccess file.

Example:

error_reporting  =  E_ALL ^ E_DEPRECATED
sepla’s picture

@Berdir:
I really appreciate your (& the community) efforts on making this compatible. tones of thanks ;) subscribed!

Xano’s picture

Subscribing.

@ Berdir in #214 and #219: I can confirm this is a problem. There is a user on our local Drupal community site that has the same problem using PHP 5.3. Is the patch from #157 indeed the last one made for D6? I'll see if I can convert ereg() to preg_match().

gpk’s picture

A temporary fix for sites running 5.3 might be to put

// Temporary workaround for PHP 5.3 incompatibility - see http://drupal.org/node/360605.
error_reporting(E_ALL ^ E_DEPRECATED);

in settings.php.

The workaround in #219 will work on many sites but not for some sites running PHP as CGI on shared hosts.

A more permanent solution might be to move the conditional declaration of E_DEPRECATED from common.inc to bootstrap.inc and also early in the bootstrap to insert error_reporting(E_ALL ^ E_DEPRECATED);. This could probably go directly in http://api.drupal.org/api/function/_drupal_bootstrap/6 in the DRUPAL_BOOTSTRAP_CONFIGURATION phase, immediately after the timer_start(). The test for E_DEPRECATED in drupal_error_handler could probably then be removed.

A problem I can see - this does mean you can't change error_reporting in .htaccess or php.ini since it will be modified during the bootstrap. Is this an issue? If so then perhaps a simple error handler is needed just for the bootstrap that filters out the E_DEPRECATEDs, until we are fully bootstrapped and the main error handler is available. That could be set with set_error_handler('bootstrap_error_handler', E_DEPRECATED) (see http://php.net/set_error_handler).

The other option would be to set the full Drupal error handler early in bootstrap, which would also enable error handing during boostrap .. though in view of the iterations needed in #325169: Move error/exception handler higher up in the bootstrap process adding something so featureish at this stage to 6.x is probably an extremely bad idea.

Berdir’s picture

- set_error_handler('bootstrap_error_handler', E_DEPRECATED) -> That's PHP 5 only, so not possible

- Agree, there is no way to backport 325169 to Drupal 6

- We could do something like this in bootstrap.inc

// Hide E_DEPRECATED messages.
if (defined('E_DEPRECATED')) {
  error_reporting(error_reporting() ^ E_DEPRECATED);
}

That would remove E_DEPRECATED from the error reporting setting but keep everything else as it has been set before.

We can leave the definition of E_DEPRECATED and error handling as it is with that change, I think.

gpk’s picture

Ah yes, that looks good.

> leave the definition of E_DEPRECATED and error handling as it is
I was forgetting that error_reporting only affects the built-in PHP error handling and that the custom error handler is called anyway regardless of the error_reporting setting. Well, that's my latest understanding - I find the docs could be a bit clearer on this :P

Berdir’s picture

Status: Needs work » Needs review
FileSize
684 bytes

The attached patch works correctly for me and hides the early DEPRECATED messages.

Aleksic’s picture

I apply patch but flowing errors array:
Deprecated: Function ereg() is deprecated in /users/drupal/www/includes/file.inc on line 902

hass’s picture

tacituseu’s picture

Another Drupal 5.x specific problem is usage of parse_ini_file() and lack of double quotes in info files (see http://drupal.org/node/722430 and http://www.php.net/manual/en/function.parse-ini-file.php#93148).

fizk’s picture

Can someone please fix includes/files.inc: 911

Before:
elseif ($depth >= $min_depth && ereg($mask, $file)) {
After:
elseif ($depth >= $min_depth && preg_match('/' . $mask . '/', $file)) {

Berdir’s picture

That *can not* be fixed, because $mask is a function parameter and changing ereg to preg_match would break compatibility.

fizk’s picture

From looking at how core 6.x and contrib modules actually use file_scan_directory(...), the only issue with converting ereg to preg_match would be the "/" character.

So we could add something like

elseif ($depth >= $min_depth && preg_match('/' . escapeSlashes($mask) . '/', $file)) { 

...
...
function escapeSlashes($s)
{
    return str_replace('/', '\/', $s);
}
carlmcdade’s picture

The / is only used as a delimiter. Change the delimeter to something more esoteric and there's no problem. I was informed that this is a duplicate of http://drupal.org/node/530940 where there is a working patch that should meet the unit test. But it does not pass due to some flaw in the test itself probably since use of preg_match on the data type string in $mask returns the proper booleans.

Heine’s picture

This issue concerns a bug in Drupal. Please do not post support requests in this issue. While I understand the problems you are wrestling with, questions on how to patch, and questions about unrelated problems interfere with getting the fix in a future release.

If your followup isn't about a patch review, proposal or testresult of this issue, please do not post here, but see http://drupal.org/support for your support options instead.

I've unpublished the offtopic comments.

Heine’s picture

Version: 6.16 » 6.x-dev

Summary of the issue so far:

- A lot of fixes regarding references went into Drupal core
- Contrib modules may still have issues with PHP 5.3 - please file such issues in the issue queue of the offending module - followups about non-core modules will be removed.

Fresh core installation sometimes throw E_DEPRECATED warnings on ereg. The patch on #360605-233: PHP 5.3 Compatibility aims to solve this.

@ #259, #260 - please respond to #68 's.

$mask is a ereg-style regex, that is a function parameter. Changing that to preg would break any module that would use that function. If we can't remove *all* calls to ereg_*, we can just leave all of them there, we need to hide E_DEPRECATED anyway.

EvanDonovan’s picture

Heine: That sounds reasonable. Hopefully, I can review the patch in #233 soon. For PHP 5.3 compatibility with contrib modules, I might create a wiki page on Groups.Drupal.org, so that it can be tracked in a table.

Heine’s picture

Patch in #360605-233: PHP 5.3 Compatibility suppressed the E_DEPRECATED errors on a test installation (with error_reporting = E_ALL | E_STRICT). This install was unusable before the patch.

The patch could use additional comments (why here, why the define check).

@Aleksic, was the patch correctly applied? Where did you see the errors after the patch?

Heine’s picture

Apparantly, another summary is in order:

Contrib modules may still have issues with PHP 5.3 - please file such issues in the issue queue of the offending module - followups about non-core modules will be removed.

Drupal 6.16 itself has (AFAICT) one remaining issue on PHP 5.3 where E_DEPRECATED errors are thrown on ereg in file_scan_directory. The patch in #360605-233: PHP 5.3 Compatibility aims to solve this.

Any followup not related to fixing this issue, will be removed.

Berdir’s picture

There is another bug, see #587568: Parameter 1 to comment_nodeapi() expected to be a reference module.inc, line 450 (php 5.3 compatibility). Patch there is basically just waiting to be RTBC'd...

NancyDru’s picture

subscribing

calebgilbert’s picture

Version: 6.x-dev » 5.x-dev

Reading the backscroll I noticed someone who was merely subscribing to this issue (annoyingly) set it from 5.x-dev to 6.x-dev (see comment #227). Gabor had changed this to 5.x-dev after committing a patch to the 6.x branch (comment #158), so I am changing the issue back to the correct/intended version number.

Heine’s picture

Version: 5.x-dev » 6.x-dev

The ereg deprecated error _is_ still an issue on Drupal 6.x-dev.

Damien Tournoud’s picture

@Heine: our error handler ignores E_DEPRECATED errors. The only way you can get those errors currently is by (1) having E_DEPRECATED enabled in php.ini (bad idea number 1), and (2) having display_errors enabled (bad idea number 2).

Berdir patch in #233 would fix this corner case, but it feels like an unnecessary protection against a server mis-configuration to me.

Damien Tournoud’s picture

Status: Needs review » Reviewed & tested by the community

That said, given the impact on Drupal's image of those silly warnings, especially to new comers evaluating Drupal on WAMP distributions shipping with PHP 5.3, let's get #233 in.

Berdir’s picture

Thanks for RTBC, yeah, it is not necessary but on many shared hosts, users probably don't have access to change display_errors/error_reporting setting (or they don't know how) so with the patch in #233 it should just work in these cases.

Heine’s picture

@Damien, thanks for the RTBC. What I get from reading the forum is that all these bad ideas happen, not just on localhost/wamp, but also on shared hosts.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Great, committed #233, marking as fixed.

RobLoach’s picture

Version: 6.x-dev » 5.x-dev
Priority: Critical » Normal
Status: Fixed » Active

Just noticed that Drupal 5 does not work on PHP 5.3 either....

    * : Function ereg() is deprecated in /home/mati/apache/htdocs/drupal5/includes/file.inc on line 646.
    * : Function ereg() is deprecated in /home/mati/apache/htdocs/drupal5/includes/file.inc on line 646.
    * : Function ereg() is deprecated in /home/mati/apache/htdocs/drupal5/includes/file.inc on line 646.
    * : Function ereg() is deprecated in /home/mati/apache/htdocs/drupal5/includes/file.inc on line 646.
    * warning: syntax error, unexpected BOOL_TRUE in modules/comment/comment.info on line 3 in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 195.
    * warning: syntax error, unexpected BOOL_TRUE in modules/drupal/drupal.info on line 3 in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 195.
    * warning: syntax error, unexpected BOOL_TRUE in modules/node/node.info on line 3 in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 195.
    * warning: syntax error, unexpected BOOL_TRUE in modules/poll/poll.info on line 3 in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 195.

Also getting the following which breaks Drupal 5 completely on PHP 5.3...

    * warning: Parameter 3 to block_user() expected to be a reference, value given in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 386.
    * warning: Parameter 3 to comment_user() expected to be a reference, value given in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 386.
    * warning: Parameter 2 to node_user() expected to be a reference, value given in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 386.
    * warning: Parameter 3 to system_user() expected to be a reference, value given in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 386.
    * warning: Parameter 2 to user_user() expected to be a reference, value given in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 386.
    * warning: Parameter 2 to watchdog_user() expected to be a reference, value given in /home/mati/apache/htdocs/drupal5/includes/module.inc on line 386.
Crell’s picture

Version: 5.x-dev » 6.x-dev
Priority: Normal » Critical
Status: Active » Fixed

I don't know if we're going to be doing D5 PHP 5.3 compatibility work. That's up to Neil. But for the love of Druplicon can we make that a new thread before this one gets to a second page?

RobLoach’s picture

onyxnz’s picture

Guys, I hope I am not just adding to the clutter here...but PLEASE tell me I have no need to panic...
http://www.php.net/archive/2010.php#id2010-07-22-1
PHP drops 5.2x branch support, urges 5.3 upgrade pronto.
I checked with my host (Downtownhost.com), and they will roll out 5.3 very soon as part of standard upgrade procedure within cPanel...which just happens to be the same software every cat and their dog in Web hosting land uses...meaning much of the shared hosting sites are about to have a problem with D6+D5 all over teh interwebs.

...ok, panic now....

EvanDonovan’s picture

@Onyx: if you are on 6.17 (or even 6.16, I think), no need to panic. I think that #853064: Alter INSTALL.txt to be clear PHP 5.3 is not compatible with Drupal 5 for 5.x should be resolved soon, but in light of D7's impending release, people should be preparing to upgrade from D5 anyway.

Can someone lock this thread?

Crell’s picture

Further PHP 5.3 issues should be resolved either in the Drupal 5 thread mentioned above or in individual issues for Drupal 6, Drupal 7, or contrib modules as appropriate. Locking this thread for posterity.

Status: Fixed » Closed (fixed)
Issue tags: -PHP, -PHP 5.3, -call by reference, -ereg deprecation

Automatically closed -- issue fixed for 2 weeks with no activity.