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

Fix framework to package /Modules files when built #230

Merged
merged 1 commit into from
Sep 12, 2020

Conversation

thedavidharris
Copy link
Contributor

Public headers were added to the xcodeproj file in a way that
a modulemap was no longer being generated, and therefore no
swiftmodule (or swiftinterface) file could be created. This adds
them as explicit public headers instead of a folder copy.

Addresses #222

Public headers were added to the xcodeproj file in a way that
a modulemap was no longer being generated, and therefore no
swiftmodule (or swiftinterface) file could be created. This adds
them as explicit public headers instead of a folder copy.
Copy link
Contributor

@adamkaplan adamkaplan left a comment

Choose a reason for hiding this comment

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

This seems safe enough considering the tests still pass.

@nabla-c0d3
Copy link
Member

The problem has been that it's difficult to get to a setup that works for the three different package managers we support:

  1. Swift Package Manager
  2. Carthage
  3. CocoaPods

When we added 1) it broke 2) and 3) (#220 #214). We fixed 3) which then broke 1). We fixed it too but now 2) is still broken. This is not validated in the test suite so I am worried that merging this will just break one of them again.

@adamkaplan
Copy link
Contributor

Ouch. That sounds super painful. Maybe the contributor can separate CI tests for Carthage and SPM

@thedavidharris
Copy link
Contributor Author

I didn't want to do anything drastic but what I would generally suggest:

  • Follow the SPM defaults and use /include for public headers. Then things should be pretty clean by default
  • CocoaPods and SPM both build off the file system, so CocoaPods can follow the SPM defaults
  • Carthage just needs a valid xcodeproj file to build off of. As was the case here, this didn't change the filesystem at all, but the way the files were added to the xcodeproj weren't valid: adding the folder doesn't actually add the files to the xcodeproj file for publich headers, but since nobody is consuming those, it works fine (this pitfall technically exists with the others as well).

I wouldn't worry about specific tests around all the package managers to be an issue unless there are going to be significant changes to the project structure.

@nabla-c0d3 nabla-c0d3 merged commit 49052a8 into datatheorem:master Sep 12, 2020
@nabla-c0d3
Copy link
Member

Thanks!

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.

4 participants