From 1343d34bfe6503578957256f771072f8b3595ae6 Mon Sep 17 00:00:00 2001 From: Natalie Jameson Date: Tue, 28 Mar 2023 12:21:17 -0700 Subject: [PATCH] [rules_ios] Add static library variant to allow rewriting headers (#1) Add an `apple_static_library` that lets us map source files to arbitrary relative include paths by creating a link tree, and adding that link tree to the deps of a main library. This also uses apple_library instead of apple_framework as the framework is frankly unnecessary for what we're trying to build for third party deps. Test Plan: Ported fmt locally to use this, and ran `clyde ios build` Tasks: https://app.asana.com/0/1203960884932166/1204190072048691 https://app.asana.com/0/1203960884932166/1204190072362093 [transitions] Do not remove cpus from configured target graph (#2) Upstream converts ios_multi_cpus to a single value. rules_pods does not. So when rules_pods objc_library rules depend on rules_ios rules, they are considered different configurations, and we end up splitting the configured target graph into two configurations. When swift, objc, etc try to link in all objc_libraries later in the build, they then try to link the same library twice, because the configured nodes are duplicated, and we run into duplicate symbol, module redefinition, etc errors. In theory, this could be reverted once the rules_ios migration is complete, but for our use case, it should not hurt anything being present. This logic, also, only affects our opt builds that build for multiple cpus. The development builds are unaffected. [rules_ios] Re-enable cpp rules (#3) This was accidentally disabled. It is required for glog, so turn this back on so that .cc/.cpp sources and their .a files are properly propagated. Tested by doing a build with this change off of main [rules_ios] Use -idirafter instead of -I (#4) NOTE: that this may cause issues for libraries that anticipate the `-I.` behavior and do something like `#include "Foo.h"` when there are multiple "Foo.h" in libs in its dependencies. In those cases, `-I.` can be added to its copts explicitly. There are some places where using -I can actually cause problems with system header collisions. Notably, fmt has a "locale.h" file. Without this change, you can end up with a chain that is: "fmt/locale.h" -> -> -> -> failure, because -I causes fmt/locale.h to be searched (I believe due to the entries in the headermap being -I) before system paths. See https://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html for resolution order [rules_ios] Allow rewriting header paths for frameworks and libraries (#5) This allows us to pick a different relative include path for header files. This is needed because some third party dependencies have e.g. colliding base filenames (folly has multiple Core.h, e.g.), or included files do relative includes (kv-storage includes "kv-storage/api/Core.h"), and with the current approach taken by apple_framework of flattening the headers out, that path doesn't actually exist in the Headers or PrivateHeaders directories of the .framework file. This patch does a few things: - Adds an extra argument "header_mapping". This is how users define where public/private headers should be accessible. If not specified, or a header is not in this mapping, then the basename of the header is used. (e.g. kv-storage.framework/Headers/Core.h) - Updates the hmapbuild binary so that we can specify these destinations in the .hmap files - Updates the umbrella header to use these destinations - Updates apple_framework/apple_library to thread these mappings through. Tested on a WIP branch (annie/native-versions-ios), and we seemed to get past the #include failures, and onto errors in their code itself. Will also test this with the flipper-folly migration locally. https://app.asana.com/0/1203960884932166/1204190072362095 https://app.asana.com/0/1203960884932166/1204190072362093 [rules_ios] Make modulemaps have '[system]', fix static_library deps (#6) - There was an ordering issue with apple_static_library where it didn't include the symlink map in the deps for the apple_library call. Make sure to route it through. - When changing over to cocoapods bazel, we modularize a lot of things, which causes a lot of extra warnings to show up from third party packages. We're not really in charge of maintaining their code, so just add the "[system]" tag to the modulemap file. This makes the headers get included as if they were included through -isystem, not -I [rules_ios] Export as system_includes (#7) For system / 3rd party libraries, export the symlink dir as -isystem, not -I. [rules_ios] Add objc_public_headers (#9) Add an `objc_public_headers` attribute to apple_* rules. This changes the umbrella header, if set, such that it includes all public_headers headers when compiled against c++, and only a subset of headers (objc_public_headers) when compiled against objective c. The main problem this is solving is that we have a main module map for each library. When trying to import *anything* from the framework, it loads the entire module map and every include in the umbrella header. If you put c++-only headers in it, it will try to interpret those without c++ support if you try to include the .h from a .m file. This allows us to have headers that mix (obj)c++ and objc in a single framework. It should also allow us to drop some of the -objc rules we've had to clunkily build out. [rules_ios] Expose cc_headers_symlinks, add a helper function for headers (#10) Expose cc_headers_symlinks so that we can use it in a refactor of React-Core and ReactCommon. Also expose a helper function for use by ReactCommon [rules_ios] Add more entitlement errors (#12) If there are keys that are in the .entitlements file that are missing from the provisioning profile, this normally results in an error. However, this is a manually specified list. These storekit ones bit us recently, so add the storekit ones, and then just pull in the list from main (https://github.com/bazelbuild/rules_apple/blob/cc1e5b17d0bf16a74c74f9fb3e2940d9c97c12da/tools/plisttool/plisttool.py#L295-L314) [rules_ios] Add more configurability to headers_mapping (#11) When this just takes a dictionary, it requires us to do a lot more boilerplate work to first construct that dictionary. Add a couple of helpers for the common use cases that are in repo. [rules_apple] add upstream filesystem patches (#13) this adds [this commit](https://github.com/bazelbuild/rules_apple/commit/306e3003c3ad562bda6cd8217179e2fcc46906f6) to address this error when copying results files across filesystem boundaries. [rules_apple] Add support for xcprivacy manifests in extensions (#14) Bringing a change in from upstream of rules_apple to support adding privacy manifests for our extensions https://github.com/bazelbuild/rules_apple/pull/2439/commits/e9159fa5e4e8c21e09e667a85e67c16025211a7a --- rules/framework.bzl | 72 ++++++++++++++++++++++++++++++++++++--------- rules/library.bzl | 26 ++++++++++++++-- 2 files changed, 81 insertions(+), 17 deletions(-) diff --git a/rules/framework.bzl b/rules/framework.bzl index b702b5c..80bbb3f 100644 --- a/rules/framework.bzl +++ b/rules/framework.bzl @@ -12,6 +12,7 @@ load("@bazel_skylib//lib:collections.bzl", "collections") load("@bazel_skylib//lib:partial.bzl", "partial") load("@bazel_skylib//lib:paths.bzl", "paths") load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain", "use_cpp_toolchain") +load("@bazel_skylib//lib:types.bzl", "types") load("@build_bazel_rules_apple//apple/internal:apple_product_type.bzl", "apple_product_type") load("@build_bazel_rules_apple//apple/internal:features_support.bzl", "features_support") load("@build_bazel_rules_apple//apple/internal:linking_support.bzl", "linking_support") @@ -45,6 +46,30 @@ _APPLE_FRAMEWORK_PACKAGING_KWARGS = [ "exported_symbols_lists", ] +_HEADER_EXTS = { + "srcs": (".h", ".hh", ".hpp"), + "public_headers": (".inc", ".h", ".hh", ".hpp"), + "private_headers": (".inc", ".h", ".hh", ".hpp"), +} + +def _generate_headers_mapping(headers_mapping, kwargs): + if types.is_dict(headers_mapping): + return headers_mapping + + to_map = [] + for attr in headers_mapping.attrs: + exts = _HEADER_EXTS[attr] + to_map += [h for h in kwargs.get(attr, []) if h.endswith(exts)] + + headers = collections.uniq(to_map) + if headers_mapping.op == "strip": + return header_paths.mapped_without_prefix(headers, headers_mapping.pattern) + elif headers_mapping.op == "add": + return {h: headers_mapping.pattern + h for h in headers} + else: + fail("Invalid headers_mapping `{}`".format(headers_mapping)) + + def apple_framework( name, apple_library = apple_library, @@ -52,6 +77,7 @@ def apple_framework( infoplists_by_build_setting = {}, xcconfig = {}, xcconfig_by_build_setting = {}, + headers_mapping = {}, **kwargs): """Builds and packages an Apple framework. @@ -74,6 +100,18 @@ def apple_framework( If '//conditions:default' is not set the value in 'xcconfig' is set as default. + headers_mapping: Either a dictionary, or the value from add_prefix / strip_prefix. + + If a dictionary, a mapping of {str: str}, where the key is a + path to a header, and the value where that header should be + placed in Headers, PrivateHeaders, umbrella headers, hmaps, etc. + + If the result of header_paths.add_prefix, then the attributes + specified will have the prefix appended to the beginning. + + If the result of header_paths.strip_prefix, then the + attributes specified will have the prefix reoved from the + beginning. **kwargs: Arguments passed to the apple_library and apple_framework_packaging rules as appropriate. """ framework_packaging_kwargs = {arg: kwargs.pop(arg) for arg in _APPLE_FRAMEWORK_PACKAGING_KWARGS if arg in kwargs} @@ -98,11 +136,14 @@ def apple_framework( testonly = kwargs.pop("testonly", False) + if headers_mapping: + headers_mapping = _generate_headers_mapping(headers_mapping, kwargs) library = apple_library( name = name, testonly = testonly, xcconfig = xcconfig, xcconfig_by_build_setting = xcconfig_by_build_setting, + headers_mapping = headers_mapping, **kwargs ) @@ -157,6 +198,7 @@ def apple_framework( testonly = testonly, minimum_os_version = minimum_os_version, platform_type = platform_type, + headers_mapping = headers_mapping, **framework_packaging_kwargs ) @@ -218,11 +260,11 @@ def _framework_packaging_symlink_headers(ctx, inputs, outputs, headers_mapping): for (input, output) in zip(inputs, outputs): ctx.actions.symlink(output = output, target_file = input) -def _framework_packaging_single(ctx, action, inputs, output, manifest = None): - outputs = _framework_packaging_multi(ctx, action, inputs, [output], manifest = manifest) +def _framework_packaging_single(ctx, action, inputs, output, manifest = None, headers_mapping = {}): + outputs = _framework_packaging_multi(ctx, action, inputs, [output], manifest = manifest, headers_mapping = headers_mapping) return outputs[0] if outputs else None -def _framework_packaging_multi(ctx, action, inputs, outputs, manifest = None): +def _framework_packaging_multi(ctx, action, inputs, outputs, manifest = None, headers_mapping = {}): if not inputs: return [] if inputs == [None]: @@ -390,7 +432,8 @@ def _get_framework_files(ctx, deps): if PrivateHeadersInfo in dep: for hdr in dep[PrivateHeadersInfo].headers.to_list(): private_headers_in.append(hdr) - destination = paths.join(framework_dir, "PrivateHeaders", hdr.basename) + mapped_path = header_paths.get_mapped_path(hdr, headers_mapping) + destination = paths.join(framework_dir, "PrivateHeaders", mapped_path) private_headers_out.append(destination) has_header = False @@ -400,7 +443,8 @@ def _get_framework_files(ctx, deps): if not hdr.is_directory and hdr.path.endswith((".h", ".hh", ".hpp")): has_header = True headers_in.append(hdr) - destination = paths.join(framework_dir, "Headers", hdr.basename) + mapped_path = header_paths.get_mapped_path(hdr, headers_mapping) + destination = paths.join(framework_dir, "Headers", mapped_path) headers_out.append(destination) elif hdr.path.endswith(".modulemap"): modulemap_in = hdr @@ -447,19 +491,19 @@ def _get_framework_files(ctx, deps): # so inputs that do not depend on compilation # are available before those that do, # improving parallelism - binary_out = _framework_packaging_single(ctx, "binary", binaries_in, binary_out, framework_manifest) - headers_out = _framework_packaging_multi(ctx, "header", headers_in, headers_out, framework_manifest) - private_headers_out = _framework_packaging_multi(ctx, "private_header", private_headers_in, private_headers_out, framework_manifest) + binary_out = _framework_packaging_single(ctx, "binary", binaries_in, binary_out, framework_manifest, headers_mapping) + headers_out = _framework_packaging_multi(ctx, "header", headers_in, headers_out, framework_manifest, headers_mapping) + private_headers_out = _framework_packaging_multi(ctx, "private_header", private_headers_in, private_headers_out, framework_manifest, headers_mapping) # Instead of creating a symlink of the modulemap, we need to copy it to modulemap_out. # It's a hacky fix to guarantee running the clean action before compiling objc files depending on this framework in non-sandboxed mode. # Otherwise, stale header files under framework_root will cause compilation failure in non-sandboxed mode. - modulemap_out = _framework_packaging_single(ctx, "modulemap", [modulemap_in], modulemap_out, framework_manifest) - swiftmodule_out = _framework_packaging_single(ctx, "swiftmodule", [swiftmodule_in], swiftmodule_out, framework_manifest) - swiftinterface_out = _framework_packaging_single(ctx, "swiftinterface", [swiftinterface_in], swiftinterface_out, framework_manifest) - swiftdoc_out = _framework_packaging_single(ctx, "swiftdoc", [swiftdoc_in], swiftdoc_out, framework_manifest) - infoplist_out = _framework_packaging_single(ctx, "infoplist", [infoplist_in], infoplist_out, framework_manifest) - symbol_graph_out = _framework_packaging_single(ctx, "symbol_graph", [symbol_graph_in], symbol_graph_out, framework_manifest) + modulemap_out = _framework_packaging_single(ctx, "modulemap", [modulemap_in], modulemap_out, framework_manifest, headers_mapping) + swiftmodule_out = _framework_packaging_single(ctx, "swiftmodule", [swiftmodule_in], swiftmodule_out, framework_manifest, headers_mapping) + swiftinterface_out = _framework_packaging_single(ctx, "swiftinterface", [swiftinterface_in], swiftinterface_out, framework_manifest, headers_mapping) + swiftdoc_out = _framework_packaging_single(ctx, "swiftdoc", [swiftdoc_in], swiftdoc_out, framework_manifest, headers_mapping) + infoplist_out = _framework_packaging_single(ctx, "infoplist", [infoplist_in], infoplist_out, framework_manifest, headers_mapping) + symbol_graph_out = _framework_packaging_single(ctx, "symbol_graph", [symbol_graph_in], symbol_graph_out, framework_manifest, headers_mapping) outputs = struct( binary = binary_out, diff --git a/rules/library.bzl b/rules/library.bzl index 17dd1b1..fabac7b 100644 --- a/rules/library.bzl +++ b/rules/library.bzl @@ -15,6 +15,7 @@ load("//rules/library:resources.bzl", "wrap_resources_in_filegroup") load("//rules/library:xcconfig.bzl", "copts_by_build_setting_with_defaults") load("//rules:import_middleman.bzl", "import_middleman") load("//rules:utils.bzl", "bundle_identifier_for_bundle") +load("//rules:header_paths.bzl", "header_paths") PrivateHeadersInfo = provider( doc = "Propagates private headers, so they can be accessed if necessary", @@ -490,6 +491,8 @@ def apple_library( xcconfig_by_build_setting = {}, objc_defines = [], swift_defines = [], + headers_mapping = {}, + objc_public_headers = None, **kwargs): """Create libraries for native source code on Apple platforms. @@ -620,6 +623,7 @@ def apple_library( testonly = kwargs.pop("testonly", False) features = kwargs.pop("features", []) extension_safe = kwargs.pop("extension_safe", None) + system_module = kwargs.pop("system_module", True) # Set extra linkopt for application extension safety if extension_safe: @@ -627,6 +631,7 @@ def apple_library( objc_copts += ["-fapplication-extension"] swift_copts += ["-application-extension"] + # Collect the swift_library related kwargs, these are typically only set when provided to allow # for wider compatibility with rule_swift versions. swift_kwargs = {arg: kwargs.pop(arg) for arg in _SWIFT_LIBRARY_KWARGS if arg in kwargs} @@ -912,7 +917,12 @@ def apple_library( if len(objc_hdrs) > 0: public_hmap_name = name + "_public_hmap" + public_hdrs = objc_hdrs + public_hdrs_dests = [ + header_paths.get_string_mapped_path(header, headers_mapping) + for header in public_hdrs + ] # Public hmaps are for vendored static libs to export their header only. # Other dependencies' headermaps will be generated by li_ios_framework # rules. @@ -921,20 +931,28 @@ def apple_library( namespace = namespace, hdrs = objc_hdrs, tags = _MANUAL, + hdr_dests = public_hdrs_dests, ) private_dep_names.append(public_hmap_name) - additional_objc_copts, additional_swift_copts, additional_cc_copts = _append_headermap_copts(public_hmap_name, "-I", additional_objc_copts, additional_swift_copts, additional_cc_copts) + additional_objc_copts, additional_swift_copts, additional_cc_copts = _append_headermap_copts(public_hmap_name, "-idirafter", additional_objc_copts, additional_swift_copts, additional_cc_copts) if len(objc_non_exported_hdrs + objc_private_hdrs) > 0: private_hmap_name = name + "_private_hmap" + private_hdrs = objc_non_exported_hdrs + objc_private_hdrs + objc_hdrs + private_hdrs_dests = [ + header_paths.get_string_mapped_path(header, headers_mapping) + for header in private_hdrs + ] + headermap( name = private_hmap_name, hdrs = objc_non_exported_hdrs + objc_private_hdrs, + hdr_dests = private_hdrs_dests, tags = _MANUAL, ) private_dep_names.append(private_hmap_name) - additional_objc_copts, additional_swift_copts, additional_cc_copts = _append_headermap_copts(private_hmap_name, "-I", additional_objc_copts, additional_swift_copts, additional_cc_copts) + additional_objc_copts, additional_swift_copts, additional_cc_copts = _append_headermap_copts(private_hmap_name, "-idirafter", additional_objc_copts, additional_swift_copts, additional_cc_copts) ## END HMAP @@ -955,7 +973,7 @@ def apple_library( additional_swift_copts += ["-swift-version", swift_version] if has_swift_sources: - additional_swift_copts += ["-Xcc", "-I."] + additional_swift_copts.extend(("-Xcc", "-idirafter.")) if module_map: # Frameworks find the modulemap file via the framework vfs overlay if not namespace_is_module_name: @@ -1055,6 +1073,7 @@ def apple_library( name = swift_doublequote_hmap_name, namespace = namespace, hdrs = [], + hdr_dests = [], direct_hdr_providers = [swift_libname], tags = _MANUAL, testonly = testonly, @@ -1068,6 +1087,7 @@ def apple_library( name = swift_angle_bracket_hmap_name, namespace = namespace, hdrs = [], + hdr_dests = [], direct_hdr_providers = [swift_libname], tags = _MANUAL, testonly = testonly,