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

Add support for swift modules with swiftinterfaces (build_library_for_distribution mode) #56

Merged
merged 3 commits into from Jan 23, 2022

Conversation

vasvf
Copy link
Contributor

@vasvf vasvf commented Jan 23, 2022

This adds .swiftinterface files to artifacts that are needed for swift-only modules that contain @objc exposing interfaces.

Also tried to modify tests to fit this new type of file, but maybe I missed something

Copy link
Collaborator

@polac24 polac24 left a comment

Choose a reason for hiding this comment

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

That seems to be a reasonable change that is useful for projects with enabled BUILD_LIBRARY_FOR_DISTRIBUTION. In the PR description you mentioned adding @objc public ... but as far as I am aware, generating .swiftinterface depends for swift-evolution enabled builds (In Xcode managed by BUILD_LIBRARY_FOR_DISTRIBUTION).

Owing to that, you PR implementation seems to be fine but we should have two unit tests (in change BuildArtifactCreatorTests.swift and ArtifactSwiftProductsBuilderImplTests.swift:

  • Mode without swift-evolution - that is the current stat of these tests
  • mode with swift-evolution: same as above + expect zipping and unzipping extra .swiftinterface

In other words, your unit testing changes should be applied in a separate functions.

@vasvf
Copy link
Contributor Author

vasvf commented Jan 23, 2022

Separated tests in two files you specified.

I also removed unused let swiftmoduleExtensionsToInclude in ArtifactSwiftProductsBuilder.swift

I also added swiftinterface file check in swiftcTests

Copy link
Collaborator

@polac24 polac24 left a comment

Choose a reason for hiding this comment

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

Thanks!

@polac24 polac24 merged commit cdddc5b into spotify:master Jan 23, 2022
@vasvf vasvf changed the title Add support for swift modules with @objc interfaces Add support for swift modules with swiftinterfaces (build_library_for_distribution mode) Feb 15, 2022
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.

None yet

2 participants