| @@ -1,8 +1,6 @@ |
| -use std::{ |
| - path::{Path, PathBuf}, |
| - process::Stdio, |
| -}; |
| +use std::process::Stdio; |
| |
| +use crate::builders::{ArtifactPath, Builder, LogEvent, LogSender}; |
| use miette::{IntoDiagnostic, Result, miette}; |
| use schemars::JsonSchema; |
| use serde::{Deserialize, Serialize}; |
| @@ -11,8 +9,6 @@ use tokio::io::{AsyncBufReadExt, BufReader}; |
| use tokio::process::Command; |
| use tokio::sync::mpsc::UnboundedSender; |
| |
| -use crate::builders::{ArtifactPath, Builder, LogEvent, LogSender}; |
| - |
| fn default_parallel() -> bool { |
| true |
| } |
| @@ -31,7 +27,7 @@ pub struct CargoBuilderConfig { |
| /// |
| /// Passed verbatim as `--manifest-path`. Defaults to the manifest in the |
| /// current working directory when absent. |
| - pub manifest_path: Option<PathBuf>, |
| + pub manifest_path: Option<std::path::PathBuf>, |
| |
| /// Restrict collected artifacts to these binary (or cdylib) target names. |
| /// |
| @@ -160,7 +156,7 @@ impl Builder for CargoBuilder { |
| line: l, |
| }); |
| |
| - let result = if config.parallel { |
| + let result = if config.parallel && !config.use_cross { |
| // Give this invocation its own target directory so it |
| // does not contend with sibling builds on cargo's lock. |
| let tmpdir = TempDir::new().into_diagnostic()?; |
| @@ -181,7 +177,7 @@ impl Builder for CargoBuilder { |
| Err(e) => Err(e), |
| } |
| } else { |
| - // Sequential mode: share the default target/ directory. |
| + // Sequential mode (or sequential cross-compilation): share the default target/ directory. |
| // Cargo's file lock ensures the invocations do not |
| // corrupt each other; they simply queue up. |
| run_cargo_build( |
| @@ -269,7 +265,7 @@ async fn run_cargo_build( |
| version: &str, |
| abbaye_version: &str, |
| line_tx: UnboundedSender<String>, |
| - target_dir: Option<&Path>, |
| + target_dir: Option<&std::path::Path>, |
| ) -> Result<Vec<ArtifactPath>> { |
| let tool = if config.use_cross { "cross" } else { "cargo" }; |
| let mut cmd = Command::new(tool); |
| @@ -348,11 +344,14 @@ async fn run_cargo_build( |
| } |
| |
| for filename in msg.filenames.unwrap_or_default() { |
| - let path = PathBuf::from(&filename); |
| + let path = std::path::PathBuf::from(&filename); |
| |
| // Skip rlib / rmeta files; we only want executables and cdylibs. |
| - let ext = path.extension().and_then(|e| e.to_str()).unwrap_or(""); |
| - if matches!(ext, "rlib" | "rmeta" | "d") { |
| + let ext = path |
| + .extension() |
| + .map(|e| e.to_string_lossy()) |
| + .unwrap_or_default(); |
| + if ext == "rlib" || ext == "rmeta" || ext == "d" { |
| continue; |
| } |
| |
| @@ -404,12 +403,12 @@ async fn run_cargo_build( |
| /// which is where a normal `cargo build --target <triple>` would place them. |
| async fn relocate_artifacts( |
| artifacts: Vec<ArtifactPath>, |
| - tmp_root: &Path, |
| + tmp_root: &std::path::Path, |
| ) -> Result<Vec<ArtifactPath>> { |
| let mut relocated = Vec::with_capacity(artifacts.len()); |
| for artifact in artifacts { |
| let relative = artifact.path.strip_prefix(tmp_root).into_diagnostic()?; |
| - let stable = PathBuf::from("target").join(relative); |
| + let stable = std::path::PathBuf::from("target").join(relative); |
| if let Some(parent) = stable.parent() { |
| tokio::fs::create_dir_all(parent).await.into_diagnostic()?; |
| } |
| @@ -452,8 +451,8 @@ async fn get_host_target() -> Result<String> { |
| |
| /// Read `[package].version` from the Cargo.toml at `manifest_path` |
| /// (defaults to `Cargo.toml` in the current directory). |
| -async fn read_crate_version(manifest_path: Option<&Path>) -> Result<String> { |
| - let path = manifest_path.unwrap_or(Path::new("Cargo.toml")); |
| +async fn read_crate_version(manifest_path: Option<&std::path::Path>) -> Result<String> { |
| + let path = manifest_path.unwrap_or(std::path::Path::new("Cargo.toml")); |
| let content = tokio::fs::read_to_string(path).await.into_diagnostic()?; |
| |
| #[derive(Deserialize)] |
| @@ -473,6 +472,300 @@ async fn read_crate_version(manifest_path: Option<&Path>) -> Result<String> { |
| .ok_or_else(|| miette!("no version field in [package] in {}", path.display())) |
| } |
| |
| +#[cfg(test)] |
| +mod tests { |
| + use super::*; |
| + use std::path::Path; |
| + |
| + // ── CargoMessage deserialization ────────────────────────────────────────── |
| + |
| + #[test] |
| + fn deserialize_compiler_artifact_message() { |
| + let json = r#"{ |
| + "reason": "compiler-artifact", |
| + "package_id": "path+file:///home/user/project#abbaye@0.10.0", |
| + "target": { "name": "abbaye", "kind": ["bin"] }, |
| + "filenames": ["/home/user/project/target/release/abbaye"] |
| + }"#; |
| + let msg: CargoMessage = serde_json::from_str(json).unwrap(); |
| + assert_eq!(msg.reason, "compiler-artifact"); |
| + assert!(msg.package_id.unwrap().contains("path+file://")); |
| + let target = msg.target.unwrap(); |
| + assert_eq!(target.name, "abbaye"); |
| + assert_eq!(target.kind, vec!["bin"]); |
| + assert_eq!( |
| + msg.filenames.unwrap(), |
| + vec!["/home/user/project/target/release/abbaye"] |
| + ); |
| + } |
| + |
| + #[test] |
| + fn deserialize_build_script_message_correctly_skipped() { |
| + let json = r#"{ |
| + "reason": "compiler-artifact", |
| + "package_id": "path+file:///home/user/project#abbaye@0.10.0", |
| + "target": { "name": "build-script-build", "kind": ["custom-build"] }, |
| + "filenames": ["/home/user/project/target/release/build-script-build"] |
| + }"#; |
| + let msg: CargoMessage = serde_json::from_str(json).unwrap(); |
| + let is_custom_build = msg |
| + .target |
| + .as_ref() |
| + .is_some_and(|t| t.kind.iter().any(|k| k == "custom-build")); |
| + assert!(is_custom_build, "custom-build target should be identified"); |
| + } |
| + |
| + #[test] |
| + fn deserialize_external_dependency_skipped() { |
| + let json = r#"{ |
| + "reason": "compiler-artifact", |
| + "package_id": "registry+https://github.com/rust-lang/crates.io-index#serde@1.0.0", |
| + "target": { "name": "serde", "kind": ["lib"] }, |
| + "filenames": ["/home/user/project/target/release/libserde.rlib"] |
| + }"#; |
| + let msg: CargoMessage = serde_json::from_str(json).unwrap(); |
| + let is_local = msg |
| + .package_id |
| + .as_deref() |
| + .is_some_and(|id| id.contains("path+file://")); |
| + assert!( |
| + !is_local, |
| + "external dependency should NOT be identified as local" |
| + ); |
| + } |
| + |
| + #[test] |
| + fn deserialize_multiple_filenames_for_bin() { |
| + let json = r#"{ |
| + "reason": "compiler-artifact", |
| + "package_id": "path+file:///home/user/project#my-app@0.1.0", |
| + "target": { "name": "my-app", "kind": ["bin"] }, |
| + "filenames": [ |
| + "/home/user/project/target/release/my-app", |
| + "/home/user/project/target/release/my-app.d" |
| + ] |
| + }"#; |
| + let msg: CargoMessage = serde_json::from_str(json).unwrap(); |
| + let filenames = msg.filenames.unwrap(); |
| + assert_eq!(filenames.len(), 2); |
| + assert!(filenames[0].ends_with("my-app")); |
| + assert!(filenames[1].ends_with("my-app.d")); |
| + } |
| + |
| + #[test] |
| + fn deserialize_cdylib_artifact() { |
| + let json = r#"{ |
| + "reason": "compiler-artifact", |
| + "package_id": "path+file:///home/user/project#libfoo@0.1.0", |
| + "target": { "name": "libfoo", "kind": ["cdylib"] }, |
| + "filenames": ["/home/user/project/target/release/liblibfoo.so"] |
| + }"#; |
| + let msg: CargoMessage = serde_json::from_str(json).unwrap(); |
| + let filename = msg.filenames.unwrap().into_iter().next().unwrap(); |
| + let path = Path::new(&filename); |
| + let ext = path |
| + .extension() |
| + .map(|e| e.to_string_lossy()) |
| + .unwrap_or_default(); |
| + // .so should NOT be filtered out (only rlib, rmeta, d) |
| + assert!( |
| + !matches!(ext.as_ref(), "rlib" | "rmeta" | "d"), |
| + "cdylib .so file should not be skipped" |
| + ); |
| + } |
| + |
| + // ── Artifact name generation ───────────────────────────────────────────── |
| + |
| + #[test] |
| + fn artifact_name_includes_version_and_triple() { |
| + let path = Path::new("/target/release/abbaye"); |
| + let stem = path |
| + .file_stem() |
| + .map(|s| s.to_string_lossy().into_owned()) |
| + .unwrap_or_default(); |
| + let dot_ext = path |
| + .extension() |
| + .map(|e| format!(".{}", e.to_string_lossy())) |
| + .unwrap_or_default(); |
| + let version = "0.10.0"; |
| + let triple = "x86_64-unknown-linux-musl"; |
| + let name = format!("{stem}-{version}-{triple}{dot_ext}"); |
| + assert_eq!(name, "abbaye-0.10.0-x86_64-unknown-linux-musl"); |
| + } |
| + |
| + #[test] |
| + fn artifact_name_with_exe_extension() { |
| + let path = Path::new("/target/release/abbaye.exe"); |
| + let stem = path |
| + .file_stem() |
| + .map(|s| s.to_string_lossy().into_owned()) |
| + .unwrap_or_default(); |
| + let dot_ext = path |
| + .extension() |
| + .map(|e| format!(".{}", e.to_string_lossy())) |
| + .unwrap_or_default(); |
| + let name = format!("{stem}-0.10.0-x86_64-pc-windows-msvc{dot_ext}"); |
| + assert_eq!(name, "abbaye-0.10.0-x86_64-pc-windows-msvc.exe"); |
| + } |
| + |
| + // ─── relocate_artifacts ────────────────────────────────────────────────── |
| + |
| + #[tokio::test] |
| + async fn test_relocate_artifacts_copies_to_target() { |
| + let tmp = tempfile::tempdir().unwrap(); |
| + let tmp_root = tmp.path().join("cross-tmp"); |
| + let triple_dir = tmp_root.join("x86_64-unknown-linux-musl").join("release"); |
| + tokio::fs::create_dir_all(&triple_dir).await.unwrap(); |
| + let binary_path = triple_dir.join("abbaye"); |
| + tokio::fs::write(&binary_path, b"binary content") |
| + .await |
| + .unwrap(); |
| + |
| + let artifacts = vec![ArtifactPath { |
| + path: binary_path, |
| + name: "abbaye-0.10.0-x86_64-unknown-linux-musl".to_owned(), |
| + hash: None, |
| + category: None, |
| + group_name: None, |
| + group_comment: None, |
| + }]; |
| + |
| + let relocated = relocate_artifacts(artifacts, &tmp_root).await.unwrap(); |
| + assert_eq!(relocated.len(), 1); |
| + let expected = Path::new("target") |
| + .join("x86_64-unknown-linux-musl") |
| + .join("release") |
| + .join("abbaye"); |
| + assert_eq!(relocated[0].path, expected); |
| + assert!(expected.exists(), "binary should exist at canonical path"); |
| + let content = tokio::fs::read_to_string(&expected).await.unwrap(); |
| + assert_eq!(content, "binary content"); |
| + } |
| + |
| + // ─── get_host_target ───────────────────────────────────────────────────── |
| + |
| + #[tokio::test] |
| + async fn test_get_host_target_returns_triple() { |
| + let triple = get_host_target().await.unwrap(); |
| + assert!(!triple.is_empty(), "host target triple should not be empty"); |
| + // Should contain at least one dash (e.g. x86_64-unknown-linux-gnu) |
| + assert!( |
| + triple.contains('-'), |
| + "triple should be dash-separated: {triple}" |
| + ); |
| + } |
| + |
| + // ─── read_crate_version ───────────────────────────────────────────────── |
| + |
| + #[tokio::test] |
| + async fn test_read_crate_version_from_toml() { |
| + let tmp = tempfile::tempdir().unwrap(); |
| + let toml_path = tmp.path().join("Cargo.toml"); |
| + tokio::fs::write( |
| + &toml_path, |
| + "[package]\nname = \"test\"\nversion = \"0.5.0\"\n", |
| + ) |
| + .await |
| + .unwrap(); |
| + |
| + let version = read_crate_version(Some(&toml_path)).await.unwrap(); |
| + assert_eq!(version, "0.5.0"); |
| + } |
| + |
| + #[tokio::test] |
| + async fn test_read_crate_version_returns_error_on_missing() { |
| + let tmp = tempfile::tempdir().unwrap(); |
| + let toml_path = tmp.path().join("Cargo.toml"); |
| + tokio::fs::write(&toml_path, "[package]\nname = \"no-version\"\n") |
| + .await |
| + .unwrap(); |
| + |
| + let result = read_crate_version(Some(&toml_path)).await; |
| + assert!( |
| + result.is_err(), |
| + "should error when version field is missing" |
| + ); |
| + } |
| + |
| + // ─── use_cross parallel condition ──────────────────────────────────────── |
| + |
| + #[test] |
| + fn use_cross_disables_parallel_isolation() { |
| + // This validates the fix: when use_cross is true, the parallel |
| + // isolation path (tempdir + relocate) must NOT be taken. |
| + // The condition is `config.parallel && !config.use_cross` -- so |
| + // when use_cross is true, the result should be false regardless |
| + // of the parallel setting. |
| + let uses_isolation = |parallel: bool, use_cross: bool| -> bool { parallel && !use_cross }; |
| + |
| + assert!( |
| + !uses_isolation(true, true), |
| + "parallel=true + use_cross=true should NOT use isolation" |
| + ); |
| + assert!( |
| + !uses_isolation(false, true), |
| + "parallel=false + use_cross=true should NOT use isolation" |
| + ); |
| + assert!( |
| + uses_isolation(true, false), |
| + "parallel=true + use_cross=false SHOULD use isolation" |
| + ); |
| + assert!( |
| + !uses_isolation(false, false), |
| + "parallel=false + use_cross=false should NOT use isolation" |
| + ); |
| + } |
| + |
| + // ─── Binary name filtering (extension check) ──────────────────────────── |
| + |
| + #[test] |
| + fn skips_rlib_and_rmeta_and_dot_d_files() { |
| + for ext in ["rlib", "rmeta", "d"] { |
| + let filename = format!("/target/release/libfoo.{ext}"); |
| + let path = Path::new(&filename); |
| + let ext_str = path |
| + .extension() |
| + .map(|e| e.to_string_lossy()) |
| + .unwrap_or_default(); |
| + assert!( |
| + ext_str == "rlib" || ext_str == "rmeta" || ext_str == "d", |
| + "{ext} should match skip condition" |
| + ); |
| + } |
| + } |
| + |
| + #[test] |
| + fn keeps_executable_and_cdylib_files() { |
| + for ext in ["", "exe", "so", "dylib", "dll"] { |
| + let filename = if ext.is_empty() { |
| + "/target/release/my-bin".to_owned() |
| + } else { |
| + format!("/target/release/my-bin.{ext}") |
| + }; |
| + let path = Path::new(&filename); |
| + let ext_str = path |
| + .extension() |
| + .map(|e| e.to_string_lossy()) |
| + .unwrap_or_default(); |
| + let is_skippable = ext_str == "rlib" || ext_str == "rmeta" || ext_str == "d"; |
| + assert!(!is_skippable, "{ext} should NOT be skipped"); |
| + } |
| + } |
| + |
| + // ─── Parallel flag default ─────────────────────────────────────────────── |
| + |
| + #[test] |
| + fn default_parallel_is_true() { |
| + assert!(default_parallel(), "parallel should default to true"); |
| + } |
| + |
| + #[test] |
| + fn use_cross_defaults_to_false() { |
| + let config = CargoBuilderConfig::default(); |
| + assert!(!config.use_cross, "use_cross should default to false"); |
| + } |
| +} |
| + |
| /// Configuration for [`CargoDocBuilder`]. |
| #[derive(Debug, Default, Clone, Deserialize, Serialize, JsonSchema)] |
| pub struct CargoDocBuilderConfig { |
| @@ -480,7 +773,7 @@ pub struct CargoDocBuilderConfig { |
| /// |
| /// Passed verbatim as `--manifest-path`. Defaults to the manifest in the |
| /// current working directory when absent. |
| - pub manifest_path: Option<PathBuf>, |
| + pub manifest_path: Option<std::path::PathBuf>, |
| |
| /// Skip building documentation for dependencies (`--no-deps`). |
| #[serde(default)] |
| @@ -527,8 +820,6 @@ impl Builder for CargoDocBuilder { |
| return Err(miette!("cargo doc failed with exit status: {status}")); |
| } |
| |
| - // Resolve the doc output directory. When a manifest path is given the |
| - // workspace root is its parent directory; otherwise fall back to CWD. |
| let doc_dir = config |
| .manifest_path |
| .as_deref() |
| @@ -540,9 +831,6 @@ impl Builder for CargoDocBuilder { |
| return Err(miette!("doc directory not found at {}", doc_dir.display())); |
| } |
| |
| - // Return the entire target/doc tree as a single artifact so that the |
| - // shared rustdoc assets (CSS, JS, fonts, search indices) that live at |
| - // the root of target/doc/ are preserved alongside the per-crate HTML. |
| Ok(vec![ArtifactPath { |
| path: doc_dir, |
| name: "doc".to_owned(), |