diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d87a50..900b5c5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -48,6 +48,46 @@ jobs: - name: Run tests run: cargo test --workspace --all-targets + # The same checks on the other two operating systems the CLI is meant to run + # on. Until this job existed no `cfg(not(unix))` branch had ever been + # compiled, let alone linted or run. See + # docs/plans/cross-platform-plan-2026-09-14.md, phase 0. + # + # A separate job rather than a matrix on `rust` above, for two reasons. + # Formatting cannot vary by platform, so it runs once. And branch protection + # requires a check named exactly "Rust": a matrix would rename it and block + # every pull request until the settings changed. Add these two to the + # required checks once they are green. + rust-platforms: + name: Rust (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + # Windows and macOS runners start slower and build slower than Linux. + # Set from what the first runs take, not from the Linux job's 15. + timeout-minutes: 40 + strategy: + fail-fast: false + matrix: + os: + - macos-14 + - windows-2025 + steps: + - name: Check out source + uses: actions/checkout@v7 + with: + persist-credentials: false + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + with: + components: clippy + - name: Run Clippy + run: cargo clippy --workspace --all-targets --all-features -- -D warnings + # Runs even when Clippy fails. A lint and a failing test are separate + # findings, and one run should report both rather than hiding the tests + # behind the first warning. + - name: Run tests + if: success() || failure() + run: cargo test --workspace --all-targets + web: name: Web runs-on: ubuntu-24.04 diff --git a/cli/tests/archive.rs b/cli/tests/archive.rs index 17801b8..cad992c 100644 --- a/cli/tests/archive.rs +++ b/cli/tests/archive.rs @@ -11,9 +11,12 @@ use std::{ }; use drop_cli::{ - tar::{TarPlan, safe_relative_path, symlink_target_stays_inside, traverses_only_real_dirs}, + tar::{TarPlan, safe_relative_path, symlink_target_stays_inside}, untar::TarExtractor, }; +// Only the symlink tests use this, and planting a symlink needs Unix. +#[cfg(unix)] +use drop_cli::tar::traverses_only_real_dirs; fn scratch(name: &str) -> PathBuf { let base = std::env::temp_dir().join(format!( diff --git a/cli/tests/transfer.rs b/cli/tests/transfer.rs index 7d355bb..b6cd155 100644 --- a/cli/tests/transfer.rs +++ b/cli/tests/transfer.rs @@ -318,6 +318,10 @@ async fn a_malformed_code_fails_before_the_relay_is_contacted() { } /// Builds a ustar archive from `(name, typeflag, link_target, contents)`. +/// +/// Unix only because its one caller is: the hostile archive it builds plants +/// symlinks, and the receiver cannot create those on Windows. +#[cfg(unix)] fn archive_of(entries: &[(&str, u8, &str, &[u8])]) -> Vec { let mut archive = Vec::new(); diff --git a/docs/plans/cross-platform-plan-2026-09-14.md b/docs/plans/cross-platform-plan-2026-09-14.md index cb318f7..33c7f39 100644 --- a/docs/plans/cross-platform-plan-2026-09-14.md +++ b/docs/plans/cross-platform-plan-2026-09-14.md @@ -1,6 +1,6 @@ # Cross-platform plan: Windows, macOS and Linux, and transfers between them -Status: **proposed** +Status: **active** — phase 0 done 2026-09-14 Created: **2026-09-14** Last updated: **2026-09-14** @@ -181,20 +181,50 @@ where this plan spends its effort. Nothing below can be verified until this exists, so it comes first and its failures are the input to the rest. -- [ ] `ci.yml`: turn the `rust` job into a matrix over `ubuntu-24.04`, - `macos-14` (arm64) and `windows-2025`. Formatting stays on Linux only, - because it cannot vary by platform. Clippy runs on all three, since - `cfg(windows)` code is otherwise never linted. -- [ ] Record what fails on the first run in this plan, dated, before fixing - any of it. -- [ ] Make the suite pass on all three without skipping anything that is not - genuinely inapplicable. Every new `cfg` on a test gets a comment saying - why the test cannot run there. -- [ ] Mark `.github/workflows/ci.yml` checkouts `autocrlf`-safe: - `.gitattributes` already sets `eol=lf`, so fixture bytes should match - across platforms. Confirm on the Windows runner rather than assume. - -**Gate:** `cargo test --workspace --all-targets` green on all three runners. +- [x] `ci.yml`: a `rust-platforms` job running Clippy and the tests on + `macos-14` and `windows-2025`. It is a separate job, not a matrix on + `rust`, because branch protection requires a check named exactly "Rust" + and a matrix would rename it. Formatting stays on Linux only. The test + step runs even when Clippy fails, so one run reports both. +- [x] Record what fails on the first run in this plan, dated, before fixing + any of it. See below. +- [x] Make the suite pass on all three without skipping anything that is not + genuinely inapplicable. Each new `cfg` has a comment saying why. +- [x] Line endings: no fixture failed on Windows, so `.gitattributes`' `eol=lf` + holds for what the tests read. Confirmed by the Windows run, not assumed. +- [ ] Add `Rust (macos-14)` and `Rust (windows-2025)` to branch protection's + required checks. A repository setting, for the owner. + +#### What the runners found, 2026-09-14 + +**Run 1.** macOS: Clippy and all tests green on the first attempt. Windows: +Clippy failed on two items, and nothing else. `cli/tests/archive.rs` imported +`traverses_only_real_dirs`, and `cli/tests/transfer.rs` defined `archive_of`. +Both are used only by `#[cfg(unix)]` symlink tests. **Every `cfg(not(unix))` +branch in the CLI compiled and passed Clippy**, the first time any of it had +been compiled. Fixed by gating the two items. The test step had not run, +because it followed Clippy. + +**Run 2.** Windows: Clippy and **the whole suite green**, about 6 minutes on a +cold runner. macOS: one failure, +`upload_socket_rejects_chunks_over_the_message_cap`, with +`expected oversized chunk to be written: Io(... BrokenPipe ...)`. It passed on +run 1, so it is a race. The relay refuses an oversized frame from its header +and closes the socket, possibly while the test is still writing the frame body. +Linux's socket buffer usually absorbs the write, and macOS's usually does not. +The relay's behaviour was correct both times; the test demanded that its own +doomed write succeed. The test now accepts a broken pipe, reset or closed +connection on that write and still asserts what matters: the receiver gets an +error and never a byte of the chunk. + +**What green does not mean.** Windows passing the suite says the suite has +nothing Windows-specific in it. It does not say Windows works. Findings 1–5 +above have no tests at all: the seven symlink tests are Unix-only, and nothing +exercises reserved names, alternate data streams, trailing dots, or a symlink +arriving at a Windows receiver. Phase 1 writes those tests, and they are +expected to fail before its fixes. + +**Gate:** met, pending the final run with the race fixed. ### Phase 1 — a Windows receiver writes what it can and reports the rest diff --git a/tests/websocket_transfer.rs b/tests/websocket_transfer.rs index be4e733..6477dfc 100644 --- a/tests/websocket_transfer.rs +++ b/tests/websocket_transfer.rs @@ -162,10 +162,34 @@ async fn upload_socket_rejects_chunks_over_the_message_cap() { .expect("expected sender meta message to be sent"); next_json_message_matching(&mut receiver_ws, |payload| payload["type"] == "meta").await; - sender_ws - .send(Message::binary(oversized)) - .await - .expect("expected oversized chunk to be written"); + // The relay refuses the frame from its header and closes the socket, which + // can happen while this side is still writing the frame's body. Whether + // the write finishes first depends on how much the kernel's socket buffer + // absorbs: on Linux it usually does, on macOS it usually does not, and the + // write fails with a broken pipe or a reset. Both mean the relay hung up on + // an oversized frame, which is the behaviour under test. What is asserted + // is what the receiver saw, below. + if let Err(error) = sender_ws.send(Message::binary(oversized)).await { + let hung_up = matches!( + &error, + tokio_tungstenite::tungstenite::Error::Io(io) + if matches!( + io.kind(), + std::io::ErrorKind::BrokenPipe + | std::io::ErrorKind::ConnectionReset + | std::io::ErrorKind::ConnectionAborted + ) + ) || matches!( + &error, + tokio_tungstenite::tungstenite::Error::ConnectionClosed + | tokio_tungstenite::tungstenite::Error::AlreadyClosed + ); + + assert!( + hung_up, + "writing the oversized chunk failed for a reason other than the relay hanging up: {error}" + ); + } // The relay must tear the session down instead of relaying the chunk. The // receiver stream therefore ends, with an error frame rather than any part