diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6379d579..7e11de9c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,3 +37,28 @@ jobs: run: bun test env: TEST_DATABASE_URL: postgres://postgres:postgres@localhost:5432/nestri + + # Scoped to `nesdoctor` deliberately. The rest of the Rust half has never + # been under CI, so widening this to `--workspace` would turn every PR red + # for reasons unrelated to the PR. Widen it one member at a time, as each is + # made to pass. + nesdoctor: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Rust toolchain + run: rustup toolchain install stable --profile minimal --component clippy,rustfmt --no-self-update + - uses: Swatinem/rust-cache@v2 + with: + workspaces: ". -> target" + - name: Format + run: cargo fmt -p nesdoctor -- --check + - name: Clippy + run: cargo clippy -p nesdoctor --all-targets -- -D warnings + - name: Test + run: cargo test -p nesdoctor + # Runs without touching the network, so this stays fast and cannot fail + # on a runner's egress rules. The network path is exercised by the + # release workflow's smoke test, where it belongs. + - name: Runs at all + run: cargo run -p nesdoctor -- --quiet --no-net --no-steam --json "$RUNNER_TEMP/nd.json" < /dev/null diff --git a/.github/workflows/release-nesdoctor.yml b/.github/workflows/release-nesdoctor.yml new file mode 100644 index 00000000..3f0ce801 --- /dev/null +++ b/.github/workflows/release-nesdoctor.yml @@ -0,0 +1,156 @@ +# Builds the binaries people actually download. +# +# `nesdoctor` is handed to strangers and asked to be trusted, so the release +# artefacts are built here and nowhere else: a binary someone produced on their +# laptop and uploaded is not auditable, however honest they are. +# +# Tag `nesdoctor-v0.1.0` to cut a release, or run it by hand to check the +# matrix still builds. +name: release nesdoctor + +on: + push: + tags: ["nesdoctor-v*"] + workflow_dispatch: + +permissions: + contents: write + +jobs: + build: + name: ${{ matrix.target }} + runs-on: ${{ matrix.os }} + strategy: + # One broken target must not suppress the others: a Windows failure + # should still leave the Linux binary available to look at. + fail-fast: false + matrix: + include: + # musl and not glibc, so one Linux binary runs on every distro + # regardless of its glibc version. Static linking is the whole reason + # this target is here. + - os: ubuntu-latest + target: x86_64-unknown-linux-musl + bin: nesdoctor + - os: windows-latest + target: x86_64-pc-windows-msvc + bin: nesdoctor.exe + - os: macos-latest + target: aarch64-apple-darwin + bin: nesdoctor + # Intel Macs are still most of the installed base and they are + # clients, which is a category we want answers from. + - os: macos-13 + target: x86_64-apple-darwin + bin: nesdoctor + + steps: + - uses: actions/checkout@v4 + + - name: Rust toolchain + run: | + rustup toolchain install stable --profile minimal --no-self-update + rustup target add ${{ matrix.target }} + + # `ring`, under rustls, compiles C. On musl that needs the musl C + # toolchain present or the build fails at link time with an error that + # does not mention TLS at all. + - name: musl toolchain + if: matrix.target == 'x86_64-unknown-linux-musl' + run: sudo apt-get update && sudo apt-get install -y musl-tools + + - name: Build + run: cargo build --release -p nesdoctor --target ${{ matrix.target }} + + # The binary is run here on purpose, and this step is the reason this + # workflow is worth having rather than a `cargo build` someone trusts. + # + # A static musl build resolves root certificates through the host trust + # store, so TLS can compile perfectly and then fail on the machine it is + # shipped to -- which would break the network test, the one feature + # anybody runs this for, silently and only for other people. Running the + # real thing here catches that class of failure before a tag exists. + - name: Smoke test — the whole run, network included + shell: bash + run: | + set -euo pipefail + BIN="target/${{ matrix.target }}/release/${{ matrix.bin }}" + test -x "$BIN" + # --no-steam because a runner has no Steam and the consent prompt + # would block; stdin is closed so any prompt reads as a skip. + OUT="$("$BIN" --quiet --no-steam --json "$RUNNER_TEMP/nd.json" < /dev/null)" + echo "$OUT" + # The summary line must exist and must not have fallen back to + # "net=unmeasured", which is what a TLS or upload failure looks like. + grep -q "nesdoctor " <<<"$OUT" + if grep -q "net=unmeasured" <<<"$OUT"; then + echo "::error::network test did not run in the built binary — \ + TLS or the upload sink failed at runtime, which is exactly the \ + failure this step exists to catch" + exit 1 + fi + test -s "$RUNNER_TEMP/nd.json" + + - name: Package + shell: bash + run: | + set -euo pipefail + mkdir -p dist + NAME="nesdoctor-${{ matrix.target }}" + cp "target/${{ matrix.target }}/release/${{ matrix.bin }}" "dist/$NAME${{ matrix.bin == 'nesdoctor.exe' && '.exe' || '' }}" + cd dist + # Checksums beside the binary, because "download this and run it" is + # only a reasonable request if the file can be verified. + if command -v sha256sum >/dev/null; then + sha256sum * > "$NAME.sha256" + else + shasum -a 256 * > "$NAME.sha256" + fi + cat *.sha256 + + - uses: actions/upload-artifact@v4 + with: + name: nesdoctor-${{ matrix.target }} + path: dist/* + if-no-files-found: error + + release: + # Only on a tag. A manual run builds and smoke-tests the matrix without + # publishing anything, which is what you want while iterating. + if: startsWith(github.ref, 'refs/tags/nesdoctor-v') + needs: build + runs-on: ubuntu-latest + steps: + - uses: actions/download-artifact@v4 + with: + path: dist + merge-multiple: true + + - name: Collect checksums + run: | + cd dist + cat *.sha256 | sort -k2 > SHA256SUMS + rm -f nesdoctor-*.sha256 + ls -la + cat SHA256SUMS + + - uses: softprops/action-gh-release@v2 + with: + files: dist/* + generate_release_notes: true + body: | + **nesdoctor** — checks whether this machine can host a Nestri box, + and measures what your connection actually does under load. + + Nothing is uploaded. There is no server to upload to: the network + test talks to Cloudflare's public speed-test sink and to `1.1.1.1`, + neither of which is ours. The output is a line on your terminal that + you may choose to paste somewhere. + + Download the file for your platform, verify it against + `SHA256SUMS`, and run it. On macOS and Linux you will need + `chmod +x` first. Source is in `apps/nesdoctor`. + + The number worth running it for is **added latency under load**. + Everybody knows their download speed; almost nobody has seen this + one, and for anything interactive it is the figure that decides it. diff --git a/apps/nesdoctor/src/ask.rs b/apps/nesdoctor/src/ask.rs index df30c881..5e228291 100644 --- a/apps/nesdoctor/src/ask.rs +++ b/apps/nesdoctor/src/ask.rs @@ -110,7 +110,9 @@ pub fn run(ctx: &Ctx) -> Answers { // program is and what it prints. if ctx.steam_present { println!(); - println!("\x1b[1mOne permission.\x1b[0m Steam keeps, on this disk, the size of each game you"); + println!( + "\x1b[1mOne permission.\x1b[0m Steam keeps, on this disk, the size of each game you" + ); println!("have installed and the time you last launched it. Reading it answers three"); println!("things we would otherwise have to ask you badly: how big a library is, what"); println!("shape it has, and what hours you actually play."); diff --git a/apps/nesdoctor/src/hostreq.rs b/apps/nesdoctor/src/hostreq.rs index f435e784..d2f42a09 100644 --- a/apps/nesdoctor/src/hostreq.rs +++ b/apps/nesdoctor/src/hostreq.rs @@ -90,13 +90,7 @@ pub fn probe(sys: &SysInfo) -> HostReport { c.push(Check { id: "kvm", what: "/dev/kvm present and openable", - state: if kvm_rw { - State::Pass - } else if kvm { - State::Fail - } else { - State::Fail - }, + state: if kvm_rw { State::Pass } else { State::Fail }, detail: match (kvm, kvm_rw) { (true, true) => "yes".into(), (true, false) => { @@ -104,7 +98,9 @@ pub fn probe(sys: &SysInfo) -> HostReport { is disabled in firmware" .into() } - _ => "missing — enable SVM/VT-x in firmware, or this is a VM without nested virt".into(), + _ => { + "missing — enable SVM/VT-x in firmware, or this is a VM without nested virt".into() + } }, blocking: true, }); @@ -131,7 +127,11 @@ pub fn probe(sys: &SysInfo) -> HostReport { State::Fail }, detail: if let Some(g) = usable.first() { - format!("{} at {}", g.name, g.render_node.clone().unwrap_or_default()) + format!( + "{} at {}", + g.name, + g.render_node.clone().unwrap_or_default() + ) } else if nvidia_only { "NVIDIA only. Nvidia needs virtio-nvgpu, which is not funded — so this card \ cannot host today. It is a fine client." @@ -293,16 +293,19 @@ pub fn probe(sys: &SysInfo) -> HostReport { detail: if io_ctrl { "present at the root".into() } else { - "not available. Without it a per-box io.max silently has nothing to attach to." - .into() + "not available. Without it a per-box io.max silently has nothing to attach to.".into() }, blocking: false, }); // --- virtiofsd -------------------------------------------------------- - let virtiofsd = ["/usr/bin/virtiofsd", "/usr/libexec/virtiofsd", "/usr/lib/virtiofsd"] - .iter() - .find(|p| sys::exists(p)); + let virtiofsd = [ + "/usr/bin/virtiofsd", + "/usr/libexec/virtiofsd", + "/usr/lib/virtiofsd", + ] + .iter() + .find(|p| sys::exists(p)); c.push(Check { id: "virtiofsd", what: "virtiofsd, for shared directories into the guest", @@ -322,9 +325,7 @@ pub fn probe(sys: &SysInfo) -> HostReport { .iter() .filter(|k| k.blocking && k.state == State::Unknown) .count(); - let could_host = !c - .iter() - .any(|k| k.blocking && k.state == State::Fail); + let could_host = !c.iter().any(|k| k.blocking && k.state == State::Fail); HostReport { checks: c, diff --git a/apps/nesdoctor/src/main.rs b/apps/nesdoctor/src/main.rs index 2c0f26de..6f7b4b77 100644 --- a/apps/nesdoctor/src/main.rs +++ b/apps/nesdoctor/src/main.rs @@ -47,7 +47,7 @@ mod steam; mod sys; mod vdf; -use std::io::Write; +use std::io::{IsTerminal, Write}; use std::path::PathBuf; use clap::Parser; @@ -142,9 +142,19 @@ fn main() { .iter() .any(|g| matches!(g.vendor.as_deref(), Some("AMD") | Some("Intel"))); - let answers = if args.yes { + // Do not ask questions that cannot be answered. `--quiet` promises a single + // parseable line, and a prompt written to stdout breaks that promise; a + // non-terminal stdin cannot answer at all, so prompting into it just prints + // the whole questionnaire and skips every item. Both cases used to print + // three questions and then "skipped" -- found by the CI smoke test, which + // is the only reason anything runs this way. + let interactive = !args.quiet && std::io::stdin().is_terminal(); + + let answers = if args.yes || !interactive { ask::Answers { - steam_consent: steam_present, + // `--yes` is a deliberate consent; a pipe is not consent to read + // somebody's library. + steam_consent: args.yes && steam_present, ..Default::default() } } else { @@ -213,7 +223,9 @@ fn main() { println!("\x1b[2mgoes nowhere unless you send it.\x1b[0m"); } println!(); - println!("\x1b[2mIf you are willing: paste that line into the thread you got this from.\x1b[0m"); + println!( + "\x1b[2mIf you are willing: paste that line into the thread you got this from.\x1b[0m" + ); println!("\x1b[2mIt is the only way we learn what the machines on the other end are.\x1b[0m"); } @@ -223,7 +235,9 @@ fn banner() { println!("\x1b[2mChecks whether this machine can host a Nestri box, measures what your\x1b[0m"); println!("\x1b[2mconnection actually does under load, and asks at most five questions.\x1b[0m"); println!(); - println!("\x1b[2mNothing is uploaded. There is no server to upload to — the output is a\x1b[0m"); + println!( + "\x1b[2mNothing is uploaded. There is no server to upload to — the output is a\x1b[0m" + ); println!("\x1b[2mline on your terminal that you may choose to paste somewhere.\x1b[0m"); println!(); } @@ -282,9 +296,7 @@ fn print_net(n: &net::NetReport) { println!( " \x1b[2m That is the round trip to the *nearest* major network, so it is a\x1b[0m" ); - println!( - " \x1b[2m floor on what any player sees. It is distance, not a fault.\x1b[0m" - ); + println!(" \x1b[2m floor on what any player sees. It is distance, not a fault.\x1b[0m"); } println!(" latency, loaded {}", f(n.loaded_rtt_ms, " ms")); println!(" latency, loaded p95 {}", f(n.loaded_rtt_p95_ms, " ms")); @@ -332,9 +344,7 @@ fn print_steam(s: &steam::SteamReport) { println!(" {}", steam::sparkline(&s.launch_hours)); println!(" \x1b[2m0h 6h 12h 18h 23h\x1b[0m"); if let Some((a, b)) = s.peak_window { - println!( - " Half of your launches fall between \x1b[1m{a:02}:00 and {b:02}:59\x1b[0m." - ); + println!(" Half of your launches fall between \x1b[1m{a:02}:00 and {b:02}:59\x1b[0m."); let width = if b >= a { b - a + 1 } else { 24 - a + b + 1 }; if width <= 6 { println!( diff --git a/apps/nesdoctor/src/net.rs b/apps/nesdoctor/src/net.rs index 6c27a250..fda22d89 100644 --- a/apps/nesdoctor/src/net.rs +++ b/apps/nesdoctor/src/net.rs @@ -21,8 +21,8 @@ use std::io::Write; use std::net::{SocketAddr, TcpStream, ToSocketAddrs}; -use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::sync::Arc; +use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::time::{Duration, Instant}; use serde::Serialize; diff --git a/apps/nesdoctor/src/steam.rs b/apps/nesdoctor/src/steam.rs index cacdb814..4ee55d9e 100644 --- a/apps/nesdoctor/src/steam.rs +++ b/apps/nesdoctor/src/steam.rs @@ -84,7 +84,9 @@ fn candidate_roots() -> Vec { } v.push(PathBuf::from(r"C:\Program Files (x86)\Steam")); v.push(PathBuf::from("/usr/lib/steam")); - v.into_iter().filter(|p| p.join("steamapps").is_dir()).collect() + v.into_iter() + .filter(|p| p.join("steamapps").is_dir()) + .collect() } /// True when there is anything to ask about. Called *before* consent so the @@ -113,7 +115,7 @@ pub fn read() -> SteamReport { if let Ok(txt) = fs::read_to_string(&lf) { let doc = vdf::parse(&txt); if let Some(folders) = doc.get(&["libraryfolders"]).and_then(vdf::Value::as_node) { - for (_, entry) in folders { + for entry in folders.values() { if let Some(p) = entry.get(&["path"]).and_then(vdf::Value::as_str) { let d = PathBuf::from(p).join("steamapps"); if d.is_dir() { @@ -129,7 +131,9 @@ pub fn read() -> SteamReport { let mut by_size: Vec<(String, u64)> = Vec::new(); for dir in &app_dirs { - let Ok(entries) = fs::read_dir(dir) else { continue }; + let Ok(entries) = fs::read_dir(dir) else { + continue; + }; for e in entries.flatten() { let name = e.file_name().to_string_lossy().into_owned(); if !(name.starts_with("appmanifest_") && name.ends_with(".acf")) { @@ -160,7 +164,7 @@ pub fn read() -> SteamReport { r.titles = by_size.len(); r.bytes_on_disk = by_size.iter().map(|(_, s)| s).sum(); - by_size.sort_by(|a, b| b.1.cmp(&a.1)); + by_size.sort_by_key(|(_, size)| std::cmp::Reverse(*size)); r.largest = by_size.into_iter().take(5).collect(); // --- when do they play ----------------------------------------------- @@ -175,18 +179,12 @@ pub fn read() -> SteamReport { }; let doc = vdf::parse(&txt); let Some(apps) = doc - .get(&[ - "UserLocalConfigStore", - "Software", - "Valve", - "Steam", - "apps", - ]) + .get(&["UserLocalConfigStore", "Software", "Valve", "Steam", "apps"]) .and_then(vdf::Value::as_node) else { continue; }; - for (_, app) in apps { + for app in apps.values() { let Some(ts) = app.get(&["LastPlayed"]).and_then(vdf::Value::as_u64) else { continue; }; diff --git a/apps/nesdoctor/src/sys.rs b/apps/nesdoctor/src/sys.rs index 28ea676f..f94da651 100644 --- a/apps/nesdoctor/src/sys.rs +++ b/apps/nesdoctor/src/sys.rs @@ -10,6 +10,13 @@ //! Every probe degrades to `None` rather than failing the run. A missing //! `lspci` costs one field. +// Every probe in this module is a stack of `#[cfg]`-gated `return`s, one per +// platform, so that exactly one compiles. The trailing `return` in each arm is +// load-bearing -- dropping it makes the arms fall through to each other and the +// function stops compiling on some targets -- so clippy's advice is wrong here +// specifically, and is not suppressed anywhere else in the crate. +#![allow(clippy::needless_return)] + use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; @@ -134,7 +141,13 @@ fn ram_gib() -> Option { / 1073741824.0, ); #[cfg(target_os = "macos")] - return Some(sh("sysctl", &["-n", "hw.memsize"])?.trim().parse::().ok()? / 1073741824.0); + return Some( + sh("sysctl", &["-n", "hw.memsize"])? + .trim() + .parse::() + .ok()? + / 1073741824.0, + ); #[cfg(not(any(target_os = "linux", windows, target_os = "macos")))] return None; } @@ -213,7 +226,9 @@ fn linux_gpus() -> Vec { .as_ref() .and_then(|p| p.file_name().map(|n| n.to_string_lossy().into_owned())) .unwrap_or_default(); - let bdf = slot.splitn(2, ':').nth(1).unwrap_or(&slot).to_string(); + let bdf = slot + .split_once(':') + .map_or(slot.clone(), |(_, r)| r.to_string()); let name = lspci .lines() diff --git a/apps/nesdoctor/src/vdf.rs b/apps/nesdoctor/src/vdf.rs index b51ca126..da87c10f 100644 --- a/apps/nesdoctor/src/vdf.rs +++ b/apps/nesdoctor/src/vdf.rs @@ -202,7 +202,10 @@ mod tests { v.get(&["appstate", "sizeondisk"]).unwrap().as_u64(), Some(38654705664) ); - assert_eq!(v.get(&["AppState", "nested", "a"]).unwrap().as_u64(), Some(1)); + assert_eq!( + v.get(&["AppState", "nested", "a"]).unwrap().as_u64(), + Some(1) + ); } #[test]