From 7e242cdc9a3e0a30f50601c525989c8c0b06bae5 Mon Sep 17 00:00:00 2001 From: Mauricio Garcia Date: Fri, 5 Aug 2022 13:43:56 -0700 Subject: [PATCH 1/2] Fix race condition between xcodebuild and symlink action for test XCFramework 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 (cherry picked from commit e3cd464a2ba86f77106a4c69beebb446c65664f8) --- apple/internal/BUILD | 1 + test/testdata/fmwk/BUILD | 11 +- test/testdata/fmwk/generate_framework.bzl | 35 ++++-- test/testdata/fmwk/generation_support.bzl | 119 ++++++++++++++---- test/testdata/xcframeworks/BUILD | 2 +- .../xcframeworks/generate_xcframework.bzl | 88 +++++-------- 6 files changed, 159 insertions(+), 97 deletions(-) diff --git a/apple/internal/BUILD b/apple/internal/BUILD index 927084b0e..d070f564e 100644 --- a/apple/internal/BUILD +++ b/apple/internal/BUILD @@ -256,6 +256,7 @@ bzl_library( srcs = ["intermediates.bzl"], visibility = [ "//apple:__subpackages__", + "//test:__subpackages__", ], deps = [ "@bazel_skylib//lib:paths", diff --git a/test/testdata/fmwk/BUILD b/test/testdata/fmwk/BUILD index ffc854325..712af13b8 100644 --- a/test/testdata/fmwk/BUILD +++ b/test/testdata/fmwk/BUILD @@ -36,16 +36,6 @@ filegroup( ], ) -filegroup( - name = "ios_framework_generators", - srcs = [ - "BUILD", - "generate_framework.bzl", - "generate_framework_dsym.bzl", - "generation_support.bzl", - ], -) - # ------------------------------- iOS Framework ------------------------------------------ generate_import_framework( @@ -201,6 +191,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", ], diff --git a/test/testdata/fmwk/generate_framework.bzl b/test/testdata/fmwk/generate_framework.bzl index 33dda02ed..5fbb41edb 100644 --- a/test/testdata/fmwk/generate_framework.bzl +++ b/test/testdata/fmwk/generate_framework.bzl @@ -70,6 +70,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, @@ -79,6 +80,7 @@ def _generate_import_framework_impl(ctx): actions = actions, apple_fragment = apple_fragment, binary = binary, + label = label, xcode_config = xcode_config, ) @@ -103,22 +105,37 @@ def _generate_import_framework_impl(ctx): files = swift_library_files, extension = "swiftinterface", ) - module_interfaces.append( - generation_support.copy_file( - actions = actions, - base_path = "intermediates", - file = swiftinterface, - target_filename = "%s.swiftinterface" % architectures[0], - ), - ) + if swiftinterface: + module_interfaces.append( + generation_support.copy_file( + actions = actions, + file = swiftinterface, + label = label, + target_filename = "%s.swiftinterface" % architectures[0], + ), + ) + else: + swiftmodule = generation_support.get_file_with_extension( + files = swift_library_files, + extension = "swiftmodule", + ) + module_interfaces.append( + generation_support.copy_file( + actions = actions, + file = swiftmodule, + label = label, + target_filename = "%s.swiftmodule" % architectures[0], + ), + ) # Create framework bundle 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, ) diff --git a/test/testdata/fmwk/generation_support.bzl b/test/testdata/fmwk/generation_support.bzl index 13a676b8f..f061c5ecf 100644 --- a/test/testdata/fmwk/generation_support.bzl +++ b/test/testdata/fmwk/generation_support.bzl @@ -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 = { @@ -91,9 +95,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) @@ -132,21 +143,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] @@ -169,6 +191,7 @@ def _create_dynamic_library( apple_fragment, archs, binary, + label, minimum_os_version, sdk, xcode_config): @@ -179,16 +202,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]) @@ -198,7 +228,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: @@ -225,6 +255,7 @@ def _create_framework( actions, base_path = "", bundle_name, + label, library, headers, include_resource_bundle = False, @@ -235,6 +266,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 @@ -246,13 +278,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), @@ -265,8 +308,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 ]) @@ -275,6 +319,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) @@ -285,6 +330,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, ), @@ -295,34 +341,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 @@ -346,6 +407,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. @@ -354,6 +416,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: @@ -365,8 +428,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, @@ -380,6 +446,7 @@ def _generate_module_map( actions, bundle_name, headers = None, + label, is_framework_module = False, module_map_path, umbrella_header = None): @@ -389,6 +456,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. @@ -415,7 +483,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 diff --git a/test/testdata/xcframeworks/BUILD b/test/testdata/xcframeworks/BUILD index 4badfbe9c..ccf9a60bc 100644 --- a/test/testdata/xcframeworks/BUILD +++ b/test/testdata/xcframeworks/BUILD @@ -13,7 +13,7 @@ load("@build_bazel_rules_swift//swift:swift.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( diff --git a/test/testdata/xcframeworks/generate_xcframework.bzl b/test/testdata/xcframeworks/generate_xcframework.bzl index 5c0b9850f..4dce8b2bf 100644 --- a/test/testdata/xcframeworks/generate_xcframework.bzl +++ b/test/testdata/xcframeworks/generate_xcframework.bzl @@ -93,7 +93,7 @@ def _create_xcframework( """ bundle_name = label.name + ".xcframework" xcframework_directory = paths.join(target_dir, bundle_name) - intermediates_directory = paths.join(label.package, "intermediates") + intermediates_directory = paths.join(label.package, "%s-intermediates" % label.name) if (frameworks and libraries) or (not frameworks and not libraries): fail("Can only generate XCFrameworks using static libraries or dynamic frameworks.") @@ -162,31 +162,6 @@ def _create_xcframework( return outputs -def _copy_modules_to_xcframework(*, actions, modulemap, xcframework_files): - """Copies generated modulemap files to each XCFramework library path. - - Args: - actions: The actions provider from `ctx.actions`. - modulemap: File referencing a generated `.modulemap` file. - xcframework_files: List of files from generated XCFramework bundle. - Returns: - List of copied modulemap files. - """ - modulemaps = [] - binary_files = [f for f in xcframework_files if f.extension == "a"] - for binary_file in binary_files: - modulemap_dir = paths.basename(modulemap.dirname) - modulemap_path = paths.join(modulemap_dir, modulemap.basename) - library_modulemap = actions.declare_file(modulemap_path, sibling = binary_file) - - actions.symlink( - output = library_modulemap, - target_file = modulemap, - ) - modulemaps.append(library_modulemap) - - return modulemaps - def _generate_dynamic_xcframework_impl(ctx): """Implementation of generate_dynamic_xcframework.""" actions = ctx.actions @@ -232,6 +207,7 @@ def _generate_dynamic_xcframework_impl(ctx): apple_fragment = apple_fragment, archs = architectures, binary = binary, + label = label, minimum_os_version = minimum_os_version, sdk = sdk, xcode_config = xcode_config, @@ -240,15 +216,15 @@ def _generate_dynamic_xcframework_impl(ctx): # Create (dynamic) framework bundle framework_files = generation_support.create_framework( actions = actions, - base_path = paths.join("intermediates", library_identifier), + base_path = library_identifier, bundle_name = label.name, - library = dynamic_library, headers = hdrs, + label = label, + library = dynamic_library, ) framework_path = paths.join( - target_dir, - "intermediates", + binary.dirname, library_identifier, label.name + ".framework", ) @@ -301,7 +277,7 @@ def _generate_static_xcframework_impl(ctx): headers = [] libraries = [] module_interfaces = [] - outputs = [] + modulemaps = [] umbrella_header = None for platform in platforms: architectures = platforms[platform] @@ -309,7 +285,8 @@ def _generate_static_xcframework_impl(ctx): platform = platform, architectures = architectures, ) - library_path = paths.join("intermediates", library_identifier) + + library_path = library_identifier headers_path = paths.join(library_path, "Headers") if not swift_library: @@ -332,6 +309,7 @@ def _generate_static_xcframework_impl(ctx): actions = actions, apple_fragment = apple_fragment, binary = binary, + label = label, parent_dir = library_identifier, xcode_config = xcode_config, ) @@ -340,16 +318,19 @@ def _generate_static_xcframework_impl(ctx): headers.extend([ generation_support.copy_file( actions = actions, - file = header, base_path = headers_path, + file = header, + label = label, ) for header in hdrs ]) + umbrella_header = generation_support.generate_umbrella_header( actions = actions, bundle_name = label.name, headers = hdrs, headers_path = headers_path, + label = label, ) headers.append(umbrella_header) else: @@ -361,17 +342,17 @@ def _generate_static_xcframework_impl(ctx): files = swift_library, extension = "a", ), + label = label, target_filename = label.name + ".a", ) # Copy Swift module files to intermediate directory if include_module_interface_files: - swiftmodule_path = paths.join(library_path, label.name + ".swiftmodule") module_interfaces = [ generation_support.copy_file( actions = actions, - base_path = swiftmodule_path, file = interface_file, + label = label, target_filename = "{architecture}.{extension}".format( architecture = architectures[0], extension = interface_file.extension, @@ -386,6 +367,7 @@ def _generate_static_xcframework_impl(ctx): generation_support.copy_file( actions = actions, base_path = headers_path, + label = label, file = generation_support.get_file_with_extension( files = swift_library, extension = "h", @@ -394,40 +376,36 @@ def _generate_static_xcframework_impl(ctx): ), ) + # Generate Clang modulemap under Headers directory. + if generate_modulemap: + modulemaps.append( + generation_support.generate_module_map( + actions = actions, + bundle_name = label.name, + headers = headers, + label = label, + module_map_path = headers_path, + umbrella_header = umbrella_header, + ), + ) + libraries.append(static_library) # Create static XCFramework xcframework_files = _create_xcframework( actions = actions, apple_fragment = apple_fragment, - headers = headers, + headers = headers + modulemaps, label = label, libraries = libraries, module_interfaces = module_interfaces, target_dir = target_dir, xcode_config = xcode_config, ) - outputs.extend(xcframework_files) - - # Generate modulemap and copy to XCFramework - if generate_modulemap: - module_map_path = paths.join(label.name, "Headers") - modulemap = generation_support.generate_module_map( - actions = actions, - bundle_name = label.name, - headers = headers, - module_map_path = module_map_path, - umbrella_header = umbrella_header, - ) - outputs.extend(_copy_modules_to_xcframework( - actions = actions, - modulemap = modulemap, - xcframework_files = xcframework_files, - )) return [ DefaultInfo( - files = depset(outputs), + files = depset(xcframework_files), ), ] @@ -474,6 +452,7 @@ def _generate_static_framework_xcframework_impl(ctx): actions = actions, apple_fragment = apple_fragment, binary = binary, + label = label, xcode_config = xcode_config, ) @@ -482,6 +461,7 @@ def _generate_static_framework_xcframework_impl(ctx): base_path = paths.join("intermediates", library_identifier), bundle_name = label.name, library = dynamic_library, + label = label, headers = hdrs, ) From f0124dd5afd840371b5640ae79467e62da732f5d Mon Sep 17 00:00:00 2001 From: Keith Smiley Date: Mon, 14 Aug 2023 13:47:49 -0700 Subject: [PATCH 2/2] compat with our public only xcframework with static framework support I redid this by copying the dynamic fw impl again and changing the small pieces needed to make it static instead --- .../xcframeworks/generate_xcframework.bzl | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/testdata/xcframeworks/generate_xcframework.bzl b/test/testdata/xcframeworks/generate_xcframework.bzl index 4dce8b2bf..f576aa56f 100644 --- a/test/testdata/xcframeworks/generate_xcframework.bzl +++ b/test/testdata/xcframeworks/generate_xcframework.bzl @@ -348,9 +348,11 @@ def _generate_static_xcframework_impl(ctx): # Copy Swift module files to intermediate directory if include_module_interface_files: + swiftmodule_path = paths.join(library_path, label.name + ".swiftmodule") module_interfaces = [ generation_support.copy_file( actions = actions, + base_path = swiftmodule_path, file = interface_file, label = label, target_filename = "{architecture}.{extension}".format( @@ -448,7 +450,8 @@ def _generate_static_framework_xcframework_impl(ctx): xcode_config = xcode_config, ) - dynamic_library = generation_support.create_static_library( + # Create static library + static_library = generation_support.create_static_library( actions = actions, apple_fragment = apple_fragment, binary = binary, @@ -456,18 +459,18 @@ def _generate_static_framework_xcframework_impl(ctx): xcode_config = xcode_config, ) + # Create (static) framework bundle framework_files = generation_support.create_framework( actions = actions, - base_path = paths.join("intermediates", library_identifier), + base_path = library_identifier, bundle_name = label.name, - library = dynamic_library, - label = label, headers = hdrs, + label = label, + library = static_library, ) framework_path = paths.join( - target_dir, - "intermediates", + binary.dirname, library_identifier, label.name + ".framework", )