From 75f5ce64991a7a436243b1c59ea7d1fdf18cac4c Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 5 Oct 2026 22:27:45 +1300 Subject: [PATCH 1/5] Require source-region coverage across supported systems. --- .agents/skills/.agent-context-skills.json | 22 +- .../SKILL.md | 6 +- .../skills/socketry-project-setup/SKILL.md | 57 +- .github/workflows/test.yml | 16 +- Cargo.lock | 4 +- bake/Cargo.toml | 2 +- bake/src/main.rs | 13 +- crates/cargo-bake/src/main.rs | 64 +- crates/cargo-bake/src/project.rs | 571 ++++++++++++++++-- crates/macros/src/expansion_tests.rs | 14 + crates/macros/src/lib.rs | 5 +- src/arguments.rs | 8 + src/output.rs | 22 +- src/registry.rs | 30 +- 14 files changed, 705 insertions(+), 129 deletions(-) diff --git a/.agents/skills/.agent-context-skills.json b/.agents/skills/.agent-context-skills.json index 062e72b..6e39fb3 100644 --- a/.agents/skills/.agent-context-skills.json +++ b/.agents/skills/.agent-context-skills.json @@ -1,17 +1,33 @@ { "version": 1, "skills": { + "bake-agent-context-usage": { + "package": "bake-agent-context", + "version": "0.3.0" + }, "socketry-project-github-repository": { "package": "socketry-project", - "version": "0.2.5" + "version": "0.3.3" }, "socketry-project-pull-requests": { "package": "socketry-project", - "version": "0.2.5" + "version": "0.3.3" + }, + "socketry-project-releasing": { + "package": "socketry-project", + "version": "0.3.3" }, "socketry-project-setup": { "package": "socketry-project", - "version": "0.2.5" + "version": "0.3.3" + }, + "socketry-project-testing": { + "package": "socketry-project", + "version": "0.3.3" + }, + "socketry-project-update": { + "package": "socketry-project", + "version": "0.3.3" } } } \ No newline at end of file diff --git a/.agents/skills/socketry-project-github-repository/SKILL.md b/.agents/skills/socketry-project-github-repository/SKILL.md index d3f6d91..bf78037 100644 --- a/.agents/skills/socketry-project-github-repository/SKILL.md +++ b/.agents/skills/socketry-project-github-repository/SKILL.md @@ -74,6 +74,6 @@ settings. Do not rename, archive, transfer, or delete a repository without explicit approval, and do not disable issues, pull requests, or required checks without approval. -The Cargo-specific release workflow, environment reviewers, and trusted -publishing are covered in the -[`socketry-project-releasing` skill](https://github.com/socketry/socketry-project-rust/blob/main/context/releasing.md). +The `socketry-project-releasing` skill describes the standard Cargo release +process and links to Bake Cargo task documentation for branch rulesets, +crates.io environment reviewers, and trusted publishing. diff --git a/.agents/skills/socketry-project-setup/SKILL.md b/.agents/skills/socketry-project-setup/SKILL.md index 7c5494f..5f9acd2 100644 --- a/.agents/skills/socketry-project-setup/SKILL.md +++ b/.agents/skills/socketry-project-setup/SKILL.md @@ -1,12 +1,13 @@ --- name: socketry-project-setup -description: Set up a new or existing Rust repository for a Socketry project, including its Cargo layout, shared Bake tasks, agent context, tests, and GitHub workflows. Use when creating a crate repository or bringing one into the standard project structure. +description: Bootstrap a Rust repository with Socketry layout, Bake tasks, agent context, tests, and workflows. Use for a new repository or initial setup. --- # Set Up a Rust Repository -Use this skill to start a Rust repository with the shared Socketry conventions, -agent context, and release tasks. +Use this skill to bootstrap a new Rust repository or add its initial shared +Socketry tooling. For an audit of an established repository, use the +`socketry-project-update` skill. ## Create the repository @@ -38,7 +39,7 @@ minimal binary. Add this dependency under the existing `[dependencies]` table in `bake/Cargo.toml`: ```toml -socketry-project = "0.2" +socketry-project = "0.3" ``` Run regeneration again to link its task registrations: @@ -58,55 +59,39 @@ The `socketry-project` dependency makes the shared tasks available to the private Bake binary. It also registers `cargo:after_version_bump`, which updates `license.md`, `releases.md`, and generated sections in `readme.md` after a version change. -It bundles the standard `test` and `test:external` task providers as well. Keep task tooling out of unrelated published libraries. Consumer projects should depend on `socketry-project` from their private `bake/` package. -Install the project's agent context: +## Agent context -```sh -cargo bake agent:context:install -``` - -This installs ordinary context guides under `.agents/context/`, installs -dependency-provided skills under `.agents/skills/`, and updates `agents.md` -with links to ordinary context. Read that index and inspect applicable skills. -Add `--package socketry-project` to install only this crate's context and -skills. See the Agent Context guide provided by `bake-agent-context` for how to -organize shared and project-only guidance. +Follow the [Agent Context section in `readme.md`](../readme.md#agent-context) +to install and discover shared context and skills. Follow +[Conventions](conventions.md#source-and-documentation) for where to keep +package guidance and project-only instructions. The `bake-agent-context` +guide documents installer behavior and options. ## Set up GitHub Configure repository metadata, collaboration features, pull request defaults, and branch protection using the `socketry-project-github-repository` skill. Generate the Cargo workflow with `cargo:setup:workflow`; follow the -[`socketry-project-releasing` skill](https://github.com/socketry/socketry-project-rust/blob/main/context/releasing.md) -before applying rulesets, environment reviewers, or crates.io trusted -publishing. - -## Standard workflows +`socketry-project-releasing` skill before applying rulesets, environment +reviewers, or crates.io trusted publishing. It links to the Bake Cargo Readme +for task-specific details. -Keep `.github/workflows/test.yml` for local workspace tests. When -`bake-test-rust` is linked, install the Cargo launcher and run -`cargo bake --locked test` so the optional `test:before` hook also runs. -Otherwise use `cargo test --workspace --locked`. Add platform or feature -matrix entries when the project needs them. +## Test workflows -List selected downstream projects under -`[workspace.metadata.bake.test.external]` in the root `Cargo.toml`. Add -`.github/workflows/external.yml` only when that list is non-empty, and run -`cargo bake --locked test:external` when `bake-test-rust` is linked. The task -keeps checkouts under `external/` and applies local workspace crates as Cargo -patches so downstream tests exercise the source being developed. +Use the `socketry-project-testing` skill for organization-wide testing +expectations. Consult the installed `bake-test-rust` context for canonical +`test.yml` and optional `external.yml` workflows, task setup, coverage options, +and downstream test configuration. Use `cargo:setup:workflow` from `bake-cargo` to generate `.github/workflows/publish.yml`. That workflow checks a release candidate on pull requests, publishes after merge through the configured `crates-io` environment, and then creates or updates the matching GitHub Release from -`releases.md`. See the [`socketry-project-releasing` skill](https://github.com/socketry/socketry-project-rust/blob/main/context/releasing.md) -for trusted publishing and repository setup. See the Rust Testing context -guide provided by `socketry-project` for test workflow details and optional -downstream compatibility workflows. +`releases.md`. Follow the `socketry-project-releasing` skill for the release +process and `bake-test-rust` context for workflow and task details. ## Work on the project diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index eaa781b..aabb31a 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -27,9 +27,6 @@ jobs: - name: Run workspace tests with Bake if: runner.os != 'Windows' run: cargo bake --locked test - - name: Run workspace tests with Cargo on Windows - if: runner.os == 'Windows' - run: cargo test --workspace --locked - name: Exercise task discovery and invocation run: | cargo bake --locked --list @@ -40,18 +37,27 @@ jobs: cargo bake --locked releases:notes v0.1.0 coverage: - runs-on: ubuntu-latest + name: Coverage (${{ matrix.runner }}) + runs-on: ${{ matrix.runner }} timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + runner: [ubuntu-latest, windows-latest] steps: - uses: actions/checkout@v7 - uses: actions-rust-lang/setup-rust-toolchain@v2 with: components: clippy, llvm-tools-preview, rustfmt + # Isolate this source-installed tool from the old prebuilt-action cache. + cache-shared-key: coverage-cargo-install-v1 - name: Install Bake launcher run: cargo install socketry-cargo-bake --locked - name: Install coverage tool run: cargo install cargo-llvm-cov --locked - run: cargo fmt --all -- --check + if: matrix.runner == 'ubuntu-latest' - run: cargo clippy --workspace --all-targets --locked -- -D warnings - - name: Run tests and require complete line coverage + if: matrix.runner == 'ubuntu-latest' + - name: Run tests and require complete source-region coverage run: cargo bake --locked test:coverage --all-targets true diff --git a/Cargo.lock b/Cargo.lock index de5563a..26dda4a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -117,9 +117,9 @@ dependencies = [ [[package]] name = "bake-test-rust" -version = "0.2.2" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d29e665d8e2373f5931adebe8ee29d36fd2a59c36c3d755a7d408d7eaed20dc" +checksum = "e9d5ceec9cc08a3bb421dd3755ad9e073831934793aa88d8f55095ad98c1dc45" dependencies = [ "bake", "serde_json", diff --git a/bake/Cargo.toml b/bake/Cargo.toml index 206768c..a81c592 100644 --- a/bake/Cargo.toml +++ b/bake/Cargo.toml @@ -8,7 +8,7 @@ publish = false socketry-project = ">=0.3.3" bake = { version = "0.19.0", path = ".." } bake-agent-context = { version = "0.3.0" } -bake-test-rust = { version = "0.2.2" } +bake-test-rust = { version = "0.3.0" } bake-releases = { version = "0.3.0" } bake-cargo = { version = "0.4.0" } bake-license = { version = "0.2.0" } diff --git a/bake/src/main.rs b/bake/src/main.rs index bb5bf28..3d56df6 100644 --- a/bake/src/main.rs +++ b/bake/src/main.rs @@ -82,7 +82,11 @@ fn prepare( } fn run() -> Result<()> { - Registry::discover()?.run() + run_registry(Registry::discover()) +} + +fn run_registry(registry: Result) -> Result<()> { + registry?.run() } fn main() -> ExitCode { @@ -128,6 +132,7 @@ mod tests { #[test] fn exercises_example_tasks_and_cargo_failures() { + assert!(run_registry(Err(Error::new("discovery failure"))).is_err()); assert_eq!(greet("Sam".into(), false, vec![]).unwrap(), "Hello, Sam."); assert_eq!( greet("Sam".into(), true, vec!["tag".into()]).unwrap(), @@ -176,6 +181,12 @@ mod tests { assert!(prepare(&mut context, "v0.1.0".into(), true).is_ok()); } + #[test] + fn release_notes_require_a_version() { + let mut context = Registry::new().context("."); + assert!(notes(&mut context, &Arguments::default()).is_err()); + } + #[test] fn maps_task_results_to_process_exit_codes() { assert_eq!(exit_code(Ok(())), ExitCode::SUCCESS); diff --git a/crates/cargo-bake/src/main.rs b/crates/cargo-bake/src/main.rs index ce8be35..e88e637 100644 --- a/crates/cargo-bake/src/main.rs +++ b/crates/cargo-bake/src/main.rs @@ -31,14 +31,22 @@ fn run() -> Result { println!("cargo-bake {}", env!("CARGO_PKG_VERSION")); return Ok(0); } + if options.regenerate && !options.arguments.is_empty() { + return Err("--regenerate cannot be combined with task arguments".into()); + } + run_project(options, std::env::current_dir()) +} + +fn run_project( + options: Options, + current_directory: std::io::Result, +) -> Result { + let current_directory = current_directory?; if options.regenerate { - if !options.arguments.is_empty() { - return Err("--regenerate cannot be combined with task arguments".into()); - } - Project::regenerate(&std::env::current_dir()?, &options)?; + Project::regenerate(¤t_directory, &options)?; return Ok(0); } - let project = Project::discover(&std::env::current_dir()?, &options)?; + let project = Project::discover(¤t_directory, &options)?; let mut command = cargo(); command .args(["run", "--quiet", "--manifest-path"]) @@ -53,7 +61,12 @@ fn run() -> Result { if options.release { command.arg("--release"); } - let status = command.arg("--").args(&options.arguments).status()?; + command.arg("--").args(&options.arguments); + command_status(&mut command) +} + +fn command_status(command: &mut Command) -> Result { + let status = command.status()?; #[cfg(unix)] { use std::os::unix::process::ExitStatusExt; @@ -81,10 +94,49 @@ fn main() -> ExitCode { #[cfg(test)] mod tests { use super::*; + use std::path::PathBuf; #[test] fn cargo_program_uses_the_environment_or_default() { assert_eq!(cargo_program(None), "cargo"); assert_eq!(cargo_program(Some("custom-cargo".into())), "custom-cargo"); } + + #[test] + fn reports_project_discovery_and_regeneration_errors() { + let directory = tempfile::tempdir().unwrap(); + assert!(run_project(Options::default(), Ok(directory.path().to_path_buf())).is_err()); + + std::fs::write( + directory.path().join("Cargo.toml"), + "[package]\nname = \"example\"\nversion = \"0.1.0\"\nedition = \"2024\"\n[package.metadata.bake]\nmanifest = \"\"\n", + ) + .unwrap(); + assert!( + run_project( + Options { + regenerate: true, + ..Options::default() + }, + Ok(directory.path().to_path_buf()), + ) + .is_err() + ); + + assert!( + run_project( + Options::default(), + Err(std::io::Error::other("no current directory")), + ) + .is_err() + ); + } + + #[test] + fn reports_child_process_start_failures() { + let directory = tempfile::tempdir().unwrap(); + let mut command = Command::new(PathBuf::from(directory.path()).join("missing-cargo")); + + assert!(command_status(&mut command).is_err()); + } } diff --git a/crates/cargo-bake/src/project.rs b/crates/cargo-bake/src/project.rs index d6c77f8..d68e4bc 100644 --- a/crates/cargo-bake/src/project.rs +++ b/crates/cargo-bake/src/project.rs @@ -56,7 +56,14 @@ struct Location { } fn metadata(manifest: &Path, options: &Options) -> Result { - let mut command = cargo(); + metadata_with_command(manifest, options, cargo()) +} + +fn metadata_with_command( + manifest: &Path, + options: &Options, + mut command: std::process::Command, +) -> Result { command .args([ "metadata", @@ -82,7 +89,11 @@ fn metadata(manifest: &Path, options: &Options) -> Result { ) .into()); } - Ok(serde_json::from_slice(&output.stdout)?) + parse_metadata(&output.stdout) +} + +fn parse_metadata(output: &[u8]) -> Result { + Ok(serde_json::from_slice(output)?) } fn configured_manifest(metadata: &Value) -> Result> { @@ -109,40 +120,7 @@ impl Project { Err(_) if options.regenerate => return locate_from_manifest(&manifest), Err(error) => return Err(error), }; - let package = project_metadata.packages.iter().find(|package| { - package - .manifest_path - .canonicalize() - .is_ok_and(|path| path == manifest) - }); - let package_manifest = package - .map(|package| configured_manifest(&package.metadata)) - .transpose()? - .flatten(); - let workspace_root = project_metadata.workspace_root.canonicalize()?; - let (root, relative_manifest) = if let Some(path) = package_manifest { - ( - manifest - .parent() - .ok_or("manifest has no parent directory")? - .to_path_buf(), - path, - ) - } else { - ( - project_metadata.workspace_root.clone(), - configured_manifest(&project_metadata.metadata)?.unwrap_or("bake/Cargo.toml"), - ) - }; - let root = root.canonicalize()?; - let task_manifest = root.join(relative_manifest); - - Ok(Location { - root, - workspace_manifest: workspace_root.join("Cargo.toml"), - workspace_root, - task_manifest, - }) + locate_with_metadata(manifest, project_metadata) } pub(crate) fn discover(directory: &Path, options: &Options) -> Result { @@ -175,12 +153,20 @@ impl Project { pub(crate) fn regenerate(directory: &Path, options: &Options) -> Result<()> { let location = Self::locate(directory, options)?; + Self::regenerate_at_location(&location, options, |path| path.canonicalize()) + } + + fn regenerate_at_location( + location: &Location, + options: &Options, + canonicalize: impl Fn(&Path) -> std::io::Result, + ) -> Result<()> { let created = !location.task_manifest.is_file(); if created { - bootstrap_task_package(&location)?; + bootstrap_task_package(location)?; } - let task_manifest = location.task_manifest.canonicalize()?; + let task_manifest = canonicalize(&location.task_manifest)?; let task_metadata = metadata(&task_manifest, options)?; let package = task_metadata .packages @@ -199,7 +185,7 @@ impl Project { let generated_path = binary .src_path .parent() - .ok_or("binary source path has no parent directory")? + .unwrap_or_else(|| unreachable!("Cargo binary source paths have a parent directory")) .join("bake_generated_tasks/mod.rs"); write_if_changed(&generated_path, &generated_source)?; add_generated_module(&binary.src_path)?; @@ -218,6 +204,51 @@ impl Project { } } +fn locate_with_metadata(manifest: PathBuf, project_metadata: Metadata) -> Result { + locate_with_metadata_using(manifest, project_metadata, |path| path.canonicalize()) +} + +fn locate_with_metadata_using( + manifest: PathBuf, + project_metadata: Metadata, + canonicalize: impl Fn(&Path) -> std::io::Result, +) -> Result { + let package = project_metadata.packages.iter().find(|package| { + package + .manifest_path + .canonicalize() + .is_ok_and(|path| path == manifest) + }); + let package_manifest = package + .map(|package| configured_manifest(&package.metadata)) + .transpose()? + .flatten(); + let workspace_root = canonicalize(&project_metadata.workspace_root)?; + let (root, relative_manifest) = if let Some(path) = package_manifest { + ( + manifest + .parent() + .unwrap_or_else(|| unreachable!("Cargo manifest paths have a parent directory")) + .to_path_buf(), + path, + ) + } else { + ( + project_metadata.workspace_root.clone(), + configured_manifest(&project_metadata.metadata)?.unwrap_or("bake/Cargo.toml"), + ) + }; + let root = canonicalize(&root)?; + let task_manifest = root.join(relative_manifest); + + Ok(Location { + root, + workspace_manifest: workspace_root.join("Cargo.toml"), + workspace_root, + task_manifest, + }) +} + fn table_item<'document>(item: Option<&'document Item>, key: &str) -> Option<&'document Item> { item.and_then(Item::as_table) .and_then(|table| table.get(key)) @@ -239,9 +270,21 @@ fn configured_manifest_from_toml(document: &DocumentMut, section: &str) -> Resul } } -fn workspace_manifest_for(manifest: &Path, document: &DocumentMut) -> Result { +#[cfg(test)] +fn workspace_manifest_for( + manifest: &Path, + document: &DocumentMut, +) -> Result<(PathBuf, DocumentMut)> { + workspace_manifest_for_using(manifest, document, |path| path.canonicalize()) +} + +fn workspace_manifest_for_using( + manifest: &Path, + document: &DocumentMut, + canonicalize: impl Fn(&Path) -> std::io::Result, +) -> Result<(PathBuf, DocumentMut)> { if document.as_table().contains_key("workspace") { - return Ok(manifest.to_path_buf()); + return Ok((manifest.to_path_buf(), document.clone())); } if let Some(workspace_path) = table_item(document.as_table().get("package"), "workspace") @@ -251,10 +294,11 @@ fn workspace_manifest_for(manifest: &Path, document: &DocumentMut) -> Result()?; + return Ok((workspace_manifest, document)); } let parent = manifest @@ -269,27 +313,35 @@ fn workspace_manifest_for(manifest: &Path, document: &DocumentMut) -> Result Result { + locate_from_manifest_using(manifest, |path| path.canonicalize()) +} + +fn locate_from_manifest_using( + manifest: &Path, + canonicalize: impl Fn(&Path) -> std::io::Result, +) -> Result { let contents = fs::read_to_string(manifest)?; let document = contents.parse::()?; - let workspace_manifest = workspace_manifest_for(manifest, &document)?; - let workspace_contents = fs::read_to_string(&workspace_manifest)?; - let workspace_document = workspace_contents.parse::()?; - let workspace_root = workspace_manifest - .parent() - .ok_or("workspace manifest has no parent directory")? - .canonicalize()?; - let package_root = manifest - .parent() - .ok_or("manifest has no parent directory")? - .canonicalize()?; + let (workspace_manifest, workspace_document) = + workspace_manifest_for_using(manifest, &document, &canonicalize)?; + let workspace_root = canonicalize( + workspace_manifest + .parent() + .unwrap_or_else(|| unreachable!("Cargo manifest paths have a parent directory")), + )?; + let package_root = canonicalize( + manifest + .parent() + .unwrap_or_else(|| unreachable!("Cargo manifest paths have a parent directory")), + )?; let package_manifest = configured_manifest_from_toml(&document, "package")?; let root = if package_manifest.is_some() { package_root @@ -369,6 +421,13 @@ fn write_if_changed(path: &Path, contents: &str) -> Result<()> { } fn add_generated_module(source_path: &Path) -> Result<()> { + add_generated_module_using(source_path, remove_legacy_generated_module) +} + +fn add_generated_module_using( + source_path: &Path, + remove_legacy: impl FnOnce(&Path) -> Result<()>, +) -> Result<()> { const PATH_ATTRIBUTE: &str = "#[path = \"bake_generated_tasks/mod.rs\"]"; const MODULE_ITEM: &str = "mod bake_generated_tasks;"; const LEGACY_PATH_ATTRIBUTE: &str = "#[path = \"__bake_generated_tasks/mod.rs\"]"; @@ -391,7 +450,7 @@ fn add_generated_module(source_path: &Path) -> Result<()> { }; source = source.replace(&legacy_module, &replacement); write_if_changed(source_path, &source)?; - remove_legacy_generated_module(source_path)?; + remove_legacy(source_path)?; return Ok(()); } if has_generated_module { @@ -417,6 +476,13 @@ fn add_generated_module(source_path: &Path) -> Result<()> { } fn remove_legacy_generated_module(source_path: &Path) -> Result<()> { + remove_legacy_generated_module_using(source_path, |path| fs::remove_file(path)) +} + +fn remove_legacy_generated_module_using( + source_path: &Path, + remove_file: impl FnOnce(&Path) -> std::io::Result<()>, +) -> Result<()> { const GENERATED_MARKER: &str = "// Generated by `cargo bake --regenerate`; do not edit."; let Some(source_directory) = source_path.parent() else { @@ -425,19 +491,26 @@ fn remove_legacy_generated_module(source_path: &Path) -> Result<()> { let legacy_directory = source_directory.join("__bake_generated_tasks"); let legacy_source = legacy_directory.join("mod.rs"); if fs::read_to_string(&legacy_source).is_ok_and(|source| source.starts_with(GENERATED_MARKER)) { - fs::remove_file(legacy_source)?; + remove_file(&legacy_source)?; let _ = fs::remove_dir(legacy_directory); } Ok(()) } fn bootstrap_task_package(location: &Location) -> Result<()> { + bootstrap_task_package_using(location, |path| path.canonicalize()) +} + +fn bootstrap_task_package_using( + location: &Location, + canonicalize: impl Fn(&Path) -> std::io::Result, +) -> Result<()> { let task_directory = location .task_manifest .parent() .ok_or("task manifest has no parent directory")?; fs::create_dir_all(task_directory)?; - let task_directory = task_directory.canonicalize()?; + let task_directory = canonicalize(task_directory)?; let member = task_directory .strip_prefix(&location.workspace_root) @@ -650,6 +723,87 @@ mod tests { } } + #[test] + fn locates_from_metadata_and_reports_metadata_path_errors() { + let directory = tempdir().unwrap(); + let manifest = directory.path().join("package/Cargo.toml"); + write( + &manifest, + "[package]\nname = \"example\"\nversion = \"0.1.0\"\nedition = \"2024\"\n", + ); + let manifest = manifest.canonicalize().unwrap(); + let package = |metadata| Package { + name: "example".to_owned(), + manifest_path: manifest.clone(), + metadata, + default_run: None, + targets: vec![], + dependencies: vec![], + }; + + assert!( + locate_with_metadata( + manifest.clone(), + Metadata { + workspace_root: directory.path().to_path_buf(), + metadata: Value::Null, + packages: vec![package(serde_json::json!({"bake": {"manifest": ""}}))], + }, + ) + .is_err() + ); + + assert!( + locate_with_metadata_using( + manifest.clone(), + Metadata { + workspace_root: directory.path().to_path_buf(), + metadata: Value::Null, + packages: vec![], + }, + |_| Err(std::io::Error::other("workspace disappeared")), + ) + .is_err() + ); + + let package_root = manifest.parent().unwrap().to_path_buf(); + assert!( + locate_with_metadata_using( + manifest.clone(), + Metadata { + workspace_root: directory.path().to_path_buf(), + metadata: Value::Null, + packages: vec![package( + serde_json::json!({"bake": {"manifest": "tasks/Cargo.toml"}}) + )], + }, + |path| { + if path == package_root { + Err(std::io::Error::other("package root disappeared")) + } else { + path.canonicalize() + } + }, + ) + .is_err() + ); + + let task_manifest = directory.path().join("tasks/Cargo.toml"); + write(&task_manifest, "not a task package yet\n"); + let location = Location { + root: directory.path().to_path_buf(), + workspace_root: directory.path().to_path_buf(), + workspace_manifest: directory.path().join("Cargo.toml"), + task_manifest, + }; + assert!( + Project::regenerate_at_location(&location, &Options::default(), |_| { + Err(std::io::Error::other("task manifest disappeared")) + }) + .is_err() + ); + } + #[test] fn locates_a_workspace_manifest_from_package_and_ancestor_metadata() { let directory = tempdir().unwrap(); @@ -659,7 +813,9 @@ mod tests { let root_document = document("[workspace]\n"); assert_eq!( - workspace_manifest_for(&workspace_manifest, &root_document).unwrap(), + workspace_manifest_for(&workspace_manifest, &root_document) + .unwrap() + .0, workspace_manifest ); @@ -672,7 +828,9 @@ mod tests { "[package]\nname = \"member\"\nversion = \"0.1.0\"\nedition = \"2024\"\nworkspace = \"..\"\n", ); assert_eq!( - workspace_manifest_for(&package_manifest, &package_document).unwrap(), + workspace_manifest_for(&package_manifest, &package_document) + .unwrap() + .0, workspace_manifest.canonicalize().unwrap() ); @@ -681,6 +839,33 @@ mod tests { ); assert!(workspace_manifest_for(&package_manifest, &missing_workspace_document).is_err()); + let unreadable_workspace = tempdir().unwrap(); + fs::create_dir_all(unreadable_workspace.path().join("Cargo.toml")).unwrap(); + let member_manifest = unreadable_workspace.path().join("member/Cargo.toml"); + write(&member_manifest, "[package]\nname = \"member\"\n"); + assert!( + workspace_manifest_for( + &member_manifest, + &document("[package]\nworkspace = \"..\"\n"), + ) + .is_err() + ); + + let malformed_workspace = tempdir().unwrap(); + write( + &malformed_workspace.path().join("Cargo.toml"), + "[workspace\n", + ); + let member_manifest = malformed_workspace.path().join("member/Cargo.toml"); + write(&member_manifest, "[package]\nname = \"member\"\n"); + assert!( + workspace_manifest_for( + &member_manifest, + &document("[package]\nworkspace = \"..\"\n"), + ) + .is_err() + ); + let nested_manifest = root.join("member/nested/Cargo.toml"); write( &nested_manifest, @@ -689,7 +874,9 @@ mod tests { let nested_document = document("[package]\nname = \"nested\"\nversion = \"0.1.0\"\nedition = \"2024\"\n"); assert_eq!( - workspace_manifest_for(&nested_manifest, &nested_document).unwrap(), + workspace_manifest_for(&nested_manifest, &nested_document) + .unwrap() + .0, workspace_manifest.canonicalize().unwrap() ); } @@ -706,13 +893,65 @@ mod tests { document("[package]\nname = \"member\"\nversion = \"0.1.0\"\nedition = \"2024\"\n"); write(&directory.path().join("Cargo.toml"), "[workspace\n"); assert_eq!( - workspace_manifest_for(&manifest, &package_document).unwrap(), + workspace_manifest_for(&manifest, &package_document) + .unwrap() + .0, manifest ); let package_workspace = document("[package]\nworkspace = \"..\"\n"); assert!(workspace_manifest_for(Path::new("/"), &package_workspace).is_err()); assert!(workspace_manifest_for(Path::new("/"), &DocumentMut::new()).is_err()); + + let workspace = directory.path().join("Cargo.toml"); + write(&workspace, "[workspace]\n"); + let nested = directory.path().join("nested/Cargo.toml"); + write(&nested, "[package]\nname = \"nested\"\n"); + let nested_document = document("[package]\nname = \"nested\"\n"); + assert!( + workspace_manifest_for_using(&nested, &nested_document, |_| { + Err(std::io::Error::other("workspace disappeared")) + }) + .is_err() + ); + + let calls = std::cell::Cell::new(0); + assert!( + locate_from_manifest_using(&workspace, |_| { + let call = calls.get(); + calls.set(call + 1); + if call == 1 { + Err(std::io::Error::other("package root disappeared")) + } else { + Ok(directory.path().to_path_buf()) + } + }) + .is_err() + ); + assert!( + locate_from_manifest_using(&workspace, |_| { + Err(std::io::Error::other("workspace root disappeared")) + }) + .is_err() + ); + } + + #[test] + fn reports_metadata_process_and_response_errors() { + let directory = tempdir().unwrap(); + let options = Options::default(); + assert!(metadata_with_command(Path::new("/"), &options, cargo()).is_err()); + + let missing_cargo = directory.path().join("missing-cargo"); + assert!( + metadata_with_command( + &directory.path().join("Cargo.toml"), + &options, + std::process::Command::new(missing_cargo), + ) + .is_err() + ); + assert!(parse_metadata(b"not json").is_err()); } #[test] @@ -796,6 +1035,120 @@ mod tests { assert!(locate_from_manifest(&manifest).is_err()); assert!(locate_from_manifest(&manifest.with_file_name("missing.toml")).is_err()); + + write(&manifest, "[workspace\n"); + assert!(locate_from_manifest(&manifest).is_err()); + } + + #[test] + fn reports_project_manifest_and_task_package_errors() { + let directory = tempdir().unwrap(); + let mut options = Options { + manifest: Some(PathBuf::from("missing.toml")), + ..Options::default() + }; + assert!(Project::locate(directory.path(), &options).is_err()); + + write( + &directory.path().join("Cargo.toml"), + "[package]\nname = \"example\"\nversion = \"0.1.0\"\nedition = \"2024\"\n[package.metadata.bake]\nmanifest = \"\"\n", + ); + options.manifest = None; + assert!(Project::locate(directory.path(), &options).is_err()); + + let workspace = tempdir().unwrap(); + write( + &workspace.path().join("Cargo.toml"), + "[workspace]\nmembers = []\n[workspace.metadata.bake]\nmanifest = \"Cargo.toml\"\n", + ); + assert!(Project::discover(workspace.path(), &Options::default()).is_err()); + + let library_workspace = tempdir().unwrap(); + write( + &library_workspace.path().join("Cargo.toml"), + "[workspace]\nmembers = [\"tasks\"]\n[workspace.metadata.bake]\nmanifest = \"tasks/Cargo.toml\"\n", + ); + write( + &library_workspace.path().join("tasks/Cargo.toml"), + "[package]\nname = \"task-library\"\nversion = \"0.1.0\"\nedition = \"2024\"\n", + ); + write( + &library_workspace.path().join("tasks/src/lib.rs"), + "// A task package without a binary.\n", + ); + assert!(Project::discover(library_workspace.path(), &Options::default()).is_err()); + assert!(Project::regenerate(library_workspace.path(), &Options::default()).is_err()); + + let invalid_task_workspace = tempdir().unwrap(); + write( + &invalid_task_workspace.path().join("Cargo.toml"), + "[workspace]\nmembers = []\n[workspace.metadata.bake]\nmanifest = \"tasks/Cargo.toml\"\n", + ); + write( + &invalid_task_workspace.path().join("tasks/Cargo.toml"), + "[package\n", + ); + assert!(Project::discover(invalid_task_workspace.path(), &Options::default()).is_err()); + + let missing_package_workspace = tempdir().unwrap(); + write( + &missing_package_workspace.path().join("Cargo.toml"), + "[workspace]\nmembers = []\n[workspace.metadata.bake]\nmanifest = \"Cargo.toml\"\n", + ); + assert!( + Project::regenerate(missing_package_workspace.path(), &Options::default()).is_err() + ); + + let blocked_task_workspace = tempdir().unwrap(); + write( + &blocked_task_workspace.path().join("Cargo.toml"), + "[workspace]\nmembers = []\n[workspace.metadata.bake]\nmanifest = \"tasks/Cargo.toml\"\n", + ); + fs::write( + blocked_task_workspace.path().join("tasks"), + "not a directory", + ) + .unwrap(); + assert!(Project::regenerate(blocked_task_workspace.path(), &Options::default()).is_err()); + + let generated_module_workspace = tempdir().unwrap(); + write( + &generated_module_workspace.path().join("Cargo.toml"), + "[workspace]\nmembers = [\"bake\"]\n[workspace.metadata.bake]\nmanifest = \"bake/Cargo.toml\"\n", + ); + write( + &generated_module_workspace.path().join("bake/Cargo.toml"), + "[package]\nname = \"example-bake\"\nversion = \"0.1.0\"\nedition = \"2024\"\n", + ); + write( + &generated_module_workspace.path().join("bake/src/main.rs"), + "fn main() {}\n", + ); + fs::write( + generated_module_workspace + .path() + .join("bake/src/bake_generated_tasks"), + "not a directory", + ) + .unwrap(); + assert!( + Project::regenerate(generated_module_workspace.path(), &Options::default()).is_err() + ); + + let manual_module_workspace = tempdir().unwrap(); + write( + &manual_module_workspace.path().join("Cargo.toml"), + "[workspace]\nmembers = [\"bake\"]\n[workspace.metadata.bake]\nmanifest = \"bake/Cargo.toml\"\n", + ); + write( + &manual_module_workspace.path().join("bake/Cargo.toml"), + "[package]\nname = \"example-bake\"\nversion = \"0.1.0\"\nedition = \"2024\"\n", + ); + write( + &manual_module_workspace.path().join("bake/src/main.rs"), + "mod bake_generated_tasks;\n", + ); + assert!(Project::regenerate(manual_module_workspace.path(), &Options::default()).is_err()); } #[test] @@ -972,6 +1325,7 @@ mod tests { fn adds_migrates_and_validates_the_generated_module_declaration() { let directory = tempdir().unwrap(); let source = directory.path().join("src/main.rs"); + assert!(add_generated_module(&source).is_err()); write(&source, "fn main() {}\n"); add_generated_module(&source).unwrap(); let generated = fs::read_to_string(&source).unwrap(); @@ -1013,6 +1367,28 @@ mod tests { ); assert!(!legacy_module.exists()); + let failed_legacy_source = directory.path().join("failed-legacy/main.rs"); + write( + &failed_legacy_source, + "#[path = \"__bake_generated_tasks/mod.rs\"]\nmod __bake_generated_tasks;\n", + ); + assert!( + add_generated_module_using(&failed_legacy_source, |_| { + Err("could not remove generated module".into()) + }) + .is_err() + ); + + let readonly_legacy_source = directory.path().join("readonly-legacy/main.rs"); + write( + &readonly_legacy_source, + "#[path = \"__bake_generated_tasks/mod.rs\"]\nmod __bake_generated_tasks;\n", + ); + let mut permissions = fs::metadata(&readonly_legacy_source).unwrap().permissions(); + permissions.set_readonly(true); + fs::set_permissions(&readonly_legacy_source, permissions).unwrap(); + assert!(add_generated_module(&readonly_legacy_source).is_err()); + let both_source = directory.path().join("both/main.rs"); write( &both_source, @@ -1051,6 +1427,17 @@ mod tests { remove_legacy_generated_module(&source).unwrap(); assert!(legacy_directory.join("keep.rs").is_file()); + write( + &legacy_directory.join("mod.rs"), + "// Generated by `cargo bake --regenerate`; do not edit.\n", + ); + assert!( + remove_legacy_generated_module_using(&source, |_| { + Err(std::io::Error::other("file is locked")) + }) + .is_err() + ); + remove_legacy_generated_module(Path::new("/")).unwrap(); } @@ -1126,6 +1513,50 @@ mod tests { }; assert!(bootstrap_task_package(&location).is_err()); + + let no_manifest_parent = Location { + root: root.clone(), + workspace_root: root.clone(), + workspace_manifest: root.join("Cargo.toml"), + task_manifest: PathBuf::new(), + }; + assert!(bootstrap_task_package(&no_manifest_parent).is_err()); + + let canonicalize_failure = Location { + root: root.clone(), + workspace_root: root.clone(), + workspace_manifest: root.join("Cargo.toml"), + task_manifest: root.join("canonicalize-failure/Cargo.toml"), + }; + assert!( + bootstrap_task_package_using(&canonicalize_failure, |_| { + Err(std::io::Error::other("task directory disappeared")) + }) + .is_err() + ); + + let missing_workspace_location = Location { + root: root.clone(), + workspace_root: root.clone(), + workspace_manifest: root.join("missing-workspace.toml"), + task_manifest: root.join("other-tasks/Cargo.toml"), + }; + assert!(bootstrap_task_package(&missing_workspace_location).is_err()); + + let manifest_directory = root.join("manifest-directory"); + write( + &manifest_directory.join("Cargo.toml"), + "[workspace]\nmembers = []\n", + ); + let task_manifest = manifest_directory.join("bake/Cargo.toml"); + fs::create_dir_all(&task_manifest).unwrap(); + let manifest_location = Location { + root: manifest_directory.canonicalize().unwrap(), + workspace_root: manifest_directory.canonicalize().unwrap(), + workspace_manifest: manifest_directory.join("Cargo.toml"), + task_manifest, + }; + assert!(bootstrap_task_package(&manifest_location).is_err()); } #[test] @@ -1188,5 +1619,15 @@ mod tests { write(&manifest, "[workspace]\nmembers = \"not an array\"\n"); assert!(add_workspace_member(&manifest, "bake").is_err()); + + write(&manifest, "[workspace\n"); + assert!(add_workspace_member(&manifest, "bake").is_err()); + assert!(add_workspace_member(&directory.path().join("missing.toml"), "bake").is_err()); + + write(&manifest, "[package]\nname = \"root\"\n"); + let mut permissions = fs::metadata(&manifest).unwrap().permissions(); + permissions.set_readonly(true); + fs::set_permissions(&manifest, permissions).unwrap(); + assert!(add_workspace_member(&manifest, "bake").is_err()); } } diff --git a/crates/macros/src/expansion_tests.rs b/crates/macros/src/expansion_tests.rs index 726b60d..d3ddbf8 100644 --- a/crates/macros/src/expansion_tests.rs +++ b/crates/macros/src/expansion_tests.rs @@ -28,6 +28,7 @@ fn rejects_unsupported_signatures() { "fn example(#[bake(help)] value: String) {}", "fn example(#[bake(help = 42)] value: String) {}", "fn example(#[bake(input)] value: String) {}", + "fn example(#[bake(input)] value: Value<>) {}", "fn example(#[bake(input)] first: Value, #[bake(input)] second: Value) {}", "fn example(#[bake(input, named)] value: Value) {}", "fn example(#[bake(positional)] value: String) {}", @@ -240,4 +241,17 @@ fn expands_context_parameters_by_convention() { assert!(output.contains("Task :: new")); assert!(!output.contains("Parameter :: new")); + + let named_context = expand( + quote!(), + syn::parse_quote! { + fn example(context: String) -> Result<()> { + let _ = context; + Ok(()) + } + }, + ) + .unwrap() + .to_string(); + assert!(named_context.contains("Parameter :: new :: < String >")); } diff --git a/crates/macros/src/lib.rs b/crates/macros/src/lib.rs index d158c76..f6c155b 100644 --- a/crates/macros/src/lib.rs +++ b/crates/macros/src/lib.rs @@ -82,7 +82,10 @@ fn inner_type<'a>(kind: &str, value: &'a Type) -> Option<&'a Type> { if arguments.args.len() != 1 { return None; } - match arguments.args.first()? { + let Some(argument) = arguments.args.first() else { + unreachable!("the single generic argument was checked above") + }; + match argument { syn::GenericArgument::Type(value) => Some(value), _ => None, } diff --git a/src/arguments.rs b/src/arguments.rs index 8775792..797cc94 100644 --- a/src/arguments.rs +++ b/src/arguments.rs @@ -237,5 +237,13 @@ mod tests { arguments.insert(&repeated, "3").unwrap(); assert_eq!(arguments.repeated::("count").unwrap(), vec![1, 2, 3]); assert!(arguments.insert(&repeated, "not a number").is_err()); + + assert!( + Arguments::extract( + &[Parameter::new::("count")], + &["not a number".to_owned()], + ) + .is_err() + ); } } diff --git a/src/output.rs b/src/output.rs index 0fabf26..2ac27fa 100644 --- a/src/output.rs +++ b/src/output.rs @@ -49,16 +49,16 @@ fn format_value(value: &Value, format: Format) -> Result { Format::Raw => match value { Value::String(text) => text.clone(), Value::Null => String::new(), - value => serde_json::to_string_pretty(value)?, + value => pretty_json(value), }, - Format::Json => serde_json::to_string_pretty(value)?, + Format::Json => pretty_json(value), Format::Ndjson => { let Value::Array(values) = value else { return Err(Error::new("ndjson output requires an array result")); }; let mut output = String::new(); for value in values { - output.push_str(&serde_json::to_string(value)?); + output.push_str(&compact_json(value)); output.push('\n'); } return Ok(output); @@ -71,6 +71,16 @@ fn format_value(value: &Value, format: Format) -> Result { Ok(output) } +fn pretty_json(value: &Value) -> String { + serde_json::to_string_pretty(value) + .unwrap_or_else(|error| unreachable!("JSON values always serialize: {error}")) +} + +fn compact_json(value: &Value) -> String { + serde_json::to_string(value) + .unwrap_or_else(|error| unreachable!("JSON values always serialize: {error}")) +} + fn inferred_format(file: &Path) -> Option { match file.extension()?.to_str()?.to_ascii_lowercase().as_str() { "json" => Some(Format::Json), @@ -138,6 +148,12 @@ mod tests { fn unknown_file_extensions_do_not_select_a_format() { assert_eq!(inferred_format(Path::new("releases.md")), None); assert_eq!(inferred_format(Path::new("notes.csv")), None); + assert_eq!(inferred_format(Path::new("notes.txt")), Some(Format::Raw)); + assert_eq!(inferred_format(Path::new("notes.text")), Some(Format::Raw)); + assert_eq!( + inferred_format(Path::new("notes.ndjson")), + Some(Format::Ndjson) + ); } #[cfg(unix)] diff --git a/src/registry.rs b/src/registry.rs index 3326695..bfcf085 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -232,12 +232,25 @@ impl Registry { /// Run the process command line and print its final result through `output`. pub fn run(self) -> Result<()> { - let tokens: Vec<_> = std::env::args_os() + self.run_with_process_environment( + std::env::args_os(), + std::env::var_os("BAKE_PROJECT_ROOT"), + std::env::current_dir, + ) + } + + fn run_with_process_environment( + self, + arguments: impl IntoIterator, + configured_root: Option, + current_directory: fn() -> io::Result, + ) -> Result<()> { + let tokens: Vec<_> = arguments + .into_iter() .skip(1) .map(task_argument) .collect::>()?; - let root = - root_from_environment(std::env::var_os("BAKE_PROJECT_ROOT"), std::env::current_dir)?; + let root = root_from_environment(configured_root, current_directory)?; self.run_with(root, &tokens, &mut io::stdout().lock()) } @@ -493,6 +506,17 @@ mod tests { assert!(Registry::new().run().is_ok()); } + #[test] + fn reports_project_root_discovery_errors() { + let error = Registry::new() + .run_with_process_environment([], None, || { + Err(io::Error::other("no current directory")) + }) + .unwrap_err(); + + assert!(error.to_string().contains("no current directory")); + } + #[test] fn runs_path_root_commands_and_reports_unknown_help_tasks() { let mut registry = Registry::new(); From 26d01fa04f6032b54b926ff651342da7cdc1e343 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 5 Oct 2026 22:48:13 +1300 Subject: [PATCH 2/5] Cover launcher and task paths across platforms. --- bake/tests/tasks.rs | 81 +++++++++------ crates/cargo-bake/src/main.rs | 164 +++++++++++++++++++++++++++++-- crates/cargo-bake/src/project.rs | 40 ++++++++ src/output.rs | 25 +++-- src/registry.rs | 31 +++++- 5 files changed, 298 insertions(+), 43 deletions(-) diff --git a/bake/tests/tasks.rs b/bake/tests/tasks.rs index d7eaf9b..516ec4a 100644 --- a/bake/tests/tasks.rs +++ b/bake/tests/tasks.rs @@ -3,7 +3,6 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Output}; -#[cfg(unix)] use tempfile::TempDir; fn root() -> PathBuf { @@ -13,7 +12,6 @@ fn root() -> PathBuf { .to_path_buf() } -#[cfg(unix)] fn release_section() -> (String, String) { let releases = std::fs::read_to_string(root().join("releases.md")).unwrap(); let mut heading = None; @@ -92,45 +90,60 @@ fn default_registry_exposes_the_builtin_tasks() { } } -#[cfg(unix)] -fn fake_cargo(directory: &TempDir, exit_code: u8) -> PathBuf { - use std::os::unix::fs::PermissionsExt; - - let path = directory.path().join("fake-cargo"); +fn fake_cargo(directory: &TempDir) -> PathBuf { + let source = directory.path().join("fake-cargo.rs"); + let path = directory.path().join(if cfg!(windows) { + "fake-cargo.exe" + } else { + "fake-cargo" + }); std::fs::write( - &path, - format!( - "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$BAKE_TEST_CARGO_ARGUMENTS\"\nexit {exit_code}\n" - ), + &source, + r#" +fn main() { + let arguments = std::env::args().skip(1).collect::>().join("\n") + "\n"; + std::fs::write(std::env::var_os("BAKE_TEST_CARGO_ARGUMENTS").unwrap(), arguments).unwrap(); + let exit_code: i32 = std::env::var("BAKE_TEST_CARGO_EXIT_CODE").unwrap().parse().unwrap(); + std::process::exit(exit_code); +} +"#, ) .unwrap(); - let mut permissions = std::fs::metadata(&path).unwrap().permissions(); - permissions.set_mode(0o755); - std::fs::set_permissions(&path, permissions).unwrap(); + let output = Command::new("rustc") + .args(["--edition=2024", "--crate-name=fake_cargo"]) + .arg(&source) + .arg("-o") + .arg(&path) + .output() + .unwrap(); + assert!( + output.status.success(), + "failed to compile fake cargo: {}", + String::from_utf8_lossy(&output.stderr) + ); path } -#[cfg(unix)] -fn run_with_cargo(arguments: &[&str], cargo: &Path, log: &Path) -> Output { +fn run_with_cargo(arguments: &[&str], cargo: &Path, log: &Path, exit_code: u8) -> Output { Command::new(env!("CARGO_BIN_EXE_bake-rust-tasks")) .args(arguments) .current_dir(root()) .env("BAKE_PROJECT_ROOT", root()) .env("CARGO", cargo) .env("BAKE_TEST_CARGO_ARGUMENTS", log) + .env("BAKE_TEST_CARGO_EXIT_CODE", exit_code.to_string()) .output() .unwrap() } -#[cfg(unix)] #[test] fn checks_workspace_with_optional_offline_mode() { let directory = tempfile::tempdir().unwrap(); - let cargo = fake_cargo(&directory, 0); + let cargo = fake_cargo(&directory); let arguments = directory.path().join("arguments.txt"); assert!( - run_with_cargo(&["build:check"], &cargo, &arguments) + run_with_cargo(&["build:check"], &cargo, &arguments, 0) .status .success() ); @@ -140,7 +153,7 @@ fn checks_workspace_with_optional_offline_mode() { ); assert!( - run_with_cargo(&["build:check", "--offline", "true"], &cargo, &arguments) + run_with_cargo(&["build:check", "--offline", "true"], &cargo, &arguments, 0) .status .success() ); @@ -150,15 +163,14 @@ fn checks_workspace_with_optional_offline_mode() { ); } -#[cfg(unix)] #[test] fn propagates_check_failures_and_runs_release_notes_hook() { let directory = tempfile::tempdir().unwrap(); - let cargo = fake_cargo(&directory, 7); + let cargo = fake_cargo(&directory); let arguments = directory.path().join("arguments.txt"); let (heading, note) = release_section(); - let output = run_with_cargo(&["build:check"], &cargo, &arguments); + let output = run_with_cargo(&["build:check"], &cargo, &arguments, 7); assert!(!output.status.success()); assert!(String::from_utf8_lossy(&output.stderr).contains("cargo check failed")); @@ -166,26 +178,39 @@ fn propagates_check_failures_and_runs_release_notes_hook() { &["release:prepare", &heading, "--offline", "true"], &cargo, &arguments, + 7, ); assert!(!output.status.success()); assert!(String::from_utf8_lossy(&output.stderr).contains("cargo check failed")); - let cargo = fake_cargo(&directory, 0); + let cargo = fake_cargo(&directory); let output = stdout(run_with_cargo( &["release:prepare", &heading, "--offline", "true"], &cargo, &arguments, + 0, )); assert!(output.contains(¬e)); } -#[cfg(unix)] +fn non_utf8_argument() -> std::ffi::OsString { + #[cfg(unix)] + { + use std::os::unix::ffi::OsStringExt; + std::ffi::OsString::from_vec(vec![0xff]) + } + + #[cfg(windows)] + { + use std::os::windows::ffi::OsStringExt; + std::ffi::OsString::from_wide(&[0xd800]) + } +} + #[test] fn rejects_non_utf8_arguments() { - use std::os::unix::ffi::OsStrExt; - let output = Command::new(env!("CARGO_BIN_EXE_bake-rust-tasks")) - .arg(std::ffi::OsStr::from_bytes(b"\xff")) + .arg(non_utf8_argument()) .current_dir(root()) .env("BAKE_PROJECT_ROOT", root()) .output() diff --git a/crates/cargo-bake/src/main.rs b/crates/cargo-bake/src/main.rs index e88e637..fc04bf5 100644 --- a/crates/cargo-bake/src/main.rs +++ b/crates/cargo-bake/src/main.rs @@ -20,7 +20,14 @@ fn cargo_program(program: Option) -> OsString { } fn run() -> Result { - let options = Options::parse(std::env::args_os().skip(1))?; + run_with_arguments(std::env::args_os().skip(1), std::env::current_dir()) +} + +fn run_with_arguments( + arguments: impl IntoIterator, + current_directory: std::io::Result, +) -> Result { + let options = Options::parse(arguments)?; if options.help { println!( "cargo bake [OPTIONS] [TASK [ARGUMENTS] [:: TASK ...]]\n\nCompile and run the project's bake/ task crate.\n\nLauncher options (before TASK):\n --manifest-path PATH Project Cargo.toml (defaults to nearest ancestor)\n --offline Disable Cargo network access\n --locked Require unchanged Cargo.lock files\n --release Use Cargo's release profile\n --regenerate Create or refresh the private Bake task crate\n --help Show this launcher help without compiling tasks\n --version Show the launcher version\n\nTask options:\n --list List registered tasks\n --json Format the final task result as JSON\n TASK --help Show task arguments\n\nDefaults: workspace-root/bake/Cargo.toml. Override with\n[workspace.metadata.bake] or [package.metadata.bake] manifest = \"...\"." @@ -34,20 +41,40 @@ fn run() -> Result { if options.regenerate && !options.arguments.is_empty() { return Err("--regenerate cannot be combined with task arguments".into()); } - run_project(options, std::env::current_dir()) + run_project(options, current_directory) } fn run_project( options: Options, current_directory: std::io::Result, ) -> Result { + let mut command = cargo(); + run_project_with_command(options, current_directory, &mut command, command_status) +} + +fn run_project_with_command( + options: Options, + current_directory: std::io::Result, + command: &mut Command, + execute: impl FnOnce(&mut Command) -> Result, +) -> Result { + match configure_project_command(options, current_directory, command)? { + Some(code) => Ok(code), + None => execute(command), + } +} + +fn configure_project_command( + options: Options, + current_directory: std::io::Result, + command: &mut Command, +) -> Result> { let current_directory = current_directory?; if options.regenerate { Project::regenerate(¤t_directory, &options)?; - return Ok(0); + return Ok(Some(0)); } let project = Project::discover(¤t_directory, &options)?; - let mut command = cargo(); command .args(["run", "--quiet", "--manifest-path"]) .arg(&project.task_manifest) @@ -57,12 +84,12 @@ fn run_project( .arg(&project.binary) .current_dir(&project.root) .env("BAKE_PROJECT_ROOT", &project.root); - options.configure(&mut command); + options.configure(command); if options.release { command.arg("--release"); } command.arg("--").args(&options.arguments); - command_status(&mut command) + Ok(None) } fn command_status(command: &mut Command) -> Result { @@ -100,6 +127,39 @@ mod tests { fn cargo_program_uses_the_environment_or_default() { assert_eq!(cargo_program(None), "cargo"); assert_eq!(cargo_program(Some("custom-cargo".into())), "custom-cargo"); + assert_eq!( + cargo().get_program(), + cargo_program(std::env::var_os("CARGO")) + ); + } + + #[test] + fn handles_help_version_and_invalid_regeneration_arguments_without_a_project() { + let directory = tempfile::tempdir().unwrap(); + let current_directory = || Ok(directory.path().to_path_buf()); + + assert_eq!( + run_with_arguments(["--help"].map(OsString::from), current_directory()).unwrap(), + 0 + ); + assert_eq!( + run_with_arguments(["--version"].map(OsString::from), current_directory()).unwrap(), + 0 + ); + assert!( + run_with_arguments( + ["--regenerate", "greet"].map(OsString::from), + current_directory(), + ) + .is_err() + ); + assert!( + run_with_arguments( + ["--help"].map(OsString::from), + Err(std::io::Error::other("no current directory")), + ) + .is_ok() + ); } #[test] @@ -139,4 +199,96 @@ mod tests { assert!(command_status(&mut command).is_err()); } + + #[test] + fn configures_the_task_command_and_returns_regeneration_results() { + let directory = tempfile::tempdir().unwrap(); + let root = directory.path(); + std::fs::create_dir_all(root.join("bake/src")).unwrap(); + std::fs::write( + root.join("Cargo.toml"), + "[workspace]\nmembers = [\"bake\"]\nresolver = \"3\"\n", + ) + .unwrap(); + std::fs::write( + root.join("bake/Cargo.toml"), + "[package]\nname = \"example-bake\"\nversion = \"0.0.0\"\nedition = \"2024\"\n", + ) + .unwrap(); + std::fs::write(root.join("bake/src/main.rs"), "fn main() {}\n").unwrap(); + + let canonical_root = root.canonicalize().unwrap(); + let canonical_manifest = root.join("bake/Cargo.toml").canonicalize().unwrap(); + let mut command = Command::new("fake-cargo"); + let options = Options { + offline: true, + locked: true, + release: true, + arguments: ["greet", "Ada"].map(OsString::from).into(), + ..Options::default() + }; + let status = + run_project_with_command(options, Ok(root.to_path_buf()), &mut command, |command| { + assert_eq!( + command + .get_args() + .map(|argument| argument.to_os_string()) + .collect::>(), + [ + "run", + "--quiet", + "--manifest-path", + canonical_manifest.to_str().unwrap(), + "--package", + "example-bake", + "--bin", + "example-bake", + "--offline", + "--locked", + "--release", + "--", + "greet", + "Ada", + ] + .map(OsString::from) + .to_vec() + ); + assert_eq!(command.get_current_dir(), Some(canonical_root.as_path())); + Ok(23) + }) + .unwrap(); + assert_eq!(status, 23); + + assert_eq!( + run_project_with_command( + Options { + regenerate: true, + ..Options::default() + }, + Ok(root.to_path_buf()), + &mut Command::new("fake-cargo"), + command_status, + ) + .unwrap(), + 0 + ); + } + + #[test] + fn command_status_preserves_successful_child_exit_codes() { + #[cfg(unix)] + let mut command = { + let mut command = Command::new("sh"); + command.args(["-c", "exit 23"]); + command + }; + #[cfg(windows)] + let mut command = { + let mut command = Command::new("cmd.exe"); + command.args(["/C", "exit 23"]); + command + }; + + assert_eq!(command_status(&mut command).unwrap(), 23); + } } diff --git a/crates/cargo-bake/src/project.rs b/crates/cargo-bake/src/project.rs index d68e4bc..dcef985 100644 --- a/crates/cargo-bake/src/project.rs +++ b/crates/cargo-bake/src/project.rs @@ -1210,6 +1210,46 @@ mod tests { assert!(error.to_string().contains("cannot open task manifest")); } + #[test] + fn regenerates_a_missing_task_package_and_reports_metadata_failures() { + let directory = tempdir().unwrap(); + let root = directory.path(); + let workspace_manifest = root.join("Cargo.toml"); + write(&workspace_manifest, "[workspace]\nmembers = []\n"); + let location = Location { + root: root.canonicalize().unwrap(), + workspace_root: root.canonicalize().unwrap(), + workspace_manifest, + task_manifest: root.join("bake/Cargo.toml"), + }; + + Project::regenerate_at_location(&location, &Options::default(), |path| path.canonicalize()) + .unwrap(); + assert!(location.task_manifest.is_file()); + + let invalid_directory = tempdir().unwrap(); + let invalid_root = invalid_directory.path(); + let invalid_workspace_manifest = invalid_root.join("Cargo.toml"); + write( + &invalid_workspace_manifest, + "[workspace]\nmembers = [\"bake\"]\n", + ); + let invalid_manifest = invalid_root.join("bake/Cargo.toml"); + write(&invalid_manifest, "[package\n"); + let invalid_location = Location { + root: invalid_root.canonicalize().unwrap(), + workspace_root: invalid_root.canonicalize().unwrap(), + workspace_manifest: invalid_workspace_manifest, + task_manifest: invalid_manifest, + }; + + assert!( + Project::regenerate_at_location(&invalid_location, &Options::default(), |path| path + .canonicalize(),) + .is_err() + ); + } + #[test] fn selects_the_requested_binary_or_requires_one_unambiguous_binary() { let directory = tempdir().unwrap(); diff --git a/src/output.rs b/src/output.rs index 2ac27fa..05fcf53 100644 --- a/src/output.rs +++ b/src/output.rs @@ -156,14 +156,27 @@ mod tests { ); } - #[cfg(unix)] #[test] fn non_utf8_file_extensions_do_not_select_a_format() { - use std::os::unix::ffi::OsStringExt; - - let path = PathBuf::from(std::ffi::OsString::from_vec(vec![ - b'n', b'o', b't', b'e', b'.', 0xff, - ])); + #[cfg(unix)] + let path = { + use std::os::unix::ffi::OsStringExt; + PathBuf::from(std::ffi::OsString::from_vec(vec![ + b'n', b'o', b't', b'e', b'.', 0xff, + ])) + }; + #[cfg(windows)] + let path = { + use std::os::windows::ffi::OsStringExt; + PathBuf::from(std::ffi::OsString::from_wide(&[ + b'n' as u16, + b'o' as u16, + b't' as u16, + b'e' as u16, + b'.' as u16, + 0xd800, + ])) + }; assert_eq!(inferred_format(&path), None); } diff --git a/src/registry.rs b/src/registry.rs index bfcf085..4c5c9a8 100644 --- a/src/registry.rs +++ b/src/registry.rs @@ -568,11 +568,36 @@ mod tests { assert_eq!(task_argument("inspect".into()).unwrap(), "inspect"); #[cfg(unix)] - { + let invalid = { use std::os::unix::ffi::OsStringExt; + OsString::from_vec(vec![0xff]) + }; - assert!(task_argument(OsString::from_vec(vec![0xff])).is_err()); - } + #[cfg(windows)] + let invalid = { + use std::os::windows::ffi::OsStringExt; + OsString::from_wide(&[0xd800]) + }; + + assert!(task_argument(invalid.clone()).is_err()); + assert!( + Registry::new() + .run_with_process_environment( + [OsString::from("bake"), invalid], + None, + std::env::current_dir, + ) + .is_err() + ); + + let error = Registry::new() + .run_with_process_environment( + [OsString::from("bake"), OsString::from("missing")], + None, + || Ok(PathBuf::from(".")), + ) + .unwrap_err(); + assert!(error.to_string().contains("unknown task")); } #[test] From 6d85de22f0997f306f3c9548163a7e9fdd9fb803 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 5 Oct 2026 22:53:15 +1300 Subject: [PATCH 3/5] Cover release and Windows launcher branches. --- crates/cargo-bake/src/main.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crates/cargo-bake/src/main.rs b/crates/cargo-bake/src/main.rs index fc04bf5..b3c436f 100644 --- a/crates/cargo-bake/src/main.rs +++ b/crates/cargo-bake/src/main.rs @@ -7,7 +7,9 @@ mod project; use options::Options; use project::Project; use std::ffi::OsString; -use std::process::{Command, ExitCode}; +use std::process::Command; +#[cfg(not(test))] +use std::process::ExitCode; type Result = std::result::Result>; @@ -19,6 +21,7 @@ fn cargo_program(program: Option) -> OsString { program.unwrap_or_else(|| "cargo".into()) } +#[cfg(not(test))] fn run() -> Result { run_with_arguments(std::env::args_os().skip(1), std::env::current_dir()) } @@ -105,6 +108,7 @@ fn command_status(command: &mut Command) -> Result { Ok(status.code().unwrap_or(1)) } +#[cfg(not(test))] fn main() -> ExitCode { match run() { Ok(code) => { @@ -259,6 +263,20 @@ mod tests { .unwrap(); assert_eq!(status, 23); + assert_eq!( + run_project_with_command( + Options::default(), + Ok(root.to_path_buf()), + &mut Command::new("fake-cargo"), + |command| { + assert!(!command.get_args().any(|argument| argument == "--release")); + Ok(17) + }, + ) + .unwrap(), + 17 + ); + assert_eq!( run_project_with_command( Options { From 9c5a331d926ddaf8ab3af713fcc9d9f1b5c1ca45 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 5 Oct 2026 22:56:36 +1300 Subject: [PATCH 4/5] Test launcher result handling directly. --- crates/cargo-bake/src/main.rs | 52 +++++++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/crates/cargo-bake/src/main.rs b/crates/cargo-bake/src/main.rs index b3c436f..51cc17f 100644 --- a/crates/cargo-bake/src/main.rs +++ b/crates/cargo-bake/src/main.rs @@ -7,9 +7,7 @@ mod project; use options::Options; use project::Project; use std::ffi::OsString; -use std::process::Command; -#[cfg(not(test))] -use std::process::ExitCode; +use std::process::{Command, ExitCode}; type Result = std::result::Result>; @@ -108,25 +106,44 @@ fn command_status(command: &mut Command) -> Result { Ok(status.code().unwrap_or(1)) } -#[cfg(not(test))] -fn main() -> ExitCode { - match run() { - Ok(code) => { - // Preserve the full process code, including Windows child exit codes. - std::process::exit(code) - } +fn process_result( + result: Result, + exit: impl FnOnce(i32) -> ExitCode, + report: impl FnOnce(String), +) -> ExitCode { + match result { + Ok(code) => exit(code), Err(error) => { - eprintln!("cargo bake: {error}"); + report(format!("cargo bake: {error}")); ExitCode::FAILURE } } } +#[cfg(not(test))] +fn main() -> ExitCode { + // Preserve the full process code, including Windows child exit codes. + process_result( + run(), + |code| std::process::exit(code), + |message| eprintln!("{message}"), + ) +} + #[cfg(test)] mod tests { use super::*; use std::path::PathBuf; + fn assert_exit_code(code: i32) -> ExitCode { + assert_eq!(code, 23); + ExitCode::SUCCESS + } + + fn assert_error_message(error: String) { + assert_eq!(error, "cargo bake: expected failure"); + } + #[test] fn cargo_program_uses_the_environment_or_default() { assert_eq!(cargo_program(None), "cargo"); @@ -137,6 +154,19 @@ mod tests { ); } + #[test] + fn process_result_preserves_exit_codes_and_reports_errors() { + let result = process_result(Ok(23), assert_exit_code, assert_error_message); + assert_eq!(result, ExitCode::SUCCESS); + + let result = process_result( + Err("expected failure".into()), + assert_exit_code, + assert_error_message, + ); + assert_eq!(result, ExitCode::FAILURE); + } + #[test] fn handles_help_version_and_invalid_regeneration_arguments_without_a_project() { let directory = tempfile::tempdir().unwrap(); From 57cff5d4e00d14f81c14027363d58b46ac7361ad Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Mon, 5 Oct 2026 23:19:43 +1300 Subject: [PATCH 5/5] Flush coverage before exiting on Windows. --- crates/cargo-bake/Cargo.toml | 3 +++ crates/cargo-bake/src/main.rs | 23 ++++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/crates/cargo-bake/Cargo.toml b/crates/cargo-bake/Cargo.toml index 13b16ec..8062c4a 100644 --- a/crates/cargo-bake/Cargo.toml +++ b/crates/cargo-bake/Cargo.toml @@ -7,6 +7,9 @@ repository.workspace = true description = "Cargo launcher for project-local Bake tasks" readme = "readme.md" +[lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = ["cfg(coverage)"] } + [[bin]] name = "cargo-bake" path = "src/main.rs" diff --git a/crates/cargo-bake/src/main.rs b/crates/cargo-bake/src/main.rs index 51cc17f..95b0de3 100644 --- a/crates/cargo-bake/src/main.rs +++ b/crates/cargo-bake/src/main.rs @@ -24,6 +24,24 @@ fn run() -> Result { run_with_arguments(std::env::args_os().skip(1), std::env::current_dir()) } +// Windows terminates directly from `std::process::exit`, before LLVM's profile +// runtime can write the child process's coverage data. +#[cfg(all(not(test), coverage, windows))] +unsafe extern "C" { + fn __llvm_profile_write_file() -> i32; +} + +#[cfg(all(not(test), coverage, windows))] +fn write_coverage_profile() { + // The coverage runtime is linked by cargo-llvm-cov for coverage builds. + unsafe { + let _ = __llvm_profile_write_file(); + } +} + +#[cfg(all(not(test), not(all(coverage, windows))))] +fn write_coverage_profile() {} + fn run_with_arguments( arguments: impl IntoIterator, current_directory: std::io::Result, @@ -125,7 +143,10 @@ fn main() -> ExitCode { // Preserve the full process code, including Windows child exit codes. process_result( run(), - |code| std::process::exit(code), + |code| { + write_coverage_profile(); + std::process::exit(code) + }, |message| eprintln!("{message}"), ) }