Skip to content
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

fix: Replace SwiftPM dependency with local Version type #1684

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

jbelkins
Copy link
Contributor

@jbelkins jbelkins commented Aug 16, 2024

Issue #

#1683

Description of changes

The AWSSDKSwiftCLI project requires the SwiftPM package as a dependency. One of SwiftPM's dependencies, swift-llbuild, has been causing failures of preview builds because of issues with symlinked folders in the project.

A review of the SwiftPM dependency shows that is is used exclusively for access to the Version structure, which represents a semantic version specifier.

To simplify the project and lighten its dependencies (including the specific dependency that is causing preview build failures), the SwiftPM package is removed as a dependency and a Version structure is provided in its place.

The Version structure represents a major, minor, and patch version. It does not accept versions that do not include all three values, and it does not accept preprelease identifiers that are allowed by the semver standard. However, this is sufficient for the current manner in which the project is versioned.

All AWSSDKSwiftCLI tests pass with this change. (They previously froze because of an issue with testing Process invocations. This PR fixes the _runReturningStdOut() function so that it may be mocked with a test runner.)

I have also tested this change by running the prepare-release CLI command on my desktop, and observing the changes made to the project and the change log that was generated.

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.

@@ -18,15 +18,6 @@
"version" : "1.3.1"
}
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bye bye, useless dependencies

try testRunner.run(process)
return nil
}
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this code fixes this process runner so that it can be mocked with a test runner. Note that when the behavior is mocked, the returned output is nil; output may be mocked as well in the future if desired.

}
}

extension Version: CustomStringConvertible {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the previous Version, this struct provides the version string when printed.


// MARK: - Decodable
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The struct below replaces the one that was previously imported from the SwiftPM package.


// MARK: - Codable

extension Version: Codable {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version encodes & decodes itself by encoding/decoding its version string.

0,
prereleaseIdentifiers: prereleaseIdentifiers,
buildMetadataIdentifiers: buildMetadataIdentifiers
0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prerelease & build metadata identifiers are removed, since our project does not use them.

self.minor = minor
self.patch = patch
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that creating a Version from a string is now a non-optional initializer that throws if the string is not a valid semantic version identifier.

@@ -16,10 +16,10 @@ class GeneratePackageManifestTests: CLITestCase {
func createPackageDependencies(
crtVersion: String,
clientRuntimeVersion: String
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Below, many tests have been marked with throws since creating a Version from a string is now a throwing operation.

@jbelkins jbelkins requested review from dayaffe and sichanyoo August 16, 2024 19:32
@jbelkins jbelkins marked this pull request as ready for review August 16, 2024 19:33
@jbelkins jbelkins merged commit 241b55d into main Aug 16, 2024
29 checks passed
@jbelkins jbelkins deleted the jbe/remove_swiftpm_package branch August 16, 2024 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants