Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build frameworks/resource_bundles behind a transition #73

Merged
merged 7 commits into from
Jun 12, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,8 @@ jobs:
- uses: actions/checkout@v1
- name: Select Xcode 11.2
run: sudo xcode-select -s /Applications/Xcode_11.2.app
- name: Generate Xcode projects that load all transitive targets and compare against existing record
run: bazelisk run tests/macos/xcodeproj:Single-Application-Project-AllTargets && git diff --exit-code tests/macos/xcodeproj
- name: Generate Xcode projects that load direct targets only and compare against existing record
run: bazelisk run tests/macos/xcodeproj:Single-Application-Project-DirectTargetsOnly && git diff --exit-code tests/macos/xcodeproj
- name: Generate Xcode projects that have a test target with a host app and compare against existing record
run: bazelisk run tests/macos/xcodeproj:Test-Target-With-Test-Host-Project && git diff --exit-code tests/macos/xcodeproj
- name: Generate Xcode projects and compare against existing record
run: (bazelisk query 'kind(xcodeproj, tests/macos/xcodeproj/...)' | xargs -n 1 bazelisk run) && git diff --exit-code tests/macos/xcodeproj
- name: Run Xcode builds
run: ./tests/macos/xcodeproj/build.sh
- name: Run Unit tests
Expand Down
56 changes: 53 additions & 3 deletions rules/framework.bzl
Original file line number Diff line number Diff line change
@@ -1,8 +1,18 @@
"""Framework rules"""

load("@build_bazel_rules_apple//apple/internal:apple_product_type.bzl", "apple_product_type")
load("@build_bazel_rules_apple//apple:providers.bzl", "AppleBundleInfo")
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@build_bazel_rules_swift//swift:swift.bzl", "SwiftInfo", "swift_common")
load("//rules:library.bzl", "PrivateHeadersInfo", "apple_library")
load("//rules:transition_support.bzl", "transition_support")

_APPLE_FRAMEWORK_PACKAGING_KWARGS = [
"visibility",
"tags",
"bundle_id",
"skip_packaging",
]

def apple_framework(name, apple_library = apple_library, **kwargs):
"""Builds and packages an Apple framework.
Expand All @@ -12,15 +22,15 @@ def apple_framework(name, apple_library = apple_library, **kwargs):
apple_library: The macro used to package sources into a library.
**kwargs: Arguments passed to the apple_library and apple_framework_packaging rules as appropriate.
"""

framework_packaging_kwargs = {arg: kwargs.pop(arg) for arg in _APPLE_FRAMEWORK_PACKAGING_KWARGS if arg in kwargs}
library = apple_library(name = name, **kwargs)
apple_framework_packaging(
name = name,
framework_name = library.namespace,
transitive_deps = library.transitive_deps,
deps = library.lib_names,
visibility = kwargs.get("visibility", None),
tags = kwargs.get("tags", None),
platforms = library.platforms,
**framework_packaging_kwargs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, cute.

)

def _find_framework_dir(outputs):
Expand Down Expand Up @@ -99,6 +109,11 @@ def _apple_framework_packaging_impl(ctx):
swiftdoc_in = None
swiftdoc_out = None

# AppleBundleInfo fields
bundle_id = ctx.attr.bundle_id
infoplist = None
current_apple_platform = transition_support.current_apple_platform(ctx.fragments.apple, ctx.attr._xcode_config)

# collect files
for dep in ctx.attr.deps:
files = dep.files.to_list()
Expand Down Expand Up @@ -284,10 +299,25 @@ def _apple_framework_packaging_impl(ctx):
cc_info_provider,
swift_common.create_swift_info(**swift_info_fields),
DefaultInfo(files = depset(framework_files)),
AppleBundleInfo(
archive = None,
archive_root = None,
binary = binary_out,
bundle_id = bundle_id,
bundle_name = framework_name,
bundle_extension = ctx.attr.bundle_extension,
entitlements = None,
infoplist = infoplist,
minimum_os_version = str(current_apple_platform.target_os_version),
platform_type = str(current_apple_platform.platform.platform_type),
product_type = ctx.attr._product_type,
uses_swift = swiftmodule_out != None,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, can you add parens here to keep the order of operations obvious?
suggestion:
uses_swift = (swiftmodule_out != None),

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope the RHS doesn't turn into an array/tuple if you do that!

),
]

apple_framework_packaging = rule(
implementation = _apple_framework_packaging_impl,
cfg = transition_support.apple_rule_transition,
fragments = ["apple"],
output_to_genfiles = True,
attrs = {
Expand Down Expand Up @@ -337,6 +367,26 @@ Valid values are:
"//rules/hmap:hmaptool",
),
),
"bundle_id": attr.string(mandatory = False),
"bundle_extension": attr.string(mandatory = False, default = "framework"),
"platforms": attr.string_dict(
mandatory = False,
default = {},
),
"_product_type": attr.string(default = apple_product_type.static_framework),
# TODO: allow customizing binary type between dynamic/static
# "binary_type": attr.string(
# default = "dylib",
# ),
"_xcode_config": attr.label(
default = configuration_field(
name = "xcode_config_label",
fragment = "apple",
),
),
"_whitelist_function_transition": attr.label(
default = "@build_bazel_rules_apple//tools/whitelists/function_transition_whitelist",
),
},
doc = "Packages compiled code into an Apple .framework package",
)
25 changes: 10 additions & 15 deletions rules/library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ load("@rules_cc//cc:defs.bzl", "cc_library", "objc_import", "objc_library")
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@bazel_skylib//lib:sets.bzl", "sets")
load("@build_bazel_rules_apple//apple:apple.bzl", "apple_dynamic_framework_import", "apple_static_framework_import")
load("@build_bazel_rules_apple//apple:resources.bzl", "apple_resource_bundle")
load("@build_bazel_rules_swift//swift:swift.bzl", "swift_library")
load("//rules:precompiled_apple_resource_bundle.bzl", "precompiled_apple_resource_bundle")
load("//rules:hmap.bzl", "headermap")
load("//rules:substitute_build_settings.bzl", "substitute_build_settings")
load("//rules/library:resources.bzl", "wrap_resources_in_filegroup")
load("//rules/library:xcconfig.bzl", "settings_from_xcconfig")

Expand Down Expand Up @@ -157,26 +156,17 @@ FOUNDATION_EXPORT const unsigned char {module_name}VersionString[];
)
return destination

def _generate_resource_bundles(name, library_tools, module_name, resource_bundles, **kwargs):
def _generate_resource_bundles(name, library_tools, module_name, resource_bundles, platforms, **kwargs):
bundle_target_names = []
for bundle_name in resource_bundles:
target_name = "%s-%s" % (name, bundle_name)
substitute_build_settings(
name = name + ".info.plist",
source = "@build_bazel_rules_ios//rules/library:resource_bundle.plist",
variables = {
"PRODUCT_BUNDLE_IDENTIFIER": "com.cocoapods.%s" % bundle_name,
"PRODUCT_NAME": bundle_name,
},
tags = _MANUAL,
)
apple_resource_bundle(
precompiled_apple_resource_bundle(
name = target_name,
bundle_name = bundle_name,
resources = [
library_tools["wrap_resources_in_filegroup"](name = target_name + "_resources", srcs = resource_bundles[bundle_name]),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a follow-up, it would be nice to get rid of this extra target as well

],
infoplists = [name + ".info.plist"],
platforms = platforms,
tags = _MANUAL,
)
bundle_target_names.append(target_name)
Expand Down Expand Up @@ -318,9 +308,12 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names
weak_sdk_frameworks = kwargs.pop("weak_sdk_frameworks", [])
sdk_includes = kwargs.pop("sdk_includes", [])
pch = kwargs.pop("pch", "@build_bazel_rules_ios//rules/library:common.pch")
deps = kwargs.pop("deps", [])
deps = [d for d in kwargs.pop("deps", [])]
data = kwargs.pop("data", [])
tags = kwargs.pop("tags", [])
platforms = kwargs.pop("platforms", {
"ios": "10.0", # TODO: remove default
segiddins marked this conversation as resolved.
Show resolved Hide resolved
})
tags_manual = tags if "manual" in tags else tags + _MANUAL
internal_deps = []
lib_names = []
Expand Down Expand Up @@ -383,6 +376,7 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names
library_tools = library_tools,
resource_bundles = kwargs.pop("resource_bundles", {}),
module_name = module_name,
platforms = platforms,
**kwargs
)
deps += resource_bundles
Expand Down Expand Up @@ -607,4 +601,5 @@ def apple_library(name, library_tools = {}, export_private_headers = True, names
launch_screen_storyboard_name = launch_screen_storyboard_name,
namespace = namespace,
linkopts = linkopts,
platforms = platforms,
)
207 changes: 207 additions & 0 deletions rules/precompiled_apple_resource_bundle.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
"""
This provides a resource bundle implementation that builds the resource bundle
only once
NOTE: This rule only exists because of this issue
https://github.com/bazelbuild/rules_apple/issues/319
if this is ever fixed in bazel it should be removed
"""

load("@bazel_skylib//lib:partial.bzl", "partial")
load("@bazel_skylib//lib:paths.bzl", "paths")
load("@build_bazel_rules_apple//apple/internal:apple_product_type.bzl", "apple_product_type")
load("@build_bazel_rules_apple//apple/internal:intermediates.bzl", "intermediates")
load("@build_bazel_rules_apple//apple/internal:partials.bzl", "partials")
load("@build_bazel_rules_apple//apple/internal:resource_actions.bzl", "resource_actions")
load("@build_bazel_rules_apple//apple/internal:rule_factory.bzl", "rule_factory")
load("//rules:transition_support.bzl", "transition_support")
load("@build_bazel_rules_apple//apple:providers.bzl", "AppleResourceBundleInfo", "AppleResourceInfo")

_FAKE_BUNDLE_PRODUCT_TYPE_BY_PLATFORM_TYPE = {
"ios": apple_product_type.application,
"tvos": apple_product_type.application,
"watchos": apple_product_type.watch2_application,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add macos here since why not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

macos is tested, it just doesnt need a fake bundle product type since the rule descriptors have one for a macos resource bundle

}

def _precompiled_apple_resource_bundle_impl(ctx):
bundle_name = ctx.attr.bundle_name or ctx.label.name

attr_dict = {}
for k in dir(ctx.attr):
if k in ("to_json", "to_proto"):
continue
attr_dict[k] = getattr(ctx.attr, k)

current_apple_platform = transition_support.current_apple_platform(apple_fragment = ctx.fragments.apple, xcode_config = ctx.attr._xcode_config)
attr_dict["platform_type"] = str(current_apple_platform.platform.platform_type)
attr_dict["minimum_os_version"] = str(current_apple_platform.target_os_version)

# TODO: only do this dance if platform is not macos
segiddins marked this conversation as resolved.
Show resolved Hide resolved
attr_dict["_product_type"] = _FAKE_BUNDLE_PRODUCT_TYPE_BY_PLATFORM_TYPE.get(attr_dict["platform_type"], ctx.attr._product_type)
ios_app_ctx_dict = {}
for k in dir(ctx):
if k in ("aspect_ids", "build_setting_value", "rule", "to_json", "to_proto"):
continue
ios_app_ctx_dict[k] = getattr(ctx, k)
ios_app_ctx_dict["attr"] = struct(**attr_dict)
ios_app_ctx = struct(**ios_app_ctx_dict)

partial_output = partial.call(partials.resources_partial(
top_level_attrs = ["resources"],
# plist_attrs = ["infoplist"]
), ios_app_ctx)

# Process the plist ourselves. This is required because
# include_executable_name defaults to True, which results in iTC rejecting
# our binary.
output_plist = ctx.actions.declare_file(
paths.join("%s-intermediates" % ctx.label.name, "Info.plist"),
)
bundle_id = ctx.attr.bundle_id or "com.cocoapods." + bundle_name
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to make the bundle_id required instead of providing a default?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe? We could push the default out to the apple_library macro if you think that's better, but we do need a default at some layer

resource_actions.merge_root_infoplists(
ios_app_ctx,
ctx.files.infoplists,
output_plist,
None,
bundle_id = bundle_id,
include_executable_name = False,
)

# This is a list of files to pass to bundletool using its format, this has
# a struct with a src and dest mapping that bundle tool uses to copy files
# https://github.com/bazelbuild/rules_apple/blob/d29df97b9652e0442ebf21f1bc0e04921b584f76/tools/bundletool/bundletool_experimental.py#L29-L35
control_files = []
input_files = []
output_files = []
output_bundle_dir = ctx.actions.declare_directory(paths.join(ctx.attr.name, bundle_name + ".bundle"))

# Need getattr since the partial_ouput struct will be empty when there are no resources.
# Even so, we want to generate a resource bundle for compatibility.
# TODO: add an attr to allow skipping when there are no resources entirely
bundle_files = getattr(partial_output, "bundle_files", [])

# `target_location` is a special identifier that tells you in a generic way
# where the resource should end up. This corresponds to:
# https://github.com/bazelbuild/rules_apple/blob/d29df97b9652e0442ebf21f1bc0e04921b584f76/apple/internal/processor.bzl#L107-L119
# in this use case both "resource" and "content" correspond to the root
# directory of the final Foo.bundle/
#
# `parent` is the directory the resource should be nested in
# (under `target_location`) for example Base.lproj would be the parent for
# a Localizable.strings file. If there is no `parent`, put it in the root
#
# `sources` is a depset of files or directories that we need to copy into
# the bundle. If it's a directory this likely means the compiler could
# output any number of files (like ibtool from a storyboard) and all the
# contents should be copied to the bundle (this is handled by bundletool)
for target_location, parent, sources in bundle_files:
sources_list = sources.to_list()
parent_output_directory = parent or ""
if target_location != "resource" and target_location != "content":
# For iOS resources these are the only ones we've hit, if we need
# to add more in the future we should be sure to double check where
# the need to end up
fail("Got unexpected target location '{}' for '{}'"
.format(target_location, sources_list))
input_files.extend(sources_list)
for source in sources_list:
target_path = parent_output_directory
if not source.is_directory:
target_path = paths.join(target_path, source.basename)
output_files.append(target_path)
control_files.append(struct(src = source.path, dest = target_path))

# Create a file for bundletool to know what files to copy
# https://github.com/bazelbuild/rules_apple/blob/d29df97b9652e0442ebf21f1bc0e04921b584f76/tools/bundletool/bundletool_experimental.py#L29-L46
bundletool_instructions = struct(
bundle_merge_files = control_files,
bundle_merge_zips = [],
output = output_bundle_dir.path,
code_signing_commands = "",
post_processor = "",
)
bundletool_instructions_file = intermediates.file(
ctx.actions,
ctx.label.name,
"bundletool_actions.json",
)
ctx.actions.write(
output = bundletool_instructions_file,
content = bundletool_instructions.to_json(),
)
ctx.actions.run(
executable = ctx.executable._bundletool_experimental,
mnemonic = "BundleResources",
progress_message = "Bundling " + bundle_name,
inputs = input_files + [bundletool_instructions_file],
outputs = [output_bundle_dir],
arguments = [bundletool_instructions_file.path],
)
return [
AppleResourceInfo(
unowned_resources = depset(),
owners = depset([
(output_bundle_dir.short_path, ctx.label),
(output_plist.short_path, ctx.label),
]),
# This is a list of the resources to propagate without changing further
# In this case the tuple parameters are:
# 1. The final directory the resources should end up in, ex Foo.bundle
# would result in Bar.app/Foo.bundle
# 2. The Swift module associated with the resources, this isn't
# required for us since we don't use customModuleProvider in IB
# 3. The resources to propagate, in our case this is just the final
# Foo.bundle directory that contains our real resources
unprocessed = [
(output_bundle_dir.basename, None, depset([output_bundle_dir, output_plist])),
],
),
AppleResourceBundleInfo(),
apple_common.new_objc_provider(),
]

precompiled_apple_resource_bundle = rule(
implementation = _precompiled_apple_resource_bundle_impl,
fragments = ["apple"],
cfg = transition_support.apple_rule_transition,
attrs = dict(
# This includes all the undocumented tool requirements for this rule
rule_factory.common_tool_attributes,
infoplists = attr.label_list(
allow_files = [".plist"],
default = [
Label("@build_bazel_rules_ios//rules/library:resource_bundle.plist"),
],
),
bundle_name = attr.string(mandatory = False),
bundle_id = attr.string(mandatory = False),
resources = attr.label_list(
allow_empty = True,
allow_files = True,
),
bundle_extension = attr.string(mandatory = False, default = "bundle"),
platforms = attr.string_dict(
mandatory = True,
),
_product_type = attr.string(default = apple_product_type.bundle),
# This badly named property is required even though this isn't an ipa
ipa_post_processor = attr.label(
allow_files = True,
executable = True,
cfg = "exec",
),
_environment_plist = attr.label(
allow_single_file = True,
# TODO: handle other platforms
default = Label("@build_bazel_rules_apple//apple/internal:environment_plist_ios"),
),
_whitelist_function_transition = attr.label(
default = Label("@bazel_tools//tools/whitelists/function_transition_whitelist"),
),
_xcode_config = attr.label(
default = configuration_field(
name = "xcode_config_label",
fragment = "apple",
),
),
),
)
Loading