Remove layering_check feature on darwin#17763
Remove layering_check feature on darwin#17763keith wants to merge 1 commit intobazelbuild:masterfrom
Conversation
|
Maybe we want to go a different path here, like gating this behind an env var or something? It feels a bit weird to just remove this but I imagine since it hasn't ever been in the default Xcode toolchain there aren't many macOS users of this today. This never hit CI because previously CI only tested the Xcode toolchain until it was removed |
|
Instead of listing each header in the module_map can we use umbrella directory? Something like: And add |
|
@googlewalt could you share the script you use for that 🙃 |
|
I don't have a script. We just have a static copy of a system module map that uses umbrella directory(s) to cover paths to all the system headers. But it shouldn't be too hard to modify generate_system_module_map.sh to generate something like that. |
Using this feature isn't supported by the Xcode based toolchain on macOS, and generating a modulemap for the entire macOS SDK takes a huge amount of time. This disables this feature on macOS until there's a better solution to those performance issues. Fixes bazelbuild#16619 (comment)
fc17c60 to
635d33a
Compare
|
hoping this works instead #17857 |
| treat_warnings_as_errors_feature, | ||
| archive_param_file_feature, | ||
| ] + layering_check_features(ctx.attr.compiler) | ||
| ] |
There was a problem hiding this comment.
It looks like this code defines the layering_check feature but does not enable it. Do you know where the feature is actually enabled? One option is to find where the feature is enabled and disabled it there. However, I see that this code does enable the module_maps feature which does have overhead and is not useful without layering check, so perhaps this is ok.
There was a problem hiding this comment.
I don't see off hand where it gets enabled 🤔 but i guess the question is do we want folks to be able to enable it at all right now on macOS even if it was opt in? I haven't ever tested this externally on any platform so I'm not sure how well it works as is
There was a problem hiding this comment.
Ideally it would be nice to leave the feature in that state: disabled but functional. We do have it enabled internally by default, but we had to individually opt out lots of rules and packages.
It is unsettling that we don't know how this gets enabled, but I don't have time to hunt that down right now. I'll approve it but make a note that we might want to revisit this in the future.
Using this feature isn't supported by the Xcode based toolchain on macOS, and generating a modulemap for the entire macOS SDK takes a huge amount of time. This disables this feature on macOS until there's a better solution to those performance issues. Fixes bazelbuild#16619 (comment) Closes bazelbuild#17763. PiperOrigin-RevId: 532080490 Change-Id: I96b77eff884fa965ed20084b90b1e0af5b80b082
Using this feature isn't supported by the Xcode based toolchain on macOS, and generating a modulemap for the entire macOS SDK takes a huge amount of time. This disables this feature on macOS until there's a better solution to those performance issues.
Fixes #16619 (comment)