apple-codesign: Replace aws-sdk-s3 with rust-s3#215
apple-codesign: Replace aws-sdk-s3 with rust-s3#215KYovchevski wants to merge 4 commits intoindygreg:mainfrom
aws-sdk-s3 with rust-s3#215Conversation
aws-sdk-s3 with rust-s3aws-sdk-s3 with rust-s3
86cdf40 to
4851f8d
Compare
indygreg
left a comment
There was a problem hiding this comment.
This looks like a meaningful improvement. The aws-* crate tree feels a bit bloated and I too don't like the longer build times.
I had this PR applied locally and was going to push it.
I saw from the Cargo.lock changes that we add a dependency on openssl-sys. That didn't sit well with me because I don't want to take a dependency on OpenSSL. We could point the rust-s3 crate at an alternate crypto implementation via a *-rustls-tls feature, which I'd prefer.
But then I was thinking about the likely future, where we gain support for using AWS KMS to perform code signing. That would almost certainly use an aws-* crate. Then we'd have the official AWS crates and a 3rd party crate for S3. That feels kludgy. And the aws-* crates would insist on using Amazon's crypto stack (aws-lc-sys), which is the thing introducing the cmake dependency. So we'd be back where we started.
Then there's the interaction with #212 and how we should handle HTTP proxies.
I'm sympathetic to the short-term build time wins. But I'm not convinced this is the best long-term direction due to likely incorporation of native KMS signing support someday.
|
The main goal with the PR was actually removing the dependency on CMake, since we are using The dependency on Admittedly, I am not very familiar with the Amazon ecosystem and AWS KMS in particular, but I see the concern with having both If possible, I would be open to refactor this with a feature that controls which S3 backend is being used ( |
We found that
aws-sdk-s3is a pretty heavy dependency which also requires CMake.Replacing it with
rust-s3removes this dependency on CMake and also speeds up the build times. It also allows us to removetokioand a few other dependencies forapple-codesign, though some are still included in the tree through other dependencies.The build times below are the quickest I could get over a few clean builds of
apple-codesign.Previous:
--features="notarize") - 126s--no-default-features- 105sNow:
--features="notarize") - 108s--no-default-features- 104sAlso includes some unrelated formatting changes because I ran
cargo fmt, but the bulk of the changes are innotarization.rs.