Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --partition M/N to distribute builds #253

Merged
merged 6 commits into from
Aug 8, 2024
Merged

Add --partition M/N to distribute builds #253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1340194
Add --partition M/N to distribute builds
ryoqun Jul 14, 2024
83f8579
Add tests
ryoqun Jul 21, 2024
7e09f89
Add another test case
ryoqun Jul 21, 2024
e31c00c
Fix ci...
ryoqun Jul 21, 2024
f66a53b
Clean up code a bit
ryoqun Jul 21, 2024
03cf795
Remove needless variable: new_count
ryoqun Jul 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,9 @@ OPTIONS:
--keep-going
Keep going on failure.

--partition <M/N>
Partition runs and execute only its subset according to M/N.

--log-group <KIND>
Log grouping: none, github-actions.

Expand Down
10 changes: 9 additions & 1 deletion src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
Expand Down Expand Up @@ -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<Partition>,
/// --print-command-list
pub(crate) print_command_list: bool,
/// --version-range/--rust-version
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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", "<M/N>", "Partition runs and execute only its subset according to M/N", &[]),
("", "--log-group", "<KIND>", "Log grouping: none, github-actions", &[
"If this option is not used, the environment will be automatically detected."
]),
Expand Down
65 changes: 55 additions & 10 deletions src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,18 @@ struct Progress {
count: usize,
}

impl Progress {
fn in_partition(&self, partition: &Partition) -> bool {
// 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
}
}

#[derive(Clone)]
enum Kind<'a> {
Normal,
Expand Down Expand Up @@ -639,6 +651,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<Self, Self::Err> {
match s.split('/').map(str::parse::<usize>).collect::<Vec<_>>()[..] {
[Ok(m), Ok(n)] if 0 < m && m <= n => Ok(Self { index: m - 1, count: n }),
_ => bail!("bad or out-of-range partition: {s}"),
}
}
}

fn exec_cargo(
cx: &Context,
id: &PackageId,
Expand Down Expand Up @@ -672,7 +700,13 @@ fn exec_cargo_inner(
if progress.count != 0 && !cx.print_command_list && cx.log_group == LogGroup::None {
eprintln!();
}
progress.count += 1;

if let Some(partition) = &cx.partition {
if !progress.in_partition(partition) {
let _guard = log_and_update_progress(cx, id, line, progress, "skipping");
return Ok(());
}
}

if cx.clean_per_run {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(unrelated to this pr: i don't think cargo_clean(...) is needed if cx.print_command_list == true..right?)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Well, both are optional and I don't know what the use case of using --clean-per-run together with -pr-int-command-list in the first place, though...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for quick reply. I'll create a small pr after this pr to fix this.

cargo_clean(cx, Some(id))?;
Expand All @@ -683,15 +717,7 @@ fn exec_cargo_inner(
return Ok(());
}

// running `<command>` (on <package>) (<count>/<total>)
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, " ({}/{})", progress.count, progress.total).unwrap();
let _guard = cx.log_group.print(&msg);
let _guard = log_and_update_progress(cx, id, line, progress, "running");

line.run()
}
Expand Down Expand Up @@ -726,3 +752,22 @@ 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,
action: &str,
) -> Option<LogGroupGuard> {
// running/skipping `<command>` (on <package>) (<count>/<total>)
let mut msg = String::new();
if term::verbose() {
write!(msg, "{action} {line}").unwrap();
} else {
write!(msg, "{action} {line} on {}", cx.packages(id).name).unwrap();
}
progress.count += 1;
write!(msg, " ({}/{})", progress.count, progress.total).unwrap();
cx.log_group.print(&msg)
}
3 changes: 3 additions & 0 deletions tests/long-help.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ OPTIONS:
--keep-going
Keep going on failure.

--partition <M/N>
Partition runs and execute only its subset according to M/N.

--log-group <KIND>
Log grouping: none, github-actions.

Expand Down
2 changes: 2 additions & 0 deletions tests/short-help.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ OPTIONS:
command
--clean-per-version Remove artifacts per Rust version
--keep-going Keep going on failure
--partition <M/N> Partition runs and execute only its subset according to
M/N
--log-group <KIND> 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)
Expand Down
107 changes: 107 additions & 0 deletions tests/test.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1898,3 +1898,110 @@ 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", "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");

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",
);
}