bazelbuild
[repro] module maps created for or on objc_library do not propagate to transitive objc_library
this is a repro of an issue i've identified while trying to move to mixed_language_library from our custom macro. Regardless of how module_map is hinted in swift_interop_info, generated, or otherwise attached to an objc_library, out-of-the-box these module maps are not propagated to downstream objc_library in a objc -> swift -> objc arrangement. this is especially problematic in scenarios where PrintAsClang, going by its own uncontrollable heuristics (in this case, the addition of a lightweight generic in a public-facing objc type), inserts an @import line for the upstream objc library in a generated header.
this is something that needs to change in either swift_clang_module_aspect or objc_library to discover or otherwise propagate the upstream module maps, as this simple example does not work without customization of the rules or introducing manual dependencies on modulemaps (which isn't possible if relying on the generated modulemaps from the aspect).
My understanding is that this is "working as intended" from the point of view of Bazel core -- module maps are only used for importing clang modules into Swift. Even if you had objc_library(a) depending on objc_library(b) (with a module map provided), modular imports wouldn't work: https://github.com/bazelbuild/bazel/issues/9429
I'm trying to use this intermediate rule (which pulls out the aspect-generated module map and kind of misuses CcInfo's headers and includes) as a workaround, though it hasn't been battle-tested yet: https://gist.github.com/jschear/721fda69fc64efb5820855def1615bc0
rules_swift_package_manager has a similar approach, generating its own modulemaps and propagating them through a CcInfo.
- https://github.com/cgrindel/rules_swift_package_manager/commit/d3f47657e54e50ce20b0785274f1515036440eaa#diff-d7638fbe2320b6f14b4515f89f0285502db480136fd78ce11c4ca2e65e231e60R56-R59
- https://github.com/cgrindel/rules_swift_package_manager/issues/208
I would support fixing this in objc_library, but I'm not sure if that would be accepted. There was an attempt long ago that stalled, pre-starlarkification. https://github.com/bazelbuild/bazel/pull/6243
So, within Google we've "fixed" this by having a tool that runs post-compile and rewrites the @import lines in the generated header with path-based #imports. You can see vestiges of that in the rules (search for generated_header_rewriter).
Unfortunately since this tool depends on LLVM/Clang's source tree to load the modules and collect the headers within them, and because it uses some internal libraries on top of that, we can't easily open-source it.
There's also a semi-recent Swift compiler flag, -emit-clang-header-nonmodular-includes, which is supposed to emit path-based #includes in generated headers instead of @import. The last time I tried it, it didn't work out of the box. It wasn't clear to me how it derived the paths for each header, given the confluence of many possible different header search paths being available. What you would want for something Bazel-based is to ensure that all the paths are workspace-relative. That's a potential path to explore, but it would probably need compiler changes to be usable.
@jschear curious what you think of the objc_library.bzl in my latest commit here. I haven't tested it beyond this codebase, but I think it's at least interesting. Heavily derived from your research, so thank you!
(note: doesn't support Bazel 7, and it appears to have some issues under Bazel 9 😭)
Amazing! I saw Keith's post on rule extensions and was thinking that might be applicable here, neat. Definitely nice to not pollute the build graph with unnecessary targets.
I was hoping it would be possible to re-use the action generated by the swift_clang_module_aspect instead of duplicating that code. But I haven't been able to get that to work -- I guess the aspect runs after the sub-rule implementation function. We could at least factor out some of that code to share it between the aspect & this sub-rule?
I can definitely play around with that to see if I can get the solution slightly cleaner. The implementation of the action in the aspect very clearly covers more edge cases that make it more robust than what I've copied here.
That said, for as common of a paper cut as this is, I have no idea where I'd actually want to land this code if we were ready to commit to it. 😂 Probably worth a discussion with the other maintainers once I get to that point.
Okay I've fixed for last_green, and for 7.x you need to set --experimental_rule_extension_api. Refactoring to better leverage the aspect is tricky because I don't think we want that code to deviate too far from upstream, and the parts I need (_generate_module_map) are private functions that take an aspect_ctx.
and for 7.x you need to set --experimental_rule_extension_api
Maybe easier to land after we drop Bazel 7 support?
I guess rules_swift_package_manager also reimplements module map generation, it isn't the end of the world to duplicate it again 😄
I guess rules_swift_package_manager also reimplements module map generation, it isn't the end of the world to duplicate it again 😄
And we did internally as well. We should make it public API by now...