diff --git a/docs/manual-checks.md b/docs/manual-checks.md index 61a3d7e..3cc78d9 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -3538,3 +3538,16 @@ Register a device, sync something, then `sudo dnf remove dodossh`. make an uninstall delete a user's un-synced work — the Linux spelling of the mistake ADR 0013 decision 2 exists to prevent. Files left behind under `/opt/DodoSSH` are files the package did not own, which means something wrote into the install directory at runtime and would survive a reinstall. + +### 19.13 A copy at the same path that rpm does not own is offered nothing + +On a distribution without rpm — Arch, say, with a locally built package that puts the nightly tree at +`/opt/DodoSSH.Nightly` — open PREFERENCES → UPDATES. + +**Pass:** **CHECK NOW** is disabled and the row says this copy was installed outside the `dodossh-nightly` +RPM and should be updated with the package manager that installed it. `~/.local/share/dodossh.nightly/updates/` +stays empty however long it runs. + +**Failure means:** a banner offering a nightly is the path being taken as proof the RPM is installed, or an +rpm that cannot answer being read as an old version — the fault this check was written for, which offered +every nightly as newer and fetched a 56 MB package the machine could never install, every six hours. diff --git a/src/DodoSSH.Client.App/Platform/LinuxUpdateChannel.cs b/src/DodoSSH.Client.App/Platform/LinuxUpdateChannel.cs index f64f3d2..ef7b759 100644 --- a/src/DodoSSH.Client.App/Platform/LinuxUpdateChannel.cs +++ b/src/DodoSSH.Client.App/Platform/LinuxUpdateChannel.cs @@ -127,17 +127,61 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel this.installedVersion = installedVersion; this.handToInstaller = handToInstaller; - packageName = string.Equals(channel, "nightly", StringComparison.Ordinal) - ? "dodossh-nightly" - : "dodossh"; + packageName = PackageFor(channel); + } + + /// The channel for a copy under the RPM's install root, if rpm says the package is there. + internal static IUpdateChannel ForInstall(string channel) => + ForInstall(channel, InstalledRpmVersion, () => new LinuxUpdateChannel(channel)); + + /// + /// The channel for a copy under the RPM's install root, with rpm supplied, which is how the tests + /// reach it. + /// + /// + /// + /// The path says where this copy is and only rpm says who put it there. + /// UpdateChannels.LinuxChannel has already established the first, and on an RPM distribution + /// that is the whole story. It is not on any other: a locally built pacman or dpkg package that mirrors + /// the spec puts the same tree at the same path, and this channel then offered every nightly as newer + /// than a version nobody could read, fetched a package that machine has no way to install, and did it + /// again every six hours. + /// + /// + /// Asked once, at composition, for the reason the rest of this decision is: who installed this copy + /// does not change while it runs. It costs one rpm launch at start on a machine whose path + /// already matched, and nothing at all anywhere else. + /// + /// + /// The build's channel, which names the package. + /// What rpm says is installed under a package name, or null. + /// The channel to use when rpm does own this copy. + internal static IUpdateChannel ForInstall( + string channel, + Func installedVersion, + Func whenOwned) + { + ArgumentException.ThrowIfNullOrEmpty(channel); + ArgumentNullException.ThrowIfNull(installedVersion); + ArgumentNullException.ThrowIfNull(whenOwned); + + var package = PackageFor(channel); + + return installedVersion(package) is null + ? new UnavailableUpdateChannel( + $"This copy of DodoSSH was installed outside the {package} RPM — rpm has no record of that " + + "package here — and an RPM is the only kind of update it can fetch. Update it with the " + + "package manager that installed it.") + : whenOwned(); } /// /// /// True unconditionally, because whether this copy is one that can be updated was already decided by - /// — a build run from a checkout never reaches this class - /// at all. The same split Velopack's side uses, where IsInstalled is checked once at - /// composition rather than at every call site. + /// and — a build run from + /// a checkout, or installed by something other than rpm, never reaches this class at all. The same + /// split Velopack's side uses, where IsInstalled is checked once at composition rather than at + /// every call site. /// public bool IsSupported => true; @@ -184,9 +228,20 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel $"The {channel} release's {manifestName} could not be read as a manifest."); } + // ◆ No answer from rpm is a failure, never a version. ForInstall only built this class because + // rpm answered, so silence now is a package removed under a running process or an rpm that + // stopped working — and any stand-in number would make every published build look newer and + // fetch a package for a machine that cannot install it, which is what this once did. + if (installedVersion(packageName) is not { } installed) + { + throw new InvalidOperationException( + $"rpm has no record of {packageName}, so there is no installed version to compare the " + + $"{channel} release against."); + } + // The one place null is returned, and it means what null is documented to mean: this build is // current. Compared the way rpm compares, because rpm is what accepts or refuses the install. - if (RpmVersion.Compare(manifest.RpmVersion, installedVersion(packageName) ?? "0") <= 0) + if (RpmVersion.Compare(manifest.RpmVersion, installed) <= 0) { return null; } @@ -324,6 +379,12 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel ? $"{RepositoryApi}/releases/tags/nightly-desktop" : $"{RepositoryApi}/releases/latest"; + /// The package a channel's builds are published as. + private static string PackageFor(string channel) => + string.Equals(channel, "nightly", StringComparison.Ordinal) + ? "dodossh-nightly" + : "dodossh"; + private static string? Asset(LinuxForgeRelease? release, string name) => release?.Assets?.FirstOrDefault( asset => string.Equals(asset.Name, name, StringComparison.Ordinal))?.DownloadUrl; @@ -365,9 +426,11 @@ internal sealed class LinuxUpdateChannel : IUpdateChannel /// scripts/ci-linux-rpm.sh performs, and two copies of one rule is a pair that can disagree. /// /// - /// A machine that cannot answer gets null, which the caller reads as 0 and makes every - /// published build look newer. That is the wrong way to fail, chosen deliberately: the failure is then - /// an install PackageKit declines rather than a security fix that never arrives. + /// A machine that cannot answer gets null — no rpm at all, or one with no such package — and + /// null means "rpm does not own this copy", never a version. It used to be read as 0, on the + /// argument that the worst case was an install PackageKit declined. That holds only where PackageKit + /// installs RPMs: on any other distribution it made every published build look newer, forever, and + /// fetched a package that machine could not install. See . /// /// private static string? InstalledRpmVersion(string packageName) diff --git a/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs b/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs index aa48656..417c501 100644 --- a/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs +++ b/src/DodoSSH.Client.App/Platform/VelopackUpdateChannel.cs @@ -66,21 +66,23 @@ internal static class UpdateChannels } } - /// The Linux channel, if this copy is one a package manager put here. + /// The Linux channel, if this copy is one the RPM put here. /// /// /// The question Velopack answers with IsInstalled, answered by looking at where this process - /// lives. The RPM owns /opt/DodoSSH or /opt/DodoSSH.Nightly — the spec puts it there - /// and nothing else writes to /opt under those names — so a build running from anywhere else is - /// a checkout somebody started, an extracted archive, or a tree copied by hand. None of those can be - /// updated by installing a package over them, and offering it would be an offer that cannot be carried - /// out. + /// lives and then asking rpm. The RPM owns /opt/DodoSSH or /opt/DodoSSH.Nightly — the + /// spec puts it there — so a build running from anywhere else is a checkout somebody started, an + /// extracted archive, or a tree copied by hand. None of those can be updated by installing a package + /// over them, and offering it would be an offer that cannot be carried out. /// /// - /// Checked by path rather than by asking rpm which package owns the executable, which would be the - /// more thorough answer and costs a process launch on every start for a distinction nothing acts on - /// differently. The install root is a constant in two places — here and the packaging script — and - /// they are named in each other's comments. + /// The path is necessary and not sufficient. It used to be the whole check, on the argument + /// that asking rpm cost a process launch for a distinction nothing acted on differently. Something + /// does: a locally built pacman package that mirrors the spec puts the same tree at the same path, + /// and that copy was offered every nightly as an RPM it could never install. So a matching path is + /// then put to rpm — see — and the launch is paid + /// only where the path already matched. The install root is a constant in two places — here and the + /// packaging script — and they are named in each other's comments. /// /// [SupportedOSPlatform("linux")] @@ -91,7 +93,7 @@ internal static class UpdateChannels var here = AppContext.BaseDirectory.TrimEnd(Path.DirectorySeparatorChar); return string.Equals(here, root, StringComparison.Ordinal) - ? new LinuxUpdateChannel(DesktopChannel.Name) + ? LinuxUpdateChannel.ForInstall(DesktopChannel.Name) : new UnavailableUpdateChannel(); } } diff --git a/src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml b/src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml index 3c77313..4a227e1 100644 --- a/src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml +++ b/src/DodoSSH.Client.App/Views/SettingsGeneralPage.axaml @@ -84,7 +84,10 @@ IsVisible="{Binding Updates.Status, Converter={x:Static StringConverters.IsNotNullOrEmpty}}" /> + IsVisible="{Binding Updates.IsUnsupportedForTheUsualReason}" /> + diff --git a/src/DodoSSH.Client.Session/ClientUpdates.cs b/src/DodoSSH.Client.Session/ClientUpdates.cs index d71a155..d24ed53 100644 --- a/src/DodoSSH.Client.Session/ClientUpdates.cs +++ b/src/DodoSSH.Client.Session/ClientUpdates.cs @@ -141,9 +141,32 @@ public interface IUpdateChannel /// : the caller then has one shape to write against, and "no /// updater here" is a thing the interface can express rather than a case every call site has to remember. /// +/// +/// One situation outside those four carries a . A Linux copy sitting exactly +/// where the RPM puts it that no RPM owns — built locally and installed by another distribution's package +/// manager. It looks installed, so the sentence about checkouts and unzipped copies would describe the +/// wrong machine, and the person reading it needs telling which updater is theirs. +/// /// public sealed class UnavailableUpdateChannel : IUpdateChannel { + /// Creates one for the situations that need no telling apart. + public UnavailableUpdateChannel() + { + } + + /// Creates one whose situation is worth a sentence of its own. + /// What to tell the user, as a sentence they can act on. + public UnavailableUpdateChannel(string reason) + { + ArgumentException.ThrowIfNullOrWhiteSpace(reason); + + Reason = reason; + } + + /// Why this copy cannot replace itself, where that is not the usual why. + public string? Reason { get; } + /// public bool IsSupported => false; diff --git a/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs index 9f94ff8..e2e30d0 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/UpdateViewModel.cs @@ -126,6 +126,7 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab this.restart = restart; CurrentVersion = updates.CurrentVersion; + UnsupportedReason = (updates as UnavailableUpdateChannel)?.Reason; isAutomatic = settings.Read().AutomaticUpdateChecks; state = updates.IsSupported ? UpdateState.Idle : UpdateState.Unsupported; } @@ -133,6 +134,16 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab /// What this build calls itself. internal string CurrentVersion { get; } + /// Why this copy cannot replace itself, when the channel had something particular to say. + /// + /// Null in the ordinary cases, where the preferences screen's own sentence about checkouts and unzipped + /// copies is the right one. See for the case where it is not. + /// + internal string? UnsupportedReason { get; } + + /// Unsupported, with nothing more particular to say than the screen's own sentence. + internal bool IsUnsupportedForTheUsualReason => IsUnsupported && UnsupportedReason is null; + [ObservableProperty] private UpdateState state; @@ -248,6 +259,7 @@ internal sealed partial class UpdateViewModel : ObservableObject, IAsyncDisposab { OnPropertyChanged(nameof(IsSupported)); OnPropertyChanged(nameof(IsUnsupported)); + OnPropertyChanged(nameof(IsUnsupportedForTheUsualReason)); OnPropertyChanged(nameof(IsChecking)); OnPropertyChanged(nameof(IsDownloading)); OnPropertyChanged(nameof(IsReady)); diff --git a/tests/DodoSSH.Client.App.Tests/LinuxUpdateChannelTests.cs b/tests/DodoSSH.Client.App.Tests/LinuxUpdateChannelTests.cs index 14fd444..6bb2530 100644 --- a/tests/DodoSSH.Client.App.Tests/LinuxUpdateChannelTests.cs +++ b/tests/DodoSSH.Client.App.Tests/LinuxUpdateChannelTests.cs @@ -86,19 +86,86 @@ public sealed class LinuxUpdateChannelTests : IDisposable } /// - /// A machine that cannot say what it has treats everything as newer. + /// A machine that cannot say what it has is told so, and is offered nothing. /// /// - /// The wrong way to fail, chosen deliberately and for the reason the android channel gives: the cost is - /// an install the package manager declines, and the cost of the other direction is a security fix that - /// never arrives. + /// This used to read the silence as 0 and offer everything, on the argument that the cost was + /// an install the package manager declined. On a machine with no rpm at all — a pacman package of the + /// same tree at the same path — that was every nightly, every six hours, as a 56 MB download nothing + /// could install. Unknown is not older. /// [Fact] - public async Task AnUnknownInstalledVersionTreatsEverythingAsNewer() + public async Task AnUnknownInstalledVersionThrowsRatherThanOfferingAnything() { - var channel = Channel(RandomNumberGenerator.GetBytes(16), published: "0.0.1~alpha.0.1-1", installed: null); + var handed = new List(); + var channel = Channel( + RandomNumberGenerator.GetBytes(16), + published: "0.0.1~alpha.0.200-1", + installed: null, + handToInstaller: handed.Add); - (await channel.CheckAsync(Token)).ShouldNotBeNull(); + await Should.ThrowAsync(async () => await channel.CheckAsync(Token)); + + await Should.ThrowAsync( + async () => await channel.DownloadAsync( + new DodoSSH.Client.Session.AvailableUpdate("0.0.1-alpha.0.200"), + new Progress(), + Token)); + + handed.ShouldBeEmpty(); + Directory.Exists(Path.Combine(directory, "updates")).ShouldBeFalse(); + } + + /// + /// A copy rpm does not own gets no channel at all, and a sentence saying whose it is to update. + /// + /// + /// The case that matters: the right path and no rpm — a locally built pacman package that mirrors the + /// spec. No binary and no such package both arrive here as null. + /// + [Fact] + public void ACopyRpmDoesNotOwn_IsUnavailableAndSaysWhy() + { + var asked = new List(); + + var updates = LinuxUpdateChannel.ForInstall( + "nightly", + package => + { + asked.Add(package); + + return null; + }, + () => throw new InvalidOperationException("Built a channel for a copy rpm does not own.")); + + asked.ShouldBe(["dodossh-nightly"]); + updates.IsSupported.ShouldBeFalse(); + + var reason = updates.ShouldBeOfType().Reason; + + reason.ShouldNotBeNull(); + reason.ShouldContain("outside the dodossh-nightly RPM"); + reason.ShouldContain("package manager that installed it"); + } + + [Fact] + public void ACopyRpmOwns_GetsTheChannel() + { + var asked = new List(); + var owned = new FakeUpdateChannel(); + + var updates = LinuxUpdateChannel.ForInstall( + "release", + package => + { + asked.Add(package); + + return "0.0.1-1"; + }, + () => owned); + + asked.ShouldBe(["dodossh"]); + updates.ShouldBeSameAs(owned); } [Fact] diff --git a/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs b/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs index 417a724..2248fe5 100644 --- a/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs +++ b/tests/DodoSSH.Client.App.Tests/UpdateFlowTests.cs @@ -80,6 +80,32 @@ public sealed class UpdateFlowTests : IDisposable Build().CurrentVersion.ShouldBe("0.4.2"); } + [Fact] + public void ABuildThatCannotReplaceItself_ForAReasonOfItsOwn_SaysThatInstead() + { + var updates = new UpdateViewModel( + new UnavailableUpdateChannel("Update it with the package manager that installed it."), + new ClientSettingsStore(Paths), + TimeProvider.System, + () => 0, + restart: _ => Task.CompletedTask); + + updates.IsUnsupported.ShouldBeTrue(); + updates.UnsupportedReason.ShouldBe("Update it with the package manager that installed it."); + updates.IsUnsupportedForTheUsualReason.ShouldBeFalse(); + } + + [Fact] + public void ABuildThatCannotReplaceItself_ForTheUsualReasons_LeavesTheScreenItsOwnSentence() + { + channel.IsSupported = false; + + var updates = Build(); + + updates.UnsupportedReason.ShouldBeNull(); + updates.IsUnsupportedForTheUsualReason.ShouldBeTrue(); + } + [Fact] public async Task ABackgroundPassThatFindsNothing_SaysNothing() {