Skip to content

Commit

Permalink
Align with semver types
Browse files Browse the repository at this point in the history
This also adds a method, Epoch::from_version_req_str, that will be
used in later CLs.

Bug: 1291994
Change-Id: If128eaa7dfa80bb74a3c08fae3fb700d1ebb49f6

Cq-Include-Trybots: luci.chromium.try:android-rust-arm-dbg,android-rust-arm-rel,linux-rust-x64-dbg,linux-rust-x64-rel
Change-Id: If128eaa7dfa80bb74a3c08fae3fb700d1ebb49f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3780800
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Collin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1028435}
  • Loading branch information
chbaker0 authored and Chromium LUCI CQ committed Jul 26, 2022
1 parent 9d34d25 commit c8ef0cd
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 19 deletions.
2 changes: 2 additions & 0 deletions tools/crates/gnrt/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ test("gnrt_unittests") {
# to a linking issue we must depend on the lib target which shouldn't exist
# in the first place, but does due to a crates.py bug.
"//third_party/rust/cargo_platform/v0_1:lib",
"//third_party/rust/semver/v1:lib",
"//third_party/rust/serde/v1:lib",
"//third_party/rust/serde_json/v1:lib",
"//third_party/rust/toml/v0_5:test_support",
Expand All @@ -83,6 +84,7 @@ if (rustc_can_link) {
":gnrt_lib",
"//third_party/rust/cargo_metadata/v0_14:test_support",
"//third_party/rust/clap/v3:test_support",
"//third_party/rust/semver/v1:lib",
"//third_party/rust/serde/v1:lib",
"//third_party/rust/serde_json/v1:lib",
"//third_party/rust/toml/v0_5:test_support",
Expand Down
61 changes: 57 additions & 4 deletions tools/crates/gnrt/crates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,63 @@ use std::str::FromStr;
pub enum Epoch {
/// Epoch with major version == 0. The field is the minor version. It is an
/// error to use 0: methods may panic in this case.
Minor(u32),
Minor(u64),
/// Epoch with major version >= 1. It is an error to use 0: methods may
/// panic in this case.
Major(u32),
Major(u64),
}

impl Epoch {
/// Get the semver version string for this Epoch. This will only have a
/// non-zero major component, or a zero major component and a non-zero minor
/// component. Note this differs from Epoch's `fmt::Display` impl.
pub fn to_version_string(&self) -> String {
match *self {
// These should never return Err since formatting an integer is
// infallible.
Epoch::Minor(minor) => {
assert_ne!(minor, 0);
format!("0.{minor}")
}
Epoch::Major(major) => {
assert_ne!(major, 0);
format!("{major}")
}
}
}

/// Compute the Epoch from a `semver::Version`. This is useful since we can
/// parse versions from `cargo_metadata` and in Cargo.toml files using the
/// `semver` library.
pub fn from_version(version: &semver::Version) -> Self {
match version.major {
0 => Self::Minor(version.minor.try_into().unwrap()),
x => Self::Major(x.try_into().unwrap()),
}
}

/// Get the requested epoch from a supported dependency version string.
/// `req` should be a version request as used in Cargo.toml's [dependencies]
/// section.
///
/// `req` must use the default strategy as defined in
/// https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-cratesio
pub fn from_version_req_str(req: &str) -> Self {
// For convenience, leverage semver::VersionReq for parsing even
// though we don't need the full expressiveness.
let req = semver::VersionReq::from_str(req).unwrap();
// We require the spec to have exactly one comparator, which must use
// the default strategy.
assert_eq!(req.comparators.len(), 1);
let comp: &semver::Comparator = &req.comparators[0];
// Caret is semver's name for the default strategy.
assert_eq!(comp.op, semver::Op::Caret);
match (comp.major, comp.minor) {
(0, Some(0) | None) => panic!("invalid version req {req}"),
(0, Some(x)) => Epoch::Minor(x),
(x, _) => Epoch::Major(x),
}
}
}

// This gives us a ToString implementation for free.
Expand Down Expand Up @@ -70,9 +123,9 @@ impl FromStr for Epoch {

// Split the major and minor version numbers.
let mut parts = s.split('_');
let major: Option<u32> =
let major: Option<u64> =
parts.next().map(|s| s.parse().map_err(EpochParseError::InvalidInt)).transpose()?;
let minor: Option<u32> =
let minor: Option<u64> =
parts.next().map(|s| s.parse().map_err(EpochParseError::InvalidInt)).transpose()?;

// Get the final epoch, checking that the (major, minor) pair is valid.
Expand Down
19 changes: 17 additions & 2 deletions tools/crates/gnrt/crates_unittest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ use gnrt_lib::crates::*;

use std::str::FromStr;

use Epoch::*;

#[gtest(EpochTest, FromStr)]
fn test() {
use Epoch::{Major, Minor};
use EpochParseError::*;
expect_eq!(Epoch::from_str("v1"), Ok(Major(1)));
expect_eq!(Epoch::from_str("v2"), Ok(Major(2)));
Expand All @@ -26,8 +27,22 @@ fn test() {

#[gtest(EpochTest, ToString)]
fn test() {
use Epoch::{Major, Minor};
expect_eq!(Major(1).to_string(), "v1");
expect_eq!(Major(2).to_string(), "v2");
expect_eq!(Minor(3).to_string(), "v0_3");
}

#[gtest(EpochTest, FromVersion)]
fn test() {
use semver::Version;

expect_eq!(Epoch::from_version(&Version::new(0, 1, 0)), Minor(1));
expect_eq!(Epoch::from_version(&Version::new(1, 2, 0)), Major(1));
}

#[gtest(EpochTest, FromVersionReqStr)]
fn test() {
expect_eq!(Epoch::from_version_req_str("0.1.0"), Minor(1));
expect_eq!(Epoch::from_version_req_str("1.0.0"), Major(1));
expect_eq!(Epoch::from_version_req_str("2.3.0"), Major(2));
}
15 changes: 2 additions & 13 deletions tools/crates/gnrt/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,24 +234,20 @@ pub fn collect_dependencies(metadata: &cargo_metadata::Metadata) -> Vec<ThirdPar
}

dep.version = package.version.clone();
dep.epoch = epoch_from_version(&package.version);
dep.epoch = Epoch::from_version(&package.version);

// Collect this package's list of resolved dependencies which will be
// needed for build file generation later.
for node_dep in iter_node_deps(node) {
let dep_pkg = dep_graph.packages.get(node_dep.pkg).unwrap();

// Get the platform filter and remove terms with unsupported
// configurations.
let mut platform = node_dep.target;
if let Some(p) = platform {
assert!(platforms::matches_supported_target(&p));
platform = platforms::filter_unsupported_platform_terms(p);
}

let dep_of_dep = DepOfDep {
normalized_name: NormalizedName::from_crate_name(&dep_pkg.name),
epoch: epoch_from_version(&dep_pkg.version),
epoch: Epoch::from_version(&dep_pkg.version),
platform,
};

Expand Down Expand Up @@ -449,10 +445,3 @@ impl std::fmt::Display for TargetType {
}
}
}

fn epoch_from_version(version: &cargo_metadata::Version) -> Epoch {
match version.major {
0 => Epoch::Minor(version.minor.try_into().unwrap()),
x => Epoch::Major(x.try_into().unwrap()),
}
}

0 comments on commit c8ef0cd

Please sign in to comment.