bazelbuild / rules_apple

Bazel rules to build apps for Apple platforms.
Apache License 2.0
505 stars 260 forks source link

Resources are rebuilt for each target that includes them #319

Open keith opened 5 years ago

keith commented 5 years ago

Currently if you have this dependency layout:

FooTests (ios_unit_test) -> Foo (swift_library) -> FooResources (apple_resource_bundle)
BarTests (ios_unit_test) -> Bar (swift_library) -> Foo (from above)

When you build both FooTests and BarTests for example with bazel build //:FooTests //:BarTests you end up building FooResources twice.

If you have a large dependency tree this can end up being a bottle neck as it will rebuild the resources for each test target for all of the test targets transitive dependencies.

Ideally FooTests and BarTests would only build FooResources once if they had the same relevant configuration (for example iOS deployment target).

There are 2 known workarounds to this problem:

  1. Make all your test targets share a test host, and add Foo as a dependency of the test host.

The issues with this for are use case are:

a. Maintaining the list of dependencies in the test host's BUILD file would be a lot of overhead as we have over 100 targets with resource bundles b. If you just wanted to test something low in the dependency tree, you would still have to build all the dependencies you don't actually need to test it (this could rebuild each time as well if the dependencies depend on the target you're actually making changes to) c. In our case we actually get linker failures about missing symbols if we try this. I haven't attempted to debug. d. You end up relying on bundle loader which may be undesirable if your test host has code that you don't want test bundles to be able to rely on e. Your test bundles can no longer be run as logic tests, making them slower

  1. Create your own rule that "pre-compiles" the resource bundle (with a specific iOS version and other configuration) that is the propagated to dependencies.
Example rule (click to expand) ```python load("@bazel_skylib//lib:dicts.bzl", "dicts") 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:rule_factory.bzl", "rule_factory") load("@build_bazel_rules_apple//apple:providers.bzl", "AppleResourceInfo") def _compiled_resource_bundle(ctx): partial_output = partial.call(partials.resources_partial( bundle_id = "com.lyft." + ctx.label.name, plist_attrs = ["infoplists"], top_level_attrs = ["resources"], ), ctx) # 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(ctx.label.name + ".bundle") # `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 partial_output.bundle_files: 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.to_list())) input_files.extend(sources.to_list()) for source in sources: 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 " + ctx.label.name, inputs = input_files + [bundletool_instructions_file], outputs = [output_bundle_dir], arguments = [bundletool_instructions_file.path], ) return [ AppleResourceInfo( owners = {output_bundle_dir.short_path: depset(direct = [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, for resource # bundles Xcode passes the resource bundle's name, this doesn't # matter for us because we don't use customModuleProvider # 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])), ], ), ] compiled_resource_bundle = rule( implementation = _compiled_resource_bundle, fragments = ["apple"], attrs = dicts.add( # This includes all the undocumented tool requirements for this rule rule_factory.common_tool_attributes, { "infoplists": attr.label_list( allow_empty = False, allow_files = [".plist"], ), "resources": attr.label_list( allow_empty = False, allow_files = True, ), "minimum_os_version": attr.string(default = "10.0"), "platform_type": attr.string(default = str(apple_common.platform_type.ios)), "_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 = "host", ), }, ), ) ```

The issues with this are:

a. Relies on a lot of implementation details in rules_apple that are likely a moving target b. Requires you to know up front for all targets what are acceptable configuration values, this breaks the ability to use the resource bundle for multiple targets that actually have different configuration, such as a macOS target and an iOS target c. This breaks at least 1 internal Google use case where resource bundles with the same name are combined at the end of the build process, in this case they would just conflict d. Since you're pre-building the bundles this implementation doesn't know what source code modules it will be related to, because of this it does not pass swift_module which is required if you use customModuleProvider for xibs / storyboards (this might still work when using static libraries if the loader falls back to the main bundle anyways) e. It relies on this patch https://github.com/bazelbuild/rules_apple/pull/318

sergiocampama commented 5 years ago

I believe this is also an issue with Xcode no? If you have multiple tests targets, the resources would be processed for all of them as well. I replied in #318 for a way to make your custom rule not require the rule descriptor.

keith commented 5 years ago

That doesn't seem to be the case, in our case when running all of our tests with bazel it would take >1 hour (I never waited for it to finish) where as with Xcode it would take <20 minutes. Using our example rule made them almost identical.

keith commented 5 years ago

Another case that's very similar to this is the environment.plist. This is built for each bundle, even though the result ends up being the same. For us this means we're building hundreds of these at ~4 seconds a piece

keith commented 5 years ago

This should fix the environment.plist issues https://github.com/bazelbuild/rules_apple/pull/522

segiddins commented 4 years ago

It's not an issue with Xcode for us, since we use resource bundles, which get compiled once like any other target and then the compiled bundle just gets copied over into each test bundle. I didn't let the build of our tests finish, but it was going over 30 minutes just on re-compiling asset catalogs, which is almost more time than we spend compiling code.

ajanuar commented 3 years ago

Is this still an issue?

keith commented 3 years ago

Yes. The workarounds are either to use a rule like the one above, or set the same test host for all your test targets

ajanuar commented 3 years ago

@keith I can't build with rule above, it requires adjustment to latest rules_apple. Can you share that rule with that adjustment? Sorry, I am still new to this rule, it may take some time to understand. Thanks.

keith commented 3 years ago
Updated rule (click to expand) ```bzl """ This provides a resource bundle implementation that builds the resource bundle only once for iOS 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:config.bzl", "MINIMUM_IOS_VERSION") load("@bazel_skylib//lib:partial.bzl", "partial") load("@bazel_skylib//lib:paths.bzl", "paths") load("@build_bazel_rules_apple//apple/internal:partials.bzl", "partials") # buildifier: disable=bzl-visibility load("@build_bazel_rules_apple//apple/internal:rule_factory.bzl", "rule_factory") # buildifier: disable=bzl-visibility load("@build_bazel_rules_apple//apple/internal:platform_support.bzl", "platform_support") # buildifier: disable=bzl-visibility load("@build_bazel_rules_apple//apple:providers.bzl", "AppleResourceInfo", "AppleSupportToolchainInfo") def _compiled_resource_bundle(ctx): apple_toolchain_info = ctx.attr._toolchain[AppleSupportToolchainInfo] platform_prerequisites = platform_support.platform_prerequisites( apple_fragment = ctx.fragments.apple, config_vars = ctx.var, device_families = ["iphone"], disabled_features = ctx.disabled_features, explicit_minimum_os = MINIMUM_IOS_VERSION, features = ctx.features, objc_fragment = None, platform_type_string = str(ctx.fragments.apple.single_arch_platform.platform_type), uses_swift = False, xcode_path_wrapper = ctx.executable._xcode_path_wrapper, xcode_version_config = ctx.attr._xcode_config[apple_common.XcodeVersionConfig], ) rule_descriptor = struct( additional_infoplist_values = None, binary_infoplist = True, bundle_extension = ".bundle", bundle_package_type = None, product_type = "com.apple.product-type.bundle", # apple_product_type.bundle requires_pkginfo = False, ) partial_output = partial.call(partials.resources_partial( actions = ctx.actions, apple_toolchain_info = apple_toolchain_info, bundle_extension = rule_descriptor.bundle_extension, bundle_id = "com.lyft." + ctx.label.name, bundle_name = ctx.label.name, executable_name = None, environment_plist = ctx.file._environment_plist, launch_storyboard = None, platform_prerequisites = platform_prerequisites, plist_attrs = ["infoplist"], rule_attrs = ctx.attr, rule_descriptor = rule_descriptor, rule_label = ctx.label, top_level_attrs = ["resources"], )) # 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(ctx.label.name + ".bundle") # `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 partial_output.bundle_files: 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.to_list())) input_files.extend(sources.to_list()) for source in sources.to_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 = ctx.actions.declare_file( paths.join( "{}-intermediates".format(ctx.label.name), "bundletool_actions.json", ), ) ctx.actions.write( output = bundletool_instructions_file, content = json.encode(bundletool_instructions), ) resolved_bundletool = apple_toolchain_info.resolved_bundletool_experimental ctx.actions.run( executable = resolved_bundletool.executable, mnemonic = "BundleResources", progress_message = "Bundling " + ctx.label.name, inputs = depset( input_files + [bundletool_instructions_file], transitive = [resolved_bundletool.inputs], ), input_manifests = resolved_bundletool.input_manifests, outputs = [output_bundle_dir], arguments = [bundletool_instructions_file.path], ) return [ AppleResourceInfo( unowned_resources = depset(), owners = depset([(output_bundle_dir.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])), ], ), ] compiled_resource_bundle = rule( implementation = _compiled_resource_bundle, fragments = ["apple"], attrs = dict( rule_factory.common_tool_attributes, infoplist = attr.label( allow_files = [".plist"], ), resources = attr.label_list( allow_empty = False, allow_files = True, ), _environment_plist = attr.label( allow_single_file = True, default = "@build_bazel_rules_apple//apple/internal:environment_plist_ios", ), ), ) ```
lucasromanomr commented 3 months ago

I ended up having the same problem, I started just using rules_apple because before I used rules_ios. When I went to run my test targets the first time I was surprised.

If anyone has the updated code it would help a lot.

For now I'm using an adaptation of https://github.com/bazel-ios/rules_ios/blob/master/rules/precompiled_apple_resource_bundle.bzl