From b6d18ab9f70ee8b7dcea0a5347f278faf83ba111 Mon Sep 17 00:00:00 2001 From: Bryan Davis Date: Thu, 26 Jun 2014 13:05:29 -0600 Subject: [PATCH] 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 --- includes/DefaultSettings.php | 6 ++++ includes/GitInfo.php | 40 +++++++++++++++++--------- includes/Setup.php | 4 +++ tests/phpunit/includes/GitInfoTest.php | 2 +- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index fb73eb3a0d..918abb7bfa 100644 --- 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 304c1bce30..acf1bf64a3 100644 --- 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 fd4465bcf7..78655a45de 100644 --- 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 7c684d51e1..e22f50507e 100644 --- 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() { -- 2.20.1