diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 37f42fc..67a448f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,6 +107,40 @@ jobs: - name: verify formatting run: dotnet format DodoSSH.slnx --verify-no-changes --no-restore + # Avalonia's headless renderer is still Skia, and libSkiaSharp.so — which the layout + # test project copies into its own output — links against libfontconfig. Without that + # one library every test in DodoSSH.Client.App.Layout.Tests dies inside + # HeadlessUnitTestSession before it measures anything, and 69 tests fail for a reason + # none of their names or assertions mention. + # + # The library, and not fonts. Verified in a container where fc-list returns zero and + # the suite passes anyway: the application carries Inter itself, so nothing here needs + # a typeface installed — only the thing that would have gone looking for one. + - name: ensure skia's native dependency + run: | + set -eu + SUDO="" + [ "$(id -u)" -eq 0 ] || SUDO="sudo" + + if ldconfig -p 2>/dev/null | grep -q 'libfontconfig\.so\.1'; then + echo "libfontconfig present" + exit 0 + fi + + echo "Installing fontconfig" + if command -v apt-get >/dev/null 2>&1; then + $SUDO apt-get update -qq + $SUDO apt-get install -y --no-install-recommends libfontconfig1 + elif command -v apk >/dev/null 2>&1; then + $SUDO apk add --no-cache fontconfig + elif command -v dnf >/dev/null 2>&1; then + $SUDO dnf install -y fontconfig + else + echo "No package manager here, so Skia cannot be given its dependency and the" >&2 + echo "layout suite will fail to start. Add fontconfig to the runner's image." >&2 + exit 1 + fi + - name: build run: dotnet build DodoSSH.slnx --no-restore --configuration Release diff --git a/README.md b/README.md index 798ab6c..709ea39 100644 --- a/README.md +++ b/README.md @@ -347,10 +347,15 @@ something the specification never said. The one value it cannot take from a committed file is `Oidc:Authority`, since the container's port is assigned at start. Everything that authority points at is still the real realm. -Development and testing are currently **Windows-only**. Anything known or suspected to differ on -Linux and macOS is tracked in [`docs/platform-flags.md`](docs/platform-flags.md), along with the -deployment gotchas that have already cost time once. Read it before assuming something works -off-Windows. +Development is Windows-first, but **the full suite now runs on Linux too**, and CI runs it there on +every change. Getting there cost three fixes rather than none, and each was a real difference instead +of a test being fussy: the local file pane built its roots bar from every mount the kernel holds, one +assertion recognised the profile directory only by its Windows capitalisation, and the layout harness +pinned a COM error that only Windows raises. macOS is still unverified. + +Anything known or suspected to differ is tracked in +[`docs/platform-flags.md`](docs/platform-flags.md), along with the deployment gotchas that have already +cost time once. Read it before assuming something works off-Windows. Android has been audited and scoped, but not started: [`docs/android-port.md`](docs/android-port.md) records what ports as it stands (most of the core), what diff --git a/docs/platform-flags.md b/docs/platform-flags.md index 0888c35..7dc117b 100644 --- a/docs/platform-flags.md +++ b/docs/platform-flags.md @@ -1,8 +1,10 @@ # Platform flags Things known or suspected to behave differently outside Windows, plus deployment gotchas that -have already cost time once. **Development and testing are currently Windows-only**, so anything -here marked *unverified* has not run on the platform in question and must not be assumed to work. +have already cost time once. Development is Windows-first, but **the full test suite now runs on +Linux in CI on every change**, so a Linux claim here is usually a measurement now rather than a +suspicion. **macOS is still untested**, and anything marked *unverified* has not run on the platform +in question and must not be assumed to work. Each entry says what the risk is, why it matters, and what to do about it. Delete an entry when it has been verified or made moot — not when it merely stops being convenient. @@ -28,6 +30,17 @@ or notarization fails with an error that does not name the offending file. ## Desktop client +**The local pane's roots bar is built differently per platform, and has to be.** On Windows it is the +ready drives, from `DriveInfo.GetDrives`. On Unix that same call answers with every mount the kernel +holds — around forty on an ordinary laptop, counting `/proc`, `/sys/fs/bpf`, one per installed snap and +`/run/user/1000/doc` — and the transfers screen draws a button per root, so the bar ran to roughly five +thousand pixels inside an eight-hundred pixel window. *Fixed* in `LocalDirectory.Roots`, which on Unix +returns the root, the user's home, and whatever is mounted under `/run/media/`, `/media`, `/mnt` +or `/Volumes`. Do not try to filter `GetDrives` instead: `DriveType` reports `Fixed` for `/` and `/home` +but also for every squashfs snap, for `efivarfs` and for `tracefs`, while `/boot/efi` comes back +`Removable`, and `DriveFormat` would need a hand-kept list of every virtual filesystem Linux may grow. +Found by the layout suite on its first Linux run, which is the argument for that suite existing. + **The WebView runs on Windows.** `Avalonia.Controls.WebView` 12.0.1 (MIT, no licence key) hosts the terminal page: WebView2 launches, navigates to the loopback page, runs its JavaScript and completes the WebSocket handshake. Verified by observing an established TCP connection from `msedgewebview2` to the data @@ -411,6 +424,14 @@ be honest about: the cache offers no protection against another process running ## Build and CI +**The layout suite needs `libfontconfig` installed, and nothing else.** Avalonia's headless renderer is +still Skia, and `libSkiaSharp.so` — which the test project copies into its own output — links against +it. On an image without it, every one of the 69 tests fails inside `HeadlessUnitTestSession` before +measuring anything, reported as a `TypeInitializationException` on `SkiaSharp.SKImageInfo` that names +none of the tests' actual subjects. The CI job installs it. Worth being precise that this is the +library and not fonts: verified in a container where `fc-list` returns zero and the suite passes anyway, +because the application carries Inter itself. + **Integration tests need a Docker daemon** (Testcontainers). They run on `ubuntu-latest` in CI. macOS runners have no Docker daemon, and the Windows CI job is deliberately build-only. So anything proved by an integration test is proved on Linux only — which is the right place for diff --git a/src/DodoSSH.Client.Transfer/LocalDirectory.cs b/src/DodoSSH.Client.Transfer/LocalDirectory.cs index c9514cc..93bfb6e 100644 --- a/src/DodoSSH.Client.Transfer/LocalDirectory.cs +++ b/src/DodoSSH.Client.Transfer/LocalDirectory.cs @@ -98,26 +98,89 @@ public static class LocalDirectory /// Where the local pane can start from: the drives on Windows, and the root elsewhere. /// /// - /// Ready drives only. An empty optical drive or a disconnected network mapping is listed by + /// + /// Ready drives only, on Windows. An empty optical drive or a disconnected network mapping is listed by /// and throws on the first attempt to read it, which would put a row on /// screen whose only behaviour is an error. + /// + /// + /// Unix does not go through at all, and the summary above was true of + /// the intent long before it was true of the code. That call returns every mount the kernel holds, which + /// on an ordinary laptop is upwards of forty: /proc, /sys/fs/bpf, one per installed snap, + /// /run/user/1000/doc. The transfers screen draws a button per root, so the bar ran to some five + /// thousand pixels inside an eight-hundred pixel window — found by the layout suite the first time it + /// ran on Linux, which is the whole reason that suite measures rather than eyeballs. + /// + /// + /// Filtering that list is not workable, and it is worth writing down why so nobody tries it again. + /// Neither property that looks like it would separate a real volume from the kernel's bookkeeping does: + /// answers Fixed for / and /home, but equally for + /// every squashfs snap, for efivarfs and for tracefs, while /boot/efi comes back + /// Removable. would need a hand-maintained list of every + /// virtual filesystem Linux might grow, which is a list that is wrong the moment it is written. + /// + /// + /// So this names what somebody would actually want rather than subtracting what they would not: the + /// root, their home, and whatever is mounted where removable media conventionally mounts. A volume + /// anywhere else is still reachable by navigating from /, which is what the pane is for. + /// /// public static IReadOnlyList Roots() { var roots = new List(); - foreach (var drive in DriveInfo.GetDrives()) + if (OperatingSystem.IsWindows()) + { + foreach (var drive in DriveInfo.GetDrives()) + { + try + { + if (drive.IsReady) + { + roots.Add(drive.RootDirectory.FullName); + } + } + catch (IOException) + { + // A drive that fails even to answer whether it is ready. Nothing to show. + } + } + + return roots; + } + + roots.Add("/"); + + var home = Environment.GetFolderPath(Environment.SpecialFolder.UserProfile); + if (!string.IsNullOrEmpty(home) && Directory.Exists(home)) + { + roots.Add(home); + } + + // Where a desktop mounts a stick. /run/media/ is udisks2's, and so the one that matters on a + // current distribution; /media and /mnt stay because a hand-written fstab still uses them, and + // /Volumes is macOS, which reaches this branch too. + string[] mountParents = + [ + "/run/media/" + Environment.UserName, + "/media", + "/mnt", + "/Volumes", + ]; + + foreach (var parent in mountParents) { try { - if (drive.IsReady) + if (Directory.Exists(parent)) { - roots.Add(drive.RootDirectory.FullName); + roots.AddRange(Directory.EnumerateDirectories(parent)); } } - catch (IOException) + catch (Exception e) when (e is IOException or UnauthorizedAccessException) { - // A drive that fails even to answer whether it is ready. Nothing to show. + // A media directory that will not be listed contributes nothing. Failing to offer a + // shortcut to a stick is not a reason to have no roots bar at all. } } diff --git a/tests/DodoSSH.Client.App.Layout.Tests/LayoutHarnessTests.cs b/tests/DodoSSH.Client.App.Layout.Tests/LayoutHarnessTests.cs index a3ca8df..fcde35b 100644 --- a/tests/DodoSSH.Client.App.Layout.Tests/LayoutHarnessTests.cs +++ b/tests/DodoSSH.Client.App.Layout.Tests/LayoutHarnessTests.cs @@ -144,6 +144,19 @@ public sealed class LayoutHarnessTests // That is the reason this harness measures the extracted controls rather than MainWindow: each of // them is a part with a height budget to blow, and none of them has a native child window in it. If a // future Avalonia makes the adapter lazy, this test starts failing and the harness can be widened. + // + // All of which is a fact about Windows. WebView2 is a Win32 component and RPC_E_CHANGED_MODE is a COM + // error code; on Linux the terminal's adapter is a different implementation with no apartment to + // disagree about, so showing the window simply works and Should.Throw has nothing to catch. Skipped + // rather than rewritten to accept either outcome, because the assertion is the documentation here — + // a version that passed on both platforms would have stopped recording the constraint it exists to + // record. What the rest of the suite relies on, that the harness measures extracted controls, holds + // on every platform regardless. + if (!OperatingSystem.IsWindows()) + { + Assert.Skip("WebView2's apartment requirement, and the COM error it raises, are Windows-only."); + } + await LayoutHarness.OnTheUiThreadAsync( () => { diff --git a/tests/DodoSSH.Client.Session.Tests/ClientPathsTests.cs b/tests/DodoSSH.Client.Session.Tests/ClientPathsTests.cs index a7f3a6a..9c8eda1 100644 --- a/tests/DodoSSH.Client.Session.Tests/ClientPathsTests.cs +++ b/tests/DodoSSH.Client.Session.Tests/ClientPathsTests.cs @@ -27,7 +27,12 @@ public sealed class ClientPathsTests Path.IsPathFullyQualified(paths.DataDirectory).ShouldBeTrue(paths.DataDirectory); - paths.DataDirectory.Contains("odoSSH", StringComparison.Ordinal) + // OrdinalIgnoreCase, and the casing is the point rather than an oversight. ClientPaths spells the + // folder DodoSSH on Windows and dodossh on Unix on purpose — one follows the platform's title-cased + // convention, the other the lower-case dotfile one. The Ordinal "odoSSH" this used to look for was + // clever enough to survive either spelling of the leading D and still only ever matched Windows, + // which went unnoticed for exactly as long as nothing ran the suite anywhere else. + paths.DataDirectory.Contains("dodossh", StringComparison.OrdinalIgnoreCase) .ShouldBeTrue($"'{paths.DataDirectory}' should be identifiable as ours"); }