-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add Swift Package Registry tools and config #1597
Conversation
var errorDescription: String? { message } | ||
|
||
init(_ message: String) { | ||
public struct Error: LocalizedError { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you will see below, many types like this one have been moved into AWSCLIUtils and been marked public, so that they can be shared between CLI utilities.
], | ||
dependencies: [ | ||
.package(url: "https://github.com/apple/swift-argument-parser", from: "1.2.0"), | ||
.package(url: "https://github.com/apple/swift-package-manager", from: "0.6.0"), | ||
.package(url: "https://github.com/apple/swift-algorithms", from: "1.0.0"), | ||
.package(url: "https://github.com/apple/swift-log.git", from: "1.0.0"), | ||
.package(url: "https://github.com/awslabs/aws-sdk-swift", from: "0.46.0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our own AWS SDK for Swift is added as a dependency so that it can be used to perform the S3 & Cloudfront operations needed for publishing to the Registry.
.product(name: "Algorithms", package: "swift-algorithms"), | ||
.product(name: "Logging", package: "swift-log"), | ||
], | ||
resources: [ | ||
.process("Resources/Package.Base.swift"), | ||
.process("Resources/DocIndex.Base.md") | ||
] | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two new CLI commands are created, spr-publish
(for publishing one package to the Registry) and spr-multi-publish
(for publishing the entire SDK). I elected to make them their own executables instead of adding more subcommands to AWSSDKSwiftCLI
target above.
"spr-publish", | ||
] | ||
), | ||
.target( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SPR
target is used for sharing registry-related code between the CLI commands.
@@ -0,0 +1,95 @@ | |||
{ | |||
"pins" : [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is committed so that when we run the command line tools, we all get the same versions of dependencies until we choose to update them.
@@ -29,14 +29,14 @@ extension Process { | |||
|
|||
/// Returns the executable and arguments combined as a string | |||
var commandString: String { | |||
let items = [executableURL?.path] + (arguments ?? []) | |||
let items = [urlPath(executableURL)] + (arguments ?? []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As you will see below, Foundation URL
path is different between Mac & Linux, hence we use this wrapper method to get it.
try _run(process) | ||
process.waitUntilExit() | ||
|
||
let data = try stdOut.fileHandleForReading.readToEnd() ?? Data() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The readToEnd()
method is used instead of the readabilityHandler
above because sometimes the data was being returned before all of it had been written. readToEnd()
guarantees that the file handle closes & all the data is read before it returns.
@@ -37,3 +37,9 @@ extension String { | |||
/// Returns the string that represents a newline | |||
static var newline: Self { "\n" } | |||
} | |||
|
|||
public func printError(_ items: Any..., separator: String = " ", terminator: String = "\n") throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A simple free function that acts like a print()
statement but writes to stdErr.
public func urlPath(_ url: URL?) -> String? { | ||
guard let url else { return nil } | ||
return urlPath(url) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that on our supported Mac versions of Swift, you get the path from a URL with the path(percentEncoded:)
method, while on Linux Foundation they still only have the .path
property.
These two helper methods use the right one for the platform in use.
@@ -7,6 +7,7 @@ | |||
|
|||
import ArgumentParser | |||
import Foundation | |||
import AWSCLIUtils |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many utility types & functions were split into a new AWSCLIUtils
module so they could be shared between the old AWSSDKSwiftCLI
tool and the new registry commands.
Hence, a lot of imports to use that module.
@@ -83,56 +83,56 @@ let package = Package( | |||
.awsSDKHTTPAuth, | |||
.awsSDKIdentity | |||
], | |||
path: "./Sources/Core/AWSClientRuntime", | |||
path: "Sources/Core/AWSClientRuntime/Sources", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both runtime modules and service clients have been arranged to match the internal layout of the standard Swift package. Paths in this file have been adjusted to accommodate.
Also, service clients now have a Resources
subfolder which is included in the target.
import AWSCLIUtils | ||
import AWSCloudFront | ||
|
||
extension SPRPublisher { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Provides a function which is used to invalidate caching for the Registry version lists after they are updated.
On a spr-multi-publish run, only one invalidation is performed at the end, containing all version lists. This is because Cloudfront will start to throttle validations if they are done one path at a time for each package.
|
||
import Foundation | ||
|
||
public struct Describe: Codable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The many Codable structures below are used to read & write the standard JSON formatted data for registry version lists and version metadata.
import PackageDescription | ||
import AWSCLIUtils | ||
|
||
extension Process { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These extensions run other utilities to perform steps in publication to the registry:
- describe the contents of a package (used to verify package identity),
- perform archiving of a package version to a .zip file, and
- get the SHA-256 checksum for a file
|
||
import PackageDescription | ||
|
||
let package = Package( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made this manifest for publishing AWSSDKHTTPAuth
as its own package.
import PackageDescription | ||
|
||
let package = Package( | ||
name: "AWSSDKIdentity", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made this manifest for publishing AWSSDKIdentity
as its own package.
resources: [.process("Resources")] | ||
), | ||
] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll see many files moved below, as the runtime is organized into standard Swift package format.
import AWSSDKIdentity | ||
import AWSSDKHTTPAuth | ||
|
||
class SigV4EventSigningTests: XCTestCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests for SigV4 signing of event stream events has been moved from AWSSDKHTTPAuth
to AWSSDKEventStreamsAuth
to break a circular dependency between those two modules.
@@ -161,13 +165,16 @@ fun discoverServices(): List<AwsService> { | |||
val serviceApi = service.getTrait(software.amazon.smithy.aws.traits.ServiceTrait::class.java).orNull() | |||
?: error { "Expected aws.api#service trait attached to model ${file.absolutePath}" } | |||
val (name, version, _) = file.name.split(".") | |||
val serviceClientVersions = Node.parse(rootProject.file("ServiceClientVersions.json").readText(Charset.forName("UTF-8"))) | |||
val packageVersion = serviceClientVersions.expectObjectNode().getStringMember(name).orNull()?.value ?: | |||
serviceClientVersions.expectObjectNode().getStringMember("sdk").get().value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reads the package version out of the new ServiceClientVersions.json
file, then sets it in the smithy-build json so it can be code-generated into the service client.
@@ -11,34 +11,38 @@ class AWSSwiftDependency { | |||
val AWS_SDK_IDENTITY = SwiftDependency( | |||
"AWSSDKIdentity", | |||
"main", | |||
"0.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All these dependencies below have been updated with their registry scope and package name so they can be rendered as dependencies in Package.swift correctly.
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator | ||
import software.amazon.smithy.swift.codegen.integration.SwiftIntegration | ||
|
||
class PackageVersionIntegration : SwiftIntegration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Writes the package version into the service client package's Resources. This can be used later for data collection purposes and for reference during publication.
import software.amazon.smithy.swift.codegen.integration.ProtocolGenerator | ||
import software.amazon.smithy.swift.codegen.integration.SwiftIntegration | ||
|
||
class RegistryConfigIntegration : SwiftIntegration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Writes the registry configuration file into the service client package, so the Swift build tools know where the registry is located.
@@ -12,6 +12,7 @@ rm -rf Tests/Services/* | |||
./gradlew --stop | |||
|
|||
# Regenerate the SDK Package.swift with all services | |||
unset AWS_SWIFT_SDK_USE_LOCAL_DEPS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AWS_SWIFT_SDK_USE_LOCAL_DEPS
is unset here & below before running the CLI tools so that the SDK used by the build tools doesn't attempt to use local smithy-swift
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some questions.
@_disfavoredOverload | ||
public func urlPath(_ url: URL?) -> String? { | ||
guard let url else { return nil } | ||
return urlPath(url) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: What's the point of this method? Why not just do guard check in the method that has compiler directives for OS type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two methods have very similar signatures but are distinct. (they differ only in whether the param type and return type are optionals or not). Mostly I want to have a non-optional return value when that is needed so I don't have to unwrap at the caller.
Swift was having problems resolving the right overload for the point of use so I added the @_disfavoredOverload
annotation to help it find the right one.
"sdk": "0.15.0", | ||
"s3": "0.15.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Where did 0.15.0 come from?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a number I was using for testing. It bears no relation to real release numbers. Before we would ship with this mechanism, there will be a script (not yet provided) that manages these numbers.
@@ -12,6 +12,7 @@ rm -rf Tests/Services/* | |||
./gradlew --stop | |||
|
|||
# Regenerate the SDK Package.swift with all services | |||
unset AWS_SWIFT_SDK_USE_LOCAL_DEPS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Why shouldn't CLI tools not use local smithy-swift
? Won't the pakages already be imported within GH action runner and made available locally?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CLI now uses the latest stable aws-sdk-swift as a dependency (to perform the S3 & CloudFront operations needed to publish.)
If you leave that env var set, it will cause this dependency to break, because it will try to use a local copy of smithy-swift that isn't there.
For context, I expect to be removing the AWS_SWIFT_SDK_USE_LOCAL_DEPS
env var entirely before GA. We'll use a different mechanism for local dependencies during development.
@@ -11,34 +11,38 @@ class AWSSwiftDependency { | |||
val AWS_SDK_IDENTITY = SwiftDependency( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: I see that github links are removed for the modules; were these git links for these dependencies even used anywhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some dependencies (the ones that remain on git) still have a URL. The ones that will be published on the registry do not.
Issue #
Description of changes
Note:
Package.swift
files for AWS runtime modules and service clients are temporarily renamed toPackage.swift.txt
. This is because Xcode hides any directory inside a Swift package that has another Swift package embedded in it. When it is time to ship these modules as their own Swift packages, these manifests will be renamed back toPackage.swift
.New/existing dependencies impact assessment, if applicable
No new dependencies were added to this change.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.