Unverified Commit f8867814 authored by John DiSanti's avatar John DiSanti Committed by GitHub
Browse files

Refactor `sdk-versioner` to operate off the versions manifest (#1420)

parent d99d3e0c
Loading
Loading
Loading
Loading
+7 −8
Original line number Diff line number Diff line
@@ -56,7 +56,6 @@ val awsServices: AwsServices by lazy { discoverServices(loadServiceMembership())
val eventStreamAllowList: Set<String> by lazy { eventStreamAllowList() }

fun getSdkVersion(): String = properties.get("aws.sdk.version") ?: throw Exception("SDK version missing")
fun getSmithyRsVersion(): String = properties.get("smithy.rs.runtime.crate.version") ?: throw Exception("smithy-rs version missing")
fun getRustMSRV(): String = properties.get("rust.msrv") ?: throw Exception("Rust MSRV missing")
fun getPreviousReleaseVersionManifestPath(): String? = properties.get("aws.sdk.previous.release.versions.manifest")

@@ -206,14 +205,14 @@ tasks.register<ExecRustBuildTool>("fixExampleManifests") {
    toolPath = sdkVersionerToolPath
    binaryName = "sdk-versioner"
    arguments = listOf(
        outputDir.resolve("examples").absolutePath,
        "use-path-and-version-dependencies",
        "--sdk-path", "../../sdk",
        "--sdk-version", getSdkVersion(),
        "--smithy-version", getSmithyRsVersion()
        "--versions-toml", outputDir.resolve("versions.toml").absolutePath,
        outputDir.resolve("examples").absolutePath
    )

    outputs.dir(outputDir)
    dependsOn("relocateExamples")
    dependsOn("relocateExamples", "generateVersionManifest")
}

/**
@@ -307,7 +306,6 @@ tasks.register<ExecRustBuildTool>("fixManifests") {
    dependsOn("relocateRuntime")
    dependsOn("relocateAwsRuntime")
    dependsOn("relocateExamples")
    dependsOn("fixExampleManifests")
}

tasks.register<ExecRustBuildTool>("hydrateReadme") {
@@ -369,9 +367,10 @@ tasks.register("finalizeSdk") {
        "relocateExamples",
        "generateIndexMd",
        "fixManifests",
        "generateVersionManifest",
        "fixExampleManifests",
        "hydrateReadme",
        "relocateChangelog",
        "generateVersionManifest"
        "relocateChangelog"
    )
}

+160 −79
Original line number Diff line number Diff line
@@ -3,9 +3,10 @@
 * SPDX-License-Identifier: Apache-2.0
 */

use anyhow::bail;
use anyhow::{bail, Result};
use clap::Parser;
use smithy_rs_tool_common::package::{PackageCategory, SDK_PREFIX};
use smithy_rs_tool_common::versions_manifest::VersionsManifest;
use std::ffi::OsStr;
use std::fs;
use std::path::{Path, PathBuf};
@@ -18,56 +19,102 @@ use toml::value::{Table, Value};
    about = "CLI tool to recursively update SDK/Smithy crate references in Cargo.toml files",
    version
)]
struct Args {
#[allow(clippy::enum_variant_names)] // Want the "use" prefix in the CLI subcommand names for clarity
enum Args {
    /// Revise crates to use paths in dependencies
    UsePathDependencies {
        /// Root SDK path the path dependencies will be based off of
        #[clap(long)]
        sdk_path: PathBuf,
        /// Path(s) to recursively update Cargo.toml files in
        #[clap()]
        crate_paths: Vec<PathBuf>,

    /// SDK version to point to
    },
    /// Revise crates to use version numbers in dependencies
    UseVersionDependencies {
        /// Path to a `versions.toml` file with crate versions to use
        #[clap(long)]
    sdk_version: Option<String>,
    /// Smithy version to point to
        versions_toml: PathBuf,
        /// Path(s) to recursively update Cargo.toml files in
        #[clap()]
        crate_paths: Vec<PathBuf>,
    },
    /// Revise crates to use version numbers AND paths in dependencies
    UsePathAndVersionDependencies {
        /// Root SDK path the path dependencies will be based off of
        #[clap(long)]
    smithy_version: Option<String>,

    /// Path to generated SDK to point to
        sdk_path: PathBuf,
        /// Path to a `versions.toml` file with crate versions to use
        #[clap(long)]
    sdk_path: Option<PathBuf>,
        versions_toml: PathBuf,
        /// Path(s) to recursively update Cargo.toml files in
        #[clap()]
        crate_paths: Vec<PathBuf>,
    },
}

impl Args {
    fn validate(self) -> anyhow::Result<Self> {
        if self.crate_paths.is_empty() {
            bail!("Must provide at least one crate path to recursively update");
    fn crate_paths(&self) -> &[PathBuf] {
        match self {
            Self::UsePathDependencies { crate_paths, .. } => crate_paths,
            Self::UseVersionDependencies { crate_paths, .. } => crate_paths,
            Self::UsePathAndVersionDependencies { crate_paths, .. } => crate_paths,
        }
        if self.sdk_version.is_none() && self.sdk_path.is_none() {
            bail!("Must provide either an SDK version or an SDK path to update to");
    }
        if self.sdk_version.is_some() != self.smithy_version.is_some() {
            bail!("Must provide a Smithy version when providing an SDK version to update to");

    fn validate(self) -> Result<Self> {
        if self.crate_paths().is_empty() {
            bail!("Must provide at least one crate path to recursively update");
        }
        Ok(self)
    }
}

fn main() -> anyhow::Result<()> {
    let opt = Args::parse().validate()?;
struct DependencyContext<'a> {
    sdk_path: Option<&'a Path>,
    versions_manifest: Option<VersionsManifest>,
}

fn main() -> Result<()> {
    let args = Args::parse().validate()?;

    let dependency_context = match &args {
        Args::UsePathDependencies { sdk_path, .. } => DependencyContext {
            sdk_path: Some(sdk_path),
            versions_manifest: None,
        },
        Args::UseVersionDependencies { versions_toml, .. } => DependencyContext {
            sdk_path: None,
            versions_manifest: Some(VersionsManifest::from_file(&versions_toml)?),
        },
        Args::UsePathAndVersionDependencies {
            sdk_path,
            versions_toml,
            ..
        } => DependencyContext {
            sdk_path: Some(sdk_path),
            versions_manifest: Some(VersionsManifest::from_file(&versions_toml)?),
        },
    };

    let start_time = Instant::now();
    let mut manifest_paths = Vec::new();
    for crate_path in &opt.crate_paths {
    for crate_path in args.crate_paths() {
        discover_manifests(&mut manifest_paths, crate_path)?;
    }

    for manifest_path in manifest_paths {
        update_manifest(&manifest_path, &opt)?;
        update_manifest(&manifest_path, &dependency_context)?;
    }

    println!("Finished in {:?}", start_time.elapsed());
    Ok(())
}

fn update_manifest(manifest_path: &Path, opt: &Args) -> anyhow::Result<()> {
fn update_manifest(
    manifest_path: &Path,
    dependency_context: &DependencyContext,
) -> anyhow::Result<()> {
    println!("Updating {:?}...", manifest_path);

    let mut metadata: Value = toml::from_slice(&fs::read(manifest_path)?)?;
@@ -81,7 +128,9 @@ fn update_manifest(manifest_path: &Path, opt: &Args) -> anyhow::Result<()> {
                    manifest_path
                );
            }
            changed = update_dependencies(dependencies.as_table_mut().unwrap(), opt)? || changed;
            changed =
                update_dependencies(dependencies.as_table_mut().unwrap(), dependency_context)?
                    || changed;
        }
    }

@@ -92,7 +141,10 @@ fn update_manifest(manifest_path: &Path, opt: &Args) -> anyhow::Result<()> {
    Ok(())
}

fn update_dependencies(dependencies: &mut Table, opt: &Args) -> anyhow::Result<bool> {
fn update_dependencies(
    dependencies: &mut Table,
    dependency_context: &DependencyContext,
) -> Result<bool> {
    let mut changed = false;
    for (key, value) in dependencies.iter_mut() {
        let category = PackageCategory::from_package_name(key);
@@ -100,7 +152,7 @@ fn update_dependencies(dependencies: &mut Table, opt: &Args) -> anyhow::Result<b
            if !value.is_table() {
                *value = Value::Table(Table::new());
            }
            update_dependency_value(key, value.as_table_mut().unwrap(), opt);
            update_dependency_value(key, value.as_table_mut().unwrap(), dependency_context)?;
            changed = true;
        }
    }
@@ -118,12 +170,11 @@ fn crate_path_name(name: &str) -> &str {
    }
}

fn update_dependency_value(crate_name: &str, value: &mut Table, opt: &Args) {
    let is_sdk_crate = matches!(
        PackageCategory::from_package_name(crate_name),
        PackageCategory::AwsSdk | PackageCategory::AwsRuntime,
    );

fn update_dependency_value(
    crate_name: &str,
    value: &mut Table,
    dependency_context: &DependencyContext,
) -> Result<()> {
    // Remove keys that will be replaced
    value.remove("git");
    value.remove("branch");
@@ -131,7 +182,7 @@ fn update_dependency_value(crate_name: &str, value: &mut Table, opt: &Args) {
    value.remove("path");

    // Set the `path` if one was given
    if let Some(path) = &opt.sdk_path {
    if let Some(path) = &dependency_context.sdk_path {
        let crate_path = path.join(crate_path_name(crate_name));
        value.insert(
            "path".to_string(),
@@ -146,22 +197,23 @@ fn update_dependency_value(crate_name: &str, value: &mut Table, opt: &Args) {
    }

    // Set the `version` if one was given
    if opt.sdk_version.is_some() {
    if let Some(manifest) = &dependency_context.versions_manifest {
        if let Some(crate_metadata) = manifest.crates.get(crate_name) {
            value.insert(
                "version".to_string(),
            Value::String(
                if is_sdk_crate {
                    &opt.sdk_version
                Value::String(crate_metadata.version.clone()),
            );
        } else {
                    &opt.smithy_version
                }
                .clone()
                .unwrap(),
            ),
            bail!(
                "Crate `{}` was missing from the `versions.toml`",
                crate_name
            );
        }
    }

    Ok(())
}

/// Recursively discovers Cargo.toml files in the given `path` and adds them to `manifests`.
fn discover_manifests(manifests: &mut Vec<PathBuf>, path: impl AsRef<Path>) -> anyhow::Result<()> {
    let path = path.as_ref();
@@ -182,10 +234,35 @@ fn discover_manifests(manifests: &mut Vec<PathBuf>, path: impl AsRef<Path>) -> a

#[cfg(test)]
mod tests {
    use crate::{update_manifest, Args};
    use crate::{update_manifest, DependencyContext};
    use pretty_assertions::assert_eq;
    use smithy_rs_tool_common::package::PackageCategory;
    use smithy_rs_tool_common::versions_manifest::{CrateVersion, VersionsManifest};
    use std::path::PathBuf;
    use toml::Value;

    fn versions_toml_for(crates: &[(&str, &str)]) -> VersionsManifest {
        VersionsManifest {
            smithy_rs_revision: "doesntmatter".into(),
            aws_doc_sdk_examples_revision: "doesntmatter".into(),
            crates: crates
                .iter()
                .map(|&(name, version)| {
                    (
                        name.to_string(),
                        CrateVersion {
                            category: PackageCategory::from_package_name(name),
                            version: version.into(),
                            source_hash: "doesntmatter".into(),
                            model_hash: None,
                        },
                    )
                })
                .collect(),
            release: None,
        }
    }

    const TEST_MANIFEST: &[u8] = br#"
        [package]
        name = "test"
@@ -200,12 +277,12 @@ mod tests {
    "#;

    #[track_caller]
    fn test_with_opt(opt: Args, expected: &[u8]) {
    fn test_with_context(context: DependencyContext, expected: &[u8]) {
        let manifest_file = tempfile::NamedTempFile::new().unwrap();
        let manifest_path = manifest_file.into_temp_path();
        std::fs::write(&manifest_path, TEST_MANIFEST).unwrap();

        update_manifest(&manifest_path, &opt).expect("success");
        update_manifest(&manifest_path, &context).expect("success");

        let actual = toml::from_slice(&std::fs::read(&manifest_path).expect("read tmp file"))
            .expect("valid toml");
@@ -215,12 +292,15 @@ mod tests {

    #[test]
    fn update_dependencies_with_versions() {
        test_with_opt(
            Args {
                crate_paths: Vec::new(),
        test_with_context(
            DependencyContext {
                sdk_path: None,
                sdk_version: Some("0.5.0".to_string()),
                smithy_version: Some("0.35.0".to_string()),
                versions_manifest: Some(versions_toml_for(&[
                    ("aws-config", "0.5.0"),
                    ("aws-sdk-s3", "0.13.0"),
                    ("aws-smithy-types", "0.10.0"),
                    ("aws-smithy-http", "0.9.0"),
                ])),
            },
            br#"
            [package]
@@ -229,9 +309,9 @@ mod tests {

            [dependencies]
            aws-config = { version = "0.5.0" }
            aws-sdk-s3 = { version = "0.5.0" }
            aws-smithy-types = { version = "0.35.0" }
            aws-smithy-http = { version = "0.35.0", features = ["test-util"] }
            aws-sdk-s3 = { version = "0.13.0" }
            aws-smithy-types = { version = "0.10.0" }
            aws-smithy-http = { version = "0.9.0", features = ["test-util"] }
            something-else = "0.1"
            "#,
        );
@@ -239,12 +319,10 @@ mod tests {

    #[test]
    fn update_dependencies_with_paths() {
        test_with_opt(
            Args {
                crate_paths: Vec::new(),
                sdk_path: Some("/foo/asdf/".into()),
                sdk_version: None,
                smithy_version: None,
        test_with_context(
            DependencyContext {
                sdk_path: Some(&PathBuf::from("/foo/asdf/")),
                versions_manifest: None,
            },
            br#"
            [package]
@@ -263,12 +341,15 @@ mod tests {

    #[test]
    fn update_dependencies_with_versions_and_paths() {
        test_with_opt(
            Args {
                crate_paths: Vec::new(),
                sdk_path: Some("/foo/asdf/".into()),
                sdk_version: Some("0.5.0".to_string()),
                smithy_version: Some("0.35.0".to_string()),
        test_with_context(
            DependencyContext {
                sdk_path: Some(&PathBuf::from("/foo/asdf/")),
                versions_manifest: Some(versions_toml_for(&[
                    ("aws-config", "0.5.0"),
                    ("aws-sdk-s3", "0.13.0"),
                    ("aws-smithy-types", "0.10.0"),
                    ("aws-smithy-http", "0.9.0"),
                ])),
            },
            br#"
            [package]
@@ -277,9 +358,9 @@ mod tests {

            [dependencies]
            aws-config = { version = "0.5.0", path = "/foo/asdf/aws-config" }
            aws-sdk-s3 = { version = "0.5.0", path = "/foo/asdf/s3" }
            aws-smithy-types = { version = "0.35.0", path = "/foo/asdf/aws-smithy-types" }
            aws-smithy-http = { version = "0.35.0", path = "/foo/asdf/aws-smithy-http", features = ["test-util"] }
            aws-sdk-s3 = { version = "0.13.0", path = "/foo/asdf/s3" }
            aws-smithy-types = { version = "0.10.0", path = "/foo/asdf/aws-smithy-types" }
            aws-smithy-http = { version = "0.9.0", path = "/foo/asdf/aws-smithy-http", features = ["test-util"] }
            something-else = "0.1"
            "#
        );