From e93a6e9d742be62877870a6b9e912b42d7683ab7 Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Sat, 16 Dec 2023 08:10:38 -0500 Subject: [PATCH 1/6] Remove shutdown handling from Drush --- drush.php | 2 +- src/Commands/core/CliCommands.php | 5 -- src/Runtime/DependencyInjection.php | 3 +- src/Runtime/RedispatchHook.php | 1 - src/Runtime/Runtime.php | 28 +-------- src/Runtime/ShutdownHandler.php | 60 ------------------- .../ShutdownAndErrorHandlerTest.php | 22 +++---- 7 files changed, 12 insertions(+), 109 deletions(-) delete mode 100644 src/Runtime/ShutdownHandler.php diff --git a/drush.php b/drush.php index b6cb544bea..cd00e708c8 100755 --- a/drush.php +++ b/drush.php @@ -134,7 +134,7 @@ // Preflight and run $preflight = new Preflight($environment); $di = new DependencyInjection(); -$di->desiredHandlers(['errorHandler', 'shutdownHandler']); +$di->desiredHandlers(['errorHandler']); $runtime = new Runtime($preflight, $di); $status_code = $runtime->run($_SERVER['argv']); diff --git a/src/Commands/core/CliCommands.php b/src/Commands/core/CliCommands.php index e08c47fb09..9563a1cdad 100644 --- a/src/Commands/core/CliCommands.php +++ b/src/Commands/core/CliCommands.php @@ -96,11 +96,6 @@ public function cli(array $options = ['version-history' => false, 'cwd' => self: $this->makeEntitiesAvailableWithShortClassNames(); - // PsySH will never return control to us, but our shutdown handler will still - // run after the user presses ^D. Mark this command as completed to avoid a - // spurious error message. - Runtime::setCompleted(); - // Run the terminate event before the shell is run. Otherwise, if the shell // is forking processes (the default), any child processes will close the // database connection when they are killed. So when we return back to the diff --git a/src/Runtime/DependencyInjection.php b/src/Runtime/DependencyInjection.php index 65cb9e5219..4743dc359c 100644 --- a/src/Runtime/DependencyInjection.php +++ b/src/Runtime/DependencyInjection.php @@ -146,9 +146,8 @@ protected function addDrushServices($container, ClassLoader $loader, DrushDrupal ->addMethodCall('addSearchLocation', ['CommandFiles']) ->addMethodCall('setSearchPattern', ['#.*(Commands|CommandFile).php$#']); - // Error and Shutdown handlers + // Error handler Robo::addShared($container, 'errorHandler', 'Drush\Runtime\ErrorHandler'); - Robo::addShared($container, 'shutdownHandler', 'Drush\Runtime\ShutdownHandler'); // Add inflectors. @see \Drush\Boot\BaseBoot::inflect $container->inflector(SiteAliasManagerAwareInterface::class) diff --git a/src/Runtime/RedispatchHook.php b/src/Runtime/RedispatchHook.php index 335368ca37..35e3ec4a2b 100644 --- a/src/Runtime/RedispatchHook.php +++ b/src/Runtime/RedispatchHook.php @@ -124,7 +124,6 @@ protected function exitEarly(int $exit_code): never // Note that RemoteCommandProxy::execute() is expecting that // the redispatch() method will not return, so that will need // to be altered if this behavior is changed. - Runtime::setCompleted(); exit($exit_code); } } diff --git a/src/Runtime/Runtime.php b/src/Runtime/Runtime.php index 288a9cff04..9977ddf92a 100644 --- a/src/Runtime/Runtime.php +++ b/src/Runtime/Runtime.php @@ -4,6 +4,7 @@ namespace Drush\Runtime; +use JetBrains\PhpStorm\Deprecated; use Symfony\Component\Console\Output\ConsoleOutput; use Drush\Application; use Drush\Commands\DrushCommands; @@ -109,40 +110,15 @@ protected function doRun($argv, $output): int // Bootstrap: bootstrap site to the level requested by the command (via a 'post-init' hook) $status = $application->run($input, $output); - // Placate the Drush shutdown handler. - Runtime::setCompleted(); - Runtime::setExitCode($status); - return $status; } /** * Mark the current request as having completed successfully. */ + #[Deprecated("Shutdown handling removed from Drush. Please remove the call to Runtime::setCompleted()")] public static function setCompleted(): void { Drush::config()->set(self::DRUSH_RUNTIME_COMPLETED_NAMESPACE, true); } - - /** - * Mark the exit code for current request. - * - * @deprecated - * Was used by backend.inc - */ - public static function setExitCode(int $code): void - { - Drush::config()->set(self::DRUSH_RUNTIME_EXIT_CODE_NAMESPACE, $code); - } - - /** - * Get the exit code for current request. - * - * @deprecated - * Was used by backend.inc - */ - public static function exitCode() - { - return Drush::config()->get(self::DRUSH_RUNTIME_EXIT_CODE_NAMESPACE, 0); - } } diff --git a/src/Runtime/ShutdownHandler.php b/src/Runtime/ShutdownHandler.php deleted file mode 100644 index 8e0494ba4c..0000000000 --- a/src/Runtime/ShutdownHandler.php +++ /dev/null @@ -1,60 +0,0 @@ -get(Runtime::DRUSH_RUNTIME_COMPLETED_NAMESPACE)) { - Drush::logger()->warning('Drush command terminated abnormally.'); - // Make sure that we will return an error code when we exit, - // even if the code that got us here did not. - if (!Runtime::exitCode()) { - Runtime::setExitCode(DrushCommands::EXIT_FAILURE); - } - } - - // This way returnStatus() will always be the last shutdown function (unless other shutdown functions register shutdown functions...) - // and won't prevent other registered shutdown functions (IE from numerous cron methods) from running by calling exit() before they get a chance. - register_shutdown_function([$this, 'returnStatus']); - } - - /** - * @deprecated. This function will be removed in Drush 10. Throw an exception to indicate an error. - */ - public function returnStatus(): never - { - exit(Runtime::exitCode()); - } -} diff --git a/tests/functional/ShutdownAndErrorHandlerTest.php b/tests/functional/ShutdownAndErrorHandlerTest.php index e869fe67db..0bcba1b28d 100644 --- a/tests/functional/ShutdownAndErrorHandlerTest.php +++ b/tests/functional/ShutdownAndErrorHandlerTest.php @@ -20,10 +20,10 @@ class ShutdownAndErrorHandlerTest extends CommandUnishTestCase */ public function testShutdownFunctionAbruptExit() { - // Run some garbage php with a syntax error. - $this->drush(PhpCommands::EVAL, ['exit(0);'], [], null, null, DrushCommands::EXIT_FAILURE); - - $this->assertStringContainsString("Drush command terminated abnormally.", $this->getErrorOutput(), 'Error handler did not log a message.'); + // Run some garbage php with a syntax error and assert correct exit code. + $this->drush(PhpCommands::EVAL, ['exit(1);'], [], null, null, DrushCommands::EXIT_FAILURE); + // Placate phpunit. If above succeeds we are done here. + $this->addToAssertionCount(1); } /** @@ -46,9 +46,9 @@ public function testShutdownFunctionExitCodePassedThrough() public function testShutdownFunctionPHPError() { // Run some garbage php with a syntax error. - $this->drush(PhpCommands::EVAL, ['\Drush\Drush::setContainer("string is the wrong type to pass here");'], [], null, null, PHP_MAJOR_VERSION == 5 ? 255 : DrushCommands::EXIT_FAILURE); - - $this->assertStringContainsString("Drush command terminated abnormally.", $this->getErrorOutput(), 'Error handler did not log a message.'); + $this->drush(PhpCommands::EVAL, ['\Drush\Drush::setContainer("string is the wrong type to pass here");'], [], null, null, DrushCommands::EXIT_FAILURE); + // Placate phpunit. If above succeeds we are done here. + $this->addToAssertionCount(1); } /** @@ -58,12 +58,6 @@ public function testErrorHandler() { // Access a missing array element $this->drush(PhpCommands::EVAL, ['$a = []; print $a["b"];']); - - if (empty($this->logLevel()) && PHP_MAJOR_VERSION <= 7) { - $this->assertEquals('', $this->getErrorOutput(), 'Error handler did not suppress deprecated message.'); - } else { - $msg = PHP_MAJOR_VERSION >= 8 ? 'Undefined array key "b" PhpCommands.php' : 'Undefined index: b PhpCommands.php'; - $this->assertStringContainsString($msg, $this->getErrorOutput()); - } + $this->assertStringContainsString('Undefined array key "b" PhpCommands.php', $this->getErrorOutput()); } } From 1d32e0a753646d3c30a0563a019da6076d1095ec Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Sat, 16 Dec 2023 08:15:18 -0500 Subject: [PATCH 2/6] Reomve constants --- src/Runtime/Runtime.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/Runtime/Runtime.php b/src/Runtime/Runtime.php index 9977ddf92a..f954312b86 100644 --- a/src/Runtime/Runtime.php +++ b/src/Runtime/Runtime.php @@ -22,9 +22,6 @@ */ class Runtime { - const DRUSH_RUNTIME_COMPLETED_NAMESPACE = 'runtime.execution.completed'; - const DRUSH_RUNTIME_EXIT_CODE_NAMESPACE = 'runtime.exit_code'; - public function __construct(protected Preflight $preflight, protected DependencyInjection $di) { } @@ -119,6 +116,6 @@ protected function doRun($argv, $output): int #[Deprecated("Shutdown handling removed from Drush. Please remove the call to Runtime::setCompleted()")] public static function setCompleted(): void { - Drush::config()->set(self::DRUSH_RUNTIME_COMPLETED_NAMESPACE, true); + // Do nothing. } } From d40fa55ad5c8fc5d3a5537935cd586a720de7ec6 Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Tue, 19 Dec 2023 12:21:39 -0500 Subject: [PATCH 3/6] Revert a few pieces per PR discussion --- src/Commands/core/CliCommands.php | 5 +++++ src/Runtime/RedispatchHook.php | 1 + src/Runtime/Runtime.php | 8 +++++++- 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Commands/core/CliCommands.php b/src/Commands/core/CliCommands.php index 9563a1cdad..e08c47fb09 100644 --- a/src/Commands/core/CliCommands.php +++ b/src/Commands/core/CliCommands.php @@ -96,6 +96,11 @@ public function cli(array $options = ['version-history' => false, 'cwd' => self: $this->makeEntitiesAvailableWithShortClassNames(); + // PsySH will never return control to us, but our shutdown handler will still + // run after the user presses ^D. Mark this command as completed to avoid a + // spurious error message. + Runtime::setCompleted(); + // Run the terminate event before the shell is run. Otherwise, if the shell // is forking processes (the default), any child processes will close the // database connection when they are killed. So when we return back to the diff --git a/src/Runtime/RedispatchHook.php b/src/Runtime/RedispatchHook.php index 35e3ec4a2b..335368ca37 100644 --- a/src/Runtime/RedispatchHook.php +++ b/src/Runtime/RedispatchHook.php @@ -124,6 +124,7 @@ protected function exitEarly(int $exit_code): never // Note that RemoteCommandProxy::execute() is expecting that // the redispatch() method will not return, so that will need // to be altered if this behavior is changed. + Runtime::setCompleted(); exit($exit_code); } } diff --git a/src/Runtime/Runtime.php b/src/Runtime/Runtime.php index f954312b86..93c80ae53a 100644 --- a/src/Runtime/Runtime.php +++ b/src/Runtime/Runtime.php @@ -22,6 +22,9 @@ */ class Runtime { + #[Deprecated("Remove for Drush 13")] + const DRUSH_RUNTIME_COMPLETED_NAMESPACE = 'runtime.execution.completed'; + public function __construct(protected Preflight $preflight, protected DependencyInjection $di) { } @@ -107,6 +110,9 @@ protected function doRun($argv, $output): int // Bootstrap: bootstrap site to the level requested by the command (via a 'post-init' hook) $status = $application->run($input, $output); + // Placate the Drush shutdown handler (@todo remove for v13). + Runtime::setCompleted(); + return $status; } @@ -116,6 +122,6 @@ protected function doRun($argv, $output): int #[Deprecated("Shutdown handling removed from Drush. Please remove the call to Runtime::setCompleted()")] public static function setCompleted(): void { - // Do nothing. + Drush::config()->set(self::DRUSH_RUNTIME_COMPLETED_NAMESPACE, true); } } From 7055d25a4cd926c250034ff327975e7b8e01d56b Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Tue, 19 Dec 2023 12:25:00 -0500 Subject: [PATCH 4/6] PHPCS --- src/Runtime/Runtime.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Runtime/Runtime.php b/src/Runtime/Runtime.php index 93c80ae53a..5e9ca04eba 100644 --- a/src/Runtime/Runtime.php +++ b/src/Runtime/Runtime.php @@ -24,7 +24,7 @@ class Runtime { #[Deprecated("Remove for Drush 13")] const DRUSH_RUNTIME_COMPLETED_NAMESPACE = 'runtime.execution.completed'; - + public function __construct(protected Preflight $preflight, protected DependencyInjection $di) { } From 5b2df566db395845179f966f6f8686853a22ab6b Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Fri, 22 Dec 2023 05:14:57 -0500 Subject: [PATCH 5/6] Undeprecate the bits that are staying in drush13 --- src/Runtime/Runtime.php | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/Runtime/Runtime.php b/src/Runtime/Runtime.php index 5e9ca04eba..2ca88fb960 100644 --- a/src/Runtime/Runtime.php +++ b/src/Runtime/Runtime.php @@ -22,7 +22,8 @@ */ class Runtime { - #[Deprecated("Remove for Drush 13")] + + // Shutdown handling removed from Drush, but 3rd party commandfiles may add it back. const DRUSH_RUNTIME_COMPLETED_NAMESPACE = 'runtime.execution.completed'; public function __construct(protected Preflight $preflight, protected DependencyInjection $di) @@ -110,7 +111,7 @@ protected function doRun($argv, $output): int // Bootstrap: bootstrap site to the level requested by the command (via a 'post-init' hook) $status = $application->run($input, $output); - // Placate the Drush shutdown handler (@todo remove for v13). + // Placate the Drush shutdown handler which can be provided via custom commandfile. Runtime::setCompleted(); return $status; @@ -118,8 +119,9 @@ protected function doRun($argv, $output): int /** * Mark the current request as having completed successfully. + * + * Shutdown handling removed from Drush, but 3rd party commandfiles may add it back. */ - #[Deprecated("Shutdown handling removed from Drush. Please remove the call to Runtime::setCompleted()")] public static function setCompleted(): void { Drush::config()->set(self::DRUSH_RUNTIME_COMPLETED_NAMESPACE, true); From 761bfc150e7d30a7d6b0cf04ff9a568a75686f63 Mon Sep 17 00:00:00 2001 From: Moshe Weitzman Date: Fri, 22 Dec 2023 05:32:14 -0500 Subject: [PATCH 6/6] optimize imports --- src/Runtime/Runtime.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Runtime/Runtime.php b/src/Runtime/Runtime.php index 2ca88fb960..71c5454869 100644 --- a/src/Runtime/Runtime.php +++ b/src/Runtime/Runtime.php @@ -4,12 +4,11 @@ namespace Drush\Runtime; -use JetBrains\PhpStorm\Deprecated; -use Symfony\Component\Console\Output\ConsoleOutput; use Drush\Application; use Drush\Commands\DrushCommands; use Drush\Drush; use Drush\Preflight\Preflight; +use Symfony\Component\Console\Output\ConsoleOutput; /** * Control the Drush runtime environment