From d18e76dbef35efeb1f735bb070e54be5dc629e4b Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Sat, 31 Aug 2019 23:43:23 +0100 Subject: [PATCH] Setup: Move MWDebug logic to MWDebug.php * Remove checks in HTMLFileCache.php and Article.php. These haven't been needed since the same check was added to Setup.php, many years ago. When FileCache is enabled, The Setup.php code disables MWDebug. There is no reason for FileCache to then also disable itself based on unused config. That means both of them lose. We now handle this logic in one place: MWDebug::setup(). * In rebuildFileCache.php, turn it off explicitly, just in case. The previous code there didn't work because finalSetup() is called after doMaintenance.php includes Setup.php, which is what checked this config var to decide on MWDebug::init. On the other hand, it's also always off in CLI mode. But, let's not depend on that, maybe we decide to enable it on CLI one day! Just keep it off explicitly here. Bug: T189966 Change-Id: I45a8f77092249751dc6f276aa5bb67ebf5b4f64c --- includes/Setup.php | 13 +------------ includes/cache/HTMLFileCache.php | 4 ---- includes/debug/MWDebug.php | 24 ++++++++++++++++++++++++ includes/page/Article.php | 4 ++-- maintenance/rebuildFileCache.php | 8 ++++---- 5 files changed, 31 insertions(+), 22 deletions(-) diff --git a/includes/Setup.php b/includes/Setup.php index 226780080d..823a81bf78 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -547,12 +547,6 @@ if ( isset( $wgSquidMaxage ) ) { $wgSquidMaxage = $wgCdnMaxAge; } -// Easy to forget to falsify $wgDebugToolbar for static caches. -// If file cache or CDN cache is on, just disable this (DWIMD). -if ( $wgUseFileCache || $wgUseCdn ) { - $wgDebugToolbar = false; -} - // Blacklisted file extensions shouldn't appear on the "allowed" list $wgFileExtensions = array_values( array_diff( $wgFileExtensions, $wgFileBlacklist ) ); @@ -620,12 +614,7 @@ if ( defined( 'MW_NO_SESSION' ) ) { $wgPHPSessionHandling = MW_NO_SESSION === 'warn' ? 'warn' : 'disable'; } -// Disable MWDebug for command line mode, this prevents MWDebug from eating up -// all the memory from logging SQL queries on maintenance scripts -global $wgCommandLineMode; -if ( $wgDebugToolbar && !$wgCommandLineMode ) { - MWDebug::init(); -} +MWDebug::setup(); // Reset the global service locator, so any services that have already been created will be // re-created while taking into account any custom settings and extensions. diff --git a/includes/cache/HTMLFileCache.php b/includes/cache/HTMLFileCache.php index a0d61b259e..ab78ee469b 100644 --- a/includes/cache/HTMLFileCache.php +++ b/includes/cache/HTMLFileCache.php @@ -94,10 +94,6 @@ class HTMLFileCache extends FileCacheBase { $config = MediaWikiServices::getInstance()->getMainConfig(); if ( !$config->get( 'UseFileCache' ) && $mode !== self::MODE_REBUILD ) { - return false; - } elseif ( $config->get( 'DebugToolbar' ) ) { - wfDebug( "HTML file cache skipped. \$wgDebugToolbar on\n" ); - return false; } diff --git a/includes/debug/MWDebug.php b/includes/debug/MWDebug.php index e8778362ec..6bcb0e6ff8 100644 --- a/includes/debug/MWDebug.php +++ b/includes/debug/MWDebug.php @@ -67,6 +67,30 @@ class MWDebug { */ protected static $deprecationWarnings = []; + /** + * @internal For use by Setup.php only. + */ + public static function setup() { + global $wgDebugToolbar, + $wgUseCdn, $wgUseFileCache, $wgCommandLineMode; + + if ( + // Easy to forget to falsify $wgDebugToolbar for static caches. + // If file cache or CDN cache is on, just disable this (DWIMD). + $wgUseCdn || + $wgUseFileCache || + // Keep MWDebug off on CLI. This prevents MWDebug from eating up + // all the memory for logging SQL queries in maintenance scripts. + $wgCommandLineMode + ) { + return; + } + + if ( $wgDebugToolbar ) { + self::init(); + } + } + /** * Enabled the debugger and load resource module. * This is called by Setup.php when $wgDebugToolbar is true. diff --git a/includes/page/Article.php b/includes/page/Article.php index d8cd1c5bc8..535aeffeb3 100644 --- a/includes/page/Article.php +++ b/includes/page/Article.php @@ -587,7 +587,7 @@ class Article implements Page { * page of the given title. */ public function view() { - global $wgUseFileCache, $wgDebugToolbar; + global $wgUseFileCache; # Get variables from query string # As side effect this will load the revision and update the title @@ -643,7 +643,7 @@ class Article implements Page { } # Try client and file cache - if ( !$wgDebugToolbar && $oldid === 0 && $this->mPage->checkTouched() ) { + if ( $oldid === 0 && $this->mPage->checkTouched() ) { # Try to stream the output from file cache if ( $wgUseFileCache && $this->tryFileCache() ) { wfDebug( __METHOD__ . ": done file cache\n" ); diff --git a/maintenance/rebuildFileCache.php b/maintenance/rebuildFileCache.php index 2cdf418b25..d484392d7e 100644 --- a/maintenance/rebuildFileCache.php +++ b/maintenance/rebuildFileCache.php @@ -43,18 +43,18 @@ class RebuildFileCache extends Maintenance { } public function finalSetup() { - global $wgDebugToolbar, $wgUseFileCache; + global $wgUseFileCache; $this->enabled = $wgUseFileCache; // Script will handle capturing output and saving it itself $wgUseFileCache = false; - // Debug toolbar makes content uncacheable so we disable it. - // Has to be done before Setup.php initialize MWDebug - $wgDebugToolbar = false; // Avoid DB writes (like enotif/counters) MediaWiki\MediaWikiServices::getInstance()->getReadOnlyMode() ->setReason( 'Building cache' ); + // Ensure no debug-specific logic ends up in the cache (must be after Setup.php) + MWDebug::deinit(); + parent::finalSetup(); } -- 2.20.1