From 40759f3b28109e448afb966d0b23ac98cfd6a4ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Mon, 27 Mar 2023 17:38:42 +0200 Subject: [PATCH] Review feedback --- utils/wasm-builder/src/lib.rs | 72 ++---------- utils/wasm-builder/src/version.rs | 180 ++++++++++++++++++++++++++++++ 2 files changed, 190 insertions(+), 62 deletions(-) create mode 100644 utils/wasm-builder/src/version.rs diff --git a/utils/wasm-builder/src/lib.rs b/utils/wasm-builder/src/lib.rs index 536e8ecf6161e..72c706161c756 100644 --- a/utils/wasm-builder/src/lib.rs +++ b/utils/wasm-builder/src/lib.rs @@ -115,9 +115,11 @@ use std::{ path::{Path, PathBuf}, process::Command, }; +use version::Version; mod builder; mod prerequisites; +mod version; mod wasm_project; pub use builder::{WasmBuilder, WasmBuilderSelectProject}; @@ -210,7 +212,7 @@ fn get_rustup_command() -> Option { let output = Command::new("rustup").args(&["toolchain", "list"]).output().ok()?.stdout; let lines = output.as_slice().lines(); - let mut found_version: Option<(String, Version)> = None; + let mut versions = Vec::new(); for line in lines.filter_map(|l| l.ok()) { let rustup_version = line.trim_end_matches(&host); @@ -222,48 +224,15 @@ fn get_rustup_command() -> Option { let Some(cargo_version) = cmd.version() else { continue; }; - if let Some((rustup_version_other, cargo_version_other)) = &mut found_version { - let is_older = cargo_version_other.is_older(&cargo_version); - - // Nightly should not overwrite a stable version. - if cargo_version_other.is_stable() && cargo_version.is_nightly { - continue - } - - // Stable versions overwrite nightly versions or otherwise just check which one is - // older. - if cargo_version_other.is_nightly && cargo_version.is_stable() || is_older { - *rustup_version_other = rustup_version.into(); - *cargo_version_other = cargo_version; - } - } else { - found_version = Some((rustup_version.into(), cargo_version)); - } + versions.push((cargo_version, rustup_version.to_string())); } - let version = found_version?.0; - Some(CargoCommand::new_with_args("rustup", &["run", &version, "cargo"])) -} - -/// The version of rustc/cargo. -#[derive(Clone, Copy, Debug)] -struct Version { - pub major: u32, - pub minor: u32, - pub patch: u32, - pub is_nightly: bool, -} - -impl Version { - /// Returns if `self` is older than `other`. - fn is_older(&self, other: &Self) -> bool { - self.major < other.major || self.minor < other.minor || self.patch < other.patch - } + // Sort by the parsed version to get the latest version (greatest version) at the end of the + // vec. + versions.sort_by_key(|v| v.0); + let version = &versions.last()?.1; - /// Returns if `self` is a stable version. - fn is_stable(&self) -> bool { - !self.is_nightly - } + Some(CargoCommand::new_with_args("rustup", &["run", &version, "cargo"])) } /// Wraps a specific command which represents a cargo invocation. @@ -305,28 +274,7 @@ impl CargoCommand { .ok() .and_then(|o| String::from_utf8(o.stdout).ok())?; - if let Some(version) = version.split(" ").nth(1) { - let mut is_nightly = false; - let parts = version - .split(".") - .filter_map(|v| { - if let Some(rest) = v.strip_suffix("-nightly") { - is_nightly = true; - rest.parse().ok() - } else { - v.parse().ok() - } - }) - .collect::>(); - - if parts.len() != 3 { - return None - } - - Some(Version { major: parts[0], minor: parts[1], patch: parts[2], is_nightly }) - } else { - None - } + Version::extract(&version) } /// Returns the version of this cargo command or `None` if it failed to extract the version. diff --git a/utils/wasm-builder/src/version.rs b/utils/wasm-builder/src/version.rs new file mode 100644 index 0000000000000..9a22f4402b1a3 --- /dev/null +++ b/utils/wasm-builder/src/version.rs @@ -0,0 +1,180 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::cmp::Ordering; + +/// The version of rustc/cargo. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Version { + pub major: u32, + pub minor: u32, + pub patch: u32, + pub is_nightly: bool, + pub year: u32, + pub month: u32, + pub day: u32, +} + +impl Version { + /// Returns if `self` is a stable version. + pub fn is_stable(&self) -> bool { + !self.is_nightly + } + + /// Return if `self` is a nightly version. + pub fn is_nightly(&self) -> bool { + self.is_nightly + } + + /// Extract from the given `version` string. + pub fn extract(version: &str) -> Option { + let mut is_nightly = false; + let version_parts = version + .split(" ") + .nth(1)? + .split(".") + .filter_map(|v| { + if let Some(rest) = v.strip_suffix("-nightly") { + is_nightly = true; + rest.parse().ok() + } else { + v.parse().ok() + } + }) + .collect::>(); + + if version_parts.len() != 3 { + return None + } + + let date = version.split(" ").nth(3)?; + + let date_parts = date + .split("-") + .filter_map(|v| v.strip_suffix(")").unwrap_or(v).parse().ok()) + .collect::>(); + + if date_parts.len() != 3 { + return None + } + + Some(Version { + major: version_parts[0], + minor: version_parts[1], + patch: version_parts[2], + is_nightly, + year: date_parts[0], + month: date_parts[1], + day: date_parts[2], + }) + } +} + +/// Ordering is done in the following way: +/// +/// 1. `stable` > `nightly` +/// 2. Then compare major, minor and patch. +/// 3. Last compare the date. +impl Ord for Version { + fn cmp(&self, other: &Self) -> Ordering { + if self == other { + return Ordering::Equal + } + + // Ensure that `stable > nightly` + if self.is_stable() && other.is_nightly() { + return Ordering::Greater + } else if self.is_nightly() && other.is_stable() { + return Ordering::Less + } + + let to_compare = [ + (self.major, other.major), + (self.minor, other.minor), + (self.patch, other.patch), + (self.year, other.year), + (self.month, other.month), + (self.day, other.day), + ]; + + to_compare + .iter() + .find_map(|(l, r)| if l != r { l.partial_cmp(&r) } else { None }) + // We already checked this right at the beginning, so we should never return here + // `Equal`. + .unwrap_or(Ordering::Equal) + } +} + +impl PartialOrd for Version { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn version_compare_and_extract_works() { + let version_1_66_0 = Version::extract("cargo 1.66.0 (d65d197ad 2022-11-15)").unwrap(); + let version_1_66_1 = Version::extract("cargo 1.66.1 (d65d197ad 2022-11-15)").unwrap(); + let version_1_66_0_nightly = + Version::extract("cargo 1.66.0-nightly (d65d197ad 2022-10-15)").unwrap(); + let version_1_66_0_nightly_older_date = + Version::extract("cargo 1.66.0-nightly (d65d197ad 2022-10-14)").unwrap(); + let version_1_65_0 = Version::extract("cargo 1.65.0 (d65d197ad 2022-10-15)").unwrap(); + let version_1_65_0_older_date = + Version::extract("cargo 1.65.0 (d65d197ad 2022-10-14)").unwrap(); + + assert!(version_1_66_1 > version_1_66_0); + assert!(version_1_66_1 > version_1_65_0); + assert!(version_1_66_1 > version_1_66_0_nightly); + assert!(version_1_66_1 > version_1_66_0_nightly_older_date); + assert!(version_1_66_1 > version_1_65_0_older_date); + + assert!(version_1_66_0 > version_1_65_0); + assert!(version_1_66_0 > version_1_66_0_nightly); + assert!(version_1_66_0 > version_1_66_0_nightly_older_date); + assert!(version_1_66_0 > version_1_65_0_older_date); + + assert!(version_1_65_0 > version_1_66_0_nightly); + assert!(version_1_65_0 > version_1_66_0_nightly_older_date); + assert!(version_1_65_0 > version_1_65_0_older_date); + + let mut versions = vec![ + version_1_66_0, + version_1_66_0_nightly, + version_1_66_0_nightly_older_date, + version_1_65_0_older_date, + version_1_65_0, + version_1_66_1, + ]; + versions.sort_by(|a, b| b.cmp(a)); + + let expected_versions_order = vec![ + version_1_66_1, + version_1_66_0, + version_1_65_0, + version_1_65_0_older_date, + version_1_66_0_nightly, + version_1_66_0_nightly_older_date, + ]; + assert_eq!(expected_versions_order, versions); + } +}