Revert and fix "Revert "Modify -—with-extensions to throw extension dependency errors""

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

diff --git a/includes/installer/CliInstaller.php b/includes/installer/CliInstaller.php
index 424c9d7..0ff34b0 100644 (file)
--- 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 6d1e211..b830b70 100644 (file)
--- 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 2412319..7bec49a 100644 (file)
--- 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 a71bb74..28a1746 100644 (file)
--- 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;
                }