From 959cb487479ac27fc002065d317c88609ce80cfa Mon Sep 17 00:00:00 2001 From: Milen Dzhumerov Date: Fri, 8 Jul 2022 08:08:50 -0700 Subject: [PATCH] Add hooks to compile resources locally Summary: # Problem At present, the `re_macdo` targets uses constraints to force local running (as macdo cannot operate on RE due to network isolation) https://www.internalfb.com/code/fbsource/[197f5593b1b27c27f01f120acbd7403fda80b25f]/xplat/toolchains/jackalope/dotslash/BUCK?lines=12-15 Because the `re_macdo` is part of the `apple_toolchain`, that means that every `apple_library` and `apple_binary` is *forced* to run locally as well. There are two very bad side effects: 1. Pika Linux builds do not use Linux RE for any compilation and linking (i.e., leads to very slow builds) 2. Pika Linux builds do not upload to cache because all actions run locally While problem #2 will disappear over time with action cache for local actions, problem #1 will never disappear as long as we force local executions via constraints. # Solution The solution is to remove the local constraints (i.e., `"fbcode//buck2/platform/execution:runs_only_local"`) from the `re_macdo` target while still executing locally for the resource compilation actions. To do that, we just need to basically pass `prefer_local = True` to the resource compilation actions when using the Linux Pika toolchain when combined with macDo. **Q&A** - Q: Can't we just add one of the local execution labels to force local execution? - A: No because the local execution labels work on `genrule`s which contains invocations of tools that should run locally. In our case, the rules that execute the `re_macdo` tools are part of the internal Apple rules (prelude), so there's nothing we can annotate with those. # This Diff We look for a rule attribute named `_compile_resources_locally` and then use that to set the `prefer_local` value to `ctx.actions.run()` for `ibtool,` `actool` and `momc` invocations. Note that `_compile_resources_locally` does not exist yet, so this will be a no-op. Reviewed By: d16r Differential Revision: D37683156 fbshipit-source-id: 0dd753dfad773b2447911875cd50aa5dfd14c21b --- prelude/apple/apple_asset_catalog.bzl | 5 +++-- prelude/apple/apple_bundle_resources.bzl | 5 +++-- prelude/apple/apple_bundle_utility.bzl | 4 ++++ prelude/apple/apple_core_data.bzl | 6 +++--- prelude/apple/apple_resource_types.bzl | 5 +++++ 5 files changed, 18 insertions(+), 7 deletions(-) diff --git a/prelude/apple/apple_asset_catalog.bzl b/prelude/apple/apple_asset_catalog.bzl index e04ab9b8e5ca..bd7a9824e561 100644 --- a/prelude/apple/apple_asset_catalog.bzl +++ b/prelude/apple/apple_asset_catalog.bzl @@ -2,7 +2,7 @@ load("@fbcode//buck2/prelude/apple:apple_toolchain_types.bzl", "AppleToolchainIn load("@fbcode//buck2/prelude/utils:utils.bzl", "flatten") load(":apple_asset_catalog_compilation_options.bzl", "AppleAssetCatalogsCompilationOptions", "get_apple_asset_catalogs_compilation_options") # @unused Used as a type load(":apple_asset_catalog_types.bzl", "AppleAssetCatalogResult", "AppleAssetCatalogSpec", "StringWithSourceTarget") -load(":apple_bundle_utility.bzl", "get_bundle_min_target_version") +load(":apple_bundle_utility.bzl", "get_bundle_min_target_version", "get_bundle_resource_processing_options") load(":apple_sdk.bzl", "get_apple_sdk_name") load(":apple_sdk_metadata.bzl", "get_apple_sdk_metadata_for_sdk_name") load(":resource_groups.bzl", "create_resource_graph") @@ -28,9 +28,10 @@ def compile_apple_asset_catalog(ctx: "context", specs: [AppleAssetCatalogSpec.ty return None plist = ctx.actions.declare_output("AssetCatalog.plist") catalog = ctx.actions.declare_output("AssetCatalogCompiled") + processing_options = get_bundle_resource_processing_options(ctx) compilation_options = get_apple_asset_catalogs_compilation_options(ctx) command = _get_actool_command(ctx, single_spec, catalog.as_output(), plist.as_output(), compilation_options) - ctx.actions.run(command, category = "apple_asset_catalog") + ctx.actions.run(command, prefer_local = processing_options.prefer_local, category = "apple_asset_catalog") return AppleAssetCatalogResult(compiled_catalog = catalog, catalog_plist = plist) def _merge_asset_catalog_specs(ctx: "context", xs: [AppleAssetCatalogSpec.type]) -> AppleAssetCatalogSpec.type: diff --git a/prelude/apple/apple_bundle_resources.bzl b/prelude/apple/apple_bundle_resources.bzl index 0a0578ca15a7..3854f88dc213 100644 --- a/prelude/apple/apple_bundle_resources.bzl +++ b/prelude/apple/apple_bundle_resources.bzl @@ -12,7 +12,7 @@ load( load(":apple_bundle_destination.bzl", "AppleBundleDestination") load(":apple_bundle_part.bzl", "AppleBundlePart") load(":apple_bundle_types.bzl", "AppleBundleInfo") -load(":apple_bundle_utility.bzl", "get_extension_attr", "get_product_name") +load(":apple_bundle_utility.bzl", "get_bundle_resource_processing_options", "get_extension_attr", "get_product_name") load(":apple_core_data.bzl", "compile_apple_core_data") load( ":apple_core_data_types.bzl", @@ -216,7 +216,8 @@ def _run_ibtool( else: command = ibtool_command - ctx.actions.run(command, category = "apple_ibtool", identifier = action_identifier) + processing_options = get_bundle_resource_processing_options(ctx) + ctx.actions.run(command, prefer_local = processing_options.prefer_local, category = "apple_ibtool", identifier = action_identifier) def _compile_ui_resource( ctx: "context", diff --git a/prelude/apple/apple_bundle_utility.bzl b/prelude/apple/apple_bundle_utility.bzl index 2398ccc833b1..ca9540cebc06 100644 --- a/prelude/apple/apple_bundle_utility.bzl +++ b/prelude/apple/apple_bundle_utility.bzl @@ -1,4 +1,5 @@ load(":apple_bundle_types.bzl", "AppleMinDeploymentVersionInfo") +load(":apple_resource_types.bzl", "AppleResourceProcessingOptions") load(":apple_target_sdk_version.bzl", "get_min_deployment_version_for_node") # `ctx` in all functions below is expected to be of `apple_bundle` or `apple_test` rule @@ -39,3 +40,6 @@ def get_bundle_min_target_version(ctx: "context") -> str.type: # TODO(T110378109): support default value from SDK `Info.plist` fail("Could not determine min target sdk version for bundle: {}".format(ctx.label)) + +def get_bundle_resource_processing_options(ctx: "context") -> AppleResourceProcessingOptions.type: + return AppleResourceProcessingOptions(prefer_local = getattr(ctx.attr, "_compile_resources_locally", False)) diff --git a/prelude/apple/apple_core_data.bzl b/prelude/apple/apple_core_data.bzl index 70427900109c..14ae4e4035d5 100644 --- a/prelude/apple/apple_core_data.bzl +++ b/prelude/apple/apple_core_data.bzl @@ -1,5 +1,5 @@ load("@fbcode//buck2/prelude/apple:apple_toolchain_types.bzl", "AppleToolchainInfo") -load(":apple_bundle_utility.bzl", "get_bundle_min_target_version") +load(":apple_bundle_utility.bzl", "get_bundle_min_target_version", "get_bundle_resource_processing_options") load(":apple_core_data_types.bzl", "AppleCoreDataSpec") load(":apple_sdk.bzl", "get_apple_sdk_name") load(":resource_groups.bzl", "create_resource_graph") @@ -43,9 +43,9 @@ def compile_apple_core_data(ctx: "context", specs: [AppleCoreDataSpec.type], pro ], allow_args = True, ) - combined_command = cmd_args(["/bin/sh", wrapper_script]).hidden(hidden + momc_commands + [output.as_output()]) - ctx.actions.run(combined_command, category = "apple_core_data") + processing_options = get_bundle_resource_processing_options(ctx) + ctx.actions.run(combined_command, prefer_local = processing_options.prefer_local, category = "apple_core_data") return output def _get_momc_command(ctx: "context", core_data_spec: AppleCoreDataSpec.type, product_name: str.type, output_directory: "cmd_args") -> "cmd_args": diff --git a/prelude/apple/apple_resource_types.bzl b/prelude/apple/apple_resource_types.bzl index d487bc100608..e260093fc289 100644 --- a/prelude/apple/apple_resource_types.bzl +++ b/prelude/apple/apple_resource_types.bzl @@ -19,3 +19,8 @@ AppleResourceSpec = record( named_variant_files = field({str.type: ["artifact"]}, {}), codesign_files_on_copy = field(bool.type, False), ) + +# Used when invoking `ibtool`, `actool` and `momc` +AppleResourceProcessingOptions = record( + prefer_local = field(bool.type, False), +)