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

Isolate SDK examples from root SDK workspace (#2458)

* Migrate `sdk-versioner` from `toml` to `toml_edit`

* Add `--isolate-crates` flag to `sdk-versioner`

* Enable example crate isolation
parent 89b5a7e5
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -232,6 +232,7 @@ tasks.register<ExecRustBuildTool>("fixExampleManifests") {
    binaryName = "sdk-versioner"
    arguments = listOf(
        "use-path-and-version-dependencies",
        "--isolate-crates",
        "--sdk-path", "../../sdk",
        "--versions-toml", outputDir.resolve("versions.toml").absolutePath,
        outputDir.resolve("examples").absolutePath,
+27 −1
Original line number Diff line number Diff line
@@ -704,7 +704,7 @@ dependencies = [
 "pretty_assertions",
 "smithy-rs-tool-common",
 "tempfile",
 "toml",
 "toml_edit",
]

[[package]]
@@ -925,6 +925,23 @@ dependencies = [
 "serde",
]

[[package]]
name = "toml_datetime"
version = "0.6.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "3ab8ed2edee10b50132aed5f331333428b011c99402b5a534154ed15746f9622"

[[package]]
name = "toml_edit"
version = "0.19.6"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "08de71aa0d6e348f070457f85af8bd566e2bc452156a423ddf22861b3a953fae"
dependencies = [
 "indexmap",
 "toml_datetime",
 "winnow",
]

[[package]]
name = "tower-service"
version = "0.3.2"
@@ -1179,6 +1196,15 @@ version = "0.36.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "c811ca4a8c853ef420abd8592ba53ddbbac90410fab6903b3e79972a631f7680"

[[package]]
name = "winnow"
version = "0.3.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "ee7b2c67f962bf5042bfd8b6a916178df33a26eec343ae064cb8e069f638fa6f"
dependencies = [
 "memchr",
]

[[package]]
name = "winreg"
version = "0.10.1"
+1 −1
Original line number Diff line number Diff line
@@ -15,7 +15,7 @@ opt-level = 0
[dependencies]
anyhow = "1.0"
clap = { version = "~3.1.18", features = ["derive"] }
toml = { version = "0.5.8", features = ["preserve_order"] }
toml_edit = { version = "0.19.6" }
smithy-rs-tool-common = { version = "0.1", path = "../smithy-rs-tool-common" }

[dev-dependencies]
+186 −77
Original line number Diff line number Diff line
@@ -3,7 +3,7 @@
 * SPDX-License-Identifier: Apache-2.0
 */

use anyhow::{bail, Result};
use anyhow::{bail, Context, Result};
use clap::Parser;
use smithy_rs_tool_common::package::{PackageCategory, SDK_PREFIX};
use smithy_rs_tool_common::versions_manifest::VersionsManifest;
@@ -11,7 +11,7 @@ use std::ffi::OsStr;
use std::fs;
use std::path::{Path, PathBuf};
use std::time::Instant;
use toml::value::{Table, Value};
use toml_edit::{Document, InlineTable, Item, Table, Value};

#[derive(Parser, Debug)]
#[clap(
@@ -29,6 +29,9 @@ enum Args {
        /// Path(s) to recursively update Cargo.toml files in
        #[clap()]
        crate_paths: Vec<PathBuf>,
        /// Makes each individual crate its own workspace
        #[clap(long)]
        isolate_crates: bool,
    },
    /// Revise crates to use version numbers in dependencies
    UseVersionDependencies {
@@ -38,6 +41,9 @@ enum Args {
        /// Path(s) to recursively update Cargo.toml files in
        #[clap()]
        crate_paths: Vec<PathBuf>,
        /// Makes each individual crate its own workspace
        #[clap(long)]
        isolate_crates: bool,
    },
    /// Revise crates to use version numbers AND paths in dependencies
    UsePathAndVersionDependencies {
@@ -50,6 +56,9 @@ enum Args {
        /// Path(s) to recursively update Cargo.toml files in
        #[clap()]
        crate_paths: Vec<PathBuf>,
        /// Makes each individual crate its own workspace
        #[clap(long)]
        isolate_crates: bool,
    },
}

@@ -62,6 +71,14 @@ impl Args {
        }
    }

    fn isolate_crates(&self) -> bool {
        *match self {
            Self::UsePathDependencies { isolate_crates, .. } => isolate_crates,
            Self::UseVersionDependencies { isolate_crates, .. } => isolate_crates,
            Self::UsePathAndVersionDependencies { isolate_crates, .. } => isolate_crates,
        }
    }

    fn validate(self) -> Result<Self> {
        if self.crate_paths().is_empty() {
            bail!("Must provide at least one crate path to recursively update");
@@ -103,7 +120,7 @@ fn main() -> Result<()> {
    }

    for manifest_path in manifest_paths {
        update_manifest(&manifest_path, &dependency_context)?;
        update_manifest(&manifest_path, &dependency_context, args.isolate_crates())?;
    }

    println!("Finished in {:?}", start_time.elapsed());
@@ -113,10 +130,16 @@ fn main() -> Result<()> {
fn update_manifest(
    manifest_path: &Path,
    dependency_context: &DependencyContext,
    isolate_crates: bool,
) -> anyhow::Result<()> {
    println!("Updating {:?}...", manifest_path);

    let mut metadata: Value = toml::from_slice(&fs::read(manifest_path)?)?;
    let mut metadata: Document = String::from_utf8(
        fs::read(manifest_path).with_context(|| format!("failed to read {manifest_path:?}"))?,
    )
    .with_context(|| format!("{manifest_path:?} has invalid UTF-8"))?
    .parse::<Document>()
    .with_context(|| format!("failed to parse {manifest_path:?}"))?;
    let mut changed = false;
    for set in ["dependencies", "dev-dependencies", "build-dependencies"] {
        if let Some(dependencies) = metadata.get_mut(set) {
@@ -132,9 +155,20 @@ fn update_manifest(
                    || changed;
        }
    }
    if isolate_crates && !metadata.contains_key("workspace") {
        let package_position = metadata["package"]
            .as_table()
            .expect("has a package")
            .position()
            .unwrap_or_default();
        let mut workspace = Table::new();
        workspace.set_position(package_position);
        metadata.insert("workspace", Item::Table(workspace));
        changed = true;
    }

    if changed {
        fs::write(manifest_path, toml::to_vec(&metadata)?)?;
        fs::write(manifest_path, metadata.to_string())?;
    }

    Ok(())
@@ -146,12 +180,18 @@ fn update_dependencies(
) -> Result<bool> {
    let mut changed = false;
    for (key, value) in dependencies.iter_mut() {
        let category = PackageCategory::from_package_name(key);
        let category = PackageCategory::from_package_name(key.get());
        if !matches!(category, PackageCategory::Unknown) {
            if !value.is_table() {
                *value = Value::Table(Table::new());
            }
            update_dependency_value(key, value.as_table_mut().unwrap(), dependency_context)?;
            let old_value = match value {
                Item::Table(table) => table.clone(),
                Item::Value(Value::InlineTable(inline)) => inline.clone().into_table(),
                _ => Table::new(),
            };
            *value = Item::Value(Value::InlineTable(updated_dependency_value(
                key.get(),
                old_value,
                dependency_context,
            )?));
            changed = true;
        }
    }
@@ -169,11 +209,13 @@ fn crate_path_name(name: &str) -> &str {
    }
}

fn update_dependency_value(
fn updated_dependency_value(
    crate_name: &str,
    value: &mut Table,
    old_value: Table,
    dependency_context: &DependencyContext,
) -> Result<()> {
) -> Result<InlineTable> {
    let mut value = old_value;

    // Remove keys that will be replaced
    value.remove("git");
    value.remove("branch");
@@ -183,25 +225,19 @@ fn update_dependency_value(
    // Set the `path` if one was given
    if let Some(path) = &dependency_context.sdk_path {
        let crate_path = path.join(crate_path_name(crate_name));
        value.insert(
            "path".to_string(),
            Value::String(
        value["path"] = toml_edit::value(
            crate_path
                .as_os_str()
                .to_str()
                .expect("valid utf-8 path")
                .to_string(),
            ),
        );
    }

    // Set the `version` if one was given
    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(crate_metadata.version.clone()),
            );
            value["version"] = toml_edit::value(crate_metadata.version.clone());
        } else {
            bail!(
                "Crate `{}` was missing from the `versions.toml`",
@@ -210,7 +246,8 @@ fn update_dependency_value(
        }
    }

    Ok(())
    value.sort_values_by(|a, _, b, _| b.cmp(a));
    Ok(value.into_inline_table())
}

/// Recursively discovers Cargo.toml files in the given `path` and adds them to `manifests`.
@@ -238,7 +275,6 @@ mod tests {
    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 {
@@ -268,31 +304,40 @@ mod tests {
name = "test"
version = "0.1.0"

# Some comment that should be preserved
[dependencies]
aws-config = "0.4.1"
aws-sdk-s3 = "0.4.1"
aws-smithy-types = "0.34.1"
aws-smithy-http = { version = "0.34.1", features = ["test-util"] }
        something-else = "0.1"
something-else = { version = "0.1", no-default-features = true }
tokio = { version = "1.18", features = ["net"] }

[dev-dependencies.another-thing]
# some comment
version = "5.0"
# another comment
features = ["foo", "baz"]
"#;

    #[track_caller]
    fn test_with_context(context: DependencyContext, expected: &[u8]) {
    fn test_with_context(isolate_crates: bool, 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, &context).expect("success");
        update_manifest(&manifest_path, &context, isolate_crates).expect("success");

        let actual = toml::from_slice(&std::fs::read(&manifest_path).expect("read tmp file"))
            .expect("valid toml");
        let expected: Value = toml::from_slice(expected).unwrap();
        let actual =
            String::from_utf8(std::fs::read(&manifest_path).expect("read tmp file")).unwrap();
        let expected = std::str::from_utf8(expected).unwrap();
        assert_eq!(expected, actual);
    }

    #[test]
    fn update_dependencies_with_versions() {
        test_with_context(
            false,
            DependencyContext {
                sdk_path: None,
                versions_manifest: Some(versions_toml_for(&[
@@ -307,12 +352,20 @@ mod tests {
name = "test"
version = "0.1.0"

# Some comment that should be preserved
[dependencies]
aws-config = { version = "0.5.0" }
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"
something-else = { version = "0.1", no-default-features = true }
tokio = { version = "1.18", features = ["net"] }

[dev-dependencies.another-thing]
# some comment
version = "5.0"
# another comment
features = ["foo", "baz"]
"#,
        );
    }
@@ -320,6 +373,7 @@ mod tests {
    #[test]
    fn update_dependencies_with_paths() {
        test_with_context(
            false,
            DependencyContext {
                sdk_path: Some(&PathBuf::from("/foo/asdf/")),
                versions_manifest: None,
@@ -329,12 +383,20 @@ mod tests {
name = "test"
version = "0.1.0"

# Some comment that should be preserved
[dependencies]
aws-config = { path = "/foo/asdf/aws-config" }
aws-sdk-s3 = { path = "/foo/asdf/s3" }
aws-smithy-types = { path = "/foo/asdf/aws-smithy-types" }
aws-smithy-http = { path = "/foo/asdf/aws-smithy-http", features = ["test-util"] }
            something-else = "0.1"
something-else = { version = "0.1", no-default-features = true }
tokio = { version = "1.18", features = ["net"] }

[dev-dependencies.another-thing]
# some comment
version = "5.0"
# another comment
features = ["foo", "baz"]
"#,
        );
    }
@@ -342,6 +404,43 @@ mod tests {
    #[test]
    fn update_dependencies_with_versions_and_paths() {
        test_with_context(
            false,
            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]
name = "test"
version = "0.1.0"

# Some comment that should be preserved
[dependencies]
aws-config = { version = "0.5.0", path = "/foo/asdf/aws-config" }
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 = { version = "0.1", no-default-features = true }
tokio = { version = "1.18", features = ["net"] }

[dev-dependencies.another-thing]
# some comment
version = "5.0"
# another comment
features = ["foo", "baz"]
"#
        );
    }

    #[test]
    fn update_dependencies_isolate_crates() {
        test_with_context(
            true,
            DependencyContext {
                sdk_path: Some(&PathBuf::from("/foo/asdf/")),
                versions_manifest: Some(versions_toml_for(&[
@@ -356,12 +455,22 @@ mod tests {
name = "test"
version = "0.1.0"

[workspace]

# Some comment that should be preserved
[dependencies]
aws-config = { version = "0.5.0", path = "/foo/asdf/aws-config" }
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"
something-else = { version = "0.1", no-default-features = true }
tokio = { version = "1.18", features = ["net"] }

[dev-dependencies.another-thing]
# some comment
version = "5.0"
# another comment
features = ["foo", "baz"]
"#
        );
    }