From fc62caf09952e4ed5f12a2f32937162ad01cba29 Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Fri, 7 Aug 2026 01:13:14 +0300 Subject: [PATCH 1/3] fix(xtask): pin the validate fidelity oracle to upstream rsync 3.4.4 --- xtask/src/cli.rs | 8 + xtask/src/commands/interop/shared/upstream.rs | 47 +++- xtask/src/commands/validate/mod.rs | 53 ++--- xtask/src/commands/validate/oracle.rs | 206 ++++++++++++++++++ 4 files changed, 272 insertions(+), 42 deletions(-) create mode 100644 xtask/src/commands/validate/oracle.rs diff --git a/xtask/src/cli.rs b/xtask/src/cli.rs index 92c8ba300b..dc79b4998a 100644 --- a/xtask/src/cli.rs +++ b/xtask/src/cli.rs @@ -92,6 +92,14 @@ pub enum Command { /// rsync across all client transports, then run the cross-platform checks /// (host `clippy -D warnings`, windows-gnu and linux-musl `cargo check`). /// Absent cross toolchains are skipped with a hint, never failed. + /// + /// The upstream side of the comparison is the pinned rsync 3.4.4 built by + /// `bash tools/ci/run_interop.sh` under + /// `target/interop/upstream-install/`, verified by its + /// `--version` banner and printed before the results. A missing or + /// wrong-version oracle aborts the run rather than falling back to + /// whatever `rsync` is on `PATH`. Set `OC_RSYNC_VALIDATE_UPSTREAM` to a + /// binary to compare against another release deliberately. Validate(ValidateMatrixArgs), } diff --git a/xtask/src/commands/interop/shared/upstream.rs b/xtask/src/commands/interop/shared/upstream.rs index 7f97046643..7ca171ec8a 100644 --- a/xtask/src/commands/interop/shared/upstream.rs +++ b/xtask/src/commands/interop/shared/upstream.rs @@ -6,14 +6,57 @@ use std::path::{Path, PathBuf}; /// Supported upstream rsync versions for interop testing. pub const UPSTREAM_VERSIONS: &[&str] = &["3.0.9", "3.1.3", "3.4.1"]; +/// Root under which `tools/ci/run_interop.sh` installs the pinned upstream +/// builds, one `/bin/rsync` per entry. This is the single place any +/// xtask command may look for an upstream binary. +pub fn install_root(workspace: &Path) -> PathBuf { + workspace.join("target/interop/upstream-install") +} + +/// Path of the pinned upstream build for `version` under `install_root`. +pub fn pinned_binary(install_root: &Path, version: &str) -> PathBuf { + install_root.join(version).join("bin/rsync") +} + +/// First line of `binary --version`, or `None` if it cannot be run. +pub fn version_banner(binary: &Path) -> Option { + let out = std::process::Command::new(binary) + .arg("--version") + .output() + .ok()?; + if !out.status.success() { + return None; + } + let stdout = String::from_utf8_lossy(&out.stdout); + stdout.lines().next().map(|l| l.trim_end().to_owned()) +} + +/// Dotted release version parsed from an rsync `--version` banner line. +/// +/// The install path is not authoritative: a directory named `3.4.4` may hold +/// any binary, and `/usr/bin/rsync` on macOS is openrsync. Mirrors +/// `upstream_release_version()` in `tools/ci/run_interop.sh`, which parses the +/// banner for the same reason. +pub fn parse_release_version(banner: &str) -> Option<&str> { + let rest = banner.strip_prefix("rsync")?.trim_start(); + let rest = rest.strip_prefix("version")?.trim_start(); + let end = rest + .find(|c: char| !c.is_ascii_digit() && c != '.') + .unwrap_or(rest.len()); + let version = &rest[..end]; + version + .starts_with(|c: char| c.is_ascii_digit()) + .then_some(version) +} + /// Detect available upstream rsync binaries. pub fn detect_upstream_binaries(workspace: &Path) -> TaskResult> { - let interop_dir = workspace.join("target/interop/upstream-install"); + let interop_dir = install_root(workspace); let mut binaries = Vec::new(); for version in UPSTREAM_VERSIONS { - let binary_path = interop_dir.join(version).join("bin/rsync"); + let binary_path = pinned_binary(&interop_dir, version); if binary_path.exists() { binaries.push(UpstreamBinary { diff --git a/xtask/src/commands/validate/mod.rs b/xtask/src/commands/validate/mod.rs index 3fc063bb46..b67d4db4d5 100644 --- a/xtask/src/commands/validate/mod.rs +++ b/xtask/src/commands/validate/mod.rs @@ -9,6 +9,10 @@ //! Each check is a self-contained [`Check`] strategy that owns its fixture and //! comparison and reports one [`CheckOutcome`] per matrix cell; the runner just //! aggregates. This keeps checks independent and individually testable. +//! +//! The upstream side of every comparison is the pinned rsync build the interop +//! harness installs, verified by its `--version` banner; see [`oracle`] for the +//! resolution rules and the `OC_RSYNC_VALIDATE_UPSTREAM` override. use std::collections::HashSet; use std::path::Path; @@ -130,6 +134,8 @@ mod checks; #[cfg(unix)] mod comparison; #[cfg(unix)] +mod oracle; +#[cfg(unix)] mod support; #[cfg(unix)] mod transport; @@ -268,7 +274,7 @@ mod unix_impl { .collect(); let oc = crate::commands::interop::shared::oc_rsync::detect_oc_rsync_binary(workspace)?; - let upstream = pick_upstream(workspace)?; + let upstream = super::oracle::resolve(workspace)?; let transports = resolve_transports(&options.transports)?; let work = workspace.join("target/validate"); @@ -284,10 +290,14 @@ mod unix_impl { .map(|c| c.label()) .collect(); cats.sort_unstable(); + // Print the oracle's path *and* its version banner before any result, + // so every PASS and FAIL below is attributable to a named upstream + // release rather than to whichever rsync the host happened to have. + eprintln!("[validate] upstream oracle: {}", upstream.banner); eprintln!( "[validate] oc-rsync={} vs upstream={} over [{}] categories [{}]", oc.binary_path().display(), - upstream.display(), + upstream.path.display(), transports .iter() .map(|t| t.label()) @@ -298,7 +308,7 @@ mod unix_impl { let ctx = ValidateCtx { oc: oc.binary_path(), - upstream: &upstream, + upstream: &upstream.path, work: &work, transports: &transports, flags: &options.flags, @@ -337,43 +347,6 @@ mod unix_impl { cross_result } - /// Ground-truth upstream rsync: prefer the system `rsync` on `PATH` (the - /// real drop-in comparison target), else an interop-built binary (3.4.x - /// preferred). - fn pick_upstream(workspace: &Path) -> TaskResult { - if let Some(system) = system_rsync() { - return Ok(system); - } - let all = crate::commands::interop::shared::upstream::detect_upstream_binaries(workspace)?; - let available: Vec<_> = all.into_iter().filter(|b| b.is_available()).collect(); - let chosen = available - .iter() - .find(|b| b.version_string().starts_with("3.4")) - .or_else(|| available.first()) - .ok_or_else(|| { - crate::error::TaskError::Validation("no upstream rsync binary found".into()) - })?; - Ok(chosen.binary_path().to_path_buf()) - } - - /// Locate a working `rsync` on `PATH`. - fn system_rsync() -> Option { - let path_var = std::env::var_os("PATH")?; - for dir in std::env::split_paths(&path_var) { - let candidate = dir.join("rsync"); - if candidate.is_file() - && std::process::Command::new(&candidate) - .arg("--version") - .output() - .map(|out| out.status.success()) - .unwrap_or(false) - { - return Some(candidate); - } - } - None - } - /// Print the matrix grouped by check, plus a summary line. fn report(outcomes: &[CheckOutcome]) { let (mut pass, mut fail, mut skip) = (0, 0, 0); diff --git a/xtask/src/commands/validate/oracle.rs b/xtask/src/commands/validate/oracle.rs new file mode 100644 index 0000000000..98421fee3e --- /dev/null +++ b/xtask/src/commands/validate/oracle.rs @@ -0,0 +1,206 @@ +//! Resolution of the upstream rsync binary the fidelity matrix compares against. +//! +//! Every check in `cargo xtask validate` asserts "oc-rsync matches upstream". +//! That claim is only meaningful if "upstream" is the release oc-rsync targets. +//! A binary picked up from `PATH` may be any version - or, on macOS, +//! openrsync - so a PASS would mean "oc matches *something*" and a FAIL could +//! be an oracle-version difference rather than an oc defect. +//! +//! The oracle is therefore an explicit, verified input: it comes from the +//! interop harness's pinned install tree, its `--version` banner must report +//! [`ORACLE_VERSION`], and both the path and the banner are printed with the +//! results. A missing or wrong-version oracle refuses to run. + +use std::path::{Path, PathBuf}; + +use crate::commands::interop::shared::upstream; +use crate::error::{TaskError, TaskResult}; + +/// The upstream release the fidelity matrix is defined against (protocol 32). +/// +/// `tools/ci/run_interop.sh` builds and installs exactly this version, so the +/// pinned tree and this constant move together. +pub const ORACLE_VERSION: &str = "3.4.4"; + +/// Overrides the oracle binary for deliberate cross-version comparison. +/// +/// When set, the named binary is used as-is and its version is reported but +/// not enforced. Unset - the default - is strict. +pub const ORACLE_ENV: &str = "OC_RSYNC_VALIDATE_UPSTREAM"; + +/// A resolved, version-verified upstream rsync to compare against. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Oracle { + /// Path to the upstream binary. + pub path: PathBuf, + /// First line of its `--version` output, printed alongside the results. + pub banner: String, +} + +/// Resolve the oracle for `workspace`, honouring [`ORACLE_ENV`]. +pub fn resolve(workspace: &Path) -> TaskResult { + let override_path = std::env::var_os(ORACLE_ENV).map(PathBuf::from); + resolve_with( + &upstream::install_root(workspace), + override_path, + upstream::version_banner, + ) +} + +/// Resolution policy, with the install root, the override, and the version +/// probe supplied by the caller so it is exercisable without a real binary. +fn resolve_with( + install_root: &Path, + override_path: Option, + probe: impl Fn(&Path) -> Option, +) -> TaskResult { + let overridden = override_path.is_some(); + let path = + override_path.unwrap_or_else(|| upstream::pinned_binary(install_root, ORACLE_VERSION)); + + if !overridden && !path.is_file() { + return Err(TaskError::Validation(format!( + "upstream oracle missing: no rsync {ORACLE_VERSION} at {}\n\ + Build it with `bash tools/ci/run_interop.sh`, or point \ + {ORACLE_ENV} at a binary to compare against a different release.", + path.display() + ))); + } + + let Some(banner) = probe(&path) else { + return Err(TaskError::Validation(format!( + "upstream oracle unusable: `{} --version` produced no output", + path.display() + ))); + }; + + let Some(found) = upstream::parse_release_version(&banner) else { + return Err(TaskError::Validation(format!( + "upstream oracle unusable: {} is not rsync - its banner reads `{banner}`", + path.display() + ))); + }; + + if !overridden && found != ORACLE_VERSION { + return Err(TaskError::Validation(format!( + "upstream oracle version mismatch: {} reports {found}, the fidelity \ + matrix is defined against rsync {ORACLE_VERSION}\n\ + Rebuild it with `bash tools/ci/run_interop.sh`, or set \ + {ORACLE_ENV} to compare against {found} deliberately.", + path.display() + ))); + } + + Ok(Oracle { path, banner }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn banner(version: &str) -> String { + format!("rsync version {version} protocol version 32") + } + + /// The pinned binary at the expected version is accepted, and its banner is + /// carried through so every result can be attributed to a named oracle. + #[test] + fn pinned_binary_at_the_expected_version_resolves() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let pinned = upstream::pinned_binary(root, ORACLE_VERSION); + std::fs::create_dir_all(pinned.parent().unwrap()).unwrap(); + std::fs::write(&pinned, b"").unwrap(); + + let oracle = resolve_with(root, None, |_| Some(banner(ORACLE_VERSION))).unwrap(); + assert_eq!(oracle.path, pinned); + assert_eq!(oracle.banner, banner(ORACLE_VERSION)); + } + + /// An absent oracle must refuse to run rather than fall back to whatever + /// rsync happens to be installed - a silent fallback is the whole defect. + #[test] + fn absent_pinned_binary_fails_with_a_named_diagnostic() { + let dir = tempfile::tempdir().unwrap(); + let err = resolve_with(dir.path(), None, |_| Some(banner(ORACLE_VERSION))).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("upstream oracle missing"), "{msg}"); + assert!(msg.contains(ORACLE_VERSION), "{msg}"); + assert!(msg.contains("run_interop.sh"), "{msg}"); + assert!(msg.contains(ORACLE_ENV), "{msg}"); + } + + /// A binary that is present but reports a different release must fail: a + /// FAIL against 3.4.1 cannot be told apart from an oc defect. + #[test] + fn wrong_version_fails_and_names_both_versions() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let pinned = upstream::pinned_binary(root, ORACLE_VERSION); + std::fs::create_dir_all(pinned.parent().unwrap()).unwrap(); + std::fs::write(&pinned, b"").unwrap(); + + let err = resolve_with(root, None, |_| Some(banner("3.4.1"))).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("version mismatch"), "{msg}"); + assert!(msg.contains("3.4.1"), "{msg}"); + assert!(msg.contains(ORACLE_VERSION), "{msg}"); + } + + /// The path is not authoritative: a directory named 3.4.4 may hold a + /// non-rsync binary (openrsync on macOS), which must not be accepted. + #[test] + fn non_rsync_banner_is_rejected() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let pinned = upstream::pinned_binary(root, ORACLE_VERSION); + std::fs::create_dir_all(pinned.parent().unwrap()).unwrap(); + std::fs::write(&pinned, b"").unwrap(); + + let err = resolve_with(root, None, |_| Some("openrsync: 3.4.4".into())).unwrap_err(); + assert!(err.to_string().contains("is not rsync"), "{err}"); + } + + /// A binary that cannot be run at all is named rather than skipped. + #[test] + fn unprobeable_binary_is_rejected() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let pinned = upstream::pinned_binary(root, ORACLE_VERSION); + std::fs::create_dir_all(pinned.parent().unwrap()).unwrap(); + std::fs::write(&pinned, b"").unwrap(); + + let err = resolve_with(root, None, |_| None).unwrap_err(); + assert!(err.to_string().contains("unusable"), "{err}"); + } + + /// The override exists for deliberate cross-version runs, so it takes any + /// rsync release - but it is still probed, so its version reaches the + /// printed banner instead of being assumed. + #[test] + fn override_accepts_another_release_but_still_probes_it() { + let dir = tempfile::tempdir().unwrap(); + let other = dir.path().join("rsync-3.1.3"); + let oracle = + resolve_with(dir.path(), Some(other.clone()), |_| Some(banner("3.1.3"))).unwrap(); + assert_eq!(oracle.path, other); + assert_eq!(oracle.banner, banner("3.1.3")); + + let err = resolve_with(dir.path(), Some(other), |_| None).unwrap_err(); + assert!(err.to_string().contains("unusable"), "{err}"); + } + + #[test] + fn release_version_is_parsed_from_the_banner_not_the_path() { + assert_eq!( + upstream::parse_release_version(&banner("3.4.4")), + Some("3.4.4") + ); + assert_eq!( + upstream::parse_release_version("rsync version 2.6.9 protocol version 29"), + Some("2.6.9") + ); + assert_eq!(upstream::parse_release_version("openrsync: 3.4.4"), None); + assert_eq!(upstream::parse_release_version("rsync version x"), None); + } +} From 1b50f489c9cf146f19c78e0d1d7983fdea6c07d5 Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Tue, 11 Aug 2026 10:55:02 +0300 Subject: [PATCH 2/3] fix(xtask): gate the oracle banner helpers to unix version_banner and parse_release_version are consumed only by commands::validate::oracle, and validate declares `mod oracle` behind #[cfg(unix)] because the fidelity matrix is unix-only. Their enclosing module, interop::shared::upstream, is not gated, so on Windows both functions had no caller and the deny(dead_code) build failed on the Windows ACL/xattr, Windows GNU cross-check and Windows IOCP cells. Gate them to match their sole consumer. Verified with cargo check -p xtask --all-targets --target x86_64-pc-windows-msvc (clean) and natively via fmt + clippy -D warnings. --- xtask/src/commands/interop/shared/upstream.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/xtask/src/commands/interop/shared/upstream.rs b/xtask/src/commands/interop/shared/upstream.rs index 7ca171ec8a..c886ca0de9 100644 --- a/xtask/src/commands/interop/shared/upstream.rs +++ b/xtask/src/commands/interop/shared/upstream.rs @@ -19,6 +19,11 @@ pub fn pinned_binary(install_root: &Path, version: &str) -> PathBuf { } /// First line of `binary --version`, or `None` if it cannot be run. +/// +/// Unix-only: the sole consumer is the fidelity matrix's oracle resolution +/// (`commands::validate::oracle`), and `validate` declares `mod oracle` behind +/// `#[cfg(unix)]`. Without the gate this is dead code on Windows. +#[cfg(unix)] pub fn version_banner(binary: &Path) -> Option { let out = std::process::Command::new(binary) .arg("--version") @@ -37,6 +42,9 @@ pub fn version_banner(binary: &Path) -> Option { /// any binary, and `/usr/bin/rsync` on macOS is openrsync. Mirrors /// `upstream_release_version()` in `tools/ci/run_interop.sh`, which parses the /// banner for the same reason. +/// +/// Unix-only for the same reason as [`version_banner`]. +#[cfg(unix)] pub fn parse_release_version(banner: &str) -> Option<&str> { let rest = banner.strip_prefix("rsync")?.trim_start(); let rest = rest.strip_prefix("version")?.trim_start(); From ce086f6ce11e0c30cf4271f8d2a25cba7775b399 Mon Sep 17 00:00:00 2001 From: Ofer Chen Date: Wed, 12 Aug 2026 22:58:57 +0300 Subject: [PATCH 3/3] fix(xtask): restore the unix gate on the validate skips module Every sibling module in the validate command carries #[cfg(unix)], and skips.rs opens with `use super::unix_impl::{CheckOutcome, Status}`, a module that is itself #[cfg(unix)]. Merging master into this branch re-ordered the module list to place `mod oracle;` alphabetically and dropped the attribute from `mod skips;` in the process, so on Windows the module compiled while the type it imports was configured out. That broke every Windows job that builds the workspace - not just the cross-check - because all of them build xtask: error[E0432]: unresolved import `super::unix_impl` --> xtask/src/commands/validate/skips.rs:30:12 Verified with cargo check -p xtask for x86_64-pc-windows-msvc and for the native unix target. --- xtask/src/commands/validate/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xtask/src/commands/validate/mod.rs b/xtask/src/commands/validate/mod.rs index adb6bc2cfd..8a62022659 100644 --- a/xtask/src/commands/validate/mod.rs +++ b/xtask/src/commands/validate/mod.rs @@ -135,7 +135,7 @@ mod checks; mod comparison; #[cfg(unix)] mod oracle; - +#[cfg(unix)] mod skips; #[cfg(unix)] mod support;