Skip to content

Commit

Permalink
Remove analysis failures for use_tree_artifacts_outputs flag.
Browse files Browse the repository at this point in the history
We started using `apple_*_xcframework_import` rules to bring in XCFrameworks and we are getting failures during a query.
We don't actually use the version macOS framework in our build, it's just shipped along side the iOS framework.

Because of the `fail` we need to remove usage of the `use_tree_artifacts_outputs` flag, which causes a big regression to our disk usage:

- bin/Code/Apps/CashApp/CashApp_archive-root/Payload/Cash.app (732 MB)
- bin/Code/Apps/CashApp/CashApp-intermediates/unprocessed_archive.zip (732 MB)
- bin/Code/Apps/CashApp/CashApp.ipa (740 MB)

With `use_tree_artifacts_outputs` flag enabled we only have:

- bin/Code/Apps/CashApp/CashApp_archive-root/Payload/Cash.app (732 MB)

We should see about better supporting this flag in the future, consider making it the default, or improve the disk usage without it.
  • Loading branch information
luispadron committed Sep 26, 2024
1 parent b4a1d1e commit c00823a
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 64 deletions.
22 changes: 1 addition & 21 deletions apple/internal/apple_framework_import.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,10 @@ load(
"@build_bazel_rules_apple//apple:utils.bzl",
"group_files_by_directory",
)
load(
"@build_bazel_rules_apple//apple/internal:apple_toolchains.bzl",
"AppleXPlatToolsToolchainInfo",
)
load(
"@build_bazel_rules_apple//apple/internal:cc_toolchain_info_support.bzl",
"cc_toolchain_info_support",
)
load(
"@build_bazel_rules_apple//apple/internal:experimental.bzl",
"is_experimental_tree_artifact_enabled",
)
load(
"@build_bazel_rules_apple//apple/internal:framework_import_support.bzl",
"framework_import_support",
Expand Down Expand Up @@ -205,7 +197,6 @@ def _debug_info_binaries(
def _apple_dynamic_framework_import_impl(ctx):
"""Implementation for the apple_dynamic_framework_import rule."""
actions = ctx.actions
apple_xplat_toolchain_info = ctx.attr._xplat_toolchain[AppleXPlatToolsToolchainInfo]
cc_toolchain = find_cpp_toolchain(ctx)
deps = ctx.attr.deps
disabled_features = ctx.disabled_features
Expand All @@ -215,19 +206,8 @@ def _apple_dynamic_framework_import_impl(ctx):

# TODO(b/258492867): Add tree artifacts support when Bazel can handle remote actions with
# symlinks. See https://github.com/bazelbuild/bazel/issues/16361.

target_triplet = cc_toolchain_info_support.get_apple_clang_triplet(cc_toolchain)
has_versioned_framework_files = framework_import_support.has_versioned_framework_files(
framework_imports,
)
tree_artifact_enabled = (
apple_xplat_toolchain_info.build_settings.use_tree_artifacts_outputs or
is_experimental_tree_artifact_enabled(config_vars = ctx.var)
)
if target_triplet.os == "macos" and has_versioned_framework_files and tree_artifact_enabled:
fail("The apple_dynamic_framework_import rule does not yet support versioned " +
"frameworks with the experimental tree artifact feature/build setting. " +
"Please ensure that the `apple.experimental.tree_artifact_outputs` variable is not " +
"set to 1 on the command line or in your active build configuration.")

providers = []
framework = framework_import_support.classify_framework_imports(
Expand Down
17 changes: 1 addition & 16 deletions apple/internal/apple_xcframework_import.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ load(
"@build_bazel_rules_apple//apple/internal:cc_toolchain_info_support.bzl",
"cc_toolchain_info_support",
)
load(
"@build_bazel_rules_apple//apple/internal:experimental.bzl",
"is_experimental_tree_artifact_enabled",
)
load(
"@build_bazel_rules_apple//apple/internal:framework_import_support.bzl",
"framework_import_support",
Expand Down Expand Up @@ -452,19 +448,8 @@ def _apple_dynamic_xcframework_import_impl(ctx):

# TODO(b/258492867): Add tree artifacts support when Bazel can handle remote actions with
# symlinks. See https://github.com/bazelbuild/bazel/issues/16361.

target_triplet = cc_toolchain_info_support.get_apple_clang_triplet(cc_toolchain)
has_versioned_framework_files = framework_import_support.has_versioned_framework_files(
xcframework_imports,
)
tree_artifact_enabled = (
apple_xplat_toolchain_info.build_settings.use_tree_artifacts_outputs or
is_experimental_tree_artifact_enabled(config_vars = ctx.var)
)
if target_triplet.os == "macos" and has_versioned_framework_files and tree_artifact_enabled:
fail("The apple_dynamic_xcframework_import rule does not yet support versioned " +
"frameworks with the experimental tree artifact feature/build setting. " +
"Please ensure that the `apple.experimental.tree_artifact_outputs` variable is not " +
"set to 1 on the command line or in your active build configuration.")

xcframework = _classify_xcframework_imports(ctx.var, xcframework_imports)
if xcframework.bundle_type == _BUNDLE_TYPE.libraries:
Expand Down
12 changes: 0 additions & 12 deletions test/starlark_tests/apple_dynamic_xcframework_import_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ load(
load(
"//test/starlark_tests/rules:analysis_failure_message_test.bzl",
"analysis_failure_message_test",
"analysis_failure_message_with_tree_artifact_outputs_test",
)
load(
"//test/starlark_tests/rules:analysis_target_actions_test.bzl",
Expand Down Expand Up @@ -451,17 +450,6 @@ def apple_dynamic_xcframework_import_test_suite(name):
tags = [name],
)

# Verify importing XCFramework with versioned frameworks and tree artifacts fails.
analysis_failure_message_with_tree_artifact_outputs_test(
name = "{}_fails_with_versioned_frameworks_and_tree_artifact_outputs_test".format(name),
target_under_test = "//test/starlark_tests/targets_under_test/macos:app_with_imported_dynamic_versioned_xcframework",
expected_error = (
"The apple_dynamic_xcframework_import rule does not yet support versioned " +
"frameworks with the experimental tree artifact feature/build setting."
),
tags = [name],
)

native.test_suite(
name = name,
tags = [name],
Expand Down
15 changes: 0 additions & 15 deletions test/starlark_tests/macos_application_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@

"""macos_application Starlark tests."""

load(
"//test/starlark_tests/rules:analysis_failure_message_test.bzl",
"analysis_failure_message_with_tree_artifact_outputs_test",
)
load(
"//test/starlark_tests/rules:analysis_output_group_info_files_test.bzl",
"analysis_output_group_info_files_test",
Expand Down Expand Up @@ -365,17 +361,6 @@ def macos_application_test_suite(name):
tags = [name],
)

# Verify importing versioned framework with tree artifacts enabled fails.
analysis_failure_message_with_tree_artifact_outputs_test(
name = "{}_fails_with_imported_versioned_framework_and_tree_artifact_outputs".format(name),
target_under_test = "//test/starlark_tests/targets_under_test/macos:app_with_imported_versioned_fmwk",
expected_error = (
"The apple_dynamic_framework_import rule does not yet support versioned " +
"frameworks with the experimental tree artifact feature/build setting."
),
tags = [name],
)

# Test app with App Intents generates and bundles Metadata.appintents bundle.
archive_contents_test(
name = "{}_contains_app_intents_metadata_bundle".format(name),
Expand Down

0 comments on commit c00823a

Please sign in to comment.