From 2f7dee8c9bcb111645c0dd9a78ed0a65c9b269cd Mon Sep 17 00:00:00 2001 From: Ed Page Date: Thu, 7 Sep 2023 10:23:21 -0500 Subject: [PATCH] refactor: Defer calculating the toolchain flag In addition to decoupling concerns (expansion of a range into discrete versions), this will make it easier to do contains checks and insertions on the discrete versions. --- src/context.rs | 2 +- src/main.rs | 11 ++++++----- src/rustup.rs | 8 +++----- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/context.rs b/src/context.rs index bfdf5a86..17eb051c 100644 --- a/src/context.rs +++ b/src/context.rs @@ -27,7 +27,7 @@ pub(crate) struct Context { pub(crate) cargo_version: u32, pub(crate) restore: restore::Manager, pub(crate) current_dir: PathBuf, - pub(crate) version_range: Option>, + pub(crate) version_range: Option>, pub(crate) use_github_action_grouping: bool, } diff --git a/src/main.rs b/src/main.rs index 7a8036f4..789e2b73 100644 --- a/src/main.rs +++ b/src/main.rs @@ -64,7 +64,7 @@ fn try_main() -> Result<()> { if let Some(range) = &cx.version_range { let total = progress.total; progress.total = 0; - for (cargo_version, _) in range { + for cargo_version in range { if cx.target.is_empty() || *cargo_version >= 64 { progress.total += total; } else { @@ -79,11 +79,12 @@ fn try_main() -> Result<()> { // 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, toolchain) in range { - rustup::install_toolchain(toolchain, &cx.target, true)?; + for cargo_version in range { + let toolchain = format!("+1.{cargo_version}"); + rustup::install_toolchain(&toolchain, &cx.target, true)?; if generate_lockfile || regenerate_lockfile_on_51_or_up && *cargo_version >= 51 { let mut line = line.clone(); - line.leading_arg(toolchain); + line.leading_arg(&toolchain); line.arg("generate-lockfile"); if let Some(pid) = cx.current_package() { let package = cx.packages(pid); @@ -110,7 +111,7 @@ fn try_main() -> Result<()> { } let mut line = line.clone(); - line.leading_arg(toolchain); + line.leading_arg(&toolchain); line.apply_context(cx); exec_on_packages( cx, diff --git a/src/rustup.rs b/src/rustup.rs index 54c867bf..1624b9c0 100644 --- a/src/rustup.rs +++ b/src/rustup.rs @@ -23,7 +23,7 @@ impl Rustup { } } -pub(crate) fn version_range(range: VersionRange, cx: &Context) -> Result> { +pub(crate) fn version_range(range: VersionRange, cx: &Context) -> Result> { let check = |version: &Version| { if version.major != 1 { bail!("major version must be 1"); @@ -98,10 +98,8 @@ pub(crate) fn version_range(range: VersionRange, cx: &Context) -> Result get_stable_version()?, }; - let versions: Vec<_> = (start_inclusive.minor..=end_inclusive.minor) - .step_by(cx.version_step as _) - .map(|minor| (minor, format!("+1.{minor}"))) - .collect(); + let versions: Vec<_> = + (start_inclusive.minor..=end_inclusive.minor).step_by(cx.version_step as _).collect(); if versions.is_empty() { bail!("specified version range `{range}` is empty"); }