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.

Sign up for GitHub

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

Jump to bottom

Include signing entity in version metadata #6363

Merged
merged 1 commit into from Apr 4, 2023
Merged

Include signing entity in version metadata #6363

merged 1 commit into from Apr 4, 2023

Conversation

neonichu
Copy link
Member

Clients who are asking for metadata prior to download an archive from the registry may still want to display information from the signature which is non-trivial to extract. This includes it if possible.

@neonichu neonichu self-assigned this Mar 31, 2023
@neonichu
Copy link
Member Author

@swift-ci please smoke test

@neonichu neonichu force-pushed the signing-entity-in-version-metadata branch from 4d2f75d to e262bd0 Compare March 31, 2023 19:39
@neonichu
Copy link
Member Author

@swift-ci please smoke test

@MaxDesiatov
Copy link
Member

@swift-ci smoke test

@neonichu neonichu force-pushed the signing-entity-in-version-metadata branch from e262bd0 to 2f14408 Compare April 3, 2023 18:38
@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test

tomerd
tomerd reviewed Apr 3, 2023
View reviewed changes
Sources/PackageRegistry/RegistryClient.swift
return nil
}
let configuration = self.configuration.signing(for: package, registry: registry)
return try? tsc_await { SignatureValidation.extractSigningEntity(
Copy link
Member

Choose a reason for hiding this comment

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

I think the method itself is async, so we wont need to use tsc_await here?

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can see, nothing in RegistryClient is async.

Copy link
Member

Choose a reason for hiding this comment

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

what I mean is that this method takes a completion handler. so we dont need to use tsc_await, we can chain the handlers

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, now I get what you mean. We could do that but I think that'll make this code much more complicated tbh. We can have zero and also more than one resource with signing info, so there'll be a bunch of logic for aggregating the results. IMO seems better to just do this for now and eventually replace with real await instead.

@neonichu neonichu force-pushed the signing-entity-in-version-metadata branch from 2f14408 to 9f465a4 Compare April 3, 2023 19:03
@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test

@neonichu
Include signing entity in version metadata
ff1de60 
Clients who are asking for metadata prior to download an archive from the registry may still want to display information from the signature which is non-trivial to extract. This includes it if possible.
@neonichu neonichu force-pushed the signing-entity-in-version-metadata branch from 9f465a4 to ff1de60 Compare April 3, 2023 20:22
@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test

@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test linux

@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test windows

2 similar comments
@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test windows

@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test windows

@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023 

/Applications/Xcode-beta.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk/usr/lib/swift/Foundation.swiftmodule/x86_64-apple-macos.swiftinterface:4:8: error: no such module 'Combine'
import Combine
       ^
/private/var/folders/pf/p5g2vb2j0yx1x6vlmxdppc700000gn/T/Miscellaneous_AtMainSupport.60lkRg/Miscellaneous_AtMainSupport/Package.swift:1:8: error: failed to build module 'Foundation'; this SDK is not supported by the compiler (the SDK is built with 'Apple Swift version 5.5 (swiftlang-1300.0.27.6 clang-1300.0.27.2)', while this compiler is 'Apple Swift version 5.9-dev (LLVM f5c0e1516308d32, Swift 756fe15bdd053a8)'). Please select a toolchain which matches the SDK.
import Foundation
       ^

@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test windows

@neonichu
Copy link
Member Author

neonichu commented Apr 3, 2023

@swift-ci smoke test macOS

@neonichu
Copy link
Member Author

neonichu commented Apr 4, 2023

@swift-ci smoke test windows

@neonichu neonichu merged commit 638cc86 into main Apr 4, 2023
4 checks passed
@neonichu neonichu deleted the signing-entity-in-version-metadata branch April 4, 2023 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaxDesiatov MaxDesiatov MaxDesiatov approved these changes

@tomerd tomerd tomerd left review comments

@yim-lee yim-lee yim-lee approved these changes

@abertelrud abertelrud Awaiting requested review from abertelrud

Assignees

@neonichu neonichu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@neonichu @MaxDesiatov @tomerd @yim-lee