diff --git a/aws/sdk/build.gradle.kts b/aws/sdk/build.gradle.kts index 936740ac19c1b884cb6fea5d76bd42397ea5f576..9c0126e219074c972b20ed27d12eec8f9edfb2c2 100644 --- a/aws/sdk/build.gradle.kts +++ b/aws/sdk/build.gradle.kts @@ -232,6 +232,7 @@ tasks.register("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, diff --git a/tools/ci-build/sdk-versioner/Cargo.lock b/tools/ci-build/sdk-versioner/Cargo.lock index 00fdd6c9973076ea027ecd7c3cc88a1ee64c5342..2862d9eeb4f34b7637b61807d8a07c12ebbab707 100644 --- a/tools/ci-build/sdk-versioner/Cargo.lock +++ b/tools/ci-build/sdk-versioner/Cargo.lock @@ -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" diff --git a/tools/ci-build/sdk-versioner/Cargo.toml b/tools/ci-build/sdk-versioner/Cargo.toml index 4cd1e37af831705c1066b2877a27b898a1cb3dd9..f7fc9477224ca01450188e416c16c0888b3ffb50 100644 --- a/tools/ci-build/sdk-versioner/Cargo.toml +++ b/tools/ci-build/sdk-versioner/Cargo.toml @@ -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] diff --git a/tools/ci-build/sdk-versioner/src/main.rs b/tools/ci-build/sdk-versioner/src/main.rs index 4a74adcc2acc5febba8da4485dcc07b7b8759cc1..77cf9ff6061b9e846966c009457a5cf3043c3a9c 100644 --- a/tools/ci-build/sdk-versioner/src/main.rs +++ b/tools/ci-build/sdk-versioner/src/main.rs @@ -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, + /// 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, + /// 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, + /// 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 { 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::() + .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 { 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 { + 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( - crate_path - .as_os_str() - .to_str() - .expect("valid utf-8 path") - .to_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 { @@ -264,35 +300,44 @@ mod tests { } const TEST_MANIFEST: &[u8] = br#" - [package] - name = "test" - version = "0.1.0" - - [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" - "#; +[package] +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 = { 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(&[ @@ -303,45 +348,99 @@ mod tests { ])), }, br#" - [package] - name = "test" - version = "0.1.0" - - [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" - "#, +[package] +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 = { 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_with_paths() { test_with_context( + false, DependencyContext { sdk_path: Some(&PathBuf::from("/foo/asdf/")), versions_manifest: None, }, br#" - [package] - name = "test" - version = "0.1.0" - - [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" - "#, +[package] +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 = { 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_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(&[ @@ -352,17 +451,27 @@ mod tests { ])), }, br#" - [package] - name = "test" - version = "0.1.0" - - [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" - "# +[package] +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 = { 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"] +"# ); } }