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 Apr 20, 2024
1 parent a7b3885 commit 296db4f
Show file tree
Hide file tree
Showing 6 changed files with 486 additions and 52 deletions.
98 changes: 75 additions & 23 deletions rules/framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ load("//rules:providers.bzl", "AvoidDepsInfo", "FrameworkInfo")
load("//rules:transition_support.bzl", "transition_support")
load("//rules:utils.bzl", "is_bazel_7")
load("//rules/internal:objc_provider_utils.bzl", "objc_provider_utils")
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 All @@ -31,6 +33,7 @@ load(
"apple_resource_aspect",
)
load("//rules:force_load_direct_deps.bzl", "force_load_direct_deps")
load("//rules:header_paths.bzl", "header_paths")

_APPLE_FRAMEWORK_PACKAGING_KWARGS = [
"visibility",
Expand All @@ -43,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 @@ -72,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 @@ -96,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 @@ -155,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 @@ -188,35 +232,39 @@ def _find_framework_dir(outputs):
return prefix + ".framework"
return None

def _framework_packaging_symlink_headers(ctx, inputs, outputs):
inputs_by_basename = {input.basename: input for input in inputs}
def _framework_packaging_symlink_headers(ctx, inputs, outputs, headers_mapping):
inputs_by_mapped_name = {header_paths.get_mapped_path(input, headers_mapping): input for input in inputs}

# If this check is true it means that multiple inputs have the same 'basename',
# an additional check is done to see if that was caused by 'action_inputs' containing
# two different paths to the same file
#
# In that case fails with a msg listing the differences found
if len(inputs_by_basename) != len(inputs):
inputs_by_basename_paths = [x.path for x in inputs_by_basename.values()]
inputs_with_duplicated_basename = [x for x in inputs if not x.path in inputs_by_basename_paths]
if len(inputs_by_mapped_name) != len(inputs):
inputs_by_mapped_name_paths = [x.path for x in inputs_by_mapped_name.values()]
inputs_with_duplicated_basename = [x for x in inputs if not x.path in inputs_by_mapped_name_paths]
if len(inputs_with_duplicated_basename) > 0:
# TODO: Fix this error message
fail("""
[Error] Multiple files with the same name exists.\n
See below for the list of paths found for each basename:\n
{}
""".format({x.basename: (x.path, inputs_by_basename[x.basename].path) for x in inputs_with_duplicated_basename}))
""".format({
header_paths.get_mapped_path(x, headers_mapping):
(x.path, inputs_by_mapped_name[header_paths.get_mapped_path(x, headers_mapping)].path)
for x in inputs_with_duplicated_basename
}))

# If no error occurs create symlinks for each output with
# each input as 'target_file'
output_input_dict = {output: inputs_by_basename[output.basename] for output in outputs}
for (output, input) in output_input_dict.items():
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 All @@ -240,7 +288,7 @@ def _framework_packaging_multi(ctx, action, inputs, outputs, manifest = None):
args.add_all("--outputs", outputs)

if action in ["header", "private_header"]:
_framework_packaging_symlink_headers(ctx, inputs, outputs)
_framework_packaging_symlink_headers(ctx, inputs, outputs, headers_mapping)
else:
ctx.actions.run(
executable = ctx.executable._framework_packaging,
Expand Down Expand Up @@ -311,6 +359,7 @@ def _get_virtual_framework_info(ctx, framework_files, compilation_context_fields
def _get_framework_files(ctx, deps):
framework_name = ctx.attr.framework_name
bundle_extension = ctx.attr.bundle_extension
headers_mapping = header_paths.stringify_mapping(ctx.attr.headers_mapping)

# declare framework directory
framework_dir = "%s/%s.%s" % (ctx.attr.name, framework_name, bundle_extension)
Expand Down Expand Up @@ -383,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 @@ -393,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 @@ -440,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 Expand Up @@ -1299,6 +1350,7 @@ The C++ toolchain from which linking flags and other tools needed by the Swift
toolchain (such as `clang`) will be retrieved.
""",
),
"headers_mapping": attr.label_keyed_string_dict(allow_files=True),
},
doc = "Packages compiled code into an Apple .framework package",
)
127 changes: 127 additions & 0 deletions rules/header_paths.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
load("@bazel_skylib//lib:paths.bzl", "paths")

def _stringify_mapping(headers_mapping):
"""
Convert a mapping of {Target: string} to a mapping of {string:string}
- Target is a target pointing to a single source file
- the value in `headers_mapping` is the path within Headers/PrivateHeaders to use
for the file.
"""
ret = {}
for (t, dest) in headers_mapping.items():
files = t.files.to_list()
if len(files) != 1:
fail("{} should be a single file".format(t))
f = files[0]
if not f.is_source:
fail("{} should be a single source file, not a generated file".format(f))
ret[f.owner.name] = dest
return ret

def _get_mapped_path(hdr, headers_mapping):
"""
Get the relative destination path for a File
This will return the value of mapping, or the basename of the file if no
mapping exists or if `hdr` is not a source file.
"""
if hdr.is_source:
return headers_mapping.get(hdr.owner.name, hdr.basename)
else:
return hdr.basename

def _get_string_mapped_path(hdr, headers_mapping):
return headers_mapping.get(hdr, paths.basename(hdr))

def _mapped_without_prefix(files, prefix):
"""
Convert a list of source files to a mapping of those files -> paths with
`prefix` removed
"""
ret = {}
for file in files:
if file.startswith(prefix):
ret[file] = file[len(prefix):]
else:
fail("File `{}` does not start with prefix `{}`".format(file, prefix))
return ret

def _glob_and_strip_prefix(src_dirs, suffix = ".h"):
"""
Takes a list of directories, and converts them to a mapping of src -> dest
For each src_dir, all files ending in `suffix`, recursively, are remapped.
The remapping is from the original path to the last component of `src_dir` +
the remainder of that file's path. The most topologically deep path gets
precedence in the returned dictionary.
For example, for ReactCommon,
`glob_and_strip_prefix(["react", "react/renderer/imagemanager/platform/ios/react"])`
might return something like:
{
...
# Matched from src_dir = "react", glob "react/**/*.h"
"react/debug/flags.h": "react/debug/flags.h",
...
# Matched from src_dir = "react/renderer/imagemanager/platform/ios/react"
# glob "react/renderer/imagemanager/platform/ios/react/**/*.h"
"react/renderer/imagemanager/platform/ios/react/renderer/imagemanager/RCTImageManager.h": "react/renderer/imagemanager/RCTImageManager.h",
...
}
This is mostly useful for merging several directories of globs into a single
mapping for use by `cc_headers_symlinks` in `@rules_ios//:apple_static_library.bzl`
"""
ret = {}
for src_dir in sorted(src_dirs):
top_level = paths.basename(src_dir)
prefix = paths.dirname(src_dir)
to_strip = len(prefix) + 1 if prefix else 0
for file in native.glob(["{}/**/*{}".format(src_dir, suffix)]):
ret[file] = file[to_strip:]
return ret

def _strip_prefix(prefix, attrs=("srcs", "private_headers", "public_headers")):
"""
When remapping headers, strip the given prefix from the destination for all headers
in the specified attrs.
Only supported as the headers_mapping attribute in apple_framework
"""
return struct(pattern = prefix, attrs = attrs, op = "strip")

def _add_prefix(prefix, attrs=("srcs", "private_headers", "public_headers")):
"""
When remapping headers, add the given prefix to the destination for all headers
in the specified attrs.
Only supported as the headers_mapping attribute in apple_framework
"""
return struct(pattern = prefix, attrs = attrs, op = "add")

def _identity_mapping(attrs=("srcs", "private_headers", "public_headers")):
"""
Create a mapping that ensures that paths are not changed
Normally apple_framework moves all headers to a Headers/PrivateHeaders directory,
and often we just to make sure the files stay put in those directories with the
full on-filesystem layout.
This replaces doing something like `{h: h for h in HEADERS}`
"""
return struct(pattern = "", attrs = attrs, op = "strip")


header_paths = struct(
stringify_mapping = _stringify_mapping,
get_mapped_path = _get_mapped_path,
get_string_mapped_path = _get_string_mapped_path,
mapped_without_prefix = _mapped_without_prefix,
glob_and_strip_prefix = _glob_and_strip_prefix,
add_prefix = _add_prefix,
strip_prefix = _strip_prefix,
identity_mapping = _identity_mapping,
)
Loading

0 comments on commit 296db4f

Please sign in to comment.