Skip to content

Commit

Permalink
fix(cargo-codspeed): handle bench build per process to fix working di…
Browse files Browse the repository at this point in the history
…rectory issues
  • Loading branch information
art049 committed Apr 22, 2024
1 parent 2e7a495 commit 1f48cee
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 87 deletions.
139 changes: 78 additions & 61 deletions crates/cargo-codspeed/src/build.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use crate::{
helpers::{clear_dir, get_codspeed_target_dir},
helpers::{clear_dir, get_codspeed_target_dir, get_target_packages},
prelude::*,
};

use std::{collections::BTreeSet, fs::create_dir_all, rc::Rc};

use cargo::{
core::{FeatureValue, Package, Verbosity, Workspace},
core::{FeatureValue, Package, Target, Verbosity, Workspace},
ops::{CompileFilter, CompileOptions, Packages},
util::{command_prelude::CompileMode, interning::InternedString},
Config,
Expand Down Expand Up @@ -50,46 +50,56 @@ fn get_compile_options(
Ok(compile_opts)
}

struct BenchToBuild<'a> {
package: &'a Package,
target: &'a Target,
}

pub fn build_benches(
ws: &Workspace,
selected_benches: Option<Vec<String>>,
package_name: Option<String>,
features: Option<Vec<String>>,
) -> Result<()> {
let is_root_package = package_name.is_none();
let package = match package_name.as_ref() {
Some(package_name) => ws.members()
.find(|p| p.name().to_string() == *package_name)
.ok_or_else(|| anyhow!("Package {} not found", package_name))?

,
None => ws.current().map_err(|_| anyhow!("No package found. If working with a workspace please use the -p option to specify a member."))?
};
let all_benches = package
.targets()
.iter()
.filter(|t| t.is_bench())
.collect_vec();
let packages_to_build = get_target_packages(&package_name, ws)?;
let mut benches_to_build = vec![];
for package in packages_to_build.iter() {
let benches = package
.targets()
.iter()
.filter(|t| t.is_bench())
.collect_vec();
benches_to_build.extend(
benches
.into_iter()
.map(|t| BenchToBuild { package, target: t }),
);
}

let all_benches_count = all_benches.len();
let all_benches_count = benches_to_build.len();

let benches = if let Some(selected_benches) = selected_benches {
all_benches
let benches_to_build = if let Some(selected_benches) = selected_benches {
benches_to_build
.into_iter()
.filter(|t| selected_benches.contains(&t.name().to_string()))
.filter(|t| selected_benches.contains(&t.target.name().to_string()))
.collect_vec()
} else {
all_benches
benches_to_build
};

let actual_benches_count = benches_to_build.len();

ws.config().shell().set_verbosity(Verbosity::Normal);
ws.config().shell().status_with_color(
"Collected",
format!(
"{} benchmark suite(s) to build{}",
benches.len(),
if all_benches_count > benches.len() {
format!(" ({} filtered out)", all_benches_count - benches.len())
benches_to_build.len(),
if all_benches_count > actual_benches_count {
format!(
" ({} filtered out)",
all_benches_count - actual_benches_count
)
} else {
"".to_string()
}
Expand All @@ -98,51 +108,58 @@ pub fn build_benches(
)?;

let config = ws.config();
let codspeed_root_target_dir = get_codspeed_target_dir(ws);
// Create and clear packages target directories
for package in packages_to_build.iter() {
let package_target_dir = codspeed_root_target_dir.join(package.name());
create_dir_all(&package_target_dir)?;
clear_dir(&package_target_dir)?;
}
let mut built_benches = 0;
for bench in benches_to_build.iter() {
ws.config().shell().status_with_color(
"Building",
format!("{} {}", bench.package.name(), bench.target.name()),
Color::Yellow,
)?;
let is_root_package = ws.current_opt().map_or(false, |p| p == bench.package);
let benches_names = vec![bench.target.name()];
let compile_opts = get_compile_options(
config,
&features,
bench.package,
benches_names,
is_root_package,
)?;
let result = cargo::ops::compile(ws, &compile_opts)?;
let built_units = result
.tests
.into_iter()
.filter(|u| u.unit.target.is_bench())
.collect_vec();
if built_units.is_empty() {
continue;
}
built_benches += 1;
let codspeed_target_dir = codspeed_root_target_dir.join(bench.package.name());
for built_unit in built_units.iter() {
let bench_dest = codspeed_target_dir
.clone()
.join(built_unit.unit.target.name());
println!("Copying {:?} to {:?}", built_unit.path, bench_dest);
std::fs::copy(built_unit.path.clone(), bench_dest)?;
}
}

let benches_names = benches.iter().map(|t| t.name()).collect_vec();
let benches_names_str = benches_names.join(", ");

ws.config()
.shell()
.status_with_color("Building", benches_names_str.clone(), Color::Yellow)?;
let compile_opts =
get_compile_options(config, &features, package, benches_names, is_root_package)?;
let result = cargo::ops::compile(ws, &compile_opts)?;
let built_benches = result
.tests
.into_iter()
.filter(|u| u.unit.target.is_bench())
.collect_vec();

if built_benches.is_empty() {
if built_benches == 0 {
bail!(
"No benchmark target found. \
Please add a benchmark target to your Cargo.toml"
);
}

ws.config()
.shell()
.status_with_color("Built", benches_names_str, Color::Green)?;

let mut codspeed_target_dir = get_codspeed_target_dir(ws);
create_dir_all(&codspeed_target_dir)?;
if let Some(name) = package_name.as_ref() {
codspeed_target_dir = codspeed_target_dir.join(name);
create_dir_all(&codspeed_target_dir)?;
}
clear_dir(&codspeed_target_dir)?;

for built_bench in built_benches.iter() {
let bench_dest = codspeed_target_dir
.clone()
.join(built_bench.unit.target.name());
std::fs::copy(built_bench.path.clone(), bench_dest)?;
}

ws.config().shell().status_with_color(
"Finished",
format!("built {} benchmark suite(s)", benches.len()),
format!("built {} benchmark suite(s)", built_benches),
Color::Green,
)?;

Expand Down
24 changes: 24 additions & 0 deletions crates/cargo-codspeed/src/helpers.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::prelude::*;
use cargo::CargoResult;
use std::path::{Path, PathBuf};

pub fn get_codspeed_target_dir(ws: &Workspace) -> PathBuf {
Expand All @@ -8,6 +9,29 @@ pub fn get_codspeed_target_dir(ws: &Workspace) -> PathBuf {
.join("codspeed")
}

/// Get the packages to run benchmarks for
/// If a package name is provided, only that package is a target
/// If no package name is provided,
/// and the current directory is a package then only that package is a target
/// Otherwise all packages in the workspace are targets
pub fn get_target_packages<'a>(
package_name: &Option<String>,
ws: &'a Workspace<'_>,
) -> Result<Vec<&'a cargo::core::Package>> {
let packages_to_run = if let Some(package) = package_name.as_ref() {
let p = ws
.members()
.find(|m| m.manifest().name().to_string().as_str() == package)
.ok_or(anyhow!("Package {} not found", package))?;
vec![p]
} else if let CargoResult::Ok(p) = ws.current() {
vec![p]
} else {
ws.members().collect::<Vec<_>>()
};
Ok(packages_to_run)
}

pub fn clear_dir<P>(dir: P) -> Result<()>
where
P: AsRef<Path>,
Expand Down
26 changes: 6 additions & 20 deletions crates/cargo-codspeed/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ use std::{io, path::PathBuf};
use anyhow::anyhow;
use termcolor::Color;

use crate::{helpers::get_codspeed_target_dir, prelude::*};
use crate::{
helpers::{get_codspeed_target_dir, get_target_packages},
prelude::*,
};

struct BenchToRun {
bench_path: PathBuf,
Expand All @@ -18,28 +21,11 @@ pub fn run_benches(
package: Option<String>,
) -> Result<()> {
let codspeed_target_dir = get_codspeed_target_dir(ws);

let packages_to_run = if let Some(package) = package.as_ref() {
let p = ws
.members()
.find(|m| m.manifest().name().to_string().as_str() == package);
if let Some(p) = p {
vec![p]
} else {
bail!("Package {} not found", package);
}
} else {
ws.members().collect::<Vec<_>>()
};
let packages_to_run = get_target_packages(&package, ws)?;
let mut benches: Vec<BenchToRun> = vec![];
for p in packages_to_run {
let package_name = p.manifest().name().to_string();
let is_root_package = p.root() == ws.root();
let package_target_dir = if is_root_package {
codspeed_target_dir.clone()
} else {
codspeed_target_dir.join(&package_name)
};
let package_target_dir = codspeed_target_dir.join(&package_name);
let working_directory = p.root().to_path_buf();
if let io::Result::Ok(read_dir) = std::fs::read_dir(&package_target_dir) {
for entry in read_dir {
Expand Down
22 changes: 22 additions & 0 deletions crates/cargo-codspeed/tests/crates_working_directory.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use predicates::str::contains;
use std::path::PathBuf;

mod helpers;
use helpers::*;
Expand Down Expand Up @@ -34,3 +35,24 @@ fn test_crates_working_directory_build_and_run_implicit() {
.stderr(contains("Finished running 1 benchmark suite(s)"));
teardown(dir);
}

#[test]
fn test_crates_working_directory_build_in_subfolder_and_run() {
let dir = setup(DIR, Project::CratesWorkingDirectory);
cargo_codspeed(&dir)
.current_dir(PathBuf::from(&dir).join("the_crate"))
.args(["build"])
.assert()
.success();
cargo_codspeed(&dir)
.arg("run")
.assert()
.success()
.stderr(contains("Finished running 1 benchmark suite(s)"));
cargo_codspeed(&dir)
.args(["run", "-p", "the_crate"])
.assert()
.success()
.stderr(contains("Finished running 1 benchmark suite(s)"));
teardown(dir);
}
17 changes: 11 additions & 6 deletions crates/cargo-codspeed/tests/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,13 @@ fn test_workspace_build_without_package_spec() {
cargo_codspeed(&dir)
.arg("build")
.assert()
.failure()
.stderr(contains(
"Error No package found. If working with a workspace please use the -p option to specify a member.",
));
.success()
.stderr(contains("Finished built 3 benchmark suite(s)"));
cargo_codspeed(&dir)
.arg("run")
.assert()
.success()
.stderr(contains("Finished running 3 benchmark suite(s)"));
teardown(dir);
}

Expand Down Expand Up @@ -91,12 +94,14 @@ fn test_workspace_build_both_and_run_all() {
cargo_codspeed(&dir)
.arg("build")
.args(["--package", "package-a"])
.assert();
.assert()
.success();

cargo_codspeed(&dir)
.arg("build")
.args(["--package", "package-b"])
.assert();
.assert()
.success();

cargo_codspeed(&dir)
.arg("run")
Expand Down

0 comments on commit 1f48cee

Please sign in to comment.