From 5bc89bf32b0602c906417e3bcf99eabbb15bdf98 Mon Sep 17 00:00:00 2001 From: Andy Miller Date: Mon, 10 Nov 2025 11:40:13 +0000 Subject: [PATCH] simplified safe-upgrade Signed-off-by: Andy Miller --- system/install.php | 18 +- system/src/Grav/Installer/Install.php | 247 ++------------------------ 2 files changed, 36 insertions(+), 229 deletions(-) diff --git a/system/install.php b/system/install.php index 37ada646b..d73b9891a 100644 --- a/system/install.php +++ b/system/install.php @@ -13,6 +13,17 @@ if (!defined('GRAV_ROOT')) { // Check if Install class is already loaded (from an older Grav version) // This happens when upgrading from older versions where the OLD Install class // was loaded via autoloader before extracting the update package (e.g., via Install::forceSafeUpgrade()) +$logInstallerSource = static function ($install, string $source) { + try { + $reflection = new \ReflectionClass($install); + $path = $reflection->getFileName() ?: 'unknown'; + } catch (\Throwable $e) { + $path = 'unknown'; + } + + error_log(sprintf('[Grav Upgrade] Installer loaded from %s: %s', $source, $path)); +}; + if (class_exists('Grav\\Installer\\Install', false)) { // OLD Install class is already loaded. We cannot load the NEW one due to PHP limitations. // However, we can work around this by: @@ -31,10 +42,15 @@ if (class_exists('Grav\\Installer\\Install', false)) { $locationProp->setValue($install, __DIR__ . '/..'); } + $logInstallerSource($install, 'existing installation'); + return $install; } // Normal case: Install class not yet loaded, load the NEW one require_once __DIR__ . '/src/Grav/Installer/Install.php'; -return Grav\Installer\Install::instance(); +$install = Grav\Installer\Install::instance(); +$logInstallerSource($install, 'extracted update package'); + +return $install; diff --git a/system/src/Grav/Installer/Install.php b/system/src/Grav/Installer/Install.php index ad4cec912..3aa97fe11 100644 --- a/system/src/Grav/Installer/Install.php +++ b/system/src/Grav/Installer/Install.php @@ -16,9 +16,6 @@ use Grav\Common\GPM\Installer; use Grav\Common\Grav; use Grav\Common\Plugins; use RuntimeException; -// NOTE: SafeUpgradeService is NOT imported via 'use' to avoid autoloader loading OLD class -// When Install.php is included during upgrade, the autoloader is from the OLD installation -// and would load the OLD SafeUpgradeService. We manually require the NEW one when needed. use function class_exists; use function dirname; use function function_exists; @@ -297,146 +294,25 @@ ERR; // Update user/config/version.yaml before copying the files to avoid frontend from setting the version schema. $this->updater->install(); - if ($this->shouldUseSafeUpgrade()) { - // CRITICAL: Check if SafeUpgradeService is already loaded from old installation - // If it is, PHP will use the OLD version instead of the NEW one from this package - $expectedPath = $this->location . '/system/src/Grav/Common/Upgrade/SafeUpgradeService.php'; + $safeUpgradeRequested = $this->shouldUseSafeUpgrade(); + $progressMessage = $safeUpgradeRequested + ? 'Safe upgrade temporarily using legacy installer...' + : 'Running legacy installer...'; + $this->relayProgress('installing', $progressMessage, null); - if (class_exists('Grav\\Common\\Upgrade\\SafeUpgradeService', false)) { - // Class is already loaded - check if it's from the correct location - $reflection = new \ReflectionClass('Grav\\Common\\Upgrade\\SafeUpgradeService'); - $loadedFrom = $reflection->getFileName(); - - $loadedFromReal = realpath($loadedFrom) ?: $loadedFrom; - $expectedReal = realpath($expectedPath) ?: $expectedPath; - - if ($loadedFromReal !== $expectedReal) { - // OLD SafeUpgradeService is already loaded - fall back to traditional upgrade - error_log(sprintf( - 'WARNING: SafeUpgradeService was loaded from old installation (%s). ' . - 'Falling back to traditional upgrade method.', - $loadedFromReal - )); - - // Force traditional upgrade by disabling safe upgrade - Install::forceSafeUpgrade(false); - - // Skip to traditional upgrade below - goto traditional_upgrade; - } - } - - // SafeUpgradeService was loaded in shouldUseSafeUpgrade() from the NEW package - $options = []; - try { - $grav = Grav::instance(); - if ($grav && isset($grav['config'])) { - $options['config'] = $grav['config']; - } - } catch (\Throwable $e) { - // ignore - } - - // Use fully qualified class name (no 'use' statement to avoid autoloader) - $service = new \Grav\Common\Upgrade\SafeUpgradeService($options); - - // CRITICAL: Verify we're using the NEW SafeUpgradeService from this package, not the old one - $this->verifySafeUpgradeServiceVersion($service); - - // Run preflight checks using the NEW SafeUpgradeService - // This ensures preflight logic is from the package being installed, not the old installation - try { - $targetVersion = $this->getVersion(); - $preflight = $service->preflight($targetVersion); - $isMajorMinorUpgrade = $preflight['is_major_minor_upgrade'] ?? false; - - // Check for pending plugin/theme updates - if (!empty($preflight['plugins_pending'])) { - $pending = $preflight['plugins_pending']; - $list = []; - foreach ($pending as $slug => $info) { - $current = $info['current'] ?? 'unknown'; - $available = $info['available'] ?? 'unknown'; - $list[] = sprintf('%s (v%s → v%s)', $slug, $current, $available); - } - - if ($isMajorMinorUpgrade) { - // For major/minor upgrades, block until plugins are updated - // This allows the NEW package to define and enforce the upgrade policy - $currentVersion = GRAV_VERSION; - $targetVersion = $this->getVersion(); - throw new RuntimeException( - sprintf( - "Major version upgrade detected (v%s → v%s).\n\n" . - "The following plugins/themes have updates available:\n - %s\n\n" . - "For major version upgrades, plugins and themes must be updated FIRST.\n" . - "Please run 'bin/gpm update' to update these packages, then retry the upgrade.\n" . - "This ensures plugins have necessary compatibility fixes for the new Grav version.", - $currentVersion, - $targetVersion, - implode("\n - ", $list) - ) - ); - } else { - // For patch upgrades, just log a warning but allow upgrade - error_log( - 'WARNING: The following plugins/themes have updates available: ' . - implode(', ', $list) . '. ' . - 'Consider updating them after upgrading Grav.' - ); - } - } - - // PSR log conflicts - log warning but don't block for now - if (!empty($preflight['psr_log_conflicts'])) { - $conflicts = $preflight['psr_log_conflicts']; - error_log( - 'WARNING: PSR/log conflicts detected in plugins: ' . - implode(', ', array_keys($conflicts)) . - '. These may need updating after Grav upgrade.' - ); - } - - // Monolog conflicts - log warning but don't block for now - if (!empty($preflight['monolog_conflicts'])) { - $conflicts = $preflight['monolog_conflicts']; - error_log( - 'WARNING: Monolog API conflicts detected in plugins: ' . - implode(', ', array_keys($conflicts)) . - '. These may need updating after Grav upgrade.' - ); - } - } catch (RuntimeException $e) { - // Preflight failed - abort upgrade with clear message - throw new RuntimeException( - 'Upgrade preflight checks failed: ' . $e->getMessage(), - 0, - $e - ); - } - - // Preflight passed - proceed with upgrade - if ($this->progressCallback) { - $service->setProgressCallback(function (string $stage, string $message, ?int $percent = null, array $extra = []) { - $this->relayProgress($stage, $message, $percent); - }); - } - $manifest = $service->promote($this->location, $this->getVersion(), $this->ignores); - if (method_exists($service, 'getLastManifest')) { - $manifest = $service->getLastManifest() ?? $manifest; - } - $this->lastManifest = $manifest; - Installer::setError(Installer::OK); - } else { - traditional_upgrade: - Installer::install( - $this->zip ?? '', - GRAV_ROOT, - ['sophisticated' => true, 'overwrite' => true, 'ignore_symlinks' => true, 'ignores' => $this->ignores], - $this->location, - !($this->zip && is_file($this->zip)) - ); + if ($safeUpgradeRequested) { + error_log('[Grav Upgrade] Safe upgrade requested but staging is currently disabled; running legacy installer.'); } + + Installer::install( + $this->zip ?? '', + GRAV_ROOT, + ['sophisticated' => true, 'overwrite' => true, 'ignore_symlinks' => true, 'ignores' => $this->ignores], + $this->location, + !($this->zip && is_file($this->zip)) + ); + + $this->relayProgress('complete', 'Legacy installer finished.', 100); } catch (Exception $e) { Installer::setError($e->getMessage()); } @@ -455,28 +331,15 @@ ERR; */ private function shouldUseSafeUpgrade(): bool { - // Check static override first (for programmatic control) if (null !== self::$forceSafeUpgrade) { return self::$forceSafeUpgrade; } - // Check environment variable set by SelfupgradeCommand (avoids early class loading) $envValue = getenv('GRAV_FORCE_SAFE_UPGRADE'); if (false !== $envValue && '' !== $envValue) { return $envValue === '1'; } - // CRITICAL CHECK: Ensure current installation supports SafeUpgradeService - // Must check this BEFORE reading config to prevent using safe-upgrade on old installations - $currentServiceFile = GRAV_ROOT . '/system/src/Grav/Common/Upgrade/SafeUpgradeService.php'; - if (!file_exists($currentServiceFile)) { - // Current installation doesn't have SafeUpgradeService (upgrading from < 1.7.50) - // Use traditional upgrade method - return false; - } - - // PRIMARY CHECK: Check Grav config setting using proper config system - // This respects the fallback chain: user/config/system.yaml -> system/config/system.yaml try { $grav = Grav::instance(); if ($grav && isset($grav['config'])) { @@ -486,28 +349,13 @@ ERR; } } } catch (\Throwable $e) { - // Grav container may not be initialised yet - // Fall through to load SafeUpgradeService and return false + // ignore bootstrap failures } - // FINAL STEP: Load SafeUpgradeService from NEW package if needed - // Only reached if current installation HAS SafeUpgradeService but config is not set - if (!class_exists('Grav\\Common\\Upgrade\\SafeUpgradeService', false)) { - // Class not loaded yet - try to load from NEW package - $serviceFile = $this->location . '/system/src/Grav/Common/Upgrade/SafeUpgradeService.php'; - if (!file_exists($serviceFile)) { - return false; // SafeUpgradeService not available in this package - } - // Load the NEW SafeUpgradeService from this package - require_once $serviceFile; - } - - // If we get here: current installation HAS SafeUpgradeService, but config is not explicitly set - // Default to FALSE (traditional upgrade) for safety - // Users must explicitly enable system.updates.safe_upgrade to use it return false; } + /** * @return void * @throws RuntimeException @@ -622,64 +470,7 @@ ERR; return $matches[1] ?? ''; } - /** - * Verify that we're using the NEW SafeUpgradeService from this package, - * not the OLD one from the current installation. - * - * This is CRITICAL to ensure bugfixes in SafeUpgradeService are actually used. - * - * @param object $service The SafeUpgradeService instance (no type hint to avoid early loading) - * @return void - * @throws RuntimeException if the wrong version is loaded - */ - protected function verifySafeUpgradeServiceVersion(object $service): void - { - // Get the file path where SafeUpgradeService was loaded from - $reflection = new \ReflectionClass($service); - $loadedFrom = $reflection->getFileName(); - // Expected: should be from THIS package in $this->location - // e.g., /tmp/grav-update-abc123/grav-update/system/src/Grav/Common/Upgrade/SafeUpgradeService.php - $expectedPath = $this->location . '/system/src/Grav/Common/Upgrade/SafeUpgradeService.php'; - - // Normalize paths for comparison - $loadedFromReal = realpath($loadedFrom) ?: $loadedFrom; - $expectedReal = realpath($expectedPath) ?: $expectedPath; - - if ($loadedFromReal !== $expectedReal) { - // CRITICAL ERROR: We're using the OLD SafeUpgradeService! - // This means bugfixes in the new version won't be applied. - error_log(sprintf( - 'CRITICAL: SafeUpgradeService loaded from WRONG location!' . "\n" . - ' Expected (NEW): %s' . "\n" . - ' Actual (OLD): %s' . "\n" . - 'This indicates a class loading issue that will prevent bugfixes from being applied.', - $expectedReal, - $loadedFromReal - )); - - throw new RuntimeException( - 'CRITICAL: SafeUpgradeService was loaded from the old installation instead of the new package. ' . - 'This is a known issue that has been fixed. Please upgrade using CLI: bin/gpm self-upgrade -f' - ); - } - - // Additional check: verify IMPLEMENTATION_VERSION constant exists - // (Added in this fix - old versions won't have it) - if (!defined('Grav\\Common\\Upgrade\\SafeUpgradeService::IMPLEMENTATION_VERSION')) { - error_log( - 'WARNING: SafeUpgradeService::IMPLEMENTATION_VERSION not defined. ' . - 'This suggests an old version is loaded.' - ); - } else { - $version = constant('Grav\\Common\\Upgrade\\SafeUpgradeService::IMPLEMENTATION_VERSION'); - error_log(sprintf( - 'SafeUpgradeService verification PASSED. Using version %s from: %s', - $version, - $loadedFromReal - )); - } - } protected function legacySupport(): void {