Fix GitInfo cache file path computation and storage location

Depending on the configuration used in LocalSettings.php, $IP can be
changed between the time that configuration is loaded and the wiki
runtime by logic in WebStart.php. Attempt to mitigate the effects of
such changes on the cache file name computation by canonicalizing both
$IP and the path using PHP's realpath() function.

Related but distinct is the possible need to configure the canonical
location for finding cache files on disk separately from
$wgCacheDirectory. This change introduces a new configuration variable
named $wgGitInfoCacheDirectory that can be set to a path that diverges
from the default location of $wgCacheDirectory/gitinfo. This will be
useful in the WMF cluster where $wgCacheDirectory points to a directory
that is not managed by the deployment system.

Finally add wfDebugLog logging to make tracking down issues such as
miscomputed cache paths easier.

Bug: 53972
Change-Id: Iceb9e1ce8d3b4bb08f89fa6ec5d5e7392aaafd46

diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index fb73eb3..918abb7 100644 (file)
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -2163,6 +2163,12 @@ $wgCachePages = true;
  */
 $wgCacheEpoch = '20030516000000';
 
+/**
+ * Directory where GitInfo will look for pre-computed cache files. If false,
+ * $wgCacheDirectory/gitinfo will be used.
+ */
+$wgGitInfoCacheDirectory = false;
+
 /**
  * Bump this number when changing the global style sheets and JavaScript.
  *

diff --git a/includes/GitInfo.php b/includes/GitInfo.php
index 304c1bc..acf1bf6 100644 (file)
--- a/includes/GitInfo.php
+++ b/includes/GitInfo.php
@@ -57,6 +57,9 @@ class GitInfo {
         */
        public function __construct( $repoDir, $usePrecomputed = true ) {
                $this->cacheFile = self::getCacheFilePath( $repoDir );
+               wfDebugLog( 'gitinfo',
+                       "Computed cacheFile={$this->cacheFile} for {$repoDir}"
+               );
                if ( $usePrecomputed &&
                        $this->cacheFile !== null &&
                        is_readable( $this->cacheFile )
@@ -65,9 +68,11 @@ class GitInfo {
                                file_get_contents( $this->cacheFile ),
                                true
                        );
+                       wfDebugLog( 'gitinfo', "Loaded git data from cache for {$repoDir}" );
                }
 
                if ( !$this->cacheIsComplete() ) {
+                       wfDebugLog( 'gitinfo', "Cache incomplete for {$repoDir}" );
                        $this->basedir = $repoDir . DIRECTORY_SEPARATOR . '.git';
                        if ( is_readable( $this->basedir ) && !is_dir( $this->basedir ) ) {
                                $GITfile = file_get_contents( $this->basedir );
@@ -90,24 +95,31 @@ class GitInfo {
         * Compute the path to the cache file for a given directory.
         *
         * @param string $repoDir The root directory of the repo where .git can be found
-        * @return string Path to GitInfo cache file in $wgCacheDirectory or null if
-        * $wgCacheDirectory is false (cache disabled).
+        * @return string Path to GitInfo cache file in $wgGitInfoCacheDirectory or
+        * null if $wgGitInfoCacheDirectory is false (cache disabled).
+        * @since 1.24
         */
        protected static function getCacheFilePath( $repoDir ) {
-               global $IP, $wgCacheDirectory;
-               if ( $wgCacheDirectory ) {
-                       // Transform path to git repo to something we can safely embed in a filename
-                       $repoName = $repoDir;
-                       if ( strpos( $repoName, $IP ) === 0 ) {
+               global $IP, $wgGitInfoCacheDirectory;
+
+               if ( $wgGitInfoCacheDirectory ) {
+                       // Convert both $IP and $repoDir to canonical paths to protect against
+                       // $IP having changed between the settings files and runtime.
+                       $realIP = realpath( $IP );
+                       $repoName = realpath( $repoDir );
+                       if ( $repoName === false ) {
+                               // Unit tests use fake path names
+                               $repoName = $repoDir;
+                       }
+                       if ( strpos( $repoName, $realIP ) === 0 ) {
                                // Strip $IP from path
-                               $repoName = substr( $repoName, strlen( $IP ) );
+                               $repoName = substr( $repoName, strlen( $realIP ) );
                        }
+                       // Transform path to git repo to something we can safely embed in
+                       // a filename
                        $repoName = strtr( $repoName, DIRECTORY_SEPARATOR, '-' );
                        $fileName = 'info' . $repoName . '.json';
-                       return implode(
-                               DIRECTORY_SEPARATOR,
-                               array( $wgCacheDirectory, 'gitinfo', $fileName )
-                       );
+                       return "{$wgGitInfoCacheDirectory}/{$fileName}";
                }
                return null;
        }
@@ -330,7 +342,9 @@ class GitInfo {
                        $this->getRemoteUrl();
 
                        if ( !$this->cacheIsComplete() ) {
-                               wfDebugLog( "Failed to compute GitInfo for \"{$this->basedir}\"" );
+                               wfDebugLog( 'gitinfo',
+                                       "Failed to compute GitInfo for \"{$this->basedir}\""
+                               );
                                return;
                        }
 

diff --git a/includes/Setup.php b/includes/Setup.php
index fd4465b..78655a4 100644 (file)
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -106,6 +106,10 @@ if ( $wgDeletedDirectory === false ) {
        $wgDeletedDirectory = "{$wgUploadDirectory}/deleted";
 }
 
+if ( $wgGitInfoCacheDirectory === false && $wgCacheDirectory !== false ) {
+       $wgGitInfoCacheDirectory = "{$wgCacheDirectory}/gitinfo";
+}
+
 if ( isset( $wgFileStore['deleted']['directory'] ) ) {
        $wgDeletedDirectory = $wgFileStore['deleted']['directory'];
 }

diff --git a/tests/phpunit/includes/GitInfoTest.php b/tests/phpunit/includes/GitInfoTest.php
index 7c684d5..e22f505 100644 (file)
--- a/tests/phpunit/includes/GitInfoTest.php
+++ b/tests/phpunit/includes/GitInfoTest.php
@@ -6,7 +6,7 @@ class GitInfoTest extends MediaWikiTestCase {
 
        protected function setUp() {
                parent::setUp();
-               $this->setMwGlobals( 'wgCacheDirectory', __DIR__ . '/../data' );
+               $this->setMwGlobals( 'wgGitInfoCacheDirectory', __DIR__ . '/../data/gitinfo' );
        }
 
        public function testValidJsonData() {