Clean up Clang module handling #113
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Makes a couple of changes to make Clang modules work better with generated libraries. Namely: - Only include public files in generated modulemap files. Previously it would include every header, which would A) potentially add headers from other modules (eg in React where multiple modules share a source directory) or include headers that aren't supposed to be compileable externally (eg all of Yoga's public headers handle both C and C++, but its internal ones assume C++ and would break when being used from a C translation unit). - Generate module maps for each target, not just the top-level target, since they will (likely) have different sources from the top-level one. - Populate the `module_map` attribute in the `objc_library` rule so Bazel doesn't generate its own module map. - Explicitly specify `-fmodules` if we have modules enabled. I'm not sure why, but I was running into cases where Bazel *wouldn't* pass that flag to Clang even when modules were enabled. - Only generate `gen_module_map` rule if module map generation is on.
FuegoFro
commented
Jan 28, 2020
| return Glob(pattern: pattern, behavior: GlobBehaviorBashV4).paths | ||
| } | ||
|
|
||
| // Expands a set of globs with the semantics of pod `source_file` globs. |
There was a problem hiding this comment.
Note: This is just the functionality that was in Sources/RepoToolsCore/RepoActions.swift, but pulled out to a function for easy reuse.
I had been testing this off an older master, which didn't have the `[system]` change. This: - Suppresses the `[system]` tag if there's nothing in the modulemap, since otherwise it complains with "expected umbrella, header, submodule, or module export". - Changes directory creation to use the FS manager rather than shell, because I was getting weird issues where the `pwd` was returning an empty string.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Makes a couple of changes to make Clang modules work better with generated libraries. Namely:
module_mapattribute in theobjc_libraryrule so Bazel doesn't generate its own module map.-fmodulesif we have modules enabled. I'm not sure why, but I was running into cases where Bazel wouldn't pass that flag to Clang even when modules were enabled.gen_module_maprule if module map generation is on.