From 0aec1401e59f0c386729792515995b319d6480e0 Mon Sep 17 00:00:00 2001 From: Brad Jorsch Date: Thu, 5 Sep 2019 10:34:36 -0400 Subject: [PATCH] =?utf8?q?Revert=20and=20fix=20"Revert=20"Modify=20-?= =?utf8?q?=E2=80=94with-extensions=20to=20throw=20extension=20dependency?= =?utf8?q?=20errors""?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit This reverts commit cc7ec36a577161bba06159abd15efddc8a4f745d. It makes the following changes from the original patch: * findExtensions() will not consider config-extension-not-found as an error. * Fixed a bug where extension info wouldn't actually be returned by findExtensions(). * When an extension's dependency doesn't exist, wrap the config-extension-not-found error in config-extension-dependency. This makes it more clear, and prevents the error from being ignored per the first bullet. * maintenance/install.php will use ->text() rather than ->parse() when printing thrown errors. * Change the stuff done to make phan happy. Bug: T225512 Change-Id: I7d29700e8b7e91841556847d669b350cbd306fe6 --- includes/installer/CliInstaller.php | 12 ++++++-- includes/installer/Installer.php | 36 ++++++++++++++++------ includes/installer/WebInstallerOptions.php | 17 +++++++--- maintenance/install.php | 2 +- 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/includes/installer/CliInstaller.php b/includes/installer/CliInstaller.php index 424c9d7b78..0ff34b086f 100644 --- a/includes/installer/CliInstaller.php +++ b/includes/installer/CliInstaller.php @@ -120,7 +120,11 @@ class CliInstaller extends Installer { } $this->setVar( '_Extensions', $status->value ); } elseif ( isset( $options['with-extensions'] ) ) { - $this->setVar( '_Extensions', array_keys( $this->findExtensions() ) ); + $status = $this->findExtensions(); + if ( !$status->isOK() ) { + throw new InstallException( $status ); + } + $this->setVar( '_Extensions', array_keys( $status->value ) ); } // Set up the default skins @@ -131,7 +135,11 @@ class CliInstaller extends Installer { } $skins = $status->value; } else { - $skins = array_keys( $this->findExtensions( 'skins' ) ); + $status = $this->findExtensions( 'skins' ); + if ( !$status->isOK() ) { + throw new InstallException( $status ); + } + $skins = array_keys( $status->value ); } $this->setVar( '_Skins', $skins ); diff --git a/includes/installer/Installer.php b/includes/installer/Installer.php index 6d1e211d68..b830b7006a 100644 --- a/includes/installer/Installer.php +++ b/includes/installer/Installer.php @@ -1273,7 +1273,8 @@ abstract class Installer { * * @param string $directory Directory to search in, relative to $IP, must be either "extensions" * or "skins" - * @return array[][] [ $extName => [ 'screenshots' => [ '...' ] ] + * @return Status An object containing an error list. If there were no errors, an associative + * array of information about the extension can be found in $status->value. */ public function findExtensions( $directory = 'extensions' ) { switch ( $directory ) { @@ -1292,33 +1293,43 @@ abstract class Installer { * * @param string $type Either "extension" or "skin" * @param string $directory Directory to search in, relative to $IP - * @return array [ $extName => [ 'screenshots' => [ '...' ] ] + * @return Status An object containing an error list. If there were no errors, an associative + * array of information about the extension can be found in $status->value. */ protected function findExtensionsByType( $type = 'extension', $directory = 'extensions' ) { if ( $this->getVar( 'IP' ) === null ) { - return []; + return Status::newGood( [] ); } $extDir = $this->getVar( 'IP' ) . '/' . $directory; if ( !is_readable( $extDir ) || !is_dir( $extDir ) ) { - return []; + return Status::newGood( [] ); } $dh = opendir( $extDir ); $exts = []; + $status = new Status; while ( ( $file = readdir( $dh ) ) !== false ) { - if ( !is_dir( "$extDir/$file" ) ) { + // skip non-dirs and hidden directories + if ( !is_dir( "$extDir/$file" ) || $file[0] === '.' ) { continue; } - $status = $this->getExtensionInfo( $type, $directory, $file ); - if ( $status->isOK() ) { - $exts[$file] = $status->value; + $extStatus = $this->getExtensionInfo( $type, $directory, $file ); + if ( $extStatus->isOK() ) { + $exts[$file] = $extStatus->value; + } elseif ( $extStatus->hasMessage( 'config-extension-not-found' ) ) { + // (T225512) The directory is not actually an extension. Downgrade to warning. + $status->warning( 'config-extension-not-found', $file ); + } else { + $status->merge( $extStatus ); } } closedir( $dh ); uksort( $exts, 'strnatcasecmp' ); - return $exts; + $status->value = $exts; + + return $status; } /** @@ -1419,11 +1430,16 @@ abstract class Installer { } elseif ( $e->missingExtensions || $e->missingSkins ) { // There's an extension missing in the dependency tree, // so add those to the dependency list and try again - return $this->readExtension( + $status = $this->readExtension( $fullJsonFile, array_merge( $extDeps, $e->missingExtensions ), array_merge( $skinDeps, $e->missingSkins ) ); + if ( !$status->isOK() && !$status->hasMessage( 'config-extension-dependency' ) ) { + $status = Status::newFatal( 'config-extension-dependency', + basename( dirname( $fullJsonFile ) ), $status->getMessage() ); + } + return $status; } // Some other kind of dependency error? return Status::newFatal( 'config-extension-dependency', diff --git a/includes/installer/WebInstallerOptions.php b/includes/installer/WebInstallerOptions.php index 2412319ea3..7bec49a369 100644 --- a/includes/installer/WebInstallerOptions.php +++ b/includes/installer/WebInstallerOptions.php @@ -104,7 +104,8 @@ class WebInstallerOptions extends WebInstallerPage { $this->getFieldsetEnd() ); - $skins = $this->parent->findExtensions( 'skins' ); + $skins = $this->parent->findExtensions( 'skins' )->value; + '@phan-var array[] $skins'; $skinHtml = $this->getFieldsetStart( 'config-skins' ); $skinNames = array_map( 'strtolower', array_keys( $skins ) ); @@ -144,7 +145,8 @@ class WebInstallerOptions extends WebInstallerPage { $this->getFieldsetEnd(); $this->addHTML( $skinHtml ); - $extensions = $this->parent->findExtensions(); + $extensions = $this->parent->findExtensions()->value; + '@phan-var array[] $extensions'; $dependencyMap = []; if ( $extensions ) { @@ -324,11 +326,16 @@ class WebInstallerOptions extends WebInstallerPage { return null; } + /** + * @param string $name + * @param array $screenshots + */ private function makeScreenshotsLink( $name, $screenshots ) { global $wgLang; if ( count( $screenshots ) > 1 ) { $links = []; $counter = 1; + foreach ( $screenshots as $shot ) { $links[] = Html::element( 'a', @@ -448,7 +455,7 @@ class WebInstallerOptions extends WebInstallerPage { * @return bool */ public function submitSkins() { - $skins = array_keys( $this->parent->findExtensions( 'skins' ) ); + $skins = array_keys( $this->parent->findExtensions( 'skins' )->value ); $this->parent->setVar( '_Skins', $skins ); if ( $skins ) { @@ -498,7 +505,7 @@ class WebInstallerOptions extends WebInstallerPage { $this->setVar( 'wgRightsIcon', '' ); } - $skinsAvailable = array_keys( $this->parent->findExtensions( 'skins' ) ); + $skinsAvailable = array_keys( $this->parent->findExtensions( 'skins' )->value ); $skinsToInstall = []; foreach ( $skinsAvailable as $skin ) { $this->parent->setVarsFromRequest( [ "skin-$skin" ] ); @@ -519,7 +526,7 @@ class WebInstallerOptions extends WebInstallerPage { $retVal = false; } - $extsAvailable = array_keys( $this->parent->findExtensions() ); + $extsAvailable = array_keys( $this->parent->findExtensions()->value ); $extsToInstall = []; foreach ( $extsAvailable as $ext ) { $this->parent->setVarsFromRequest( [ "ext-$ext" ] ); diff --git a/maintenance/install.php b/maintenance/install.php index a71bb74a5c..28a1746d9e 100644 --- a/maintenance/install.php +++ b/maintenance/install.php @@ -118,7 +118,7 @@ class CommandLineInstaller extends Maintenance { try { $installer = InstallerOverrides::getCliInstaller( $siteName, $adminName, $this->mOptions ); } catch ( \MediaWiki\Installer\InstallException $e ) { - $this->output( $e->getStatus()->getMessage()->parse() . "\n" ); + $this->output( $e->getStatus()->getMessage()->text() . "\n" ); return false; } -- 2.20.1