From b873e9294b87ee256df581e2cc9e83ef814871c0 Mon Sep 17 00:00:00 2001 From: RazeSoldier Date: Sat, 27 Jul 2019 23:38:54 +0800 Subject: [PATCH] Make CliInstaller control the processing logic of the error Previously, if there was an error during CLI installation, CliInstaller::showStatusMessage() exited the script directly. The exit timing of the script should be given to the caller, not the callee. So, I coding: [1] Remove `exit()` from CliInstaller::showStatusMessage() [2] Make the callee to return Status, the caller determine how to handle these Status [3] Strictly check the key database type instead of just outputting message Bug: T46511 Change-Id: I72ffd33fe5c592b9ea78f37bae5a9c081295c624 --- autoload.php | 1 + includes/installer/CliInstaller.php | 27 +++++++------ includes/installer/InstallException.php | 51 +++++++++++++++++++++++++ includes/installer/Installer.php | 18 +++++---- maintenance/install.php | 25 ++++++++---- 5 files changed, 96 insertions(+), 26 deletions(-) create mode 100644 includes/installer/InstallException.php diff --git a/autoload.php b/autoload.php index e6e650492f..c4179869b1 100644 --- a/autoload.php +++ b/autoload.php @@ -884,6 +884,7 @@ $wgAutoloadLocalClasses = [ 'MediaWiki\\Diff\\WordAccumulator' => __DIR__ . '/includes/diff/WordAccumulator.php', 'MediaWiki\\HeaderCallback' => __DIR__ . '/includes/HeaderCallback.php', 'MediaWiki\\Http\\HttpRequestFactory' => __DIR__ . '/includes/http/HttpRequestFactory.php', + 'MediaWiki\\Installer\\InstallException' => __DIR__ . '/includes/installer/InstallException.php', 'MediaWiki\\Interwiki\\ClassicInterwikiLookup' => __DIR__ . '/includes/interwiki/ClassicInterwikiLookup.php', 'MediaWiki\\Interwiki\\InterwikiLookup' => __DIR__ . '/includes/interwiki/InterwikiLookup.php', 'MediaWiki\\Interwiki\\InterwikiLookupAdapter' => __DIR__ . '/includes/interwiki/InterwikiLookupAdapter.php', diff --git a/includes/installer/CliInstaller.php b/includes/installer/CliInstaller.php index 567fb10a69..99d594df97 100644 --- a/includes/installer/CliInstaller.php +++ b/includes/installer/CliInstaller.php @@ -21,6 +21,7 @@ * @ingroup Deployment */ +use MediaWiki\Installer\InstallException; use MediaWiki\MediaWikiServices; /** @@ -51,6 +52,7 @@ class CliInstaller extends Installer { * @param string $siteName * @param string|null $admin * @param array $options + * @throws InstallException */ function __construct( $siteName, $admin = null, array $options = [] ) { global $wgContLang; @@ -114,7 +116,7 @@ class CliInstaller extends Installer { $status = $this->validateExtensions( 'extension', 'extensions', $options['extensions'] ); if ( !$status->isOK() ) { - $this->showStatusMessage( $status ); + throw new InstallException( $status ); } $this->setVar( '_Extensions', $status->value ); } elseif ( isset( $options['with-extensions'] ) ) { @@ -125,7 +127,7 @@ class CliInstaller extends Installer { if ( isset( $options['skins'] ) ) { $status = $this->validateExtensions( 'skin', 'skins', $options['skins'] ); if ( !$status->isOK() ) { - $this->showStatusMessage( $status ); + throw new InstallException( $status ); } $skins = $status->value; } else { @@ -176,15 +178,23 @@ class CliInstaller extends Installer { $vars = Installer::getExistingLocalSettings(); if ( $vars ) { - $this->showStatusMessage( - Status::newFatal( "config-localsettings-cli-upgrade" ) - ); + $status = Status::newFatal( "config-localsettings-cli-upgrade" ); + $this->showStatusMessage( $status ); + return $status; } - $this->performInstallation( + $result = $this->performInstallation( [ $this, 'startStage' ], [ $this, 'endStage' ] ); + // PerformInstallation bails on a fatal, so make sure the last item + // completed before giving 'next.' Likewise, only provide back on failure + $lastStepStatus = end( $result ); + if ( $lastStepStatus->isOk() ) { + return Status::newGood(); + } else { + return $lastStepStatus; + } } /** @@ -248,11 +258,6 @@ class CliInstaller extends Installer { $this->showMessage( ...$w ); } } - - if ( !$status->isOK() ) { - echo "\n"; - exit( 1 ); - } } public function envCheckPath() { diff --git a/includes/installer/InstallException.php b/includes/installer/InstallException.php new file mode 100644 index 0000000000..a03a5aaa6c --- /dev/null +++ b/includes/installer/InstallException.php @@ -0,0 +1,51 @@ +status = $status; + } + + public function getStatus() : \Status { + return $this->status; + } +} diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php index f6a5c41fb2..7452b7c4b9 100644 --- a/includes/installer/Installer.php +++ b/includes/installer/Installer.php @@ -756,6 +756,8 @@ abstract class Installer { */ protected function envCheckDB() { global $wgLang; + /** @var string|null $dbType The user-specified database type */ + $dbType = $this->getVar( 'wgDBtype' ); $allNames = []; @@ -768,25 +770,27 @@ abstract class Installer { $databases = $this->getCompiledDBs(); $databases = array_flip( $databases ); + $ok = true; foreach ( array_keys( $databases ) as $db ) { $installer = $this->getDBInstaller( $db ); $status = $installer->checkPrerequisites(); if ( !$status->isGood() ) { + if ( !$this instanceof WebInstaller && $db === $dbType ) { + // Strictly check the key database type instead of just outputting message + // Note: No perform this check run from the web installer, since this method always called by + // the welcome page under web installation, so $dbType will always be 'mysql' + $ok = false; + } $this->showStatusMessage( $status ); - } - if ( !$status->isOK() ) { unset( $databases[$db] ); } } $databases = array_flip( $databases ); if ( !$databases ) { $this->showError( 'config-no-db', $wgLang->commaList( $allNames ), count( $allNames ) ); - - // @todo FIXME: This only works for the web installer! return false; } - - return true; + return $ok; } /** @@ -1586,7 +1590,7 @@ abstract class Installer { * @param callable $startCB A callback array for the beginning of each step * @param callable $endCB A callback array for the end of each step * - * @return array Array of Status objects + * @return Status[] Array of Status objects */ public function performInstallation( $startCB, $endCB ) { $installResults = []; diff --git a/maintenance/install.php b/maintenance/install.php index 1dd1909497..a71bb74a5c 100644 --- a/maintenance/install.php +++ b/maintenance/install.php @@ -115,7 +115,12 @@ class CommandLineInstaller extends Maintenance { $this->setPassOption(); } - $installer = InstallerOverrides::getCliInstaller( $siteName, $adminName, $this->mOptions ); + try { + $installer = InstallerOverrides::getCliInstaller( $siteName, $adminName, $this->mOptions ); + } catch ( \MediaWiki\Installer\InstallException $e ) { + $this->output( $e->getStatus()->getMessage()->parse() . "\n" ); + return false; + } $status = $installer->doEnvironmentChecks(); if ( $status->isGood() ) { @@ -123,17 +128,21 @@ class CommandLineInstaller extends Maintenance { } else { $installer->showStatusMessage( $status ); - return; + return false; } if ( !$envChecksOnly ) { - $installer->execute(); + $status = $installer->execute(); + if ( !$status->isGood() ) { + return false; + } $installer->writeConfigurationFile( $this->getOption( 'confpath', $IP ) ); + $installer->showMessage( + 'config-install-success', + $installer->getVar( 'wgServer' ), + $installer->getVar( 'wgScriptPath' ) + ); } - $installer->showMessage( - 'config-install-success', - $installer->getVar( 'wgServer' ), - $installer->getVar( 'wgScriptPath' ) - ); + return true; } private function setDbPassOption() { -- 2.20.1