Skip to content

Conversation

@AndrewKostka
Copy link
Contributor

No description provided.

array_push( $out, $line );
}

while( $line = fgets( $stderrProc ) ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a little unclear to me if this means that we end up batching up all the internal stdout lines and output each of those to the main stdout (and also store in $out) before only then addressing the stderr batch.

e.g. if we have a command that runs for an hour and the first thing to happen is something on STDERR would we see nothing at all until STDOUT has got to the end of the stream. I suspect we might find this a little misleading if it's the case.

My attempts to test this but putting something like:

                $stdout = fopen( 'php://stderr', 'w');
		fwrite( $stdout, “hello, world\n”);
		fclose($stdout);

in update.php simply resulted in having no output of at all from the inner script

Copy link
Contributor

@deer-wmde deer-wmde Apr 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sadly only can confirm your assumption. I tried to find a way around it but not successfully. For example, I wrapped the two while() loops in a while(proc_get_status( $proc ) )[ 'running' ]) and turned the while($line = fgets( ... into if statements. And while that alternates between writing stdout and stderr output, it doesn't make sure that you get everything in the right order and (even worse) that you get every line at all.

$mwPath = realpath( __DIR__ . '/../../../' );
$cmd = 'WBS_DOMAIN=' . $GLOBALS[WBSTACK_INFO_GLOBAL]->requestDomain . ' php ' . $mwPath . '/maintenance/update.php --quick';
exec($cmd, $out, $return);
$cmd = 'WBS_DOMAIN=' . $domain . ' php ' . $mwPath . '/maintenance/update.php --quick';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be worth trying if we can have the command itself redirect stderr to stdout like this:

$cmd = 'WBS_DOMAIN=' . $domain . ' php ' . $mwPath . '/maintenance/update.php --quick 2>&1';

which in case it does work could save us a few hoops to jump through in here.

@dati18 dati18 force-pushed the pipe-update-to-stdout branch from 4747ff3 to 895ba21 Compare November 6, 2025 13:10
exec($cmd, $out, $return);
$cmd = 'WBS_DOMAIN=' . $domain . ' php ' . $mwPath . '/maintenance/update.php --quick 2>&1';

$stdout = fopen( 'php://stdout', 'w' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is opened, but never closed. I suggest adding fclose($stdout) at the end to close this.

class PreApiWbStackUpdate {
public function execute() {
@set_time_limit( 60*60 ); // 60 mins maybe D:
@ini_set( 'memory_limit', '-1' ); // also try to disable the memory limit? Is this even a good idea?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using @ to suppress errors might hide important diagnostic errors. Can we try something like this:

if ( !set_time_limit( 60*60 ) ) {
			fwrite( STDERR, "Warning: Unable to set time limit.\n" );
}
if ( ini_set( 'memory_limit', '-1' ) === false ) {
			fwrite( STDOUT, "Warning: Unable to set memory_limit to -1\n" );
} 

fwrite( $stdout, "DOMAIN: " . $domain . "\n" );

$spec = [ [ 'pipe', 'r' ], [ 'pipe', 'w' ],[ 'pipe', 'w' ] ];
$proc = proc_open( $cmd, $spec, $pipes );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: Should we check if proc_open is executed successfully?

if ($proc === false) {
  fwrite($stdout, "Error: Failed to start process \n");
} else{
...
}


$out = [];

$pid = ( proc_get_status( $proc ) )[ 'pid' ];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, if $proc returns false then this will fail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants