Chad has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/88764


Change subject: Remove wfDl() and cleanup everything it touched
......................................................................

Remove wfDl() and cleanup everything it touched

wfDl() is a wrapper around dl(), which is an evil function and
basically only works from the command line of Zend. Luckily
no extension has ever used this thing, so let's just remove it
outright.

For comparison, here's a list of places it does not work:
- hhvm
- php as apache module
- php compiled with zts support
- safe_mode
- Basically any shared host that cares about security

Most callers are using it to check for extension support and are
actually failing gracefully when wfDl() returns false. In these
places we're just going to use extension_loaded().

While we're at it, clean up some of the test skip logic in the
media tests so we can bail as early as possible if we know we
can't complete the test.

This also immediately removes $wgLoadFileinfoExtension. It's been
enabled by default since 5.3 and falls back gracefully when the
support isn't available.

Change-Id: Ieb430dfc74483731dde51d6e20fa700d641ba1f4
---
M includes/DefaultSettings.php
M includes/GlobalFunctions.php
M includes/MimeMagic.php
M includes/db/DatabaseMysql.php
M includes/diff/DifferenceEngine.php
M includes/installer/DatabaseInstaller.php
M maintenance/backup.inc
M maintenance/sqlite.inc
M tests/phpunit/includes/CollationTest.php
M tests/phpunit/includes/media/BitmapMetadataHandlerTest.php
M tests/phpunit/includes/media/ExifBitmapTest.php
M tests/phpunit/includes/media/ExifRotationTest.php
M tests/phpunit/includes/media/ExifTest.php
M tests/phpunit/includes/media/FormatMetadataTest.php
M tests/phpunit/includes/media/JpegTest.php
M tests/phpunit/includes/media/TiffTest.php
M tests/phpunit/includes/media/XMPTest.php
M tests/phpunit/phpunit.php
18 files changed, 33 insertions(+), 112 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/64/88764/1

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 98c583b..21cd1ff5 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -1126,13 +1126,6 @@
 $wgMimeInfoFile = 'includes/mime.info';
 
 /**
- * Switch for loading the FileInfo extension by PECL at runtime.
- * This should be used only if fileinfo is installed as a shared object
- * or a dynamic library.
- */
-$wgLoadFileinfoExtension = false;
-
-/**
  * Sets an external mime detector program. The command must print only
  * the mime type to standard output.
  * The name of the file to process will be appended to the command given here.
diff --git a/includes/GlobalFunctions.php b/includes/GlobalFunctions.php
index b11bce9..8241d81 100644
--- a/includes/GlobalFunctions.php
+++ b/includes/GlobalFunctions.php
@@ -2635,38 +2635,6 @@
 }
 
 /**
- * Wrapper function for PHP's dl(). This doesn't work in most situations from
- * PHP 5.3 onward, and is usually disabled in shared environments anyway.
- *
- * @param string $extension A PHP extension. The file suffix (.so or .dll)
- *                          should be omitted
- * @param string $fileName Name of the library, if not $extension.suffix
- * @return Bool - Whether or not the extension is loaded
- */
-function wfDl( $extension, $fileName = null ) {
-       if ( extension_loaded( $extension ) ) {
-               return true;
-       }
-
-       $canDl = false;
-       if ( PHP_SAPI == 'cli' || PHP_SAPI == 'cgi' || PHP_SAPI == 'embed' ) {
-               $canDl = ( function_exists( 'dl' ) && is_callable( 'dl' )
-               && wfIniGetBool( 'enable_dl' ) && !wfIniGetBool( 'safe_mode' ) 
);
-       }
-
-       if ( $canDl ) {
-               $fileName = $fileName ? $fileName : $extension;
-               if ( wfIsWindows() ) {
-                       $fileName = 'php_' . $fileName;
-               }
-               wfSuppressWarnings();
-               dl( $fileName . '.' . PHP_SHLIB_SUFFIX );
-               wfRestoreWarnings();
-       }
-       return extension_loaded( $extension );
-}
-
-/**
  * Windows-compatible version of escapeshellarg()
  * Windows doesn't recognise single-quotes in the shell, but the 
escapeshellarg()
  * function puts single quotes in regardless of OS.
diff --git a/includes/MimeMagic.php b/includes/MimeMagic.php
index 44fafcaf..8220e92 100644
--- a/includes/MimeMagic.php
+++ b/includes/MimeMagic.php
@@ -169,10 +169,6 @@
         */
        private static $instance;
 
-       /** True if the fileinfo extension has been loaded
-        */
-       private static $extensionLoaded = false;
-
        /** Initializes the MimeMagic object. This is called by 
MimeMagic::singleton().
         *
         * This constructor parses the mime.types and mime.info files and build 
internal mappings.
@@ -182,17 +178,12 @@
                 *   --- load mime.types ---
                 */
 
-               global $wgMimeTypeFile, $IP, $wgLoadFileinfoExtension;
+               global $wgMimeTypeFile, $IP;
 
                $types = MM_WELL_KNOWN_MIME_TYPES;
 
                if ( $wgMimeTypeFile == 'includes/mime.types' ) {
                        $wgMimeTypeFile = "$IP/$wgMimeTypeFile";
-               }
-
-               if ( $wgLoadFileinfoExtension && !self::$extensionLoaded ) {
-                       self::$extensionLoaded = true;
-                       wfDl( 'fileinfo' );
                }
 
                if ( $wgMimeTypeFile ) {
diff --git a/includes/db/DatabaseMysql.php b/includes/db/DatabaseMysql.php
index b75d615..956bb69 100644
--- a/includes/db/DatabaseMysql.php
+++ b/includes/db/DatabaseMysql.php
@@ -43,12 +43,9 @@
        }
 
        protected function mysqlConnect( $realServer ) {
-               # Load mysql.so if we don't have it
-               wfDl( 'mysql' );
-
                # Fail now
                # Otherwise we get a suppressed fatal error, which is very hard 
to track down
-               if ( !function_exists( 'mysql_connect' ) ) {
+               if ( !extension_loaded( 'mysql' ) ) {
                        throw new DBConnectionError( $this, "MySQL functions 
missing, have you compiled PHP with the --with-mysql option?\n" );
                }
 
diff --git a/includes/diff/DifferenceEngine.php 
b/includes/diff/DifferenceEngine.php
index 0c9086b..e436f58 100644
--- a/includes/diff/DifferenceEngine.php
+++ b/includes/diff/DifferenceEngine.php
@@ -696,24 +696,6 @@
        }
 
        /**
-        * Make sure the proper modules are loaded before we try to
-        * make the diff
-        */
-       private function initDiffEngines() {
-               global $wgExternalDiffEngine;
-               if ( $wgExternalDiffEngine == 'wikidiff' && !function_exists( 
'wikidiff_do_diff' ) ) {
-                       wfProfileIn( __METHOD__ . '-php_wikidiff.so' );
-                       wfDl( 'php_wikidiff' );
-                       wfProfileOut( __METHOD__ . '-php_wikidiff.so' );
-               }
-               elseif ( $wgExternalDiffEngine == 'wikidiff2' && 
!function_exists( 'wikidiff2_do_diff' ) ) {
-                       wfProfileIn( __METHOD__ . '-php_wikidiff2.so' );
-                       wfDl( 'wikidiff2' );
-                       wfProfileOut( __METHOD__ . '-php_wikidiff2.so' );
-               }
-       }
-
-       /**
         * Generate a diff, no caching.
         *
         * This implementation uses generateTextDiffBody() to generate a diff 
based on the default
@@ -778,8 +760,6 @@
 
                $otext = str_replace( "\r\n", "\n", $otext );
                $ntext = str_replace( "\r\n", "\n", $ntext );
-
-               $this->initDiffEngines();
 
                if ( $wgExternalDiffEngine == 'wikidiff' && function_exists( 
'wikidiff_do_diff' ) ) {
                        # For historical reasons, external diff engine expects
diff --git a/includes/installer/DatabaseInstaller.php 
b/includes/installer/DatabaseInstaller.php
index 3739bb5..8bb7826 100644
--- a/includes/installer/DatabaseInstaller.php
+++ b/includes/installer/DatabaseInstaller.php
@@ -321,16 +321,11 @@
         * Convenience function.
         * Check if a named extension is present.
         *
-        * @see wfDl
         * @param $name
         * @return bool
         */
        protected static function checkExtension( $name ) {
-               wfSuppressWarnings();
-               $compiled = wfDl( $name );
-               wfRestoreWarnings();
-
-               return $compiled;
+               return extension_loaded( $name );
        }
 
        /**
diff --git a/maintenance/backup.inc b/maintenance/backup.inc
index db045cf..3dc94c8 100644
--- a/maintenance/backup.inc
+++ b/maintenance/backup.inc
@@ -170,11 +170,8 @@
                                        break;
                                case "force-normal":
                                        if ( !function_exists( 'utf8_normalize' 
) ) {
-                                               wfDl( "php_utfnormal.so" );
-                                               if ( !function_exists( 
'utf8_normalize' ) ) {
-                                                       $this->fatalError( 
"Failed to load UTF-8 normalization extension. " .
-                                                               "Install or 
remove --force-normal parameter to use slower code." );
-                                               }
+                                               $this->fatalError( "UTF-8 
normalization extension not loaded. " .
+                                                       "Install or remove 
--force-normal parameter to use slower code." );
                                        }
                                        break;
                                default:
diff --git a/maintenance/sqlite.inc b/maintenance/sqlite.inc
index 49f4e00..08188ca 100644
--- a/maintenance/sqlite.inc
+++ b/maintenance/sqlite.inc
@@ -33,10 +33,7 @@
         * @return bool
         */
        public static function isPresent() {
-               wfSuppressWarnings();
-               $compiled = wfDl( 'pdo_sqlite' );
-               wfRestoreWarnings();
-               return $compiled;
+               return extension_loaded( 'pdo_sqlite' );
        }
 
        /**
diff --git a/tests/phpunit/includes/CollationTest.php 
b/tests/phpunit/includes/CollationTest.php
index ae35fd7..f1004fb 100644
--- a/tests/phpunit/includes/CollationTest.php
+++ b/tests/phpunit/includes/CollationTest.php
@@ -2,7 +2,7 @@
 class CollationTest extends MediaWikiLangTestCase {
        protected function setUp() {
                parent::setUp();
-               if ( !wfDl( 'intl' ) ) {
+               if ( !extension_loaded( 'intl' ) ) {
                        $this->markTestSkipped( 'These tests require intl 
extension' );
                }
        }
diff --git a/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php 
b/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php
index ffa6084..43792c1 100644
--- a/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php
+++ b/tests/phpunit/includes/media/BitmapMetadataHandlerTest.php
@@ -18,10 +18,10 @@
         * translation (to en) where XMP should win.
         */
        public function testMultilingualCascade() {
-               if ( !wfDl( 'exif' ) ) {
+               if ( !extension_loaded( 'exif' ) ) {
                        $this->markTestSkipped( "This test needs the exif 
extension." );
                }
-               if ( !wfDl( 'xml' ) ) {
+               if ( !extension_loaded( 'xml' ) ) {
                        $this->markTestSkipped( "This test needs the xml 
extension." );
                }
 
@@ -115,7 +115,7 @@
        }
 
        public function testPNGXMP() {
-               if ( !wfDl( 'xml' ) ) {
+               if ( !extension_loaded( 'xml' ) ) {
                        $this->markTestSkipped( "This test needs the xml 
extension." );
                }
                $handler = new BitmapMetadataHandler();
diff --git a/tests/phpunit/includes/media/ExifBitmapTest.php 
b/tests/phpunit/includes/media/ExifBitmapTest.php
index 1109c47..532182c 100644
--- a/tests/phpunit/includes/media/ExifBitmapTest.php
+++ b/tests/phpunit/includes/media/ExifBitmapTest.php
@@ -4,13 +4,14 @@
 
        protected function setUp() {
                parent::setUp();
+               if ( !extension_loaded( 'exif' ) ) {
+                       $this->markTestSkipped( "This test needs the exif 
extension." );
+               }
 
                $this->setMwGlobals( 'wgShowEXIF', true );
 
                $this->handler = new ExifBitmapHandler;
-               if ( !wfDl( 'exif' ) ) {
-                       $this->markTestSkipped( "This test needs the exif 
extension." );
-               }
+
        }
 
        public function testIsOldBroken() {
diff --git a/tests/phpunit/includes/media/ExifRotationTest.php 
b/tests/phpunit/includes/media/ExifRotationTest.php
index f02e8b9..c16de8b 100644
--- a/tests/phpunit/includes/media/ExifRotationTest.php
+++ b/tests/phpunit/includes/media/ExifRotationTest.php
@@ -8,6 +8,10 @@
 
        protected function setUp() {
                parent::setUp();
+               if ( !extension_loaded( 'exif' ) ) {
+                       $this->markTestSkipped( "This test needs the exif 
extension." );
+               }
+
                $this->handler = new BitmapHandler();
                $filePath = __DIR__ . '/../../data/media';
 
@@ -22,9 +26,6 @@
                                'containerPaths' => array( 'temp-thumb' => 
$tmpDir, 'data' => $filePath )
                        ) )
                ) );
-               if ( !wfDl( 'exif' ) ) {
-                       $this->markTestSkipped( "This test needs the exif 
extension." );
-               }
 
                $this->setMwGlobals( array(
                        'wgShowEXIF' => true,
diff --git a/tests/phpunit/includes/media/ExifTest.php 
b/tests/phpunit/includes/media/ExifTest.php
index 6ad28ac..b84ed56 100644
--- a/tests/phpunit/includes/media/ExifTest.php
+++ b/tests/phpunit/includes/media/ExifTest.php
@@ -3,12 +3,13 @@
 
        protected function setUp() {
                parent::setUp();
+               if ( !extension_loaded( 'exif' ) ) {
+                       $this->markTestSkipped( "This test needs the exif 
extension." );
+               }
 
                $this->mediaPath = __DIR__ . '/../../data/media/';
 
-               if ( !wfDl( 'exif' ) ) {
-                       $this->markTestSkipped( "This test needs the exif 
extension." );
-               }
+
 
                $this->setMwGlobals( 'wgShowEXIF', true );
        }
diff --git a/tests/phpunit/includes/media/FormatMetadataTest.php 
b/tests/phpunit/includes/media/FormatMetadataTest.php
index f26d27e..bee0906 100644
--- a/tests/phpunit/includes/media/FormatMetadataTest.php
+++ b/tests/phpunit/includes/media/FormatMetadataTest.php
@@ -4,7 +4,7 @@
        protected function setUp() {
                parent::setUp();
 
-               if ( !wfDl( 'exif' ) ) {
+               if ( !extension_loaded( 'exif' ) ) {
                        $this->markTestSkipped( "This test needs the exif 
extension." );
                }
                $filePath = __DIR__ . '/../../data/media';
diff --git a/tests/phpunit/includes/media/JpegTest.php 
b/tests/phpunit/includes/media/JpegTest.php
index 05d3661..7775c41 100644
--- a/tests/phpunit/includes/media/JpegTest.php
+++ b/tests/phpunit/includes/media/JpegTest.php
@@ -3,12 +3,13 @@
 
        protected function setUp() {
                parent::setUp();
-
-               $this->filePath = __DIR__ . '/../../data/media/';
-               if ( !wfDl( 'exif' ) ) {
+               if ( !extension_loaded( 'exif' ) ) {
                        $this->markTestSkipped( "This test needs the exif 
extension." );
                }
 
+               $this->filePath = __DIR__ . '/../../data/media/';
+
+
                $this->setMwGlobals( 'wgShowEXIF', true );
        }
 
diff --git a/tests/phpunit/includes/media/TiffTest.php 
b/tests/phpunit/includes/media/TiffTest.php
index 91c35c4..1ec34d5 100644
--- a/tests/phpunit/includes/media/TiffTest.php
+++ b/tests/phpunit/includes/media/TiffTest.php
@@ -3,6 +3,9 @@
 
        protected function setUp() {
                parent::setUp();
+               if ( !extension_loaded( 'exif' ) ) {
+                       $this->markTestSkipped( "This test needs the exif 
extension." );
+               }
 
                $this->setMwGlobals( 'wgShowEXIF', true );
 
@@ -11,17 +14,11 @@
        }
 
        public function testInvalidFile() {
-               if ( !wfDl( 'exif' ) ) {
-                       $this->markTestIncomplete( "This test needs the exif 
extension." );
-               }
                $res = $this->handler->getMetadata( null, $this->filePath . 
'README' );
                $this->assertEquals( ExifBitmapHandler::BROKEN_FILE, $res );
        }
 
        public function testTiffMetadataExtraction() {
-               if ( !wfDl( 'exif' ) ) {
-                       $this->markTestIncomplete( "This test needs the exif 
extension." );
-               }
                $res = $this->handler->getMetadata( null, $this->filePath . 
'test.tiff' );
                $expected = 
'a:16:{s:10:"ImageWidth";i:20;s:11:"ImageLength";i:20;s:13:"BitsPerSample";a:3:{i:0;i:8;i:1;i:8;i:2;i:8;}s:11:"Compression";i:5;s:25:"PhotometricInterpretation";i:2;s:16:"ImageDescription";s:17:"Created
 with 
GIMP";s:12:"StripOffsets";i:8;s:11:"Orientation";i:1;s:15:"SamplesPerPixel";i:3;s:12:"RowsPerStrip";i:64;s:15:"StripByteCounts";i:238;s:11:"XResolution";s:19:"1207959552/16777216";s:11:"YResolution";s:19:"1207959552/16777216";s:19:"PlanarConfiguration";i:1;s:14:"ResolutionUnit";i:2;s:22:"MEDIAWIKI_EXIF_VERSION";i:2;}';
                // Re-unserialize in case there are subtle differences between 
how versions
diff --git a/tests/phpunit/includes/media/XMPTest.php 
b/tests/phpunit/includes/media/XMPTest.php
index 25a43eb..aa0cdd2 100644
--- a/tests/phpunit/includes/media/XMPTest.php
+++ b/tests/phpunit/includes/media/XMPTest.php
@@ -3,7 +3,7 @@
 
        protected function setUp() {
                parent::setUp();
-               if ( !wfDl( 'xml' ) ) {
+               if ( !extension_loaded( 'xml' ) ) {
                        $this->markTestSkipped( 'Requires libxml to do XMP 
parsing' );
                }
        }
diff --git a/tests/phpunit/phpunit.php b/tests/phpunit/phpunit.php
index 1d65e52..e91f5cd 100755
--- a/tests/phpunit/phpunit.php
+++ b/tests/phpunit/phpunit.php
@@ -11,6 +11,8 @@
 // Set a flag which can be used to detect when other scripts have been entered 
through this entry point or not
 define( 'MW_PHPUNIT_TEST', true );
 
+set_include_path( get_include_path() . PATH_SEPARATOR . 
'/usr/local/share/pear/' );
+
 // Start up MediaWiki in command-line mode
 require_once dirname( dirname( __DIR__ ) ) . "/maintenance/Maintenance.php";
 

-- 
To view, visit https://gerrit.wikimedia.org/r/88764
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ieb430dfc74483731dde51d6e20fa700d641ba1f4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Chad <ch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to