diff --git a/README.md b/README.md index 48218083c..0e4202e7b 100644 --- a/README.md +++ b/README.md @@ -2536,7 +2536,10 @@ kcap feedback --feedback # send feedback; prompts > Releases and checks its sha256 against the manifest, installing nothing on a > mismatch. Each release gets its own `versions/` directory and the > `current` link is switched to it, so running processes keep the files they -> started from; then the new binary runs `kcap refresh`. A daemon service +> started from; then the new binary runs `kcap refresh`. The version it replaced +> stays; older ones are deleted unless something still runs from them. A daemon +> started from a script install restarts on the new version once it is idle, +> Windows included. A daemon service > installed from a script install is moved onto the `current` path, so it > follows updates. Switching from npm, a daemon service installed under npm keeps > running the npm binary: run `kcap daemon service install` again once the diff --git a/src/Capacitor.Cli.Core/Resources/help-update.txt b/src/Capacitor.Cli.Core/Resources/help-update.txt index ac40ff76b..8d8a0a8f1 100644 --- a/src/Capacitor.Cli.Core/Resources/help-update.txt +++ b/src/Capacitor.Cli.Core/Resources/help-update.txt @@ -14,8 +14,11 @@ npm, downloads this platform's archive from GitHub Releases and checks its sha256 against the manifest; any mismatch installs nothing. The release goes into its own versions/ directory and the `current` link is switched to it, so running processes keep the files they started from. -The new binary then runs `kcap refresh`. A daemon service installed from a -script install is moved onto the `current` path, so it follows updates. +The new binary then runs `kcap refresh`. The version it replaced stays; +older ones are deleted unless something still runs from them. A daemon +started from a script install restarts on the new version once it is idle, +Windows included. A daemon service installed from a script install is moved +onto the `current` path, so it follows updates. A daemon service installed under npm keeps running the npm binary after a switch to the script install: run `kcap daemon service install` again. diff --git a/src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs b/src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs index 8be6e6bc2..f55fc52f3 100644 --- a/src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs +++ b/src/Capacitor.Cli.Daemon/Services/RestartCoordinator.cs @@ -22,8 +22,8 @@ internal enum RestartRequestResult { /// Polls the running binary; a size/mtime change queues a restart-after-update that /// fires the chosen the moment the daemon is idle /// (no hosted agents, no in-flight eval). Also handles explicit requests from the -/// control socket. Self-detection is macOS/Linux only (the poll is a no-op on Windows, -/// where a running binary can't be replaced); the explicit request path works anywhere. +/// control socket. On Windows self-detection covers script installs only; the explicit +/// request path works anywhere. /// internal sealed partial class RestartCoordinator : BackgroundService { readonly TimeProvider _time; @@ -78,18 +78,20 @@ internal void PrimeBaseline() { lock (_gate) _baseline = StatBinary(); } - static BinaryStat? StatProcessBinary() { - // Windows can't replace a running binary in place, so self-detection is a no-op - // there: returning null means BinaryChanged() never sees a change. Explicit - // restart requests still work (they set _pending directly). Keeping the OS check - // here — rather than in Tick() — leaves the gate logic OS-agnostic and testable. - if (OperatingSystem.IsWindows()) return null; + static BinaryStat? StatProcessBinary() => StatBinaryAt(Environment.ProcessPath, OperatingSystem.IsWindows(), File.Exists); + + /// + /// What the poll compares: the binary reached through a script install's current, which an + /// update switches rather than overwriting this file. Outside a script install Windows never replaces + /// a running binary in place, so there it returns null and BinaryChanged() never fires; explicit + /// restart requests still work. + /// + internal static BinaryStat? StatBinaryAt(string? processPath, bool isWindows, Func fileExists) { + if (processPath is null) return null; + if (isWindows && ScriptInstallLayout.FromBinary(processPath, fileExists) is null) return null; try { - // A script install flips `current` rather than replacing this file, so watch the path through it. - var p = Environment.ProcessPath is { } exe ? ScriptInstallLayout.Stabilize(exe) : null; - if (p is null) return null; - var fi = new FileInfo(p); + var fi = new FileInfo(ScriptInstallLayout.Stabilize(processPath, fileExists)); return fi.Exists ? new BinaryStat(fi.Length, fi.LastWriteTimeUtc.Ticks) : null; } catch { return null; // transient (file being swapped mid-install) diff --git a/src/Capacitor.Cli/Commands/ScriptUpdater.cs b/src/Capacitor.Cli/Commands/ScriptUpdater.cs index 653ec664f..aaca91e14 100644 --- a/src/Capacitor.Cli/Commands/ScriptUpdater.cs +++ b/src/Capacitor.Cli/Commands/ScriptUpdater.cs @@ -13,7 +13,7 @@ namespace Capacitor.Cli.Commands; /// Installs one release into a script install, the way the install script does: download the RID's /// archive, verify it against the release manifest's sha256, extract it into its own /// versions/<v> and switch current to it. A running kcap keeps its own files, because -/// no version directory that is in use is ever written. +/// no version directory that is in use is ever written or pruned. /// /// Verification fails closed: a manifest that cannot be read, has no valid checksum for this RID, /// or does not match the downloaded bytes installs nothing. @@ -23,6 +23,9 @@ public sealed partial class ScriptUpdater(KcapReleaseClient releases) { /// a test cannot make one off Windows. internal Action SwitchCurrent { get; init; } = DefaultSwitch; + /// Whether something runs from a version directory, so pruning must keep it. + internal Func InUse { get; init; } = AnyFileInUse; + internal async Task InstallAsync( ScriptInstallLayout layout, string version, string channel, string rid, TextWriter stdout, TextWriter stderr, CancellationToken ct) { @@ -53,8 +56,9 @@ internal async Task InstallAsync( return false; } - var dest = layout.VersionDir(version); - if (ActiveVersionDir(layout) is { } active && PathsEqual(active, dest)) { + var dest = layout.VersionDir(version); + var previous = ActiveVersionDir(layout); + if (previous is not null && PathsEqual(previous, dest)) { await stdout.WriteLineAsync($"kcap {version} is already the active version."); TryWriteMarker(layout, channel, stderr); return true; @@ -117,6 +121,9 @@ internal async Task InstallAsync( TryWriteMarker(layout, channel, stderr); await stdout.WriteLineAsync($"Installed kcap {version} to {layout.Root}"); + var pruned = PruneVersions(layout, keep: [dest, previous]); + if (pruned > 0) await stdout.WriteLineAsync($"Removed {pruned} unused kcap version{(pruned == 1 ? "" : "s")}."); + return true; } finally { try { Directory.Delete(work, recursive: true); } catch { /* best-effort */ } @@ -155,6 +162,57 @@ internal static void TryWriteMarker(ScriptInstallLayout layout, string channel, } } + /// + /// Deletes the version directories and moved-aside copies an update leaves behind, keeping + /// : the new version and the one it replaced, which a daemon or agent started + /// before the switch is likely still running. Anything else in use stays too. Returns how many went. + /// + internal int PruneVersions(ScriptInstallLayout layout, IEnumerable keep) { + var kept = keep.OfType().ToArray(); + var pruned = 0; + + IEnumerable candidates; + try { + candidates = new DirectoryInfo(layout.Versions).EnumerateDirectories() + .Where(d => d.LinkTarget is null) + .Where(d => ReleaseManifest.IsValidVersion(d.Name) || d.Name.StartsWith(".replaced-", StringComparison.Ordinal)) + .Where(d => !kept.Any(k => PathsEqual(k, d.FullName))) + .ToArray(); + } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { + return 0; + } + + foreach (var dir in candidates) { + if (InUse(dir.FullName)) continue; + + try { + dir.Delete(recursive: true); + pruned++; + } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { + // Started after the probe, or not ours to delete: the next update tries again. + } + } + + return pruned; + } + + /// + /// True when a file in cannot be opened exclusively for writing. A running + /// executable refuses that on Windows (sharing violation) and Linux (ETXTBSY); macOS does not, but + /// deleting a running binary there leaves the process its file. + /// + internal static bool AnyFileInUse(string dir) { + try { + foreach (var file in Directory.EnumerateFiles(dir, "*", SearchOption.AllDirectories)) { + using var _ = new FileStream(file, FileMode.Open, FileAccess.ReadWrite, FileShare.None); + } + + return false; + } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { + return true; + } + } + /// The version directory current points at, or null when it is not a link. internal static string? ActiveVersionDir(ScriptInstallLayout layout) => new DirectoryInfo(layout.Current).LinkTarget is { } target diff --git a/test/Capacitor.Cli.Daemon.Tests.Unit/Services/RestartCoordinatorTests.cs b/test/Capacitor.Cli.Daemon.Tests.Unit/Services/RestartCoordinatorTests.cs index 3bde09c57..3c00d8db0 100644 --- a/test/Capacitor.Cli.Daemon.Tests.Unit/Services/RestartCoordinatorTests.cs +++ b/test/Capacitor.Cli.Daemon.Tests.Unit/Services/RestartCoordinatorTests.cs @@ -120,4 +120,31 @@ public async Task Request_restart_when_busy_queues_then_fires_when_idle() { c.Tick(); await Assert.That(spy.Calls).IsEqualTo(1); } + + /// On Windows only a script install is watched: an update switches its current, so the + /// binary reached through it changes while the file the daemon runs from does not. + [Test] + public async Task On_Windows_a_script_install_is_watched_through_current() { + using var tmp = new TempDir(); + tmp.CreateFile("kcap/install.json", "{}"); + var running = tmp.CreateFile("kcap/versions/1.0.0/bin/kcap-daemon", "old"); + var viaCurrent = tmp.CreateFile("kcap/current/bin/kcap-daemon", "old"); + + var before = RestartCoordinator.StatBinaryAt(running, isWindows: true, File.Exists); + File.WriteAllText(viaCurrent, "the new version"); + var after = RestartCoordinator.StatBinaryAt(running, isWindows: true, File.Exists); + + await Assert.That(before).IsNotNull(); + await Assert.That(RestartDecision.BinaryChanged(before, after)).IsTrue(); + await Assert.That(File.ReadAllText(running)).IsEqualTo("old"); + } + + [Test] + public async Task On_Windows_a_binary_outside_a_script_install_is_not_watched() { + using var tmp = new TempDir(); + var exe = tmp.CreateFile("npm/bin/kcap-daemon", "x"); + + await Assert.That(RestartCoordinator.StatBinaryAt(exe, isWindows: true, File.Exists)).IsNull(); + await Assert.That(RestartCoordinator.StatBinaryAt(exe, isWindows: false, File.Exists)).IsNotNull(); + } } diff --git a/test/Capacitor.Cli.Tests.Unit/Commands/ScriptUpdaterTests.cs b/test/Capacitor.Cli.Tests.Unit/Commands/ScriptUpdaterTests.cs index 1615c67d1..2901ca696 100644 --- a/test/Capacitor.Cli.Tests.Unit/Commands/ScriptUpdaterTests.cs +++ b/test/Capacitor.Cli.Tests.Unit/Commands/ScriptUpdaterTests.cs @@ -1,4 +1,5 @@ using Capacitor.Cli.Commands; +using Capacitor.Cli.Core.Install; namespace Capacitor.Cli.Tests.Unit.Commands; @@ -11,9 +12,10 @@ public class ScriptUpdaterTests { static bool Unsupported => OperatingSystem.IsWindows(); static async Task<(bool Ok, string Out, string Err)> Install( - ScriptReleaseFixture f, string version, string channel = "latest", TimeSpan? downloadTimeout = null) { + ScriptReleaseFixture f, string version, string channel = "latest", TimeSpan? downloadTimeout = null, + Func? inUse = null) { var (stdout, stderr) = (new StringWriter(), new StringWriter()); - var ok = await new ScriptUpdater(f.Client(downloadTimeout)) + var ok = await new ScriptUpdater(f.Client(downloadTimeout)) { InUse = inUse ?? ScriptUpdater.AnyFileInUse } .InstallAsync(f.Layout, version, channel, ScriptReleaseFixture.Rid, stdout, stderr, CancellationToken.None); return (ok, stdout.ToString(), stderr.ToString()); } @@ -144,13 +146,13 @@ public async Task The_active_version_is_left_in_place_and_nothing_is_downloaded( /// A process started from a version before current moved away may still run there, so /// installing that version again moves the old directory aside instead of deleting it. [Test] - public async Task A_leftover_version_directory_is_moved_aside_not_deleted() { + public async Task A_leftover_version_directory_in_use_is_moved_aside_not_deleted() { Skip.When(Unsupported, "the fixture's current is a symlink"); using var f = new ScriptReleaseFixture(); f.LeaveVersion("1.5.0", "leftover"); f.Publish("1.5.0"); - var (ok, _, _) = await Install(f, "1.5.0"); + var (ok, _, _) = await Install(f, "1.5.0", inUse: dir => Path.GetFileName(dir).StartsWith(".replaced-", StringComparison.Ordinal)); await Assert.That(ok).IsTrue(); await Assert.That(File.ReadAllText(f.Layout.CurrentBin("kcap"))).IsEqualTo("kcap 1.5.0"); @@ -201,4 +203,50 @@ public async Task Switching_replaces_the_link_and_leaves_no_temporary_one() { await Assert.That(Directory.EnumerateFileSystemEntries(tmp.Path).Select(p => Path.GetFileName(p)!).ToArray()) .IsEquivalentTo(["versions", "current"]); } + + /// An update keeps the version it replaced, which a daemon started before the switch still runs, + /// and deletes the older ones. + [Test] + public async Task An_update_prunes_versions_older_than_the_one_it_replaced() { + Skip.When(Unsupported, "the fixture's current is a symlink"); + using var f = new ScriptReleaseFixture(); + f.LeaveVersion("0.9.0", "older"); + f.Publish("1.5.0"); + + var (ok, output, _) = await Install(f, "1.5.0"); + + await Assert.That(ok).IsTrue(); + await Assert.That(output).Contains("Removed 1 unused kcap version."); + await Assert.That(Directory.EnumerateDirectories(f.Layout.Versions).Select(p => Path.GetFileName(p)!).ToArray()) + .IsEquivalentTo(["1.0.0", "1.5.0"]); + } + + [Test] + public async Task Pruning_keeps_what_it_is_told_to_and_what_is_in_use() { + using var tmp = new TempDir(); + var layout = new ScriptInstallLayout(tmp.Path); + foreach (var name in new[] { "1.0.0", "1.1.0", "1.2.0", "1.3.0-beta.1", ".replaced-1.2.0-abc", ".update-12-def", ".staging-34", "notes" }) + tmp.CreateFile($"versions/{name}/bin/kcap"); + var updater = new ScriptUpdater(null!) { InUse = dir => Path.GetFileName(dir) == "1.1.0" }; + + var pruned = updater.PruneVersions(layout, keep: [layout.VersionDir("1.3.0-beta.1"), layout.VersionDir("1.2.0"), null]); + + await Assert.That(pruned).IsEqualTo(2); + await Assert.That(Directory.EnumerateDirectories(layout.Versions).Select(p => Path.GetFileName(p)!).ToArray()) + .IsEquivalentTo(["1.1.0", "1.2.0", "1.3.0-beta.1", ".update-12-def", ".staging-34", "notes"]); + } + + /// A file someone holds open marks its directory in use; a running binary refuses the same + /// exclusive write open on Windows and Linux. + [Test] + public async Task A_directory_with_a_file_held_open_is_in_use() { + using var tmp = new TempDir(); + var dir = tmp.CreateDir("1.0.0"); + var exe = tmp.CreateFile("1.0.0/bin/kcap"); + + await Assert.That(ScriptUpdater.AnyFileInUse(dir)).IsFalse(); + using (new FileStream(exe, FileMode.Open, FileAccess.Read, FileShare.Read)) + await Assert.That(ScriptUpdater.AnyFileInUse(dir)).IsTrue(); + await Assert.That(ScriptUpdater.AnyFileInUse(dir)).IsFalse(); + } }