Skip to content

Commit

Permalink
Add hooks to compile resources locally
Browse files Browse the repository at this point in the history
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
  • Loading branch information
milend authored and facebook-github-bot committed Jul 8, 2022
1 parent 76f5f73 commit 959cb48
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 7 deletions.
5 changes: 3 additions & 2 deletions prelude/apple/apple_asset_catalog.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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:
Expand Down
5 changes: 3 additions & 2 deletions prelude/apple/apple_bundle_resources.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
4 changes: 4 additions & 0 deletions prelude/apple/apple_bundle_utility.bzl
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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))
6 changes: 3 additions & 3 deletions prelude/apple/apple_core_data.bzl
Original file line number Diff line number Diff line change
@@ -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")
Expand Down Expand Up @@ -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":
Expand Down
5 changes: 5 additions & 0 deletions prelude/apple/apple_resource_types.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)

0 comments on commit 959cb48

Please sign in to comment.