Skip to content

Commit

Permalink
feat: Support mixed MSRV in --version-range
Browse files Browse the repository at this point in the history 
This expands on the approach taken in #212 bucketing packages into
rust-versions to run.  If we skipped the MSRV (due to `--version-step`),
we automatically inject it.  If a package's MSRV isn't within the range,
we skip it.

Benefits
- Relatively simple to implement and to explain
- We keep the number of runs to a minimum by walking in lock-step the
  `--version-step`, independent of what each package' MSRV

I did have to specialize `--rust-version` vs `--version-range` to avoid
`--rust-version` range users walking more than they needed.

To keep the progress total accurate, I shifted the calculating of the
total from `determine_package_list` to after we have bucketed
everything.  To make this feasible, I saved off the how many iterations
a package will have without the version range being taken into account.

As a byproduct, this fixes a bug in #212 where it didn't take the
rust-version into account when determining the total.

Fixes #199
  • Loading branch information
@epage @taiki-e
epage authored and taiki-e committed Sep 9, 2023
1 parent af30888 commit 6b607d3
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 71 deletions.
114 changes: 59 additions & 55 deletions src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,68 +66,70 @@ fn try_main() -> Result<()> {
let mut progress = Progress::default();
let mut keep_going = KeepGoing::default();
if let Some(range) = cx.version_range {
if range == VersionRange::msrv() {
let total = packages.iter().map(|p| p.feature_count).sum();
progress.total = total;

let mut versions = BTreeMap::new();
for pkg in packages {
let v = cx
.rust_version(pkg.id)
.map(str::parse::<Version>)
.transpose()?
.ok_or_else(|| {
format_err!("no rust-version field in Cargo.toml is specified")
})?;
versions.entry(v.strip_patch()).or_insert_with(Vec::new).push(pkg);
}

// First, generate the lockfile using the oldest cargo specified.
// https://github.com/taiki-e/cargo-hack/issues/105
let mut generate_lockfile = true;
// Workaround for spurious "failed to select a version" error.
// (This does not work around the underlying cargo bug: https://github.com/rust-lang/cargo/issues/10623)
let mut regenerate_lockfile_on_51_or_up = false;
for (cargo_version, packages) in versions {
versioned_cargo_exec_on_packages(
cx,
&packages,
cargo_version.minor,
&mut progress,
&mut keep_going,
&mut generate_lockfile,
&mut regenerate_lockfile_on_51_or_up,
)?;
let mut versions = BTreeMap::new();
let steps = rustup::version_range(range, cx.version_step, &packages, cx)?;
for pkg in packages {
let msrv = cx
.rust_version(pkg.id)
.map(str::parse::<Version>)
.transpose()?
.map(Version::strip_patch);
if range == VersionRange::msrv() {
let msrv = msrv.ok_or_else(|| {
format_err!("no rust-version field in Cargo.toml is specified")
})?;
versions.entry(msrv).or_insert_with(Vec::new).push(pkg);
} else {
let mut seen = false;
for cargo_version in &steps {
if msrv.is_some() && Some(*cargo_version) < msrv {
continue;
}
if !seen {
if Some(*cargo_version) != msrv {
if let Some(msrv) = msrv {
versions.entry(msrv).or_insert_with(Vec::new).push(pkg.clone());
}
}
seen = true;
}
versions.entry(*cargo_version).or_insert_with(Vec::new).push(pkg.clone());
}
if !seen {
let package = cx.packages(pkg.id);
let name = &package.name;
let msrv = msrv.expect("always `seen` if no msrv");
warn!("skipping {name}, rust-version ({msrv}) is not in specified range ({range})");
}
}
} else {
let range = rustup::version_range(range, cx.version_step, &packages, cx)?;
}

let total: usize = packages.iter().map(|p| p.feature_count).sum();
for cargo_version in &range {
for (cargo_version, packages) in &versions {
for package in packages {
if cx.target.is_empty() || cargo_version.minor >= 64 {
progress.total += total;
progress.total += package.feature_count;
} else {
progress.total += total * cx.target.len();
progress.total += package.feature_count * cx.target.len();
}
}
}

// First, generate the lockfile using the oldest cargo specified.
// https://github.com/taiki-e/cargo-hack/issues/105
let mut generate_lockfile = true;
// Workaround for spurious "failed to select a version" error.
// (This does not work around the underlying cargo bug: https://github.com/rust-lang/cargo/issues/10623)
let mut regenerate_lockfile_on_51_or_up = false;
for cargo_version in range {
versioned_cargo_exec_on_packages(
cx,
&packages,
cargo_version.minor,
&mut progress,
&mut keep_going,
&mut generate_lockfile,
&mut regenerate_lockfile_on_51_or_up,
)?;
}
// First, generate the lockfile using the oldest cargo specified.
// https://github.com/taiki-e/cargo-hack/issues/105
let mut generate_lockfile = true;
// Workaround for spurious "failed to select a version" error.
// (This does not work around the underlying cargo bug: https://github.com/rust-lang/cargo/issues/10623)
let mut regenerate_lockfile_on_51_or_up = false;
for (cargo_version, packages) in versions {
versioned_cargo_exec_on_packages(
cx,
&packages,
cargo_version.minor,
&mut progress,
&mut keep_going,
&mut generate_lockfile,
&mut regenerate_lockfile_on_51_or_up,
)?;
}
} else {
let total = packages.iter().map(|p| p.feature_count).sum();
Expand All @@ -148,6 +150,7 @@ struct Progress {
count: usize,
}

#[derive(Clone)]
enum Kind<'a> {
Normal,
Each { features: Vec<&'a Feature> },
Expand Down Expand Up @@ -263,6 +266,7 @@ fn determine_kind<'a>(
}
}

#[derive(Clone)]
struct PackageRuns<'a> {
id: &'a PackageId,
kind: Kind<'a>,
Expand Down
35 changes: 21 additions & 14 deletions src/rustup.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,23 +60,30 @@ pub(crate) fn version_range(
if let Some(rust_version) = rust_version {
Ok(rust_version)
} else {
let mut version = None;
let mut lowest_msrv = None;
for pkg in packages {
let v = cx.rust_version(pkg.id);
if v.is_none() || v == version {
// no-op
} else if version.is_none() {
version = v;
} else {
bail!("automatic detection of the lower bound of the version range is not yet supported when the minimum supported Rust version of the crates in the workspace do not match")
}
let pkg_msrv = cx
.rust_version(pkg.id)
.map(str::parse::<Version>)
.transpose()?
.map(Version::strip_patch);
lowest_msrv = match (lowest_msrv, pkg_msrv) {
(Some(workspace), Some(pkg)) => {
if workspace < pkg {
Some(workspace)
} else {
Some(pkg)
}
}
(Some(msrv), None) | (None, Some(msrv)) => Some(msrv),
(None, None) => None,
};
}
let version = match version {
Some(v) => v.parse()?,
None => bail!("no rust-version field in Cargo.toml is specified"),
let Some(lowest_msrv) = lowest_msrv else {
bail!("no rust-version field in Cargo.toml is specified")
};
rust_version = Some(version);
Ok(version)
rust_version = Some(lowest_msrv);
Ok(lowest_msrv)
}
};

Expand Down
23 changes: 21 additions & 2 deletions tests/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1353,11 +1353,30 @@ fn version_range() {
running `rustup run 1.64 cargo check` on member2 (4/4)
",
);
// Skips `real` because it isn't in range
cargo_hack(["check", "--version-range", "..=1.64", "--workspace"])
.assert_failure("rust-version")
.assert_failure("rust-version") // warn
.stderr_contains(
"
automatic detection of the lower bound of the version range is not yet supported when the minimum supported Rust version of the crates in the workspace do not match
warning: skipping real, rust-version (1.65) is not in specified range (..=1.64)
running `rustup run 1.63 cargo check` on member1 (1/5)
running `rustup run 1.63 cargo check` on member2 (2/5)
running `rustup run 1.64 cargo check` on member1 (3/5)
running `rustup run 1.64 cargo check` on member2 (4/5)
running `rustup run 1.64 cargo check` on member3 (5/5)
",
);
cargo_hack(["check", "--version-range", "..=1.66", "--version-step", "2", "--workspace"])
.assert_success("rust-version")
.stderr_contains(
"
running `rustup run 1.63 cargo check` on member1 (1/7)
running `rustup run 1.63 cargo check` on member2 (2/7)
running `rustup run 1.64 cargo check` on member3 (3/7)
running `rustup run 1.65 cargo check` on member1 (4/7)
running `rustup run 1.65 cargo check` on member2 (5/7)
running `rustup run 1.65 cargo check` on member3 (6/7)
running `rustup run 1.65 cargo check` on real (7/7)
",
);
}
Expand Down

0 comments on commit 6b607d3

Please sign in to comment.