Skip to content

Commit

Permalink
[rules_ios] Add static library variant to allow rewriting headers (#1)
Browse files Browse the repository at this point in the history
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" -> <locale> -> <clocale> -> <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](bazelbuild/rules_apple@306e300)
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
bazelbuild/rules_apple@e9159fa
  • Loading branch information
nataliejameson committed Aug 13, 2024
1 parent 4e09120 commit 1343d34
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 17 deletions.
72 changes: 58 additions & 14 deletions rules/framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -45,13 +46,38 @@ _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,
infoplists = [],
infoplists_by_build_setting = {},
xcconfig = {},
xcconfig_by_build_setting = {},
headers_mapping = {},
**kwargs):
"""Builds and packages an Apple framework.
Expand All @@ -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}
Expand All @@ -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
)

Expand Down Expand Up @@ -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
)

Expand Down Expand Up @@ -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]:
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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,
Expand Down
26 changes: 23 additions & 3 deletions rules/library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -620,13 +623,15 @@ 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:
linkopts += ["-fapplication-extension"]
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}
Expand Down Expand Up @@ -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.
Expand All @@ -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

Expand All @@ -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:
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down

0 comments on commit 1343d34

Please sign in to comment.