From 13401947c6e26228603383a7bd28e1f931f23da9 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sun, 14 Jul 2024 16:25:28 +0900 Subject: [PATCH 1/6] Add --partition M/N to distribute builds --- README.md | 3 +++ src/cli.rs | 10 +++++++++- src/main.rs | 39 +++++++++++++++++++++++++++++++++++++-- tests/long-help.txt | 3 +++ tests/short-help.txt | 2 ++ 5 files changed, 54 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index b9d7ca25..752b314a 100644 --- a/README.md +++ b/README.md @@ -216,6 +216,9 @@ OPTIONS: --keep-going Keep going on failure. + --partition + Partition runs and execute only its subset according to M/N. + --log-group Log grouping: none, github-actions. diff --git a/src/cli.rs b/src/cli.rs index f6933e33..3df1c97a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -13,7 +13,7 @@ use lexopt::{ ValueExt, }; -use crate::{term, version::VersionRange, Feature, LogGroup, Rustup}; +use crate::{term, version::VersionRange, Feature, LogGroup, Partition, Rustup}; pub(crate) struct Args { pub(crate) leading_args: Vec, @@ -53,6 +53,8 @@ pub(crate) struct Args { pub(crate) clean_per_version: bool, /// --keep-going pub(crate) keep_going: bool, + /// --partition + pub(crate) partition: Option, /// --print-command-list pub(crate) print_command_list: bool, /// --version-range/--rust-version @@ -155,6 +157,7 @@ impl Args { let mut clean_per_run = false; let mut clean_per_version = false; let mut keep_going = false; + let mut partition = None; let mut print_command_list = false; let mut no_manifest_path = false; let mut locked = false; @@ -308,6 +311,7 @@ impl Args { Long("clean-per-run") => parse_flag!(clean_per_run), Long("clean-per-version") => parse_flag!(clean_per_version), Long("keep-going") => parse_flag!(keep_going), + Long("partition") => parse_opt!(partition, false), Long("print-command-list") => parse_flag!(print_command_list), Long("no-manifest-path") => parse_flag!(no_manifest_path), Long("locked") => parse_flag!(locked), @@ -571,6 +575,8 @@ impl Args { None => LogGroup::auto(), }; + let partition = partition.as_deref().map(str::parse).transpose()?; + if no_dev_deps || no_private { let flag = if no_dev_deps && no_private { "--no-dev-deps and --no-private modify" @@ -620,6 +626,7 @@ impl Args { clean_per_run, clean_per_version, keep_going, + partition, print_command_list, no_manifest_path, include_features: include_features.into_iter().map(Into::into).collect(), @@ -813,6 +820,7 @@ const HELP: &[HelpText<'_>] = &[ "This flag can only be used together with --version-range flag.", ]), ("", "--keep-going", "", "Keep going on failure", &[]), + ("", "--partition", "", "Partition runs and execute only its subset according to M/N", &[]), ("", "--log-group", "", "Log grouping: none, github-actions", &[ "If this option is not used, the environment will be automatically detected." ]), diff --git a/src/main.rs b/src/main.rs index 46eded90..0ddf2496 100644 --- a/src/main.rs +++ b/src/main.rs @@ -639,6 +639,22 @@ impl FromStr for LogGroup { } } +pub(crate) struct Partition { + index: usize, + count: usize, +} + +impl FromStr for Partition { + type Err = Error; + + fn from_str(s: &str) -> std::result::Result { + match s.split('/').map(str::parse::).collect::>()[..] { + [Ok(index), Ok(count)] if 0 < index && index <= count => Ok(Self { index, count }), + _ => bail!("bad or out-of-range partition: {s}"), + } + } +} + fn exec_cargo( cx: &Context, id: &PackageId, @@ -672,7 +688,26 @@ fn exec_cargo_inner( if progress.count != 0 && !cx.print_command_list && cx.log_group == LogGroup::None { eprintln!(); } - progress.count += 1; + + let new_count = progress.count + 1; + let mut skip = false; + if let Some(partition) = &cx.partition { + if progress.count % partition.count != partition.index - 1 { + let mut msg = String::new(); + if term::verbose() { + write!(msg, "skipping {line}").unwrap(); + } else { + write!(msg, "skipping {line} on {}", cx.packages(id).name).unwrap(); + } + write!(msg, " ({}/{})", new_count, progress.total).unwrap(); + let _guard = cx.log_group.print(&msg); + skip = true; + } + } + progress.count = new_count; + if skip { + return Ok(()); + } if cx.clean_per_run { cargo_clean(cx, Some(id))?; @@ -690,7 +725,7 @@ fn exec_cargo_inner( } else { write!(msg, "running {line} on {}", cx.packages(id).name).unwrap(); } - write!(msg, " ({}/{})", progress.count, progress.total).unwrap(); + write!(msg, " ({}/{})", new_count, progress.total).unwrap(); let _guard = cx.log_group.print(&msg); line.run() diff --git a/tests/long-help.txt b/tests/long-help.txt index 54350367..c7b07fa9 100644 --- a/tests/long-help.txt +++ b/tests/long-help.txt @@ -186,6 +186,9 @@ OPTIONS: --keep-going Keep going on failure. + --partition + Partition runs and execute only its subset according to M/N. + --log-group Log grouping: none, github-actions. diff --git a/tests/short-help.txt b/tests/short-help.txt index a06b7c67..5185a78f 100644 --- a/tests/short-help.txt +++ b/tests/short-help.txt @@ -46,6 +46,8 @@ OPTIONS: command --clean-per-version Remove artifacts per Rust version --keep-going Keep going on failure + --partition Partition runs and execute only its subset according to + M/N --log-group Log grouping: none, github-actions --print-command-list Print commands without run (Unstable) --no-manifest-path Do not pass --manifest-path option to cargo (Unstable) From 83f85795c4635683a5ed234ff46e17102f402a06 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sun, 21 Jul 2024 15:09:43 +0900 Subject: [PATCH 2/6] Add tests --- src/main.rs | 11 +++++- tests/test.rs | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 2 deletions(-) diff --git a/src/main.rs b/src/main.rs index 0ddf2496..ae691220 100644 --- a/src/main.rs +++ b/src/main.rs @@ -158,6 +158,13 @@ struct Progress { count: usize, } +impl Progress { + fn in_partition(&self, partition: &Partition) -> bool { + let current_index = self.count / self.total.div_ceil(partition.count); + current_index == partition.index + } +} + #[derive(Clone)] enum Kind<'a> { Normal, @@ -649,7 +656,7 @@ impl FromStr for Partition { fn from_str(s: &str) -> std::result::Result { match s.split('/').map(str::parse::).collect::>()[..] { - [Ok(index), Ok(count)] if 0 < index && index <= count => Ok(Self { index, count }), + [Ok(m), Ok(n)] if 0 < m && m <= n => Ok(Self { index: m - 1, count: n }), _ => bail!("bad or out-of-range partition: {s}"), } } @@ -692,7 +699,7 @@ fn exec_cargo_inner( let new_count = progress.count + 1; let mut skip = false; if let Some(partition) = &cx.partition { - if progress.count % partition.count != partition.index - 1 { + if !progress.in_partition(partition) { let mut msg = String::new(); if term::verbose() { write!(msg, "skipping {line}").unwrap(); diff --git a/tests/test.rs b/tests/test.rs index d889766f..8d3f98ae 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1898,3 +1898,106 @@ fn print_command_list() { ) .stdout_not_contains("`"); } + +#[test] +fn partition() { + cargo_hack(["check", "--feature-powerset", "--partition", "1/3"]) + .assert_success("real") + .stderr_contains( + " + running `cargo check --all-features` on real (1/17) + running `cargo check --no-default-features` on real (2/17) + running `cargo check --no-default-features --features a` on real (3/17) + running `cargo check --no-default-features --features b` on real (4/17) + running `cargo check --no-default-features --features a,b` on real (5/17) + running `cargo check --no-default-features --features c` on real (6/17) + skipping `cargo check --no-default-features --features a,c` on real (7/17) + skipping `cargo check --no-default-features --features b,c` on real (8/17) + skipping `cargo check --no-default-features --features a,b,c` on real (9/17) + skipping `cargo check --no-default-features --features default` on real (10/17) + skipping `cargo check --no-default-features --features a,default` on real (11/17) + skipping `cargo check --no-default-features --features b,default` on real (12/17) + skipping `cargo check --no-default-features --features a,b,default` on real (13/17) + skipping `cargo check --no-default-features --features c,default` on real (14/17) + skipping `cargo check --no-default-features --features a,c,default` on real (15/17) + skipping `cargo check --no-default-features --features b,c,default` on real (16/17) + skipping `cargo check --no-default-features --features a,b,c,default` on real (17/17) + ", + ); + + cargo_hack(["check", "--feature-powerset", "--partition", "2/3"]) + .assert_success("real") + .stderr_contains( + " + skipping `cargo check --all-features` on real (1/17) + skipping `cargo check --no-default-features` on real (2/17) + skipping `cargo check --no-default-features --features a` on real (3/17) + skipping `cargo check --no-default-features --features b` on real (4/17) + skipping `cargo check --no-default-features --features a,b` on real (5/17) + skipping `cargo check --no-default-features --features c` on real (6/17) + running `cargo check --no-default-features --features a,c` on real (7/17) + running `cargo check --no-default-features --features b,c` on real (8/17) + running `cargo check --no-default-features --features a,b,c` on real (9/17) + running `cargo check --no-default-features --features default` on real (10/17) + running `cargo check --no-default-features --features a,default` on real (11/17) + running `cargo check --no-default-features --features b,default` on real (12/17) + skipping `cargo check --no-default-features --features a,b,default` on real (13/17) + skipping `cargo check --no-default-features --features c,default` on real (14/17) + skipping `cargo check --no-default-features --features a,c,default` on real (15/17) + skipping `cargo check --no-default-features --features b,c,default` on real (16/17) + skipping `cargo check --no-default-features --features a,b,c,default` on real (17/17) + ", + ); + + cargo_hack(["check", "--feature-powerset", "--partition", "3/3"]) + .assert_success("real") + .stderr_contains( + " + skipping `cargo check --all-features` on real (1/17) + skipping `cargo check --no-default-features` on real (2/17) + skipping `cargo check --no-default-features --features a` on real (3/17) + skipping `cargo check --no-default-features --features b` on real (4/17) + skipping `cargo check --no-default-features --features a,b` on real (5/17) + skipping `cargo check --no-default-features --features c` on real (6/17) + skipping `cargo check --no-default-features --features a,c` on real (7/17) + skipping `cargo check --no-default-features --features b,c` on real (8/17) + skipping `cargo check --no-default-features --features a,b,c` on real (9/17) + skipping `cargo check --no-default-features --features default` on real (10/17) + skipping `cargo check --no-default-features --features a,default` on real (11/17) + skipping `cargo check --no-default-features --features b,default` on real (12/17) + running `cargo check --no-default-features --features a,b,default` on real (13/17) + running `cargo check --no-default-features --features c,default` on real (14/17) + running `cargo check --no-default-features --features a,c,default` on real (15/17) + running `cargo check --no-default-features --features b,c,default` on real (16/17) + running `cargo check --no-default-features --features a,b,c,default` on real (17/17) + ", + ); +} + +#[test] +fn partition_bad() { + cargo_hack(["check", "--each-feature", "--partition", "foo/bar"]) + .assert_failure("real") + .stderr_contains("bad or out-of-range partition: foo/bar"); + + cargo_hack(["check", "--each-feature", "--partition", "0/3"]) + .assert_failure("real") + .stderr_contains("bad or out-of-range partition: 0/3"); + + cargo_hack(["check", "--each-feature", "--partition", "4/3"]) + .assert_failure("real") + .stderr_contains("bad or out-of-range partition: 4/3"); + + cargo_hack([ + "check", + "--each-feature", + "--partition", + "1/3", + "--partition", + "2/3", + ]) + .assert_failure("real") + .stderr_contains( + "The argument '--partition' was provided more than once, but cannot be used multiple times", + ); +} From 7e09f892637d954237a7f0b7e257e43dbf0f31fb Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sun, 21 Jul 2024 16:00:51 +0900 Subject: [PATCH 3/6] Add another test case --- tests/test.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test.rs b/tests/test.rs index 8d3f98ae..05f370e7 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -1980,6 +1980,10 @@ fn partition_bad() { .assert_failure("real") .stderr_contains("bad or out-of-range partition: foo/bar"); + cargo_hack(["check", "--each-feature", "--partition", "2/0"]) + .assert_failure("real") + .stderr_contains("bad or out-of-range partition: 2/0"); + cargo_hack(["check", "--each-feature", "--partition", "0/3"]) .assert_failure("real") .stderr_contains("bad or out-of-range partition: 0/3"); From e31c00cae0abcca364255d51a04736c589e256e7 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sun, 21 Jul 2024 16:01:00 +0900 Subject: [PATCH 4/6] Fix ci... --- src/main.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/main.rs b/src/main.rs index ae691220..36904307 100644 --- a/src/main.rs +++ b/src/main.rs @@ -160,7 +160,12 @@ struct Progress { impl Progress { fn in_partition(&self, partition: &Partition) -> bool { - let current_index = self.count / self.total.div_ceil(partition.count); + // div_ceil (stabilized at 1.73) can't be used due to MSRV = 1.70... + let mut chunk_count = self.total / partition.count; + if self.total % partition.count != 0 { + chunk_count += 1; + } + let current_index = self.count / chunk_count; current_index == partition.index } } From f66a53be8be6d444a0d0cc4fb0d3a4b7562bf138 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sun, 21 Jul 2024 16:26:44 +0900 Subject: [PATCH 5/6] Clean up code a bit --- src/main.rs | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/main.rs b/src/main.rs index 36904307..71fdc456 100644 --- a/src/main.rs +++ b/src/main.rs @@ -702,24 +702,12 @@ fn exec_cargo_inner( } let new_count = progress.count + 1; - let mut skip = false; if let Some(partition) = &cx.partition { if !progress.in_partition(partition) { - let mut msg = String::new(); - if term::verbose() { - write!(msg, "skipping {line}").unwrap(); - } else { - write!(msg, "skipping {line} on {}", cx.packages(id).name).unwrap(); - } - write!(msg, " ({}/{})", new_count, progress.total).unwrap(); - let _guard = cx.log_group.print(&msg); - skip = true; + let _guard = log_and_update_progress(cx, id, line, progress, new_count, "skipping"); + return Ok(()); } } - progress.count = new_count; - if skip { - return Ok(()); - } if cx.clean_per_run { cargo_clean(cx, Some(id))?; @@ -730,15 +718,7 @@ fn exec_cargo_inner( return Ok(()); } - // running `` (on ) (/) - let mut msg = String::new(); - if term::verbose() { - write!(msg, "running {line}").unwrap(); - } else { - write!(msg, "running {line} on {}", cx.packages(id).name).unwrap(); - } - write!(msg, " ({}/{})", new_count, progress.total).unwrap(); - let _guard = cx.log_group.print(&msg); + let _guard = log_and_update_progress(cx, id, line, progress, new_count, "running"); line.run() } @@ -773,3 +753,23 @@ fn print_command(mut line: ProcessBuilder<'_>) { let l = line.to_string(); println!("{}", &l[1..l.len() - 1]); } + +fn log_and_update_progress( + cx: &Context, + id: &PackageId, + line: &ProcessBuilder<'_>, + progress: &mut Progress, + new_count: usize, + action: &str, +) -> Option { + // running/skipping `` (on ) (/) + let mut msg = String::new(); + if term::verbose() { + write!(msg, "{action} {line}").unwrap(); + } else { + write!(msg, "{action} {line} on {}", cx.packages(id).name).unwrap(); + } + write!(msg, " ({}/{})", new_count, progress.total).unwrap(); + progress.count = new_count; + cx.log_group.print(&msg) +} From 03cf795630eebe8196917d07db3f7ac96d729d55 Mon Sep 17 00:00:00 2001 From: Ryo Onodera Date: Sun, 21 Jul 2024 21:30:21 +0900 Subject: [PATCH 6/6] Remove needless variable: new_count --- src/main.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/main.rs b/src/main.rs index 71fdc456..4fb7e236 100644 --- a/src/main.rs +++ b/src/main.rs @@ -701,10 +701,9 @@ fn exec_cargo_inner( eprintln!(); } - let new_count = progress.count + 1; if let Some(partition) = &cx.partition { if !progress.in_partition(partition) { - let _guard = log_and_update_progress(cx, id, line, progress, new_count, "skipping"); + let _guard = log_and_update_progress(cx, id, line, progress, "skipping"); return Ok(()); } } @@ -718,7 +717,7 @@ fn exec_cargo_inner( return Ok(()); } - let _guard = log_and_update_progress(cx, id, line, progress, new_count, "running"); + let _guard = log_and_update_progress(cx, id, line, progress, "running"); line.run() } @@ -759,7 +758,6 @@ fn log_and_update_progress( id: &PackageId, line: &ProcessBuilder<'_>, progress: &mut Progress, - new_count: usize, action: &str, ) -> Option { // running/skipping `` (on ) (/) @@ -769,7 +767,7 @@ fn log_and_update_progress( } else { write!(msg, "{action} {line} on {}", cx.packages(id).name).unwrap(); } - write!(msg, " ({}/{})", new_count, progress.total).unwrap(); - progress.count = new_count; + progress.count += 1; + write!(msg, " ({}/{})", progress.count, progress.total).unwrap(); cx.log_group.print(&msg) }