Unverified Commit f2041a42 authored by ysaito1001's avatar ysaito1001 Committed by GitHub
Browse files

Specify tilde version requirements for runtime crates in manifest files (#3009)

## Motivation and Context
This PR converts version requirements for runtime crates (those under
`aws/rust-runtime` and those under `rust-runtime`) in manifest files
into tilde requirements, allowing customers to safely incorporate only
patch versions (from [The Cargo
Book](https://doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html)).

## Description
Prior to the PR, if one opens the manifest file for an SDK create, a
version requirement for a runtime crate looks like
[this](https://github.com/awslabs/aws-sdk-rust/blob/655d490682e97ef799da85703520b5501fe91574/sdk/s3/Cargo.toml#L24-L27).
Post general availability (GA), those runtime crates will have versions
like `1.X.X` and if specified as such in a manifest file, it will be
interpreted as `>=1.X.X, <2.0.0`. To protect customers from minor
version bumps from runtime crates, we want to change their version
requirements to [tilde
requirements](https://doc.rust-lang.org/nightly/cargo/reference/specifying-dependencies.html#tilde-requirements)
`~1.X`, which then supports a version range `>=1.X.0, <1.X+1.0`.

Note that the change being discussed here is only concerned with version
requirements of runtime crates. There is a runtime crate that depends on
SDK crates (e.g. `aws-config` depending on `aws-sdk-sts`). In that case,
we do _not_ want to turn the version requirement of the SDK crate into a
tilde version because customers may depend on that SDK crate by
themselves, causing multiple versions of it to be brought in to their
crate graph.

To support the above functionality, `publisher` has been updated.
Specifically, when the `fix-manifests` subcommand runs, for runtime
crates it will render the tilde version requirements into manifest
files. Regarding a implementation detail, since the `semver::Version`
used in `publisher` requires three components, `major.minor.patch`, to
appear when version requirements are turned into in-memory data
structure, it does not quite work well with non-semver version
requirements. To address it, our own `crate::package::Version` has been
introduced to handle both semver and versions that do not adhere to the
semver specification. It then boils down to the `parse_version` function
being updated to be able to parse an input string into
`crate::package::Version`, as specified at call sites through a generic
parameter `P: ParseIntoVersion`.

## Testing
- Updated an existing unit test `test_validate_packages` in the
`package` module.
- Added a new unit test
`sdk_crate_version_should_not_be_turned_into_tilde_requirement` to the
`fix-manifests` subcommand.
- Ran our internal release pipeline and confirmed the generated SDK
crates have tilde dependencies for runtime crates in their manifest
files.

## Checklist
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the AWS
SDK, generated SDK code, or SDK runtime crates

----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent 398ad9e5
Loading
Loading
Loading
Loading
+139 −39
Original line number Diff line number Diff line
@@ -10,14 +10,77 @@ use crate::sort::dependency_order;
use crate::RUST_SDK_CI_OWNER;
use anyhow::{Context, Result};
use cargo_toml::{Dependency, DepsSet, Manifest};
use semver::Version;
use smithy_rs_tool_common::package::PackageCategory;
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::error::Error as StdError;
use std::fmt;
use std::fmt::Formatter;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use tokio::fs;
use tracing::warn;
use tracing::{error, warn};

/// A trait that marks a type that can be created from `&str` and turned into
/// [`Version`]
///
/// This trait is purely for convenience to reduce the number of generic parameters for functions,
/// that is, the functions only need to have a single generic parameter that implements this trait
/// instead of having to specify `ParsedT` and `Err` separately.
pub trait ParseIntoVersion {
    type ParsedT: FromStr<Err = Self::Err> + Into<Version>;
    type Err: Into<BoxError>;
}

/// A type that indicates the `SemVer` variant of [`Version`] will be created as a result of parsing
pub struct SemVer;
impl ParseIntoVersion for SemVer {
    type ParsedT = semver::Version;
    type Err = semver::Error;
}

/// A type that indicates the `Lenient` variant of [`Version`] will be created as a result of parsing
pub struct VersionRequirement;
impl ParseIntoVersion for VersionRequirement {
    type ParsedT = String;
    type Err = std::convert::Infallible;
}

/// An enum that handles both semver as well as string values that do not adhere to the semver
/// specification.
///
/// Most of the time, the `SemVer` variant will be used when manifest files are parsed into an
/// in-memory data structure. Those version strings can appear under the `[package]` section as
/// well as the dependency table within a manifest file.
/// The `VersionRequirement` variant is used when the `fix_manifests` subcommand and the
/// `generate_version_manifest` subcommand are executed, allowing version strings to be parsed into
/// tilde version requirements.
#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
pub enum Version {
    SemVer(semver::Version),
    VersionRequirement(String),
}

impl From<semver::Version> for Version {
    fn from(version: semver::Version) -> Self {
        Version::SemVer(version)
    }
}

impl From<String> for Version {
    fn from(s: String) -> Self {
        Version::VersionRequirement(s)
    }
}

impl fmt::Display for Version {
    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        let version = match self {
            Version::SemVer(v) => v.to_string(),
            Version::VersionRequirement(s) => s.clone(),
        };
        write!(f, "{}", version)
    }
}

/// Information required to identify a package (crate).
#[derive(Clone, Debug, Eq, PartialEq, Ord, PartialOrd)]
@@ -27,10 +90,10 @@ pub struct PackageHandle {
}

impl PackageHandle {
    pub fn new(name: impl Into<String>, version: Version) -> Self {
    pub fn new(name: impl Into<String>, version: impl Into<Version>) -> Self {
        Self {
            name: name.into(),
            version,
            version: version.into(),
        }
    }
}
@@ -145,7 +208,7 @@ pub async fn discover_and_validate_package_batches(
    fs: Fs,
    path: impl AsRef<Path>,
) -> Result<(Vec<PackageBatch>, PackageStats)> {
    let packages = discover_packages(fs, path.as_ref().into())
    let packages = discover_packages::<VersionRequirement>(fs, path.as_ref().into())
        .await?
        .into_iter()
        .filter(|package| package.publish == Publish::Allowed)
@@ -171,6 +234,8 @@ pub enum Error {
    MissingVersion(PathBuf, String),
    #[error("crate {0} has multiple versions: {1} and {2}")]
    MultipleVersions(String, Version, Version),
    #[error("multiple version requirements have been specified for crate {0}: {1} and {2}")]
    MultipleVersionRequirements(String, Version, Version),
}

/// Discovers all Cargo.toml files under the given path recursively
@@ -192,18 +257,24 @@ pub async fn discover_manifests(path: PathBuf) -> Result<Vec<PathBuf>> {
}

/// Discovers and parses all Cargo.toml files that are packages (as opposed to being exclusively workspaces)
pub async fn discover_packages(fs: Fs, path: PathBuf) -> Result<Vec<Package>> {
pub async fn discover_packages<P: ParseIntoVersion>(fs: Fs, path: PathBuf) -> Result<Vec<Package>> {
    let manifest_paths = discover_manifests(path).await?;
    read_packages(fs, manifest_paths).await
    read_packages::<P>(fs, manifest_paths).await
}

/// Parses a semver version number and adds additional error context when parsing fails.
pub fn parse_version(manifest_path: &Path, version: &str) -> Result<Version, Error> {
    Version::parse(version)
/// Parses `version` into [`Version`] and adds additional error context when parsing fails.
pub fn parse_version<P: ParseIntoVersion>(
    manifest_path: &Path,
    version: &str,
) -> Result<P::ParsedT, Error> {
    P::ParsedT::from_str(version)
        .map_err(|err| Error::InvalidCrateVersion(manifest_path.into(), version.into(), err.into()))
}

fn read_dependencies(path: &Path, dependencies: &DepsSet) -> Result<Vec<PackageHandle>> {
fn read_dependencies<P: ParseIntoVersion>(
    path: &Path,
    dependencies: &DepsSet,
) -> Result<Vec<PackageHandle>> {
    let mut result = Vec::new();
    for (name, metadata) in dependencies {
        match metadata {
@@ -213,7 +284,7 @@ fn read_dependencies(path: &Path, dependencies: &DepsSet) -> Result<Vec<PackageH
                    let version = detailed
                        .version
                        .as_ref()
                        .map(|version| parse_version(path, version))
                        .map(|version| parse_version::<P>(path, version))
                        .ok_or_else(|| Error::MissingVersion(path.into(), name.into()))??;
                    result.push(PackageHandle::new(name, version));
                }
@@ -224,23 +295,28 @@ fn read_dependencies(path: &Path, dependencies: &DepsSet) -> Result<Vec<PackageH
}

/// Returns `Ok(None)` when the Cargo.toml is a workspace rather than a package
fn read_package(path: &Path, manifest_bytes: &[u8]) -> Result<Option<Package>> {
fn read_package<P: ParseIntoVersion>(
    path: &Path,
    manifest_bytes: &[u8],
) -> Result<Option<Package>> {
    let manifest = Manifest::from_slice(manifest_bytes)
        .with_context(|| format!("failed to load package manifest for {:?}", path))?;
    if let Some(package) = manifest.package {
        let name = package.name;
        let version = parse_version(path, &package.version)?;
        let handle = PackageHandle { name, version };
        let version = parse_version::<P>(path, &package.version)?;
        let handle = PackageHandle::new(name, version);
        let publish = match package.publish {
            cargo_toml::Publish::Flag(true) => Publish::Allowed,
            _ => Publish::NotAllowed,
        };

        let mut local_dependencies = BTreeSet::new();
        local_dependencies.extend(read_dependencies(path, &manifest.dependencies)?.into_iter());
        local_dependencies.extend(read_dependencies(path, &manifest.dev_dependencies)?.into_iter());
        local_dependencies
            .extend(read_dependencies(path, &manifest.build_dependencies)?.into_iter());
            .extend(read_dependencies::<P>(path, &manifest.dependencies)?.into_iter());
        local_dependencies
            .extend(read_dependencies::<P>(path, &manifest.dev_dependencies)?.into_iter());
        local_dependencies
            .extend(read_dependencies::<P>(path, &manifest.build_dependencies)?.into_iter());
        Ok(Some(Package::new(
            handle,
            path,
@@ -255,38 +331,63 @@ fn read_package(path: &Path, manifest_bytes: &[u8]) -> Result<Option<Package>> {
/// Validates that all of the publishable crates use consistent version numbers
/// across all of their local dependencies.
fn validate_packages(packages: &[Package]) -> Result<()> {
    let mut versions: BTreeMap<String, Version> = BTreeMap::new();
    let track_version = &mut |handle: &PackageHandle| -> Result<(), Error> {
        if let Some(version) = versions.get(&handle.name) {
    fn track_version<F>(
        handle: &PackageHandle,
        map: &mut BTreeMap<String, Version>,
        error_generator: F,
    ) -> Result<(), Error>
    where
        F: FnOnce(String, Version, Version) -> Result<(), Error>,
    {
        if let Some(version) = map.get(&handle.name) {
            if *version != handle.version {
                Err(Error::MultipleVersions(
                error_generator(
                    (&handle.name).into(),
                    versions[&handle.name].clone(),
                    map[&handle.name].clone(),
                    handle.version.clone(),
                ))
                )
            } else {
                Ok(())
            }
        } else {
            versions.insert(handle.name.clone(), handle.version.clone());
            map.insert(handle.name.clone(), handle.version.clone());
            Ok(())
        }
    };
    }
    let mut versions: BTreeMap<String, Version> = BTreeMap::new();
    let mut version_requirements: BTreeMap<String, Version> = BTreeMap::new();
    for package in packages {
        track_version(&package.handle)?;
        track_version(
            &package.handle,
            &mut versions,
            |crate_name, version_a, version_b| {
                Err(Error::MultipleVersions(crate_name, version_a, version_b))
            },
        )?;
        for dependency in &package.local_dependencies {
            track_version(dependency)?;
            track_version(
                dependency,
                &mut version_requirements,
                |crate_name, version_a, version_b| {
                    Err(Error::MultipleVersionRequirements(
                        crate_name, version_a, version_b,
                    ))
                },
            )?;
        }
    }

    Ok(())
}

pub async fn read_packages(fs: Fs, manifest_paths: Vec<PathBuf>) -> Result<Vec<Package>> {
pub async fn read_packages<P: ParseIntoVersion>(
    fs: Fs,
    manifest_paths: Vec<PathBuf>,
) -> Result<Vec<Package>> {
    let mut result = Vec::new();
    for path in &manifest_paths {
        let contents: Vec<u8> = fs.read_file(path).await?;
        if let Some(package) = read_package(path, &contents)? {
        if let Some(package) = read_package::<P>(path, &contents)? {
            result.push(package);
        }
    }
@@ -335,11 +436,10 @@ fn batch_packages(packages: Vec<Package>) -> Result<Vec<PackageBatch>> {
#[cfg(test)]
mod tests {
    use super::*;
    use semver::Version;
    use std::path::PathBuf;

    fn version(version: &str) -> Version {
        Version::parse(version).unwrap()
        semver::Version::parse(version).unwrap().into()
    }

    #[test]
@@ -363,7 +463,7 @@ mod tests {
        "#;
        let path: PathBuf = "test/Cargo.toml".into();

        let package = read_package(&path, manifest)
        let package = read_package::<SemVer>(&path, manifest)
            .expect("parse success")
            .expect("is a package");
        assert_eq!("test", package.handle.name);
@@ -393,7 +493,7 @@ mod tests {

        let error = format!(
            "{}",
            read_package(&path, manifest).expect_err("should fail")
            read_package::<SemVer>(&path, manifest).expect_err("should fail")
        );
        assert!(
            error.contains("Invalid crate version"),
@@ -404,11 +504,11 @@ mod tests {

    fn package(name: &str, dependencies: &[&str]) -> Package {
        Package::new(
            PackageHandle::new(name, Version::parse("1.0.0").unwrap()),
            PackageHandle::new(name, semver::Version::parse("1.0.0").unwrap()),
            format!("{}/Cargo.toml", name),
            dependencies
                .iter()
                .map(|d| PackageHandle::new(*d, Version::parse("1.0.0").unwrap()))
                .map(|d| PackageHandle::new(*d, semver::Version::parse("1.0.0").unwrap()))
                .collect(),
            Publish::Allowed,
        )
@@ -486,11 +586,11 @@ mod tests {

    fn pkg_ver(name: &str, version: &str, dependencies: &[(&str, &str)]) -> Package {
        Package::new(
            PackageHandle::new(name, Version::parse(version).unwrap()),
            PackageHandle::new(name, semver::Version::parse(version).unwrap()),
            format!("{}/Cargo.toml", name),
            dependencies
                .iter()
                .map(|p| PackageHandle::new(p.0, Version::parse(p.1).unwrap()))
                .map(|p| PackageHandle::new(p.0, semver::Version::parse(p.1).unwrap()))
                .collect(),
            Publish::Allowed,
        )
@@ -526,7 +626,7 @@ mod tests {
        ])
        .expect_err("fail");
        assert_eq!(
            "crate A has multiple versions: 1.1.0 and 1.0.0",
            "multiple version requirements have been specified for crate A: 1.1.0 and 1.0.0",
            format!("{}", error)
        );
    }
+14 −13
Original line number Diff line number Diff line
@@ -12,9 +12,9 @@ use std::collections::{BTreeMap, BTreeSet};
/// Determines the dependency order of the given packages.
pub fn dependency_order(packages: Vec<Package>) -> Result<Vec<Package>> {
    let mut order = Vec::new();
    let mut packages: BTreeMap<PackageHandle, Package> = packages
    let mut packages: BTreeMap<String, Package> = packages
        .into_iter()
        .map(|p| (p.handle.clone(), p))
        .map(|p| (p.handle.name.clone(), p))
        .collect();
    let mut visited = BTreeSet::new();

@@ -22,7 +22,7 @@ pub fn dependency_order(packages: Vec<Package>) -> Result<Vec<Package>> {
    to_visit.sort_by(|a, b| a.local_dependencies.len().cmp(&b.local_dependencies.len()));

    // Depth-first search topological sort
    while let Some(package) = to_visit.iter().find(|e| !visited.contains(&e.handle)) {
    while let Some(package) = to_visit.iter().find(|e| !visited.contains(&e.handle.name)) {
        dependency_order_visit(
            &package.handle,
            &packages,
@@ -34,27 +34,28 @@ pub fn dependency_order(packages: Vec<Package>) -> Result<Vec<Package>> {

    Ok(order
        .into_iter()
        .map(&mut |handle| packages.remove(&handle).unwrap())
        .map(&mut |handle: PackageHandle| packages.remove(&handle.name).unwrap())
        .collect())
}

fn dependency_order_visit(
    package_handle: &PackageHandle,
    packages: &BTreeMap<PackageHandle, Package>,
    stack: &mut BTreeSet<PackageHandle>,
    visited: &mut BTreeSet<PackageHandle>,
    packages: &BTreeMap<String, Package>,
    stack: &mut BTreeSet<String>,
    visited: &mut BTreeSet<String>,
    result: &mut Vec<PackageHandle>,
) -> Result<()> {
    if visited.contains(package_handle) {
    let crate_name = &package_handle.name;
    if visited.contains(crate_name) {
        return Ok(());
    }
    if stack.contains(package_handle) {
    if stack.contains(crate_name) {
        tracing::error!(stack = ?stack, handle = ?package_handle, "dependency cycle!");
        bail!("dependency cycle detected");
    }
    stack.insert(package_handle.clone());
    stack.insert(crate_name.clone());
    let local_dependencies = &packages
        .get(package_handle)
        .get(crate_name)
        .ok_or_else(|| {
            dbg!(packages);
            anyhow!("packages to publish doesn't contain {:?}", package_handle)
@@ -63,8 +64,8 @@ fn dependency_order_visit(
    for dependency in local_dependencies {
        dependency_order_visit(dependency, packages, stack, visited, result)?;
    }
    stack.remove(package_handle);
    visited.insert(package_handle.clone());
    stack.remove(crate_name);
    visited.insert(crate_name.clone());
    result.push(package_handle.clone());
    Ok(())
}
+3 −4
Original line number Diff line number Diff line
@@ -3,13 +3,12 @@
 * SPDX-License-Identifier: Apache-2.0
 */
use crate::fs::Fs;
use crate::package::{discover_packages, PackageHandle, Publish};
use crate::package::{discover_packages, PackageHandle, Publish, SemVer};
use crate::publish::{has_been_published_on_crates_io, publish};
use crate::subcommand::publish::correct_owner;
use crate::{cargo, SDK_REPO_NAME};
use clap::Parser;
use dialoguer::Confirm;
use semver::Version;
use smithy_rs_tool_common::git;
use smithy_rs_tool_common::package::PackageCategory;
use std::collections::HashSet;
@@ -62,7 +61,7 @@ async fn claim_crate_name(name: &str) -> anyhow::Result<()> {
    create_dummy_lib_crate(Fs::Real, name, crate_dir_path.to_path_buf()).await?;

    let category = PackageCategory::from_package_name(name);
    let package_handle = PackageHandle::new(name, Version::new(0, 0, 1));
    let package_handle = PackageHandle::new(name, semver::Version::new(0, 0, 1));
    publish(&package_handle, crate_dir_path).await?;
    // Keep things slow to avoid getting throttled by crates.io
    tokio::time::sleep(Duration::from_secs(2)).await;
@@ -77,7 +76,7 @@ async fn discover_publishable_crate_names(repository_root: &Path) -> anyhow::Res
        fs: Fs,
        path: PathBuf,
    ) -> anyhow::Result<HashSet<String>> {
        let packages = discover_packages(fs, path).await?;
        let packages = discover_packages::<SemVer>(fs, path).await?;
        let mut publishable_package_names = HashSet::new();
        for package in packages {
            if let Publish::Allowed = package.publish {
+87 −15
Original line number Diff line number Diff line
@@ -10,12 +10,12 @@
//! version numbers in addition to the dependency path.

use crate::fs::Fs;
use crate::package::{discover_manifests, parse_version};
use crate::package::{discover_manifests, parse_version, SemVer};
use crate::SDK_REPO_NAME;
use anyhow::{bail, Context, Result};
use clap::Parser;
use semver::Version;
use smithy_rs_tool_common::ci::running_in_ci;
use smithy_rs_tool_common::package::PackageCategory;
use std::collections::BTreeMap;
use std::ffi::OsStr;
use std::path::{Path, PathBuf};
@@ -93,7 +93,7 @@ enum FilterType {
}
struct VersionView<'a>(&'a Versions, FilterType);
impl VersionView<'_> {
    fn get(&self, crate_name: &str) -> Option<&Version> {
    fn get(&self, crate_name: &str) -> Option<&semver::Version> {
        let version = match (self.1, self.0 .0.get(crate_name)) {
            (FilterType::AllCrates, version) => version,
            (FilterType::PublishedOnly, v @ Some(VersionWithMetadata { publish: true, .. })) => v,
@@ -112,20 +112,20 @@ impl Versions {
        VersionView(self, FilterType::PublishedOnly)
    }

    fn published_crates(&self) -> impl Iterator<Item = (&str, &Version)> + '_ {
    fn published_crates(&self) -> impl Iterator<Item = (&str, &semver::Version)> + '_ {
        self.0
            .iter()
            .filter(|(_, v)| v.publish)
            .map(|(k, v)| (k.as_str(), &v.version))
    }

    fn get(&self, crate_name: &str) -> Option<&Version> {
    fn get(&self, crate_name: &str) -> Option<&semver::Version> {
        self.0.get(crate_name).map(|v| &v.version)
    }
}

struct VersionWithMetadata {
    version: Version,
    version: semver::Version,
    publish: bool,
}

@@ -162,7 +162,7 @@ fn package_versions(manifests: &[Manifest]) -> Result<Versions> {
            .ok_or_else(|| {
                anyhow::Error::msg(format!("{:?} is missing a package version", manifest.path))
            })?;
        let version = parse_version(&manifest.path, version)?;
        let version = parse_version::<SemVer>(&manifest.path, version)?;
        versions.insert(name.into(), VersionWithMetadata { version, publish });
    }
    Ok(Versions(versions))
@@ -188,17 +188,32 @@ fn fix_dep_set(versions: &VersionView, key: &str, metadata: &mut Value) -> Resul
    Ok(changed)
}

// Update a version of `dep_name` that has a path dependency to be that appearing in `versions`.
//
// While doing so, we will use the tilde version requirement so customers can update patch versions
// automatically. Specifically, we use tilde versions of the form `~major.minor` (e.g. `~1.2`) so
// it can support the range of versions `>=1.2.0, <1.3.0`. See
// https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#tilde-requirements
fn update_dep(table: &mut Table, dep_name: &str, versions: &VersionView) -> Result<usize> {
    if !table.contains_key("path") {
        return Ok(0);
    }
    let package_version = match versions.get(dep_name) {
        Some(version) => version.to_string(),
        None => bail!("version not found for crate {}", dep_name),
    let package_version = versions
        .get(dep_name)
        .ok_or_else(|| anyhow::Error::msg(format!("version not found for crate {dep_name}")))?;
    let package_version = if PackageCategory::from_package_name(dep_name) == PackageCategory::AwsSdk
    {
        // For a crate that depends on an SDK crate (e.g. `aws-config` depending on `aws-sdk-sts`),
        // we do _not_ want to turn the version of the SDK crate into a tilde version because
        // customers may depend on that SDK crate by themselves, causing multiple versions of it
        // to be brought in to their crate graph.
        package_version.to_string()
    } else {
        convert_to_tilde_requirement(package_version)?
    };
    let previous_version = table.insert(
        "version".into(),
        toml::Value::String(package_version.to_string()),
        toml::Value::String(package_version.clone()),
    );
    match previous_version {
        None => Ok(1),
@@ -210,6 +225,32 @@ fn update_dep(table: &mut Table, dep_name: &str, versions: &VersionView) -> Resu
    }
}

// Convert `package_version` into a tilde version requirement
//
// For instance, given `package_version` like `0.12.3`, the function returns `~0.12`.
// The fact that this function takes a `semver::Version` means one can only convert a complete
// semver `x.y.z` into a tilde version requirement, but not those like `0.21.0-alpha.1`.
fn convert_to_tilde_requirement(package_version: &semver::Version) -> Result<String> {
    // `package_version` is from the `semver` crate which requires versions to have 3 components,
    // major, minor, and patch. So it is safe to assume its string value follows that format.
    let package_version = package_version.to_string();
    let package_version = match package_version.rfind('.') {
        // Here, we're interested in the `major.minor` part.
        Some(index) => {
            assert_eq!(
                2,
                package_version.chars().filter(|&c| c == '.').count(),
                "The number of `.` in {} is not 2",
                package_version
            );
            &package_version[0..index]
        }
        None => bail!("{} did not have any dots in it", package_version),
    };

    Ok("~".to_string() + package_version)
}

fn fix_dep_sets(versions: &VersionView, metadata: &mut toml::Value) -> Result<usize> {
    let mut changed = fix_dep_set(versions, "dependencies", metadata)?;
    // allow dev dependencies to be unpublished
@@ -326,7 +367,7 @@ mod tests {
                (
                    name.to_string(),
                    VersionWithMetadata {
                        version: Version::parse(&version).unwrap(),
                        version: semver::Version::parse(&version).unwrap(),
                        publish,
                    },
                )
@@ -412,7 +453,7 @@ mod tests {
                \n\
                [local_something]\n\
                path = \"../local_something\"\n\
                version = \"1.1.3\"\n\
                version = \"~1.1\"\n\
            ",
            actual_deps.to_string()
        );
@@ -424,7 +465,7 @@ mod tests {
                \n\
                [local_dev_something]\n\
                path = \"../local_dev_something\"\n\
                version = \"0.1.0\"\n\
                version = \"~0.1\"\n\
            ",
            actual_dev_deps.to_string()
        );
@@ -436,12 +477,43 @@ mod tests {
                \n\
                [local_build_something]\n\
                path = \"../local_build_something\"\n\
                version = \"0.2.0\"\n\
                version = \"~0.2\"\n\
            ",
            actual_build_deps.to_string()
        );
    }

    #[test]
    fn sdk_crate_version_should_not_be_turned_into_tilde_requirement() {
        let manifest = br#"
            [package]
            name = "test"
            version = "1.2.0-preview"

            [dependencies]
            aws-sdk-example = { path = "../aws/sdk/example" }
        "#;
        let metadata = toml::from_slice(manifest).unwrap();
        let mut manifest = Manifest {
            path: "test".into(),
            metadata,
        };
        let versions = &[("aws-sdk-example", "0.2.0", true)];
        let versions = make_versions(versions.iter());

        fix_dep_sets(&versions.published(), &mut manifest.metadata).expect("success");

        let actual_deps = &manifest.metadata["dependencies"];
        assert_eq!(
            "\
                [aws-sdk-example]\n\
                path = \"../aws/sdk/example\"\n\
                version = \"0.2.0\"\n\
            ",
            actual_deps.to_string()
        );
    }

    #[test]
    fn test_is_example_manifest() {
        assert!(!is_example_manifest("aws-sdk-rust/sdk/s3/Cargo.toml"));
+2 −3
Original line number Diff line number Diff line
@@ -7,7 +7,6 @@ use crate::fs::Fs;
use crate::package::discover_and_validate_package_batches;
use crate::subcommand::fix_manifests::Versions;
use anyhow::{anyhow, bail, Result};
use semver::Version;
use smithy_rs_tool_common::package::PackageCategory;
use std::path::Path;
use tracing::info;
@@ -39,7 +38,7 @@ pub(super) fn validate_before_fixes(
    Ok(())
}

fn confirm_version(name: &str, expected: &Version, actual: &Version) -> Result<()> {
fn confirm_version(name: &str, expected: &semver::Version, actual: &semver::Version) -> Result<()> {
    if expected != actual {
        bail!(
            "Crate named `{}` should be at version `{}` but is at `{}`",
@@ -73,7 +72,7 @@ mod test {
            map.insert(
                (*name).into(),
                VersionWithMetadata {
                    version: Version::from_str(version).unwrap(),
                    version: semver::Version::from_str(version).unwrap(),
                    publish: true,
                },
            );
Loading