Skip to content

refactor(toolchain/names): replace toolchain_sort() with ToolchainName's Ord instance #3892

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.

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

Merged
merged 3 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion src/cli/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub enum CLIError {
fn maybe_suggest_toolchain(bad_name: &str) -> String {
let bad_name = &bad_name.to_ascii_lowercase();
static VALID_CHANNELS: &[&str] = &["stable", "beta", "nightly"];
static NUMBERED: Lazy<Regex> = Lazy::new(|| Regex::new(r"^\d+\.\d+$").unwrap());
static NUMBERED: Lazy<Regex> = Lazy::new(|| Regex::new(r"^[0-9]+\.[0-9]+$").unwrap());
if NUMBERED.is_match(bad_name) {
return format!(". Toolchain numbers tend to have three parts, e.g. {bad_name}.0");
}
Expand Down
3 changes: 2 additions & 1 deletion src/cli/self_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,7 +1086,8 @@ fn parse_new_rustup_version(version: String) -> String {
use once_cell::sync::Lazy;
use regex::Regex;

static RE: Lazy<Regex> = Lazy::new(|| Regex::new(r"\d+.\d+.\d+[0-9a-zA-Z-]*").unwrap());
static RE: Lazy<Regex> =
Lazy::new(|| Regex::new(r"[0-9]+.[0-9]+.[0-9]+[0-9a-zA-Z-]*").unwrap());

let capture = RE.captures(&version);
let matched_version = match capture {
Expand Down
2 changes: 1 addition & 1 deletion src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,7 @@ impl<'a> Cfg<'a> {
.filter_map(|n| ToolchainName::try_from(&n).ok())
.collect();

crate::toolchain::toolchain_sort(&mut toolchains);
toolchains.sort();

Ok(toolchains)
} else {
Expand Down
110 changes: 89 additions & 21 deletions src/dist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ pub enum DistError {

#[derive(Debug, PartialEq)]
struct ParsedToolchainDesc {
channel: String,
channel: Channel,
date: Option<String>,
target: Option<String>,
}
Expand All @@ -132,8 +132,7 @@ struct ParsedToolchainDesc {
// are nearly-arbitrary strings.
#[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Ord)]
pub struct PartialToolchainDesc {
// Either "nightly", "stable", "beta", or an explicit version number
pub channel: String,
pub channel: Channel,
pub date: Option<String>,
pub target: PartialTargetTriple,
}
Expand All @@ -146,12 +145,81 @@ pub struct PartialToolchainDesc {
/// 1.55-x86_64-pc-windows-msvc
#[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Ord)]
pub struct ToolchainDesc {
// Either "nightly", "stable", "beta", or an explicit version number
pub channel: String,
pub channel: Channel,
pub date: Option<String>,
pub target: TargetTriple,
}

#[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Ord)]
pub enum Channel {
Stable,
Beta,
Nightly,
Version(PartialVersion),
}

impl fmt::Display for Channel {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match self {
Self::Stable => write!(f, "stable"),
Self::Beta => write!(f, "beta"),
Self::Nightly => write!(f, "nightly"),
Self::Version(ver) => write!(f, "{ver}"),
}
}
}

impl FromStr for Channel {
type Err = anyhow::Error;
fn from_str(chan: &str) -> Result<Self> {
match chan {
"stable" => Ok(Self::Stable),
"beta" => Ok(Self::Beta),
"nightly" => Ok(Self::Nightly),
ver => ver.parse().map(Self::Version),
}
}
}

#[derive(Debug, Clone, Eq, PartialEq, PartialOrd, Ord)]
pub struct PartialVersion {
pub major: u64,
pub minor: Option<u64>,
pub patch: Option<u64>,
pub pre: semver::Prerelease,
}

impl fmt::Display for PartialVersion {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{}", self.major)?;
if let Some(minor) = self.minor {
write!(f, ".{minor}")?;
}
if let Some(patch) = self.patch {
write!(f, ".{patch}")?;
}
if !self.pre.is_empty() {
write!(f, "-{}", self.pre)?;
}
Ok(())
}
}

impl FromStr for PartialVersion {
type Err = anyhow::Error;
fn from_str(ver: &str) -> Result<Self> {
let (ver, pre) = ver.split_once('-').unwrap_or((ver, ""));
let comparator =
semver::Comparator::from_str(ver).context("error parsing `PartialVersion`")?;
Ok(Self {
major: comparator.major,
minor: comparator.minor,
patch: comparator.patch,
pre: semver::Prerelease::new(pre).context("error parsing `PartialVersion`")?,
})
}
}

#[derive(Debug, Clone, Deserialize, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize)]
#[serde(transparent)]
pub struct TargetTriple(String);
Expand Down Expand Up @@ -188,15 +256,15 @@ impl FromStr for ParsedToolchainDesc {
// and an optional match of the date (2) and target (3)
static TOOLCHAIN_CHANNEL_RE: Lazy<Regex> = Lazy::new(|| {
Regex::new(&format!(
r"^({})(?:-(\d{{4}}-\d{{2}}-\d{{2}}))?(?:-(.+))?$",
r"^({})(?:-([0-9]{{4}}-[0-9]{{2}}-[0-9]{{2}}))?(?:-(.+))?$",
// The channel patterns we support
[
"nightly",
"beta",
"stable",
// Allow from 1.0.0 through to 9.999.99 with optional patch version
// and optional beta tag
r"\d{1}\.\d{1,3}(?:\.\d{1,2})?(?:-beta(?:\.\d{1,2})?)?",
r"[0-9]{1}\.[0-9]{1,3}(?:\.[0-9]{1,2})?(?:-beta(?:\.[0-9]{1,2})?)?",
]
.join("|")
))
Expand Down Expand Up @@ -229,7 +297,7 @@ impl FromStr for ParsedToolchainDesc {
};

Self {
channel: channel.to_owned(),
channel: Channel::from_str(channel).unwrap(),
date: c.get(2).map(|s| s.as_str()).and_then(fn_map),
target: c.get(3).map(|s| s.as_str()).and_then(fn_map),
}
Expand Down Expand Up @@ -579,7 +647,7 @@ impl ToolchainDesc {
/// Either "$channel" or "channel-$date"
pub fn manifest_name(&self) -> String {
match self.date {
None => self.channel.clone(),
None => self.channel.to_string(),
Some(ref date) => format!("{}-{}", self.channel, date),
}
}
Expand All @@ -595,11 +663,11 @@ impl ToolchainDesc {
/// such as `stable`, or is an incomplete version such as `1.48`, and the
/// date field is empty.
pub(crate) fn is_tracking(&self) -> bool {
let channels = ["nightly", "beta", "stable"];
static TRACKING_VERSION: Lazy<Regex> =
Lazy::new(|| Regex::new(r"^\d{1}\.\d{1,3}$").unwrap());
(channels.iter().any(|x| *x == self.channel) || TRACKING_VERSION.is_match(&self.channel))
&& self.date.is_none()
match &self.channel {
_ if self.date.is_some() => false,
Channel::Stable | Channel::Beta | Channel::Nightly => true,
Channel::Version(ver) => ver.patch.is_none() || &*ver.pre == "beta",
}
}
}

Expand Down Expand Up @@ -752,7 +820,7 @@ pub(crate) async fn update_from_dist(

let mut fetched = String::new();
let mut first_err = None;
let backtrack = opts.toolchain.channel == "nightly" && opts.toolchain.date.is_none();
let backtrack = opts.toolchain.channel == Channel::Nightly && opts.toolchain.date.is_none();
// We want to limit backtracking if we do not already have a toolchain
let mut backtrack_limit: Option<i32> = if opts.toolchain.date.is_some() {
None
Expand Down Expand Up @@ -1099,13 +1167,10 @@ async fn dl_v1_manifest(
) -> Result<Vec<String>> {
let root_url = toolchain.package_dir(download.dist_root);

if !["nightly", "beta", "stable"].contains(&&*toolchain.channel) {
if let Channel::Version(ver) = &toolchain.channel {
// This is an explicit version. In v1 there was no manifest,
// you just know the file to download, so synthesize one.
let installer_name = format!(
"{}/rust-{}-{}.tar.gz",
root_url, toolchain.channel, toolchain.target
);
let installer_name = format!("{}/rust-{}-{}.tar.gz", root_url, ver, toolchain.target);
return Ok(vec![installer_name]);
}

Expand Down Expand Up @@ -1191,7 +1256,7 @@ mod tests {
);

let expected = ParsedToolchainDesc {
channel: channel.into(),
channel: Channel::from_str(channel).unwrap(),
date: date.map(String::from),
target: target.map(String::from),
};
Expand Down Expand Up @@ -1226,6 +1291,9 @@ mod tests {
("nightly-2020-10-04", false),
("1.48", true),
("1.47.0", false),
("1.23-beta", true),
("1.23.0-beta", true),
("1.23.0-beta.2", false),
];
for case in CASES {
let full_tcn = format!("{}-x86_64-unknown-linux-gnu", case.0);
Expand Down
4 changes: 2 additions & 2 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use url::Url;
use crate::{
dist::{
manifest::{Component, Manifest},
{TargetTriple, ToolchainDesc},
Channel, TargetTriple, ToolchainDesc,
},
toolchain::{PathBasedToolchainName, ToolchainName},
};
Expand Down Expand Up @@ -97,7 +97,7 @@ pub enum RustupError {
ToolchainNotSelected(String),
#[error("toolchain '{}' does not contain component {}{}{}", .desc, .component, suggest_message(.suggestion), if .component.contains("rust-std") {
format!("\nnote: not all platforms have the standard library pre-compiled: https://doc.rust-lang.org/nightly/rustc/platform-support.html{}",
if desc.channel == "nightly" { "\nhelp: consider using `cargo build -Z build-std` instead" } else { "" }
if desc.channel == Channel::Nightly { "\nhelp: consider using `cargo build -Z build-std` instead" } else { "" }
)
} else { "".to_string() })]
UnknownComponent {
Expand Down
2 changes: 1 addition & 1 deletion src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub(crate) use distributable::DistributableToolchain;

mod names;
pub(crate) use names::{
toolchain_sort, CustomToolchainName, LocalToolchainName, MaybeOfficialToolchainName,
CustomToolchainName, LocalToolchainName, MaybeOfficialToolchainName,
MaybeResolvableToolchainName, PathBasedToolchainName, ResolvableLocalToolchainName,
ResolvableToolchainName, ToolchainName,
};
Expand Down
51 changes: 18 additions & 33 deletions src/toolchain/names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,10 +248,10 @@ impl Display for MaybeOfficialToolchainName {
/// ToolchainName can be used in calls to Cfg that alter configuration,
/// like setting overrides, or that depend on configuration, like calculating
/// the toolchain directory.
#[derive(Clone, Debug, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
pub enum ToolchainName {
Custom(CustomToolchainName),
Official(ToolchainDesc),
Custom(CustomToolchainName),
}

impl ToolchainName {
Expand Down Expand Up @@ -280,31 +280,6 @@ impl Display for ToolchainName {
}
}

/// Sorts [`ToolchainName`]s in the following order:
/// 1. `stable`/`beta`/`nightly`-prefixed names, in this exact order.
/// 2. `X.Y.Z-suffix` names, sorted by semver rules on `X.Y.Z`, then by `suffix`.
/// 3. Other names, sorted alphanumerically.
pub(crate) fn toolchain_sort(v: &mut [ToolchainName]) {
v.sort_by_key(|name| {
let s = name.to_string();
if s.starts_with("stable") {
return (0, None, s);
}
if s.starts_with("beta") {
return (1, None, s);
}
if s.starts_with("nightly") {
return (2, None, s);
}
if let Some((ver_str, suffix)) = s.split_once('-') {
if let Ok(ver) = semver::Version::parse(ver_str) {
return (3, Some(ver), suffix.to_owned());
}
}
(4, None, s)
})
}

/// ResolvableLocalToolchainName is used to process values set in
/// RUSTUP_TOOLCHAIN: resolvable and resolved official names, custom names and
/// absolute paths.
Expand Down Expand Up @@ -505,11 +480,7 @@ mod tests {
LIST_OSES.join("|"),
LIST_ENVS.join("|")
);
let partial_toolchain_desc_re = format!(
r"(nightly|beta|stable|\d{{1}}\.\d{{1,3}}(\.\d{{1,2}})?)(-(\d{{4}}-\d{{2}}-\d{{2}}))?{triple_re}"
);

partial_toolchain_desc_re
r"(nightly|beta|stable|[0-9]{1}(\.(0|[1-9][0-9]{0,2}))(\.(0|[1-9][0-9]{0,1}))?(-beta(\.(0|[1-9][1-9]{0,1}))?)?)(-([0-9]{4}-[0-9]{2}-[0-9]{2}))?".to_owned() + &triple_re
}

prop_compose! {
Expand Down Expand Up @@ -582,12 +553,18 @@ mod tests {
"stable-x86_64-unknown-linux-gnu",
"beta-x86_64-unknown-linux-gnu",
"nightly-x86_64-unknown-linux-gnu",
"nightly-2015-01-01-x86_64-unknown-linux-gnu",
"1.0.0-x86_64-unknown-linux-gnu",
"1.2.0-x86_64-unknown-linux-gnu",
"1.8-beta-x86_64-apple-darwin",
"1.8.0-beta-x86_64-apple-darwin",
"1.8.0-beta.2-x86_64-apple-darwin",
"1.8.0-x86_64-apple-darwin",
"1.8.0-x86_64-unknown-linux-gnu",
"1.10.0-x86_64-unknown-linux-gnu",
"bar(baz)",
"foo#bar",
"the cake is a lie",
"this.is.not-a+semver",
]
.into_iter()
Expand All @@ -598,20 +575,28 @@ mod tests {
"1.8.0-x86_64-unknown-linux-gnu",
"1.0.0-x86_64-unknown-linux-gnu",
"nightly-x86_64-unknown-linux-gnu",
"nightly-2015-01-01-x86_64-unknown-linux-gnu",
"stable-x86_64-unknown-linux-gnu",
"1.10.0-x86_64-unknown-linux-gnu",
"beta-x86_64-unknown-linux-gnu",
"1.2.0-x86_64-unknown-linux-gnu",
// https://github.com/rust-lang/rustup/issues/1329
"1.8.0-x86_64-apple-darwin",
"1.8-beta-x86_64-apple-darwin",
"1.8.0-beta-x86_64-apple-darwin",
"1.8.0-beta.2-x86_64-apple-darwin",
// https://github.com/rust-lang/rustup/issues/3517
"foo#bar",
"bar(baz)",
"this.is.not-a+semver",
// https://github.com/rust-lang/rustup/issues/3168
"the cake is a lie",
]
.into_iter()
.map(|s| ToolchainName::try_from(s).unwrap())
.collect::<Vec<_>>();

super::toolchain_sort(&mut v);
v.sort();

assert_eq!(expected, v);
}
Expand Down
6 changes: 3 additions & 3 deletions tests/suite/cli_rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1218,12 +1218,12 @@ rustup home: {1}

installed toolchains
--------------------
nightly-2015-01-01-{0}
1.2.0 (hash-nightly-1)

nightly-{0} (active, default)
1.3.0 (hash-nightly-2)

nightly-2015-01-01-{0}
1.2.0 (hash-nightly-1)

active toolchain
----------------
name: nightly-{0}
Expand Down
Loading