r103179 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r103178‎ | r103179 | r103180 >
Date:15:04, 15 November 2011
Author:demon
Status:resolved (Comments)
Tags:
Comment:
Address fixme on r101644 (bug 32325, bug 32263), originally for bug 31822. PHP 5.2 below 5.2.7 throws a warning when you try to fopen() in append mode. Fixed by only fwrite()ing to STDIN|STDERR when in cli, use print otherwise (per Tim's suggestion).

People didn't seem to like the idea of bumping the minimum version to 5.2.7 since some distros like being behind the times.
Modified paths:
  • /trunk/phase3/maintenance/Maintenance.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/Maintenance.php
@@ -309,10 +309,11 @@
310310 }
311311 if ( $channel === null ) {
312312 $this->cleanupChanneled();
313 -
314 - $f = fopen( 'php://stdout', 'a' );
315 - fwrite( $f, $out );
316 - fclose( $f );
 313+ if( php_sapi_name() == 'cli' ) {
 314+ fwrite( STDOUT, $out );
 315+ } else {
 316+ print( $out );
 317+ }
317318 }
318319 else {
319320 $out = preg_replace( '/\n\z/', '', $out );
@@ -331,9 +332,7 @@
332333 if ( php_sapi_name() == 'cli' ) {
333334 fwrite( STDERR, $err . "\n" );
334335 } else {
335 - $f = fopen( 'php://stderr', 'a' );
336 - fwrite( $f, $err . "\n" );
337 - fclose( $f );
 336+ print $err;
338337 }
339338 $die = intval( $die );
340339 if ( $die > 0 ) {
@@ -349,9 +348,11 @@
350349 */
351350 public function cleanupChanneled() {
352351 if ( !$this->atLineStart ) {
353 - $handle = fopen( 'php://stdout', 'w' );
354 - fwrite( $handle, "\n" );
355 - fclose( $handle );
 352+ if( php_sapi_name() == 'cli' ) {
 353+ fwrite( STDOUT, "\n" );
 354+ } else {
 355+ print "\n";
 356+ }
356357 $this->atLineStart = true;
357358 }
358359 }
@@ -370,25 +371,34 @@
371372 return;
372373 }
373374
374 - $handle = fopen( 'php://stdout', 'a' );
 375+ $cli = php_sapi_name() == 'cli';
375376
376377 // End the current line if necessary
377378 if ( !$this->atLineStart && $channel !== $this->lastChannel ) {
378 - fwrite( $handle, "\n" );
 379+ if( $cli ) {
 380+ fwrite( STDOUT, "\n" );
 381+ } else {
 382+ print "\n";
 383+ }
379384 }
380385
381 - fwrite( $handle, $msg );
 386+ if( $cli ) {
 387+ fwrite( STDOUT, $msg );
 388+ } else {
 389+ print $msg;
 390+ }
382391
383392 $this->atLineStart = false;
384393 if ( $channel === null ) {
385394 // For unchanneled messages, output trailing newline immediately
386 - fwrite( $handle, "\n" );
 395+ if( $handle ) {
 396+ fwrite( STDOUT, "\n" );
 397+ } else {
 398+ print "\n";
 399+ }
387400 $this->atLineStart = true;
388401 }
389402 $this->lastChannel = $channel;
390 -
391 - // Cleanup handle
392 - fclose( $handle );
393403 }
394404
395405 /**

Follow-up revisions

RevisionCommit summaryAuthorDate
r103188Follow-up r103179: $handled variable undefinedplatonides16:27, 15 November 2011
r103196MFT r101644, 103179. Release notes seem to be already mergeddemon17:26, 15 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r101644(bug 31822) Error during upgrade due to output buffer reset in stdout....demon18:03, 2 November 2011

Comments

#Comment by G.Hagedorn (talk | contribs)   15:59, 15 November 2011

Please consider tagging for 1.18, see r101644

#Comment by Reedy (talk | contribs)   16:02, 15 November 2011

That revision isn't part of 1.18, isn't actually merged, or ever been tagged for 1.18... Though it adds to the 1.18 release notes, which suggest it should be

#Comment by G.Hagedorn (talk | contribs)   16:13, 15 November 2011

Apologies, you are right, I did not check the revision properly enough, just saw a 1.18 :-)! I also wrongly remembered having it in the 1.18 branch, but actually I reported Bug 32325 on the testing branch 1.19svn (I tried to read carefully this time).

Status & tagging log