From: Clara Andrew-Wani <candrewwani@gmail.com>
Date: Thu, 29 Aug 2019 14:56:27 +0000 (-0400)
Subject: Modify -—with-extensions to throw extension dependency errors
X-Git-Tag: 1.34.0-rc.0~421
X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=d9eec3c9124d87fd44e6917d5b1512b78352afb3

Modify -—with-extensions to throw extension dependency errors

Changes the behavior of -—with-extensions from silently skipping
extensions that lack another extension dependency to throwing an
install exception.

Bug: T225512
Change-Id: Ia5b41841aaf55fa5c4971ae67218f8231f8adcb8
---

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 c719c762e3..289a721e92 100644
--- a/includes/installer/Installer.php
+++ b/includes/installer/Installer.php
@@ -1270,7 +1270,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 ) {
@@ -1289,33 +1290,40 @@ 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() ) {
+			$extStatus = $this->getExtensionInfo( $type, $directory, $file );
+			if ( $extStatus->isOK() ) {
 				$exts[$file] = $status->value;
+			} else {
+				$status->merge( $extStatus );
 			}
 		}
 		closedir( $dh );
 		uksort( $exts, 'strnatcasecmp' );
 
-		return $exts;
+		$status->value = $exts;
+
+		return $status;
 	}
 
 	/**
diff --git a/includes/installer/WebInstallerOptions.php b/includes/installer/WebInstallerOptions.php
index 2412319ea3..7c1619ab5a 100644
--- a/includes/installer/WebInstallerOptions.php
+++ b/includes/installer/WebInstallerOptions.php
@@ -104,7 +104,7 @@ class WebInstallerOptions extends WebInstallerPage {
 			$this->getFieldsetEnd()
 		);
 
-		$skins = $this->parent->findExtensions( 'skins' );
+		$skins = $this->parent->findExtensions( 'skins' )->value;
 		$skinHtml = $this->getFieldsetStart( 'config-skins' );
 
 		$skinNames = array_map( 'strtolower', array_keys( $skins ) );
@@ -118,6 +118,7 @@ class WebInstallerOptions extends WebInstallerPage {
 				'value' => $chosenSkinName,
 			] );
 
+			// @phan-suppress-next-line PhanTypeNoAccessiblePropertiesForeach
 			foreach ( $skins as $skin => $info ) {
 				if ( isset( $info['screenshots'] ) ) {
 					$screenshotText = $this->makeScreenshotsLink( $skin, $info['screenshots'] );
@@ -144,7 +145,7 @@ class WebInstallerOptions extends WebInstallerPage {
 			$this->getFieldsetEnd();
 		$this->addHTML( $skinHtml );
 
-		$extensions = $this->parent->findExtensions();
+		$extensions = $this->parent->findExtensions()->value;
 		$dependencyMap = [];
 
 		if ( $extensions ) {
@@ -153,6 +154,7 @@ class WebInstallerOptions extends WebInstallerPage {
 			$extByType = [];
 			$types = SpecialVersion::getExtensionTypes();
 			// Sort by type first
+			// @phan-suppress-next-line PhanTypeNoAccessiblePropertiesForeach
 			foreach ( $extensions as $ext => $info ) {
 				if ( !isset( $info['type'] ) || !isset( $types[$info['type']] ) ) {
 					// We let extensions normally define custom types, but
@@ -329,6 +331,8 @@ class WebInstallerOptions extends WebInstallerPage {
 		if ( count( $screenshots ) > 1 ) {
 			$links = [];
 			$counter = 1;
+
+			// @phan-suppress-next-line PhanTypeNoAccessiblePropertiesForeach
 			foreach ( $screenshots as $shot ) {
 				$links[] = Html::element(
 					'a',
@@ -448,7 +452,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 +502,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 +523,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" ] );