Skip to content

Commit

Permalink
Fix race condition between xcodebuild and symlink action for test XCF…
Browse files Browse the repository at this point in the history
…ramework

This change updates when modulemap files are generated XCFramework
with static libraries. Previously, modulemap were generated after
the XCFramework bundle was created with xcodebuild. Now, it's being
included as part of the headers directory.

Additionally, this change adopts the intermediates module ahead of a
rule migration to test/starlark_tests/.

PiperOrigin-RevId: 465638790
  • Loading branch information
stravinskii authored and swiple-rules-gardener committed Aug 5, 2022
1 parent 774d8da commit e3cd464
Show file tree
Hide file tree
Showing 6 changed files with 136 additions and 87 deletions.
1 change: 1 addition & 0 deletions apple/internal/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,7 @@ bzl_library(
srcs = ["intermediates.bzl"],
visibility = [
"//apple:__subpackages__",
"//test:__subpackages__",
],
deps = [
"@bazel_skylib//lib:paths",
Expand Down
10 changes: 1 addition & 9 deletions test/testdata/fmwk/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -32,15 +32,6 @@ filegroup(
],
)

filegroup(
name = "ios_framework_generators",
srcs = [
"BUILD",
"generate_framework.bzl",
"generation_support.bzl",
],
)

generate_import_framework(
name = "iOSDynamicFramework",
archs = ["x86_64"],
Expand Down Expand Up @@ -107,6 +98,7 @@ bzl_library(
srcs = ["generation_support.bzl"],
visibility = ["//test:__subpackages__"],
deps = [
"//apple/internal:intermediates",
"@bazel_skylib//lib:paths",
"@build_bazel_apple_support//lib:apple_support",
],
Expand Down
7 changes: 5 additions & 2 deletions test/testdata/fmwk/generate_framework.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ def _generate_import_framework_impl(ctx):
apple_fragment = apple_fragment,
archs = architectures,
binary = binary,
label = label,
minimum_os_version = minimum_os_version,
sdk = sdk,
xcode_config = xcode_config,
Expand All @@ -77,6 +78,7 @@ def _generate_import_framework_impl(ctx):
actions = actions,
apple_fragment = apple_fragment,
binary = binary,
label = label,
xcode_config = xcode_config,
)

Expand Down Expand Up @@ -104,8 +106,8 @@ def _generate_import_framework_impl(ctx):
module_interfaces.append(
generation_support.copy_file(
actions = actions,
base_path = "intermediates",
file = swiftinterface,
label = label,
target_filename = "%s.swiftinterface" % architectures[0],
),
)
Expand All @@ -114,9 +116,10 @@ def _generate_import_framework_impl(ctx):
framework_files = generation_support.create_framework(
actions = actions,
bundle_name = label.name,
library = library,
headers = headers,
include_resource_bundle = include_resource_bundle,
label = label,
library = library,
module_interfaces = module_interfaces,
)

Expand Down
119 changes: 96 additions & 23 deletions test/testdata/fmwk/generation_support.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
"""Apple Frameworks and XCFramework generation support methods for testing."""

load("@build_bazel_apple_support//lib:apple_support.bzl", "apple_support")
load(
"@build_bazel_rules_apple//apple/internal:intermediates.bzl", # buildifier: disable=bzl-visibility
"intermediates",
)
load("@bazel_skylib//lib:paths.bzl", "paths")

_SDK_TO_VERSION_ARG = {
Expand Down Expand Up @@ -87,9 +91,16 @@ def _compile_binary(
Returns:
A compiled binary file.
"""
binary_name = "{label}.o".format(label = label.name)
binary_dir = "{sdk}_{archs}".format(sdk = sdk, archs = "_".join(archs))
binary = actions.declare_file(paths.join("intermediates", binary_dir, binary_name))
binary = intermediates.file(
actions = actions,
file_name = "{sdk}_{archs}_{label}.o".format(
archs = "_".join(archs),
label = label.name,
sdk = sdk,
),
output_discriminator = None,
target_name = label.name,
)

inputs = []
inputs.extend(srcs)
Expand Down Expand Up @@ -122,21 +133,32 @@ def _compile_binary(

return binary

def _create_static_library(*, actions, apple_fragment, parent_dir = "", binary, xcode_config):
def _create_static_library(
*,
actions,
apple_fragment,
label,
parent_dir = "",
binary,
xcode_config):
"""Creates an Apple static library using libtool.
Args:
actions: The actions provider from `ctx.actions`.
apple_fragment: An Apple fragment (ctx.fragments.apple).
binary: A binary file to use for the archive file.
label: Label of the target being built.
parent_dir: Optional parent directory name for the generated archive file.
xcode_config: The `apple_common.XcodeVersionConfig` provider from the context.
Returns:
A static library (archive) file.
"""
static_library_name = paths.replace_extension(binary.basename, ".a")
static_library_path = paths.join("intermediates", parent_dir, static_library_name)
static_library = actions.declare_file(static_library_path)
static_library = intermediates.file(
actions = actions,
file_name = paths.join(parent_dir, "{}.a".format(label.name)),
output_discriminator = None,
target_name = label.name,
)

args = ["/usr/bin/xcrun", "libtool", "-static", binary.path, "-o", static_library.path]

Expand All @@ -159,6 +181,7 @@ def _create_dynamic_library(
apple_fragment,
archs,
binary,
label,
minimum_os_version,
sdk,
xcode_config):
Expand All @@ -169,16 +192,23 @@ def _create_dynamic_library(
apple_fragment: An Apple fragment (ctx.fragments.apple).
archs: List of architectures to compile (e.g. ['arm64', 'x86_64']).
binary: A binary file to use for the archive file.
label: Label of the target being built.
minimum_os_version: Dotted version string for minimum OS version supported by the target.
sdk: A string representing an Apple SDK.
xcode_config: The `apple_common.XcodeVersionConfig` provider from the context.
Returns:
A dynamic library file.
"""
dylib_name, _ = binary.basename.split(".")
dylib_dir = "{sdk}_{archs}".format(sdk = sdk, archs = "_".join(archs))
dylib_path = paths.join("intermediates", dylib_dir, dylib_name)
dylib_binary = actions.declare_file(dylib_path)
dylib_binary = intermediates.file(
actions = actions,
file_name = "{sdk}_{archs}_{name}".format(
archs = "_".join(archs),
name = label.name,
sdk = sdk,
),
output_discriminator = None,
target_name = label.name,
)

args = ["/usr/bin/xcrun"]
args.extend(["-sdk", sdk])
Expand All @@ -188,7 +218,7 @@ def _create_dynamic_library(
args.append("-dynamiclib")
args.extend([
"-install_name",
"@rpath/{}.framework/{}".format(dylib_name, dylib_name),
"@rpath/{}.framework/{}".format(label.name, label.name),
])

for arch in archs:
Expand All @@ -215,6 +245,7 @@ def _create_framework(
actions,
base_path = "",
bundle_name,
label,
library,
headers,
include_resource_bundle = False,
Expand All @@ -225,6 +256,7 @@ def _create_framework(
actions: The actions provider from `ctx.actions`.
base_path: Base path for the generated archive file (optional).
bundle_name: Name of the Framework bundle.
label: Label of the target being built.
library: The library for the Framework bundle.
headers: List of header files for the Framework bundle.
include_resource_bundle: Boolean to indicate if a resource bundle should be added to
Expand All @@ -236,13 +268,24 @@ def _create_framework(
framework_files = []
framework_directory = paths.join(base_path, bundle_name + ".framework")

framework_binary = actions.declare_file(paths.join(framework_directory, bundle_name))
framework_binary = intermediates.file(
actions = actions,
file_name = paths.join(framework_directory, bundle_name),
output_discriminator = None,
target_name = label.name,
)

actions.symlink(
output = framework_binary,
target_file = library,
)

framework_plist = actions.declare_file(paths.join(framework_directory, "Info.plist"))
framework_plist = intermediates.file(
actions = actions,
file_name = paths.join(framework_directory, "Info.plist"),
output_discriminator = None,
target_name = label.name,
)
actions.write(
output = framework_plist,
content = _FRAMEWORK_PLIST_TEMPLATE.format(bundle_name),
Expand All @@ -255,8 +298,9 @@ def _create_framework(
framework_files.extend([
_copy_file(
actions = actions,
file = header,
base_path = headers_path,
file = header,
label = label,
)
for header in headers
])
Expand All @@ -265,6 +309,7 @@ def _create_framework(
bundle_name = bundle_name,
headers = headers,
headers_path = headers_path,
label = label,
is_framework_umbrella_header = True,
)
framework_files.append(umbrella_header)
Expand All @@ -275,6 +320,7 @@ def _create_framework(
actions = actions,
bundle_name = bundle_name,
is_framework_module = True,
label = label,
module_map_path = module_map_path,
umbrella_header = umbrella_header,
),
Expand All @@ -285,34 +331,49 @@ def _create_framework(
framework_files.extend([
_copy_file(
actions = actions,
file = interface_file,
base_path = modules_path,
file = interface_file,
label = label,
)
for interface_file in module_interfaces
])

if include_resource_bundle:
resources_path = paths.join(framework_directory, "Resources", bundle_name + ".bundle")
resource_file = actions.declare_file(paths.join(resources_path, "Info.plist"))

resource_file = intermediates.file(
actions = actions,
file_name = paths.join(resources_path, "Info.plist"),
output_discriminator = None,
target_name = label.name,
)
actions.write(output = resource_file, content = "Mock resource bundle")
framework_files.append(resource_file)

return framework_files

def _copy_file(*, actions, base_path, file, target_filename = None):
def _copy_file(*, actions, base_path = "", file, label, target_filename = None):
"""Copies file to a target directory.
Args:
actions: The actions provider from `ctx.actions`.
base_path: Base path for the copied files.
base_path: Base path for the copied files (optional).
file: File to copy.
label: Label of the target being built.
target_filename: (optional) String for target filename. If None, file basename is used.
Returns:
List of copied files.
"""
filename = target_filename if target_filename else file.basename
copied_file_path = paths.join(base_path, filename)
copied_file = actions.declare_file(copied_file_path)

copied_file = intermediates.file(
actions = actions,
file_name = copied_file_path,
output_discriminator = None,
target_name = label.name,
)

actions.symlink(output = copied_file, target_file = file)
return copied_file

Expand All @@ -336,6 +397,7 @@ def _generate_umbrella_header(
bundle_name,
headers,
headers_path,
label,
is_framework_umbrella_header = False):
"""Generates a single umbrella header given a sequence of header files.
Expand All @@ -344,6 +406,7 @@ def _generate_umbrella_header(
bundle_name: Name of the Framework/XCFramework bundle.
headers: List of header files for the Framework bundle.
headers_path: Base path for the generated umbrella header file.
label: Label of the target being built.
is_framework_umbrella_header: Boolean to indicate if the generated umbrella header is for an
Apple framework. Defaults to `False`.
Returns:
Expand All @@ -355,8 +418,11 @@ def _generate_umbrella_header(
for header in headers:
header_text += "#import <{}>\n".format(paths.join(header_prefix, header.basename))

umbrella_header = actions.declare_file(
paths.join(headers_path, bundle_name + ".h"),
umbrella_header = intermediates.file(
actions = actions,
file_name = paths.join(headers_path, bundle_name + ".h"),
output_discriminator = None,
target_name = label.name,
)
actions.write(
output = umbrella_header,
Expand All @@ -370,6 +436,7 @@ def _generate_module_map(
actions,
bundle_name,
headers = None,
label,
is_framework_module = False,
module_map_path,
umbrella_header = None):
Expand All @@ -379,6 +446,7 @@ def _generate_module_map(
actions: The actions provider from `ctx.actions`.
bundle_name: Name of the Framework/XCFramework bundle.
headers: List of header files to use for the generated modulemap file.
label: Label of the target being built.
is_framework_module: Boolean to indicate if the generated modulemap is for a framework.
Defaults to `False`.
module_map_path: Base path for the generated modulemap file.
Expand All @@ -405,7 +473,12 @@ def _generate_module_map(

modulemap_content.add("}")

modulemap_file = actions.declare_file(paths.join(module_map_path, "module.modulemap"))
modulemap_file = intermediates.file(
actions = actions,
file_name = paths.join(module_map_path, "module.modulemap"),
output_discriminator = None,
target_name = label.name,
)
actions.write(output = modulemap_file, content = modulemap_content)

return modulemap_file
Expand Down
2 changes: 1 addition & 1 deletion test/testdata/xcframeworks/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ load("@build_bazel_rules_swift//swift:swift_library.bzl", "swift_library")

licenses(["notice"])

# TODO(b/232578557): Migrate targets to starlark_tests/
# TODO(b/241460068): Migrate targets to starlark_tests/
package(default_visibility = ["//test/starlark_tests:__subpackages__"])

generate_dynamic_xcframework(
Expand Down
Loading

1 comment on commit e3cd464

@keith
Copy link
Member

@keith keith commented on e3cd464 Oct 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.