Skip to content

Commit

Permalink
Fix Apple frameworks bundling propagated from data with tree artifa…
Browse files Browse the repository at this point in the history
…cts.

Current logic incorrectly assumes `files` is a single framework,
and thus bundles all frameworks to the same location. If there
are multiple frameworks bundled, the partial should iterate each
bundle and source the correct bundle name to set as a parent
directory when tree artifacts are enabled.

PiperOrigin-RevId: 514821470
  • Loading branch information
stravinskii authored and swiple-rules-gardener committed Mar 7, 2023
1 parent 3db87ec commit 644face
Show file tree
Hide file tree
Showing 3 changed files with 68 additions and 9 deletions.
22 changes: 14 additions & 8 deletions apple/internal/partials/support/resources_support.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ All methods in this file follow this convention:
bundle's Info.plist.
"""

load(
"@build_bazel_rules_apple//apple/internal:experimental.bzl",
"is_experimental_tree_artifact_enabled",
)
load(
"@build_bazel_rules_apple//apple/internal:intermediates.bzl",
"intermediates",
Expand Down Expand Up @@ -656,16 +660,18 @@ def _apple_bundle(bundle_type):
if not hasattr(processor.location, bundle_type):
fail("Bundle type location not supported: ", bundle_type)

def _bundle_at_location(*, files, **_kwargs):
bundle = files.to_list().pop()
def _bundle_at_location(*, files, platform_prerequisites, **_kwargs):
location = getattr(processor.location, bundle_type)

# If tree artifacts are enabled, set the bundle name as
# the parent directory. Otherwise, let bundletool unzip
# the bundle directly.
if bundle.is_directory:
parent_dir = paths.basename(bundle.short_path)
return struct(files = [(location, parent_dir, files)])
# If tree artifacts are enabled, iterate each bundle and set the bundle name
# as the parent directory. Otherwise, let bundletool unzip the bundle as is.
if is_experimental_tree_artifact_enabled(platform_prerequisites = platform_prerequisites):
bundle_files = []
for bundle in files.to_list():
# TODO(b/271899726): Prepend parent_dir if embeddeding frameworks inside a resource bundle is allowed.
basename = paths.basename(bundle.short_path)
bundle_files.append((location, basename, depset([bundle])))
return struct(files = bundle_files)
else:
return struct(archives = [(location, None, files)])

Expand Down
36 changes: 36 additions & 0 deletions test/starlark_tests/ios_framework_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ def ios_framework_test_suite(name):
contains = [
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/basic.bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_no_version.framework/fmwk_no_version",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/fmwk_with_resources",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/Another.plist",
],
Expand All @@ -395,6 +396,36 @@ def ios_framework_test_suite(name):
],
binary_test_file = "$BUNDLE_ROOT/app_with_fmwks_from_objc_swift_libraries_using_data",
macho_load_commands_not_contain = [
"name @rpath/fmwk_with_resources.framework/fmwk_with_resources (offset 24)",
"name @rpath/fmwk_no_version.framework/fmwk_no_version (offset 24)",
"name @rpath/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle (offset 24)",
],
tags = [name],
)
archive_contents_test(
name = "{}_includes_and_does_not_link_transitive_data_ios_frameworks_with_tree_artifacts".format(name),
build_type = "simulator",
target_under_test = "//test/starlark_tests/targets_under_test/ios:app_with_fmwks_from_objc_swift_libraries_using_data",
apple_generate_dsym = True,
build_settings = {
build_settings_labels.use_tree_artifacts_outputs: "True",
},
contains = [
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/basic.bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_no_version.framework/fmwk_no_version",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/fmwk_with_resources",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/Another.plist",
],
not_contains = [
"$BUNDLE_ROOT/Another.plist",
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/Another.plist",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/basic.bundle",
"$BUNDLE_ROOT/basic.bundle",
],
binary_test_file = "$BUNDLE_ROOT/app_with_fmwks_from_objc_swift_libraries_using_data",
macho_load_commands_not_contain = [
"name @rpath/fmwk_no_version.framework/fmwk_no_version (offset 24)",
"name @rpath/fmwk_with_resources.framework/fmwk_with_resources (offset 24)",
"name @rpath/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle (offset 24)",
],
Expand All @@ -414,6 +445,7 @@ def ios_framework_test_suite(name):
contains = [
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/basic.bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_no_version.framework/fmwk_no_version",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/Another.plist",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/fmwk_with_resources",
],
Expand All @@ -425,6 +457,7 @@ def ios_framework_test_suite(name):
],
binary_test_file = "$BUNDLE_ROOT/app_with_fmwks_from_transitive_objc_swift_libraries_using_data",
macho_load_commands_not_contain = [
"name @rpath/fmwk_no_version.framework/fmwk_no_version (offset 24)",
"name @rpath/fmwk_with_resources.framework/fmwk_with_resources (offset 24)",
"name @rpath/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle (offset 24)",
],
Expand All @@ -445,6 +478,7 @@ def ios_framework_test_suite(name):
"$BUNDLE_ROOT/Frameworks/fmwk.framework/fmwk",
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/basic.bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_no_version.framework/fmwk_no_version",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/Another.plist",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/fmwk_with_resources",
],
Expand All @@ -458,6 +492,7 @@ def ios_framework_test_suite(name):
"name @rpath/fmwk.framework/fmwk (offset 24)",
],
macho_load_commands_not_contain = [
"name @rpath/fmwk_no_version.framework/fmwk_no_version (offset 24)",
"name @rpath/fmwk_with_resources.framework/fmwk_with_resources (offset 24)",
"name @rpath/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle (offset 24)",
],
Expand All @@ -475,6 +510,7 @@ def ios_framework_test_suite(name):
"$BUNDLE_ROOT/Another.plist",
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/basic.bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_min_os_baseline_with_bundle.framework/fmwk_min_os_baseline_with_bundle",
"$BUNDLE_ROOT/Frameworks/fmwk_no_version.framework/fmwk_no_version",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/Another.plist",
"$BUNDLE_ROOT/Frameworks/fmwk_with_resources.framework/fmwk_with_resources",
"$BUNDLE_ROOT/basic.bundle",
Expand Down
19 changes: 18 additions & 1 deletion test/starlark_tests/targets_under_test/ios/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ load(
"ios_ui_test",
"ios_unit_test",
)
load(
"//apple:resources.bzl",
"apple_resource_group",
)
load(
"//test/starlark_tests/rules:generate_framework.bzl",
"generate_import_framework",
Expand Down Expand Up @@ -2982,6 +2986,7 @@ ios_application(
objc_library(
name = "objc_lib_with_inner_lib_with_ios_framework",
srcs = ["//test/starlark_tests/resources:main.m"],
tags = common.fixture_tags,
deps = [":objc_lib_with_ios_framework"],
)

Expand All @@ -2991,13 +2996,24 @@ objc_library(
"//test/starlark_tests/resources:shared.h",
"//test/starlark_tests/resources:shared.m",
],
data = [":fmwk_with_resources"],
data = [
":fmwk_with_resources",
":resource_group_with_framework",
],
tags = common.fixture_tags,
)

apple_resource_group(
name = "resource_group_with_framework",
resources = [":fmwk_no_version"],
tags = common.fixture_tags,
)

swift_library(
name = "swift_lib_with_ios_framework",
srcs = ["//test/starlark_tests/resources:DummySwiftFile"],
data = [":fmwk_min_os_baseline_with_bundle"],
tags = common.fixture_tags,
)

ios_application(
Expand Down Expand Up @@ -3063,6 +3079,7 @@ objc_library(
"//test/starlark_tests/resources:shared.m",
],
data = [":fmwk_with_fmwk"],
tags = common.fixture_tags,
)

# ---------------------------------------------------------------------------------------
Expand Down

1 comment on commit 644face

@keith
Copy link
Member

@keith keith commented on 644face Mar 21, 2023

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.