resourceloader: Convert FileModule to use version hashing

Enabling the module content versionining is not feasible for FileModule as that
would involve Lessc and CSSJanus just to compute the version hash.

Instead, we can keep the existing logic that exists for the timestamp-based
versioning (which already has a comprehensive grip on tracking all involved
factors that cause a module to change) and convert it to use hashing instead.

This way the version hashes will be deterministic. Currently module versions
tend to be invalidated too often (and sometimes not often enough) due to Git and
other transport mechanisms not preserving file timestamps.

== Research ==

* <https://phabricator.wikimedia.org/T98087#1412712>
  It'd take upto 10 seconds to run startup if FileModule enables build versioning.

* <https://phabricator.wikimedia.org/T104950#1433142>
  Checking all files in resources/** took only ~30m longer in total with
  sha1_file compared to filemtime.

Bug: T104950
Change-Id: I732fa4db32258c634e32b507952f76eac7fc9395

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index b734def..efb151e 100644 (file)
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -526,19 +526,16 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
        }
 
        /**
-        * Helper method to gather file mtimes for getDefinitionSummary.
+        * Helper method to gather file hashes for getDefinitionSummary.
         *
-        * Last modified timestamps are calculated from the highest last modified
-        * timestamp of this module's constituent files as well as the files it
-        * depends on. This function is context-sensitive, only performing
-        * calculations on files relevant to the given language, skin and debug
-        * mode.
+        * This function is context-sensitive, only computing hashes of files relevant to the
+        * given language, skin, etc.
         *
         * @see ResourceLoaderModule::getFileDependencies
         * @param ResourceLoaderContext $context
         * @return array
         */
-       protected function getFileMtimes( ResourceLoaderContext $context ) {
+       protected function getFileHashes( ResourceLoaderContext $context ) {
                $files = array();
 
                // Flatten style files into $files
@@ -577,13 +574,10 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                // entry point Less file we already know about.
                $files = array_values( array_unique( $files ) );
 
-               // Don't max() because older files are significant.
-               // While the associated file names are significant, that is already taken care of by the
-               // definition summary. Avoid creating an array keyed by file path here because those are
-               // absolute file paths. Including that would needlessly cause global cache invalidation
-               // when the MediaWiki installation path changes (which is quite common in cases like
-               // Wikimedia where the installation path reflects the MediaWiki branch name).
-               return array_map( array( __CLASS__, 'safeFilemtime' ), $files );
+               // Don't include keys or file paths here, only the hashes. Including that would needlessly
+               // cause global cache invalidation when files move or if e.g. the MediaWiki path changes.
+               // Any significant ordering is already detected by the definition summary.
+               return array_map( array( __CLASS__, 'safeFileHash' ), $files );
        }
 
        /**
@@ -597,6 +591,9 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
 
                $options = array();
                foreach ( array(
+                       // T104950: Do not include localBasePath! That path may vary over time and needlessly
+                       // invalidate cache. If the path changes in a way that makes relative file paths point
+                       // to something else, getFileHashes() will incorporate that already.
                        'scripts',
                        'debugScripts',
                        'loaderScripts',
@@ -611,9 +608,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        'group',
                        'position',
                        'skipFunction',
-                       // FIXME: localBasePath includes the MediaWiki installation path and
-                       // needlessly causes cache invalidation.
-                       'localBasePath',
                        'remoteBasePath',
                        'debugRaw',
                        'raw',
@@ -623,7 +617,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
 
                $summary[] = array(
                        'options' => $options,
-                       'fileMtimes' => $this->getFileMTimes( $context ),
+                       'fileHashes' => $this->getFileHashes( $context ),
                        'msgBlobMtime' => $this->getMsgBlobMtime( $context->getLanguage() ),
                );
                return $summary;

diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php
index 46b786d..8e53c3e 100644 (file)
--- a/includes/resourceloader/ResourceLoaderModule.php
+++ b/includes/resourceloader/ResourceLoaderModule.php
@@ -835,16 +835,30 @@ abstract class ResourceLoaderModule {
        }
 
        /**
-        * Safe version of filemtime(), which doesn't throw a PHP warning if the file doesn't exist
-        * but returns 1 instead.
-        * @param string $filename File name
+        * Safe version of filemtime(), which doesn't throw a PHP warning if the file doesn't exist.
+        * Defaults to 1.
+        *
+        * @param string $filePath File path
         * @return int UNIX timestamp
         */
-       protected static function safeFilemtime( $filename ) {
+       protected static function safeFilemtime( $filePath ) {
                MediaWiki\suppressWarnings();
-               $mtime = filemtime( $filename ) ?: 1;
+               $mtime = filemtime( $filePath ) ?: 1;
                MediaWiki\restoreWarnings();
-
                return $mtime;
        }
+
+       /**
+        * Safe version of sha1_file(), which doesn't throw a PHP warning if the file doesn't exist.
+        * Defaults to empty string.
+        *
+        * @param string $filePath File path
+        * @return string Hash
+        */
+       protected static function safeFileHash( $filePath ) {
+               MediaWiki\suppressWarnings();
+               $hash = sha1_file( $filename ) ?: '';
+               MediaWiki\restoreWarnings();
+               return $hash;
+       }
 }