I wanna complain that the function drush_error_handler() logs every errors and notices when error_reporting() is not 0.

I think it should pay respect to the value of error_reporting().

Or for a quick fix, just don't log E_DEPRECATED errors. Because when you install Drupal 6 sites with Aegir, it sucks too much time, CPU and memory and can break the install.

CommentFileSizeAuthor
#6 1120848_drush_error_handler_1.patch1010 bytesmakara
#3 1120848_drush_error_handler.patch777 bytesmakara
#1 error_handler.diff516 bytesmoshe weitzman

Comments

moshe weitzman’s picture

moshe weitzman commented
Version: All-versions-4.x-dev »
Status: Active » Reviewed & tested by the community
StatusFileSize
new error_handler.diff516 bytes

I'm seeing these now too, possibly since we started using the drush error handler instead of the drupal error handler. Attached is a proposed fix for supressing DEPRECATED errors. Does it look OK to folks? boolean operators are not my forte.

Patch is against 4.x. Should apply fine to both.

moshe weitzman’s picture

moshe weitzman commented
Status: Reviewed & tested by the community » Fixed

Committed slight different fix to 5 and 4 branches. Now we match drupal core with if ($errno & (E_ALL ^ E_DEPRECATED)) {

Sorry I didn't figure out what the problem was until after the 4.5 release.

makara’s picture

makara commented
Status: Fixed » Needs review
StatusFileSize
new 1120848_drush_error_handler.patch777 bytes

E_DEPRECATED was added in PHP 5.3.

Drupal 6 is using

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

And Drupal 7's error handler is like

if ($error_level & error_reporting()) {

I'd suggest both. See the patch.

bibo’s picture

bibo commented

Running the latest drush dev version seems to generate this error (on my PHP 5.2.12 / Zend Server / Mac OS):

PHP Notice: Use of undefined constant E_DEPRECATED - assumed 'E_DEPRECATED' in /Users/ilmari/drush-HEAD/includes/environment.inc on line 34

makara's patch in the previous comment probably fixes this error.

moshe weitzman’s picture

moshe weitzman commented

The comment you removed was helpful IMO. lets also add a comment like 'drush suppresses deprecated errors, just as drupal does.'

makara’s picture

makara commented
StatusFileSize
new 1120848_drush_error_handler_1.patch1010 bytes

I don't speak good English. Please check it and feel free to correct me.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Fixed

Committed to 4 and 5. Thanks.

msonnabaum’s picture

msonnabaum commented
Status: Fixed » Needs work

Both branch's tests are failing with this patch. No time to look into it at the moment.

msonnabaum’s picture

msonnabaum commented
Status: Needs work » Fixed

Both passing now. False alarm perhaps.

anarcat’s picture

anarcat commented

I thought this was fixed in 5.x - i was asking for a merge for another fix like this one in #1129332: drush doesn't respect existing error_reporting settings. I am surprised this was broken in 5.x... But for sanity's sake, we should probably merge the other patches too...

Status: Fixed » Closed (fixed)

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

blueprint’s picture

blueprint commented

Hejo a note from an aegir user:

PHP 5.3.3-7+squeeze3 with Suhosin-Patch (cli) (built: Jun 28 2011 08:24:40)
Copyright (c) 1997-2009 The PHP Group
Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
with XCache v1.3.0, Copyright (c) 2005-2009, by mOo
with Suhosin v0.9.32.1, Copyright (c) 2007-2010, by SektionEins GmbH

Aegir 1.4 (from the kuombit debian repos 1.4)
Drush 4.5 (from squeeze-backports)

... without the patch /usr/share/drush/includes/environment.inc (in #6) aegir was really flaking out. After patching, one could install/delete etc (was functional with 3.x drush?!!!!)

I'm a bit confused by this, since version 4.4 on other Debian squeeze hosts with Aegir 1.3 did not have this kind of problem ... hmmm

chrisschaub’s picture

chrisschaub commented

Just to add a note in case anyone finds this. If you are having issues with php 5.3.2 and Aegir/Drush provisioning drupal 6 sites, this patch in #6 might fix the issue -- especially if it looks like a memory issue. I seems that these php 5.3 warnings can cause issues with Drush and Aegir finishing a site install.

blueprint’s picture

blueprint commented

Just confirming (as in #12) that #13 works.