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

[Xcodeproj] Avoid generating module.modulemap if umbrella header exists, on creating Clang Module targets #955

Conversation

norio-nomura
Copy link
Contributor

This PR increases the portability of generated Xcode project by reducing the use of module.modulemap which absolute path contains.

  • If clang module has umbrella header:
    • Add headers to HeadersBuildPhase
    • Set CLANG_ENABLE_MODULES = YES
    • Set DEFINES_MODULE = YES
    • Don’t generate module.modulemap
  • Set CLANG_ENABLE_MODULES = YES to test target so that we can import clang module.

In the course of testing to create a package written only in Objective-C, I came up with this change.
e.g: https://github.com/norio-nomura/GTMNSString_HTML

…ating the clangModule target

If clang module has umbrella header:
- Add headers to `HeadersBuildPhase`
- Set CLANG_ENABLE_MODULES = YES
- Set DEFINES_MODULE = YES
- Don’t generate `module.modulemap`
`Xcode.BuildFile.Settings` and` Xcode.BuildSettingsTable.BuildSettings` conform to that.
@norio-nomura norio-nomura changed the title [Xcodeproj] Avoid generating module.modulemap if umbrella header exists, on creating the clangModule target [Xcodeproj] Avoid generating module.modulemap if umbrella header exists, on creating Clang Module targets Feb 13, 2017
@aciidgh aciidgh requested a review from ddunbar February 13, 2017 17:12
@ddunbar
Copy link
Contributor

ddunbar commented Feb 14, 2017

Thanks! I will take a look, but it might take me a couple days I want to think about the possible fallout...

@abertelrud
Copy link
Contributor

The technical changes to the project model representation and serialization look fine. It might be good to add a unit test specifically for the new settings property on BuildFile (e.g. that it gets serialized properly). IIRC, there are unit tests on the serialization of the other model properties, to make sure they make it into the plist in proper form.

Regarding the semantics and what the potential fallout might be, I'll defer to others who are more knowledgeable about the nuances of module maps.

@norio-nomura
Copy link
Contributor Author

Thanks for feedbacks.
I added serialization test for settings property on Xcode.BuildFile.

@abertelrud
Copy link
Contributor

I have no objections to the actual diffs, but would want to hear the opinion of someone more familiar with modules about likely fallout.

@norio-nomura
Copy link
Contributor Author

With this fix, the setting of the target generated by the PM will be more like the setting of the framework target generated by Xcode's template.
So, even if there is fallout, I expect that is about the same level as that generated by Xcode's template.

@yonaskolb
Copy link
Contributor

Are there any updates on this?

@norio-nomura
Copy link
Contributor Author

Close this since I opened #1406 against master.

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.

5 participants