Skip to content

Commit ac95a5d

Browse files
authored
Migrate to a new SDK example workspace structure (#2811)
## Motivation and Context When the WebAssembly SDK example was added some months ago, we changed the build process to make each SDK example its own Cargo workspace. This allowed the `.cargo/config.toml` that changed the compiler target to work correctly. However, this has led to other issues: dependency compilation is no longer shared between examples which greatly increases the time it takes for that CI step to run, and now it is even running out of disk space on the GitHub Actions runners. This PR adds support for a new workspace layout where the [`aws-doc-sdk-examples`](https://github.com/awsdocs/aws-doc-sdk-examples) repo gets to decide how the workspaces are logically grouped. If a `Cargo.toml` file exists at the example root, then the build system assumes that the _old_ "one example, one workspace" layout should be used. If there is no root `Cargo.toml`, then it assumes the new layout should be used. The `sdk-versioner` tool had to be adapted to support more complex relative path resolution to make this work, and the `publisher fix-manifests` subcommand had to be fixed to ignore workspace-only `Cargo.toml` files. The build system in this PR needs to work for both the old and new examples layout so that the `sdk-sync` process will succeed. #2810 has been filed to track removing the old example layout at a later date. [aws-doc-sdk-examples#4997](awsdocs/aws-doc-sdk-examples#4997) changes the workspace structure of the actual examples to the new one. ## Testing - [x] Generated a full SDK with the old example layout, manually examined the output, and spot checked that some examples compile - [x] Generated a full SDK with the new example layout, manually examined the output, and spot checked that some examples compile - [x] Examples pass in CI with the old example layout - [x] Examples pass in CI with the new example layout ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent ab2ea0d commit ac95a5d

File tree

11 files changed

+317
-100
lines changed

11 files changed

+317
-100
lines changed

aws/sdk/build.gradle.kts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6+
import aws.sdk.AwsExamplesLayout
67
import aws.sdk.AwsServices
78
import aws.sdk.Membership
89
import aws.sdk.discoverServices
@@ -201,6 +202,7 @@ tasks.register("relocateExamples") {
201202
}
202203
into(outputDir)
203204
exclude("**/target")
205+
exclude("**/rust-toolchain.toml")
204206
filter { line -> line.replace("build/aws-sdk/sdk/", "sdk/") }
205207
}
206208
}
@@ -242,13 +244,22 @@ tasks.register<ExecRustBuildTool>("fixExampleManifests") {
242244

243245
toolPath = sdkVersionerToolPath
244246
binaryName = "sdk-versioner"
245-
arguments = listOf(
246-
"use-path-and-version-dependencies",
247-
"--isolate-crates",
248-
"--sdk-path", "../../sdk",
249-
"--versions-toml", outputDir.resolve("versions.toml").absolutePath,
250-
outputDir.resolve("examples").absolutePath,
251-
)
247+
arguments = when (AwsExamplesLayout.detect(project)) {
248+
AwsExamplesLayout.Flat -> listOf(
249+
"use-path-and-version-dependencies",
250+
"--isolate-crates",
251+
"--sdk-path", "../../sdk",
252+
"--versions-toml", outputDir.resolve("versions.toml").absolutePath,
253+
outputDir.resolve("examples").absolutePath,
254+
)
255+
AwsExamplesLayout.Workspaces -> listOf(
256+
"use-path-and-version-dependencies",
257+
"--isolate-crates",
258+
"--sdk-path", sdkOutputDir.absolutePath,
259+
"--versions-toml", outputDir.resolve("versions.toml").absolutePath,
260+
outputDir.resolve("examples").absolutePath,
261+
)
262+
}
252263

253264
outputs.dir(outputDir)
254265
dependsOn("relocateExamples", "generateVersionManifest")

buildSrc/src/main/kotlin/aws/sdk/ServiceLoader.kt

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,38 @@ data class RootTest(
1919
val manifestName: String,
2020
)
2121

22+
// TODO(https://github.com/awslabs/smithy-rs/issues/2810): We can remove the `Flat` layout after the switch
23+
// to `Workspaces` has been released. This can be checked by looking at the `examples/` directory in aws-sdk-rust's
24+
// main branch.
25+
//
26+
// The `Flat` layout is retained for backwards compatibility so that the next release process can succeed.
27+
enum class AwsExamplesLayout {
28+
/**
29+
* Directory layout for examples used prior to June 26, 2023,
30+
* where each example was in the `rust_dev_preview/` root directory and
31+
* was considered to be its own workspace.
32+
*
33+
* This layout had issues with CI in terms of time to compile and disk space required
34+
* since the dependencies would get recompiled for every example.
35+
*/
36+
Flat,
37+
38+
/**
39+
* Current directory layout where there are a small number of workspaces
40+
* rooted in `rust_dev_preview/`.
41+
*/
42+
Workspaces,
43+
;
44+
45+
companion object {
46+
fun detect(project: Project): AwsExamplesLayout = if (project.projectDir.resolve("examples/Cargo.toml").exists()) {
47+
AwsExamplesLayout.Flat
48+
} else {
49+
AwsExamplesLayout.Workspaces
50+
}
51+
}
52+
}
53+
2254
class AwsServices(
2355
private val project: Project,
2456
services: List<AwsService>,
@@ -44,10 +76,16 @@ class AwsServices(
4476
}
4577

4678
val examples: List<String> by lazy {
47-
project.projectDir.resolve("examples")
48-
.listFiles { file -> !file.name.startsWith(".") }.orEmpty().toList()
49-
.filter { file -> manifestCompatibleWithGeneratedServices(file) }
50-
.map { "examples/${it.name}" }
79+
val examplesRoot = project.projectDir.resolve("examples")
80+
if (AwsExamplesLayout.detect(project) == AwsExamplesLayout.Flat) {
81+
examplesRoot.listFiles { file -> !file.name.startsWith(".") }.orEmpty().toList()
82+
.filter { file -> manifestCompatibleWithGeneratedServices(file) }
83+
.map { "examples/${it.name}" }
84+
} else {
85+
examplesRoot.listFiles { file ->
86+
!file.name.startsWith(".") && file.isDirectory() && file.resolve("Cargo.toml").exists()
87+
}.orEmpty().toList().map { "examples/${it.name}" }
88+
}
5189
}
5290

5391
/**

tools/ci-build/publisher/src/package.rs

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,7 @@ pub async fn discover_and_validate_package_batches(
145145
fs: Fs,
146146
path: impl AsRef<Path>,
147147
) -> Result<(Vec<PackageBatch>, PackageStats)> {
148-
let manifest_paths = discover_package_manifests(path.as_ref().into()).await?;
149-
let packages = read_packages(fs, manifest_paths)
148+
let packages = discover_packages(fs, path.as_ref().into())
150149
.await?
151150
.into_iter()
152151
.filter(|package| package.publish == Publish::Allowed)
@@ -176,7 +175,7 @@ pub enum Error {
176175

177176
/// Discovers all Cargo.toml files under the given path recursively
178177
#[async_recursion::async_recursion]
179-
pub async fn discover_package_manifests(path: PathBuf) -> Result<Vec<PathBuf>> {
178+
pub async fn discover_manifests(path: PathBuf) -> Result<Vec<PathBuf>> {
180179
let mut manifests = Vec::new();
181180
let mut read_dir = fs::read_dir(&path).await?;
182181
while let Some(entry) = read_dir.next_entry().await? {
@@ -185,14 +184,19 @@ pub async fn discover_package_manifests(path: PathBuf) -> Result<Vec<PathBuf>> {
185184
let manifest_path = package_path.join("Cargo.toml");
186185
if manifest_path.exists() {
187186
manifests.push(manifest_path);
188-
} else {
189-
manifests.extend(discover_package_manifests(package_path).await?.into_iter());
190187
}
188+
manifests.extend(discover_manifests(package_path).await?.into_iter());
191189
}
192190
}
193191
Ok(manifests)
194192
}
195193

194+
/// Discovers and parses all Cargo.toml files that are packages (as opposed to being exclusively workspaces)
195+
pub async fn discover_packages(fs: Fs, path: PathBuf) -> Result<Vec<Package>> {
196+
let manifest_paths = discover_manifests(path).await?;
197+
read_packages(fs, manifest_paths).await
198+
}
199+
196200
/// Parses a semver version number and adds additional error context when parsing fails.
197201
pub fn parse_version(manifest_path: &Path, version: &str) -> Result<Version, Error> {
198202
Version::parse(version)
@@ -219,26 +223,33 @@ fn read_dependencies(path: &Path, dependencies: &DepsSet) -> Result<Vec<PackageH
219223
Ok(result)
220224
}
221225

222-
fn read_package(path: &Path, manifest_bytes: &[u8]) -> Result<Package> {
226+
/// Returns `Ok(None)` when the Cargo.toml is a workspace rather than a package
227+
fn read_package(path: &Path, manifest_bytes: &[u8]) -> Result<Option<Package>> {
223228
let manifest = Manifest::from_slice(manifest_bytes)
224229
.with_context(|| format!("failed to load package manifest for {:?}", path))?;
225-
let package = manifest
226-
.package
227-
.ok_or_else(|| Error::InvalidManifest(path.into()))
228-
.context("crate manifest doesn't have a `[package]` section")?;
229-
let name = package.name;
230-
let version = parse_version(path, &package.version)?;
231-
let handle = PackageHandle { name, version };
232-
let publish = match package.publish {
233-
cargo_toml::Publish::Flag(true) => Publish::Allowed,
234-
_ => Publish::NotAllowed,
235-
};
236-
237-
let mut local_dependencies = BTreeSet::new();
238-
local_dependencies.extend(read_dependencies(path, &manifest.dependencies)?.into_iter());
239-
local_dependencies.extend(read_dependencies(path, &manifest.dev_dependencies)?.into_iter());
240-
local_dependencies.extend(read_dependencies(path, &manifest.build_dependencies)?.into_iter());
241-
Ok(Package::new(handle, path, local_dependencies, publish))
230+
if let Some(package) = manifest.package {
231+
let name = package.name;
232+
let version = parse_version(path, &package.version)?;
233+
let handle = PackageHandle { name, version };
234+
let publish = match package.publish {
235+
cargo_toml::Publish::Flag(true) => Publish::Allowed,
236+
_ => Publish::NotAllowed,
237+
};
238+
239+
let mut local_dependencies = BTreeSet::new();
240+
local_dependencies.extend(read_dependencies(path, &manifest.dependencies)?.into_iter());
241+
local_dependencies.extend(read_dependencies(path, &manifest.dev_dependencies)?.into_iter());
242+
local_dependencies
243+
.extend(read_dependencies(path, &manifest.build_dependencies)?.into_iter());
244+
Ok(Some(Package::new(
245+
handle,
246+
path,
247+
local_dependencies,
248+
publish,
249+
)))
250+
} else {
251+
Ok(None)
252+
}
242253
}
243254

244255
/// Validates that all of the publishable crates use consistent version numbers
@@ -275,7 +286,9 @@ pub async fn read_packages(fs: Fs, manifest_paths: Vec<PathBuf>) -> Result<Vec<P
275286
let mut result = Vec::new();
276287
for path in &manifest_paths {
277288
let contents: Vec<u8> = fs.read_file(path).await?;
278-
result.push(read_package(path, &contents)?);
289+
if let Some(package) = read_package(path, &contents)? {
290+
result.push(package);
291+
}
279292
}
280293
Ok(result)
281294
}
@@ -350,7 +363,9 @@ mod tests {
350363
"#;
351364
let path: PathBuf = "test/Cargo.toml".into();
352365

353-
let package = read_package(&path, manifest).expect("parse success");
366+
let package = read_package(&path, manifest)
367+
.expect("parse success")
368+
.expect("is a package");
354369
assert_eq!("test", package.handle.name);
355370
assert_eq!(version("1.2.0-preview"), package.handle.version);
356371

tools/ci-build/publisher/src/subcommand/claim_crate_names.rs

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,10 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55
use crate::fs::Fs;
6-
use crate::package::{discover_package_manifests, Error, PackageHandle};
6+
use crate::package::{discover_packages, PackageHandle, Publish};
77
use crate::publish::{has_been_published_on_crates_io, publish};
88
use crate::subcommand::publish::correct_owner;
99
use crate::{cargo, SDK_REPO_NAME};
10-
use anyhow::Context;
11-
use cargo_toml::Manifest;
1210
use clap::Parser;
1311
use dialoguer::Confirm;
1412
use semver::Version;
@@ -79,20 +77,11 @@ async fn discover_publishable_crate_names(repository_root: &Path) -> anyhow::Res
7977
fs: Fs,
8078
path: PathBuf,
8179
) -> anyhow::Result<HashSet<String>> {
82-
let manifest_paths = discover_package_manifests(path).await?;
80+
let packages = discover_packages(fs, path).await?;
8381
let mut publishable_package_names = HashSet::new();
84-
for manifest_path in manifest_paths {
85-
let contents: Vec<u8> = fs.read_file(&manifest_path).await?;
86-
let manifest = Manifest::from_slice(&contents).with_context(|| {
87-
format!("failed to load package manifest for {:?}", manifest_path)
88-
})?;
89-
let package = manifest
90-
.package
91-
.ok_or(Error::InvalidManifest(manifest_path))
92-
.context("crate manifest doesn't have a `[package]` section")?;
93-
let name = package.name;
94-
if let cargo_toml::Publish::Flag(true) = package.publish {
95-
publishable_package_names.insert(name);
82+
for package in packages {
83+
if let Publish::Allowed = package.publish {
84+
publishable_package_names.insert(package.handle.name);
9685
}
9786
}
9887
Ok(publishable_package_names)

tools/ci-build/publisher/src/subcommand/fix_manifests.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
//! version numbers in addition to the dependency path.
1111
1212
use crate::fs::Fs;
13-
use crate::package::{discover_package_manifests, parse_version};
13+
use crate::package::{discover_manifests, parse_version};
1414
use crate::SDK_REPO_NAME;
1515
use anyhow::{bail, Context, Result};
1616
use clap::Parser;
@@ -20,6 +20,7 @@ use std::collections::BTreeMap;
2020
use std::ffi::OsStr;
2121
use std::path::{Path, PathBuf};
2222
use toml::value::Table;
23+
use toml::Value;
2324
use tracing::info;
2425

2526
mod validate;
@@ -55,7 +56,7 @@ pub async fn subcommand_fix_manifests(
5556
true => Mode::Check,
5657
false => Mode::Execute,
5758
};
58-
let manifest_paths = discover_package_manifests(location.into()).await?;
59+
let manifest_paths = discover_manifests(location.into()).await?;
5960
let mut manifests = read_manifests(Fs::Real, manifest_paths).await?;
6061
let versions = package_versions(&manifests)?;
6162

@@ -91,6 +92,15 @@ fn package_versions(manifests: &[Manifest]) -> Result<BTreeMap<String, Version>>
9192
Some(package) => package,
9293
None => continue,
9394
};
95+
// ignore non-publishable crates
96+
if let Some(Value::Boolean(false)) = manifest
97+
.metadata
98+
.get("package")
99+
.expect("checked above")
100+
.get("publish")
101+
{
102+
continue;
103+
}
94104
let name = package
95105
.get("name")
96106
.and_then(|name| name.as_str())

tools/ci-build/publisher/src/subcommand/generate_version_manifest.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*/
55

66
use crate::fs::Fs;
7-
use crate::package::{discover_package_manifests, read_packages};
7+
use crate::package::discover_packages;
88
use anyhow::{bail, Context, Result};
99
use clap::Parser;
1010
use semver::Version;
@@ -70,10 +70,7 @@ pub async fn subcommand_generate_version_manifest(
7070
(None, Some(output_location)) => output_location,
7171
_ => bail!("Only one of `--location` or `--output-location` should be provided"),
7272
};
73-
let manifests = discover_package_manifests(input_location.into())
74-
.await
75-
.context("discover package manifests")?;
76-
let packages = read_packages(Fs::Real, manifests)
73+
let packages = discover_packages(Fs::Real, input_location.into())
7774
.await
7875
.context("read packages")?;
7976

tools/ci-build/sdk-versioner/Cargo.lock

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tools/ci-build/sdk-versioner/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ opt-level = 0
1515
[dependencies]
1616
anyhow = "1.0"
1717
clap = { version = "~3.1.18", features = ["derive"] }
18+
pathdiff = "0.2.1"
1819
smithy-rs-tool-common = { version = "0.1", path = "../smithy-rs-tool-common" }
1920
toml_edit = { version = "0.19.6" }
2021

0 commit comments

Comments
 (0)