From 9aedd0d55964f18de9abe081fdc59b9d44064ee0 Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 1 Nov 2023 20:49:38 +0800 Subject: [PATCH 1/7] Refine `test_toolchain_sort` --- src/toolchain/names.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index f24764c7c6..48f6b0cb4b 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -675,6 +675,9 @@ mod tests { "1.2.0-x86_64-unknown-linux-gnu", "1.8.0-x86_64-unknown-linux-gnu", "1.10.0-x86_64-unknown-linux-gnu", + "bar(baz)", + "foo#bar", + "this.is.not-a+semver", ] .into_iter() .map(|s| ToolchainName::try_from(s).unwrap()) @@ -688,6 +691,10 @@ mod tests { "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/3517 + "foo#bar", + "bar(baz)", + "this.is.not-a+semver", ] .into_iter() .map(|s| ToolchainName::try_from(s).unwrap()) From 5d6d1cb3f44fe71c9b8f5a2c88d7a7226916b314 Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 1 Nov 2023 20:54:33 +0800 Subject: [PATCH 2/7] Replace `sort_by` with `sort_by_key` in `toolchain_sort` --- src/toolchain/names.rs | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 48f6b0cb4b..6c8da9168e 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -333,13 +333,7 @@ pub(crate) fn toolchain_sort(v: &mut [ToolchainName]) { } } - v.sort_by(|a, b| { - let a_str = &format!("{a}"); - let b_str = &format!("{b}"); - let a_key = toolchain_sort_key(a_str); - let b_key = toolchain_sort_key(b_str); - a_key.cmp(&b_key) - }); + v.sort_by_key(|name| toolchain_sort_key(&format!("{name}"))); } /// ResolvableLocalToolchainName is used to process values set in From 5ab766a9aa0963a1cb6cbcd6c870302363f41bd0 Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 1 Nov 2023 20:55:53 +0800 Subject: [PATCH 3/7] Inline `toolchain_sort_key` in `toolchain_sort` --- src/toolchain/names.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 6c8da9168e..1825027589 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -321,7 +321,8 @@ pub(crate) fn toolchain_sort(v: &mut [ToolchainName]) { } } - fn toolchain_sort_key(s: &str) -> Version { + v.sort_by_key(|name| { + let s: &str = &format!("{name}"); if s.starts_with("stable") { special_version(0, s) } else if s.starts_with("beta") { @@ -331,9 +332,7 @@ pub(crate) fn toolchain_sort(v: &mut [ToolchainName]) { } else { Version::parse(&s.replace('_', "-")).unwrap_or_else(|_| special_version(3, s)) } - } - - v.sort_by_key(|name| toolchain_sort_key(&format!("{name}"))); + }); } /// ResolvableLocalToolchainName is used to process values set in From 7768bc682a99c879404e7c0be621973ee1412cb7 Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 1 Nov 2023 21:09:43 +0800 Subject: [PATCH 4/7] Inline `special_version` in `toolchain_sort` --- src/toolchain/names.rs | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 1825027589..033f399287 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -311,27 +311,33 @@ impl Display for ToolchainName { pub(crate) fn toolchain_sort(v: &mut [ToolchainName]) { use semver::{BuildMetadata, Prerelease, Version}; - fn special_version(ord: u64, s: &str) -> Version { - Version { + v.sort_by_key(|name| { + let s: &str = &format!("{name}"); + let default_ver = Version { major: 0, minor: 0, patch: 0, - pre: Prerelease::new(&format!("pre.{}.{}", ord, s.replace('_', "-"))).unwrap(), + pre: Prerelease::EMPTY, build: BuildMetadata::EMPTY, - } - } + }; - v.sort_by_key(|name| { - let s: &str = &format!("{name}"); if s.starts_with("stable") { - special_version(0, s) - } else if s.starts_with("beta") { - special_version(1, s) - } else if s.starts_with("nightly") { - special_version(2, s) - } else { - Version::parse(&s.replace('_', "-")).unwrap_or_else(|_| special_version(3, s)) + let pre = Prerelease::new(&format!("pre.{}.{}", 0, s.replace('_', "-"))).unwrap(); + return Version { pre, ..default_ver }; + } + if s.starts_with("beta") { + let pre = Prerelease::new(&format!("pre.{}.{}", 1, s.replace('_', "-"))).unwrap(); + return Version { pre, ..default_ver }; + } + if s.starts_with("nightly") { + let pre = Prerelease::new(&format!("pre.{}.{}", 2, s.replace('_', "-"))).unwrap(); + return Version { pre, ..default_ver }; + } + if let Ok(v) = Version::parse(&s.replace('_', "-")) { + return v; } + let pre = Prerelease::new(&format!("pre.{}.{}", 3, s.replace('_', "-"))).unwrap(); + Version { pre, ..default_ver } }); } From c27f007695d7776b1e88fd58c0056790682eab86 Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 1 Nov 2023 21:15:49 +0800 Subject: [PATCH 5/7] Change key used in `toolchain_sort` --- src/toolchain/names.rs | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 033f399287..0331756c04 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -309,36 +309,26 @@ impl Display for ToolchainName { } pub(crate) fn toolchain_sort(v: &mut [ToolchainName]) { - use semver::{BuildMetadata, Prerelease, Version}; + use semver::Version; v.sort_by_key(|name| { - let s: &str = &format!("{name}"); - let default_ver = Version { - major: 0, - minor: 0, - patch: 0, - pre: Prerelease::EMPTY, - build: BuildMetadata::EMPTY, - }; - + let s = name.to_string(); if s.starts_with("stable") { - let pre = Prerelease::new(&format!("pre.{}.{}", 0, s.replace('_', "-"))).unwrap(); - return Version { pre, ..default_ver }; + return (0, None, s); } if s.starts_with("beta") { - let pre = Prerelease::new(&format!("pre.{}.{}", 1, s.replace('_', "-"))).unwrap(); - return Version { pre, ..default_ver }; + return (1, None, s); } if s.starts_with("nightly") { - let pre = Prerelease::new(&format!("pre.{}.{}", 2, s.replace('_', "-"))).unwrap(); - return Version { pre, ..default_ver }; + return (2, None, s); } - if let Ok(v) = Version::parse(&s.replace('_', "-")) { - return v; + if let Some((ver_str, suffix)) = s.split_once('-') { + if let Ok(ver) = Version::parse(ver_str) { + return (3, Some(ver), suffix.to_owned()); + } } - let pre = Prerelease::new(&format!("pre.{}.{}", 3, s.replace('_', "-"))).unwrap(); - Version { pre, ..default_ver } - }); + (4, None, s) + }) } /// ResolvableLocalToolchainName is used to process values set in From b2fb2586684c120d900cdc166c58af35a680f1fe Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 1 Nov 2023 20:51:19 +0800 Subject: [PATCH 6/7] Add docs specifying `toolchain_sort`'s expected behavior --- src/toolchain/names.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 0331756c04..8ac13c5017 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -308,6 +308,10 @@ 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]) { use semver::Version; From 5d531efd6be033b9c51614438c620f4c88163cc0 Mon Sep 17 00:00:00 2001 From: rami3l Date: Wed, 1 Nov 2023 21:18:31 +0800 Subject: [PATCH 7/7] Inline `semver::Version` in `toolchain_sort` --- src/toolchain/names.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/toolchain/names.rs b/src/toolchain/names.rs index 8ac13c5017..37b1819788 100644 --- a/src/toolchain/names.rs +++ b/src/toolchain/names.rs @@ -313,8 +313,6 @@ impl Display for ToolchainName { /// 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]) { - use semver::Version; - v.sort_by_key(|name| { let s = name.to_string(); if s.starts_with("stable") { @@ -327,7 +325,7 @@ pub(crate) fn toolchain_sort(v: &mut [ToolchainName]) { return (2, None, s); } if let Some((ver_str, suffix)) = s.split_once('-') { - if let Ok(ver) = Version::parse(ver_str) { + if let Ok(ver) = semver::Version::parse(ver_str) { return (3, Some(ver), suffix.to_owned()); } }