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
Generate Clang module maps during the build #6983
Conversation
@swift-ci please smoke test |
@swift-ci please test |
CMake build needs adjusting. |
d063c9a
to
8126a3f
Compare
@swift-ci please test |
@swift-ci please test windows |
1 similar comment
@swift-ci please test windows |
This looks relevant |
Ah I think I understand the problem: we previously relied on module maps to always be present to allow compilation of dependent targets, but now we actually need to add a dependency on the module map generation in the dependent targets as well. This is somewhat more involved since the dependent targets do not (and should not) know whether their dependencies have generated module maps, so we need a way to depend on the generation without depending on a concrete file. I believe Xcode's build system has a "modules ready" gate task for that purpose, but we don't have any such concept so far in SwiftPM's build system. |
8126a3f
to
f5d2c54
Compare
@swift-ci please test package compatibility |
Looks like infra failure? |
@swift-ci please test package compatibility |
Looks like this failed similar to the reverted changes from #6988, but this should be using main? |
@swift-ci please test package compatibility |
1 similar comment
@swift-ci please test package compatibility |
f5d2c54
to
d210502
Compare
@swift-ci please test package compatibility |
@swift-ci please test |
Ah yes, need to update a few tests because the raw LLBuild manifest has changed. |
de5a4d3
to
70715ef
Compare
@swift-ci please test |
@swift-ci please test windows |
@swift-ci please test package compatibility |
|
70715ef
to
bd19ab7
Compare
@swift-ci please test |
@swift-ci please test windows |
@swift-ci please test package compatibility |
Looks like there's still a race here (most likely due to an unexpressed dependency on the generated module map):
|
We're missing a dependency from Swift targets to the |
This moves generation of module maps for Clang targets into the build process.
bd19ab7
to
75fc214
Compare
@swift-ci please test |
@swift-ci please test windows |
@swift-ci please test package compatibility |
Also cc @m-i-r-z-a since this touches the build system. |
@swift-ci please test package compatibility |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not familiar with the details but the change conceptually makes sense and i assume the logic is the same and just moved into the build graph execution. do we feel test coverage is enough?
It's quite difficult for tests to discover issues with this since the more tricky mistakes will just result in a race which the typical small test case will almost always win. Even with compatibility test suite plus toolchain build, the missing dependency in Swift tasks only showed up once in several builds. So I wouldn't say it's enough, but I also don't have any concrete ideas on what to add. |
One failure that looks very much unrelated, it's a compiler crash:
|
Reverts #6983 We're seeing errors like ``` fatal error: module map file '/Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swiftdriver-macosx-x86_64/x86_64-apple-macosx/release/llbuildBasic.build/module.modulemap' not found ``` on various CI jobs and should revert this to unblock.
This moves generation of module maps for Clang targets into the build process.