Skip to content

feat(PBXFileReference): support XCFramework expectedSignature property #156

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
Merged
pepicrft merged 4 commits into from
Apr 30, 2025
Merged

feat(PBXFileReference): support XCFramework expectedSignature property #156

pepicrft merged 4 commits into from
Apr 30, 2025

Conversation

@TamarMilchtaich
Copy link
Contributor

@TamarMilchtaich TamarMilchtaich commented Mar 17, 2025 •
edited
Loading

Support XCFrameworks expectedSignature property, and add the expected expectedSignature to the target dependency, to be verified against the actual signature.

More about XCFramework's siganture:
https://developer.apple.com/documentation/Xcode/verifying-the-origin-of-your-xcframeworks

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Mar 17, 2025
@TamarMilchtaich TamarMilchtaich marked this pull request as draft March 17, 2025 12:51
TamarMilchtaich
TamarMilchtaich commented Mar 17, 2025
View reviewed changes
Sources/XcodeGraphMapper/Utilities/PathDependencyMapper.swift Outdated
return .framework(path: path, status: status, condition: condition)
case .xcframework:
return .xcframework(path: path, status: status, condition: condition)
// TODO: throw some error? log warning? this is very general, but from what I can tell it's only called from a non-xcframework contexts
Copy link
Contributor Author

@TamarMilchtaich TamarMilchtaich Mar 17, 2025

Choose a reason for hiding this comment

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

WDYT? Not sure how to treat this.

Copy link
Contributor

@pepicrft pepicrft Apr 7, 2025

Choose a reason for hiding this comment

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

Since this utility is being used from here where we can access the file reference through the build file, wouldn't we have the information that we need to set the right value here?

Copy link
Contributor Author

@TamarMilchtaich TamarMilchtaich Apr 7, 2025

Choose a reason for hiding this comment

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

Actually this seems OK now, there were some changes here following the review on tuist/tuist, and it makes sense now

From what I can tell, the path dependency mapper is used here (frameworks, no signature is required) and here (signature propagated, as expected)

So probably just need to remove the TODO

@dosubot dosubot bot added the enhancement New feature or request label Mar 17, 2025
@TamarMilchtaich TamarMilchtaich mentioned this pull request Mar 17, 2025
Closed
@pepicrft pepicrft requested review from ajkolean and Copilot April 7, 2025 11:07
Copilot AI reviewed Apr 7, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

pepicrft
pepicrft reviewed Apr 7, 2025
View reviewed changes
@TamarMilchtaich TamarMilchtaich force-pushed the xcframeworks-expected-signature branch from c367a1d to 5da3a82 Compare April 7, 2025 12:42
@TamarMilchtaich
Copy link
Contributor Author

TamarMilchtaich commented Apr 7, 2025

Fixed & squashed the fixes into one commit

@pepicrft
Copy link
Contributor

pepicrft commented Apr 7, 2025

Fixed & squashed the fixes into one commit

Let's then wait until tuist/tuist is ready.

@TamarMilchtaich TamarMilchtaich force-pushed the xcframeworks-expected-signature branch from 5da3a82 to 67d8b0c Compare April 9, 2025 08:06
@TamarMilchtaich
Copy link
Contributor Author

TamarMilchtaich commented Apr 9, 2025

Pushed the following change (and only this):
notSigned -> unsigned

@TamarMilchtaich TamarMilchtaich force-pushed the xcframeworks-expected-signature branch 2 times, most recently from ab00ad3 to d609f81 Compare April 10, 2025 14:51
@TamarMilchtaich TamarMilchtaich force-pushed the xcframeworks-expected-signature branch from d609f81 to 2fae634 Compare April 23, 2025 08:07
@TamarMilchtaich
Copy link
Contributor Author

TamarMilchtaich commented Apr 23, 2025

Pushed the following change (and only this):
signedByApple -> signedWithAppleCertificate

@TamarMilchtaich TamarMilchtaich force-pushed the xcframeworks-expected-signature branch from 2fae634 to b1094a9 Compare April 29, 2025 18:51
@TamarMilchtaich TamarMilchtaich marked this pull request as ready for review April 29, 2025 18:51
TamarMilchtaich and others added 3 commits April 30, 2025 14:50
@TamarMilchtaich @pepicrft
feat(PBXFileReference): support XCFramework expectedSignature property
33754cc 
Support XCFrameworks expectedSignature property, and add the expected
expectedSignature to the target dependency, to be verified against the
actual signature.
@pepicrft
Fix linting issues
b93a0c1
@pepicrft
Make the tests compile and pass
f703041
@pepicrft pepicrft force-pushed the xcframeworks-expected-signature branch from 5ce3dd5 to f703041 Compare April 30, 2025 12:50
@pepicrft pepicrft merged commit 81df161 into tuist:main Apr 30, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pepicrft pepicrft pepicrft left review comments

Copilot code review Copilot Copilot left review comments

@ajkolean ajkolean Awaiting requested review from ajkolean

Assignees

No one assigned

Labels

enhancement New feature or request size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@TamarMilchtaich @pepicrft