Update to bazel5 #385
Update to bazel5 #385
Conversation
| @@ -971,7 +971,8 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names | |||
| "@build_bazel_rules_ios//:virtualize_frameworks": framework_vfs_objc_copts, | |||
| "//conditions:default": framework_vfs_objc_copts if enable_framework_vfs else [], | |||
| }) | |||
|
|
|||
| if module_map: | |||
There was a problem hiding this comment.
We solve a problem with modulemaps by appending the module_map to the objc headers list. An alternative approach is here: 7f4fe87
However this approach may be preferable because bazel sees that modulemap is a dependency of the objc-library target.
There was a problem hiding this comment.
For this change, we return module_map target from the apple_library macro, since the objc-provider in bazel5 no longer supports it. See bazelbuild/bazel@31bec27#diff-eb9c179576423931e9db781a5acff548a82a4361a23ce1c1dc0c6b73b30d114bL35
amberdixon
force-pushed
the
amber/bazel5-changes
branch
from
69a1ba8 to
a4c3e01
Compare
amberdixon
force-pushed
the
amber/bazel5-changes
branch
from
7876a89 to
bd11d35
Compare
| @@ -89,7 +89,7 @@ cc_toolchain_suite( | |||
| compiler = "compiler", | |||
| cpu = arch, | |||
| cxx_builtin_include_directories = [ | |||
| "/Applications/", | |||
| "/", | |||
There was a problem hiding this comment.
This is a temporary change to workaround a problem here (bazelbuild/bazel#14346) with inclusion validation logic. We won't need this bazel5 toolchain stuff once we update to the next bazel5.0 release candidate. We need the following change (bazelbuild/bazel@005361c) which the bazel release team accidentally omitted from the rc3 cut. see bazelbuild/bazel#14440 (comment) as well.
The current bazel5 release candidate is rc3, which can be determined by checking the bazel-discuss email list: https://groups.google.com/g/bazel-discuss/
There was a problem hiding this comment.
@amberdixon LGTM and great this is working. We should have a followup to ensure the M1 CPU is working natively: removing the hack that we needed to make it work under Bazel 4.
| @@ -1 +1 @@ | |||
| 4.1.0 | |||
| 5.0.0rc3 | |||
There was a problem hiding this comment.
Great that this is all working now!
rules/framework.bzl
Outdated
| @@ -279,6 +279,8 @@ def _get_framework_files(ctx, deps): | |||
| header_in.append(hdr) | |||
| destination = paths.join(framework_dir, "Headers", hdr.basename) | |||
| header_out.append(destination) | |||
| elif hdr.path.endswith(".modulemap") and hdr.owner != dep.label: | |||
There was a problem hiding this comment.
So we're guaranteed that the owner isn't going to be the dep? Would you mind dropping a comment about why this is so? If we make a dep of some modulemap rule is that going to work?
There was a problem hiding this comment.
Thanks for catching this. The check for hdr.owner != dep.label is no longer necessary, so I will remove it.
This logic was introduced by https://github.com/bazel-ios/rules_ios/pull/180/files when traversing the direct_module_maps attribute. It seems that in bazel4, module_maps are sometimes generated by objc_library. In bazel5, we don't expect objc_library to be returning modulemaps in its providers, and the direct_headers won't be containing auto generated modulemaps!
- remove filtering with
hdr.owner != dep.label
amberdixon
force-pushed
the
amber/bazel5-changes
branch
from
6490782 to
4aa0885
Compare
amberdixon
force-pushed
the
amber/minor-bazel5-tweaks
branch
from
2fba56d to
1dc8594
Compare
…r be modulemaps generated by objecitve-c library that appear in the direct_headers
amberdixon
force-pushed
the
amber/bazel5-changes
branch
from
4aa0885 to
841fc01
Compare
|
@amberdixon are we able to merge this and the other other Bazel 5 fix? It looks like a problem with github actions - e.g. the check ran but it didn't ping back with status? https://github.com/bazel-ios/rules_ios/actions/runs/1664400771 |
No description provided.