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

Support exposing enums from ObjC via Bridging headers #149

Merged
merged 9 commits into from
Aug 25, 2022

Conversation

polac24
Copy link
Collaborator

@polac24 polac24 commented Jun 6, 2022

Part III (out of IV) of #140 fix

  • an artifact .zip for a swift target, stores two ObjC headers: -Swift.h and -Swift.h.md5. Because the former could contain absolute paths, the latter file is used in a dependency fingerprint (just like .swiftmodule files) - its content is just the same as the .swiftmodule's override (a fingerprint of the target)
  • Producer and consumers for each .h dependency, check if an override is available. If so, its content is used for the fingerprint
  • Because -Swift.h.md5 is automatically placed next to the -Swift.h only for a .framework, a developer that wants to cover that for a static library, has to manually add an extra ditto (similar to cp) build step to move -Swift.h.md5 along -Swift.h. Required steps are documented in Readme.md. This scenario can be considered (only static library + exposing enum via a bridging header) as an edge case - very unlikely to happen.

This part unblocks rake e2e:run_standalone added in #147.

@polac24 polac24 changed the title 20220606 publish swifth md5 Support exposing enums from ObjC via Bridging headers Jun 6, 2022
PatrikBillgren
PatrikBillgren previously approved these changes Jun 29, 2022
Copy link
Contributor

@PatrikBillgren PatrikBillgren left a comment

Choose a reason for hiding this comment

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

Looks really good! Approved, I just noticed one minor nit in a comment

Sources/XCRemoteCache/Artifacts/ArtifactProcessor.swift Outdated Show resolved Hide resolved
@polac24
Copy link
Collaborator Author

polac24 commented Jul 29, 2022

@PatrikBillgren, can you please reapprove after my typo fix?

@CognitiveDisson
Copy link
Contributor

@polac24 Should we merge it?

@polac24
Copy link
Collaborator Author

polac24 commented Aug 25, 2022

@CognitiveDisson, yes. let me merge it.

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.

3 participants