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

norio-nomura · 2017-12-01T08:46:20Z

This was rewritten #955 for master.

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.

aciidgh · 2017-12-01T23:52:24Z

This looks fine to me but I am not completely sure if there would be any unexpected fallout from this.

@ddunbar @abertelrud Can you guys review this patch?

ddunbar · 2017-12-02T00:36:11Z

Sources/Xcodeproj/pbxproj().swift

-                isGenerated = true
+                let umbrellaHeaderName = clangTarget.c99name + ".h"
+                if includeGroup.subitems.contains(where: { $0.path == umbrellaHeaderName }) {
+                    // if umbrellaHeader exists, we can use module.


I would say something like:

// If an umbrella header exists, enable Xcode's builtin module's feature rather than generating // a custom module map. This increases the compatibility of generated Xcode projects.

That said...

Why can't we always generate a module.modulemap file, and then set the MODULEMAP_FILE build setting to point at it. That should let Xcode's builtin logic work, but can work universally without needing the else branch here.

ddunbar · 2017-12-02T00:39:11Z

This change seems reasonable to me, but I would prefer if we could always generate a file for MODULEMAP_FILE (I may have gotten the name wrong for this setting) so this works for all frameworks, not just ones with an umbrella header.

ddunbar · 2017-12-02T00:39:45Z

My main concern here is I don't want to land a change which improves compatibility, but only for some targets, and in a way which wouldn't be discoverable to a user.

aciidgh · 2017-12-04T18:17:58Z

Sources/Xcodeproj/pbxproj().swift

+                if includeGroup.subitems.contains(where: { $0.path == umbrellaHeaderName }) {
+                    // If an umbrella header exists, enable Xcode's builtin module's feature rather than generating
+                    // a custom module map. This increases the compatibility of generated Xcode projects.
+                    targetSettings.common.CLANG_ENABLE_MODULES = "YES"


Is this different from DEFINES_MODULE?

Xcode's Quick Help said:

CLANG_ENABLE_MODULES
--
Boolean
Enables the use of modules for system APIs. System headers are imported as semantic modules instead of raw headers. This can result in faster builds and project indexing.

I am setting this by mimicking the framework target settings generated by Xcode.
I do not know if it is essential to set this to YES, but I have never had any troubles with doing this.

I do not know if it is essential to set this to YES,

I confirmed that this setting is required by testing on https://github.com/jpsim/Yams

Ah cool. Can you also implement the MODULEMAP_FILE setting that @ddunbar suggested?

…_SWIFT_FLAGS`

norio-nomura · 2017-12-06T03:00:56Z

Updated to:

Set modulemap to MODULEMAP_FILE if exists or generated
Stop using -fmodule-map-file in OTHER_SWIFT_FLAGS
Set CLANG_ENABLE_MODULES=YES and DEFINES_MODULE=YES if user provided modulemap does not exist

norio-nomura · 2017-12-06T03:03:48Z

Sources/Xcodeproj/pbxproj().swift

                }
+                targetSettings.common.CLANG_ENABLE_MODULES = "YES"
+                targetSettings.common.DEFINES_MODULE = "YES"


It does not set DEFINES_MODULE=YES if user defined modulemap exists, since I don't know about the issue mentioned in following comment:

// Disable defines target for clang target because our clang targets are not proper framework targets. // Also see: <rdar://problem/29825757> targetSettings.common.DEFINES_MODULE = "NO"

rehatkathuria · 2017-12-20T16:25:13Z

Thank you so much for taking the initiative to open up this pull request, @norio-nomura!

Curious to know your thoughts on the request as it sits, @ddunbar.

ddunbar · 2018-01-04T18:38:19Z

Sorry for the very long delay, I am testing and experimenting with this today...

olbrichj · 2018-02-05T22:25:15Z

Any updates on this?

aciidgh · 2018-06-12T21:17:20Z

Sorry that this PR has been sitting for so long on me. I created a new PR based on @norio-nomura work. Thank you for working on this!

norio-nomura added 7 commits December 1, 2017 15:46

Add CLANG_ENABLE_MODULES to Xcode.BuildSettingsTable.BuildSettings

7f4ea0a

Add Xcode.BuildFile.Settings

711b77e

Avoid generating module.modulemap if umbrella header exists, on cre…

87b36fc

…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`

Add CLANG_ENABLE_MODULES = YES to test targets

c349667

Update tests

0942ea1

Add PropertyListDictionaryConvertible

dee56ba

`Xcode.BuildFile.Settings` and` Xcode.BuildSettingsTable.BuildSettings` conform to that.

Add serialization test for settings property on Xcode.BuildFile

d063bff

norio-nomura mentioned this pull request Dec 1, 2017

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

Closed

ddunbar reviewed Dec 2, 2017

View reviewed changes

Update comment to suggested by review

deeb186

aciidgh reviewed Dec 4, 2017

View reviewed changes

Use MODULEMAP_FILE instead of using -fmodule-map-file=… in `OTHER…

cea6e01

…_SWIFT_FLAGS`

norio-nomura commented Dec 6, 2017

View reviewed changes

ikesyo mentioned this pull request Feb 13, 2018

Fix CYaml/module.modulemap using relative path ishkawa/DIKit#41

Merged

aciidgh added the pending evolution proposal label Mar 1, 2018

aciidgh mentioned this pull request Mar 12, 2018

Set CLANG_ENABLE_MODULES to YES when generating projects #1422

Closed

aciidgh mentioned this pull request Jun 12, 2018

[Xcodeproj] Use Xcode's modulemap generation when umbrella header is … #1602

Merged

aciidgh closed this Jun 12, 2018

norio-nomura deleted the use-define-module-if-umbrella-header-exists-take-2 branch June 12, 2018 22:57

This was referenced Dec 1, 2017

[SR-4972] xcodeproj generated by swift package generate-xcodeproj can not be used as a dependency of another xcode project #5013

Closed

[SR-6514] Using libSwiftPM in Xcode Project is blocked by error: missing required module 'clibc'. #4850

Closed

aciidgh mentioned this pull request May 1, 2018

[SR-7531] SwiftPM doesn't set CLANG_ENABLE_MODULES #4821

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

norio-nomura commented Dec 1, 2017

aciidgh commented Dec 1, 2017

ddunbar Dec 2, 2017

ddunbar Dec 2, 2017

ddunbar commented Dec 2, 2017

ddunbar commented Dec 2, 2017

aciidgh Dec 4, 2017

norio-nomura Dec 5, 2017

norio-nomura Dec 5, 2017

aciidgh Dec 5, 2017

norio-nomura Dec 6, 2017

norio-nomura commented Dec 6, 2017

norio-nomura Dec 6, 2017

rehatkathuria commented Dec 20, 2017

ddunbar commented Jan 4, 2018

olbrichj commented Feb 5, 2018

aciidgh commented Jun 12, 2018

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

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

Conversation

norio-nomura commented Dec 1, 2017

aciidgh commented Dec 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ddunbar commented Dec 2, 2017

ddunbar commented Dec 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

norio-nomura commented Dec 6, 2017

Choose a reason for hiding this comment

rehatkathuria commented Dec 20, 2017

ddunbar commented Jan 4, 2018

olbrichj commented Feb 5, 2018

aciidgh commented Jun 12, 2018

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

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