From c3808d2635448d0417984b20add46b85af834264 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Fri, 22 May 2026 14:19:58 -0700 Subject: [PATCH] TryUpdateHooks: also update pre-command/post-command hook loaders on mount TryUpdateHooks (called at mount time) updated the 3 native hooks (read-object, virtual-filesystem, post-index-change) but skipped the pre-command.exe and post-command.exe copies of GitHooksLoader.exe. Those were only deployed at clone time via InstallHooks, so upgrading GVFS and remounting left stale loader binaries in the enlistment. Refactor TryUpdateHook into an overload that takes explicit source and target paths, then call it from TryUpdateHooks for both command hook loaders using GitHooksLoader.exe as the source. The HookData overload becomes a thin wrapper that delegates to the new one. Add Assert-HookVersionsMatch to the staging-upgrade and clean-upgrade test scenarios: after upgrade completes, remount and verify all 5 hook binaries match the installed versions. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella --- .github/workflows/upgrade-tests.yaml | 40 ++++++++++++++++ GVFS/GVFS.Common/FileSystem/HooksInstaller.cs | 47 +++++++++++++++---- 2 files changed, 79 insertions(+), 8 deletions(-) diff --git a/.github/workflows/upgrade-tests.yaml b/.github/workflows/upgrade-tests.yaml index a398d6666..73eba2243 100644 --- a/.github/workflows/upgrade-tests.yaml +++ b/.github/workflows/upgrade-tests.yaml @@ -173,6 +173,36 @@ jobs: } } + function Assert-HookVersionsMatch { + Write-Host "Verifying hook binary versions in enlistment..." + $hooksDir = Join-Path $enlistment "src\.git\hooks" + + # Native hooks: each has its own source binary in the install directory + # Command hooks: copies of GitHooksLoader.exe + $hooks = @( + @{ Source = "GVFS.ReadObjectHook.exe"; Target = "read-object.exe" } + @{ Source = "GVFS.VirtualFileSystemHook.exe"; Target = "virtual-filesystem.exe" } + @{ Source = "GVFS.PostIndexChangedHook.exe"; Target = "post-index-change.exe" } + @{ Source = "GitHooksLoader.exe"; Target = "pre-command.exe" } + @{ Source = "GitHooksLoader.exe"; Target = "post-command.exe" } + ) + + foreach ($hook in $hooks) { + $sourcePath = Join-Path $installDir $hook.Source + $targetPath = Join-Path $hooksDir $hook.Target + if (-not (Test-Path $targetPath)) { + throw "Hook not found: $targetPath" + } + $sourceVer = [System.Diagnostics.FileVersionInfo]::GetVersionInfo($sourcePath).FileVersion + $targetVer = [System.Diagnostics.FileVersionInfo]::GetVersionInfo($targetPath).FileVersion + if ($sourceVer -ne $targetVer) { + throw "Hook version mismatch: $($hook.Target) is $targetVer, expected $sourceVer (from $($hook.Source))" + } + Write-Host " $($hook.Target): $targetVer OK" + } + Write-Host "All hook binary versions match" + } + # ============================================= # Test scenarios # ============================================= @@ -193,6 +223,11 @@ jobs: Unmount-TestRepo Restart-Service Assert-PendingUpgrade $false + + # Remount and verify all hooks were updated to new version + $null = Mount-TestRepo + Assert-HookVersionsMatch + Unmount-TestRepo Write-Host "PASS: Staging upgrade completed" } @@ -205,6 +240,11 @@ jobs: Install-GVFS $newInstaller @("/STAGEIFMOUNTED=false") Assert-PendingUpgrade $false Assert-ServiceRunning + + # Remount and verify all hooks were updated to new version + $null = Mount-TestRepo + Assert-HookVersionsMatch + Unmount-TestRepo Write-Host "PASS: Clean upgrade completed" } diff --git a/GVFS/GVFS.Common/FileSystem/HooksInstaller.cs b/GVFS/GVFS.Common/FileSystem/HooksInstaller.cs index 7ebd28eec..f7dfae0fc 100644 --- a/GVFS/GVFS.Common/FileSystem/HooksInstaller.cs +++ b/GVFS/GVFS.Common/FileSystem/HooksInstaller.cs @@ -103,6 +103,27 @@ public static bool TryUpdateHooks(GVFSContext context, out string errorMessage) } } + // Update the pre-command and post-command hook loaders (GitHooksLoader copies). + // These are deployed at clone time by InstallHooks but also need updating on + // mount so that upgrading GVFS and remounting refreshes all hooks. + string loaderSourcePath = Path.Combine(ExecutingDirectory, GVFSConstants.DotGit.Hooks.LoaderExecutable); + + string precommandHookPath = Path.Combine( + context.Enlistment.WorkingDirectoryBackingRoot, + GVFSConstants.DotGit.Hooks.PreCommandPath + GVFSPlatform.Instance.Constants.ExecutableExtension); + if (!TryUpdateHook(context, GVFSConstants.DotGit.Hooks.PreCommandHookName, loaderSourcePath, precommandHookPath, out errorMessage)) + { + return false; + } + + string postcommandHookPath = Path.Combine( + context.Enlistment.WorkingDirectoryBackingRoot, + GVFSConstants.DotGit.Hooks.PostCommandPath + GVFSPlatform.Instance.Constants.ExecutableExtension); + if (!TryUpdateHook(context, GVFSConstants.DotGit.Hooks.PostCommandHookName, loaderSourcePath, postcommandHookPath, out errorMessage)) + { + return false; + } + return true; } @@ -161,13 +182,23 @@ private static bool TryUpdateHook( HookData hook, out string errorMessage) { - bool copyHook = false; string enlistmentHookPath = Path.Combine(context.Enlistment.WorkingDirectoryBackingRoot, hook.Path + GVFSPlatform.Instance.Constants.ExecutableExtension); string installedHookPath = Path.Combine(ExecutingDirectory, hook.ExecutableName); + return TryUpdateHook(context, hook.Name, installedHookPath, enlistmentHookPath, out errorMessage); + } + + private static bool TryUpdateHook( + GVFSContext context, + string hookName, + string installedHookPath, + string enlistmentHookPath, + out string errorMessage) + { + bool copyHook = false; if (!context.FileSystem.FileExists(installedHookPath)) { - errorMessage = hook.ExecutableName + " cannot be found at " + installedHookPath; + errorMessage = Path.GetFileName(installedHookPath) + " cannot be found at " + installedHookPath; return false; } @@ -179,8 +210,8 @@ private static bool TryUpdateHook( metadata.Add("Area", "Mount"); metadata.Add(nameof(enlistmentHookPath), enlistmentHookPath); metadata.Add(nameof(installedHookPath), installedHookPath); - metadata.Add(TracingConstants.MessageKey.WarningMessage, hook.Name + " not found in enlistment, copying from installation folder"); - context.Tracer.RelatedWarning(hook.Name + " MissingFromEnlistment", metadata); + metadata.Add(TracingConstants.MessageKey.WarningMessage, hookName + " not found in enlistment, copying from installation folder"); + context.Tracer.RelatedWarning(hookName + " MissingFromEnlistment", metadata); } else { @@ -197,8 +228,8 @@ private static bool TryUpdateHook( metadata.Add(nameof(enlistmentHookPath), enlistmentHookPath); metadata.Add(nameof(installedHookPath), installedHookPath); metadata.Add("Exception", e.ToString()); - context.Tracer.RelatedError(metadata, "Failed to compare " + hook.Name + " version"); - errorMessage = "Error comparing " + hook.Name + " versions. " + ConsoleHelper.GetGVFSLogMessage(context.Enlistment.EnlistmentRoot); + context.Tracer.RelatedError(metadata, "Failed to compare " + hookName + " version"); + errorMessage = "Error comparing " + hookName + " versions. " + ConsoleHelper.GetGVFSLogMessage(context.Enlistment.EnlistmentRoot); return false; } } @@ -216,8 +247,8 @@ private static bool TryUpdateHook( metadata.Add(nameof(enlistmentHookPath), enlistmentHookPath); metadata.Add(nameof(installedHookPath), installedHookPath); metadata.Add("Exception", e.ToString()); - context.Tracer.RelatedError(metadata, "Failed to copy " + hook.Name + " to enlistment"); - errorMessage = "Error copying " + hook.Name + " to enlistment. " + ConsoleHelper.GetGVFSLogMessage(context.Enlistment.EnlistmentRoot); + context.Tracer.RelatedError(metadata, "Failed to copy " + hookName + " to enlistment"); + errorMessage = "Error copying " + hookName + " to enlistment. " + ConsoleHelper.GetGVFSLogMessage(context.Enlistment.EnlistmentRoot); return false; } }