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

Cherry pick "Add support for alternate app icons in UIKit via actool in Xcode 14+..." #5

Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions apple/internal/ios_rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ def _ios_application_impl(ctx):
launch_storyboard = ctx.file.launch_storyboard,
locales_to_include = ctx.attr.locales_to_include,
platform_prerequisites = platform_prerequisites,
primary_icon_name = ctx.attr.primary_app_icon,
resource_deps = resource_deps,
rule_descriptor = rule_descriptor,
rule_label = label,
Expand Down Expand Up @@ -2518,6 +2519,7 @@ ios_application = rule_factory.create_apple_rule(
rule_attrs.app_icon_attrs(
icon_extension = ".appiconset",
icon_parent_extension = ".xcassets",
supports_alternate_icons = True,
Copy link
Member

Choose a reason for hiding this comment

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

[question] Is this safe and backward compatible?

Copy link
Author

@vakhidbetrakhmadov vakhidbetrakhmadov Jul 26, 2024

Choose a reason for hiding this comment

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

It gotta be. I assume it wouldn't have landed in upstream otherwise.

Btw, this is a clean cherry-pick of the original commit 6fc1952, with only formatting and docs being changed from my side (through repo's automation).

),
rule_attrs.app_intents_attrs(
deps_cfg = transition_support.apple_platform_split_transition,
Expand Down
6 changes: 6 additions & 0 deletions apple/internal/partials/resources.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ def _resources_partial_impl(
locales_to_include,
output_discriminator,
platform_prerequisites,
primary_icon_name,
resource_deps,
rule_descriptor,
rule_label,
Expand Down Expand Up @@ -296,6 +297,7 @@ def _resources_partial_impl(
"output_discriminator": output_discriminator,
"parent_dir": parent_dir,
"platform_prerequisites": platform_prerequisites,
"primary_icon_name": primary_icon_name,
"product_type": rule_descriptor.product_type,
"rule_label": rule_label,
}
Expand Down Expand Up @@ -398,6 +400,7 @@ def resources_partial(
locales_to_include = [],
output_discriminator = None,
platform_prerequisites,
primary_icon_name = None,
resource_deps,
rule_descriptor,
rule_label,
Expand Down Expand Up @@ -438,6 +441,8 @@ def resources_partial(
output_discriminator: A string to differentiate between different target intermediate files
or `None`.
platform_prerequisites: Struct containing information on the platform being targeted.
primary_icon_name: An optional String to identify the name of the primary app icon when
alternate app icons have been provided for the app.
resource_deps: A list of dependencies that the resource aspect has been applied to.
rule_descriptor: A rule descriptor for platform and product types from the rule context.
rule_label: The label of the target being analyzed.
Expand Down Expand Up @@ -471,6 +476,7 @@ def resources_partial(
locales_to_include = locales_to_include,
output_discriminator = output_discriminator,
platform_prerequisites = platform_prerequisites,
primary_icon_name = primary_icon_name,
resource_deps = resource_deps,
rule_descriptor = rule_descriptor,
rule_label = rule_label,
Expand Down
3 changes: 2 additions & 1 deletion apple/internal/partials/support/resources_support.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ def _asset_catalogs(
output_discriminator,
parent_dir,
platform_prerequisites,
primary_icon_name,
product_type,
rule_label,
**_kwargs):
Expand All @@ -159,7 +160,6 @@ def _asset_catalogs(
assets_plist = None
infoplists = []
if not parent_dir:
# TODO(kaipi): Merge this into the top level Info.plist.
Copy link
Member

Choose a reason for hiding this comment

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

[question] Why remove this TODO?

Copy link
Author

@vakhidbetrakhmadov vakhidbetrakhmadov Jul 26, 2024

Choose a reason for hiding this comment

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

I don't know.

This is a clean cherry-pick of the original commit 6fc1952, with only formatting and docs being changed from my side (through repo's automation).

assets_plist_path = paths.join(parent_dir or "", "xcassets-info.plist")
assets_plist = intermediates.file(
actions = actions,
Expand Down Expand Up @@ -211,6 +211,7 @@ def _asset_catalogs(
output_dir = assets_dir,
output_plist = assets_plist,
platform_prerequisites = platform_prerequisites,
primary_icon_name = primary_icon_name,
product_type = product_type,
rule_label = rule_label,
xctoolrunner = apple_mac_toolchain_info.xctoolrunner,
Expand Down
1 change: 1 addition & 0 deletions apple/internal/resource_actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ bzl_library(
"//apple/internal/utils:xctoolrunner",
"@bazel_skylib//lib:collections",
"@bazel_skylib//lib:paths",
"@bazel_skylib//lib:sets",
"@build_bazel_apple_support//lib:apple_support",
],
)
Expand Down
64 changes: 58 additions & 6 deletions apple/internal/resource_actions/actool.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ load(
"@build_bazel_rules_apple//apple/internal:intermediates.bzl",
"intermediates",
)
load(
"@bazel_skylib//lib:sets.bzl",
"sets",
)
load(
"@build_bazel_apple_support//lib:apple_support.bzl",
"apple_support",
Expand Down Expand Up @@ -50,6 +54,7 @@ def _actool_args_for_special_file_types(
asset_files,
bundle_id,
platform_prerequisites,
primary_icon_name,
product_type):
"""Returns command line arguments needed to compile special assets.

Expand All @@ -63,6 +68,8 @@ def _actool_args_for_special_file_types(
asset_files: The asset catalog files.
bundle_id: The bundle ID to configure for this target.
platform_prerequisites: Struct containing information on the platform being targeted.
primary_icon_name: An optional String to identify the name of the primary app icon when
alternate app icons have been provided for the app.
product_type: The product type identifier used to describe the current bundle type.

Returns:
Expand Down Expand Up @@ -124,14 +131,55 @@ def _actool_args_for_special_file_types(
[appicon_extension],
attr = "app_icons",
).keys()
if len(icon_dirs) != 1:
if len(icon_dirs) != 1 and not primary_icon_name:
formatted_dirs = "[\n %s\n]" % ",\n ".join(icon_dirs)
fail("The asset catalogs should contain exactly one directory named " +
"*.%s among its asset catalogs, " % appicon_extension +
"but found the following: " + formatted_dirs, "app_icons")

app_icon_name = paths.split_extension(paths.basename(icon_dirs[0]))[0]
args += ["--app-icon", app_icon_name]
# Alternate icons are only supported for UIKit applications on iOS, tvOS, visionOS and
# iOS-on-macOS (Catalyst)
if (platform_prerequisites.platform_type == apple_common.platform_type.watchos or
platform_prerequisites.platform_type == apple_common.platform_type.macos or
product_type != apple_product_type.application):
fail("The asset catalogs should contain exactly one directory named " +
"*.%s among its asset catalogs, " % appicon_extension +
"but found the following: " + formatted_dirs, "app_icons")
else:
fail("""
Found multiple app icons among the asset catalogs with no primary_app_icon assigned.

If you intend to assign multiple app icons to this target, please declare which of these is intended
to be the primary app icon with the primary_app_icon attribute on the rule itself.

app_icons was assigned the following: {formatted_dirs}
""".format(formatted_dirs = formatted_dirs))
elif primary_icon_name:
# Check that primary_icon_name matches one of the icon sets, then add actool arguments
# for `--alternate-app-icon` and `--app_icon` as appropriate. These do NOT overlap.
app_icon_names = sets.make()
for icon_dir in icon_dirs:
app_icon_names = sets.insert(
app_icon_names,
paths.split_extension(paths.basename(icon_dir))[0],
)
app_icon_name_list = sets.to_list(app_icon_names)
found_primary = False
for app_icon_name in app_icon_name_list:
if app_icon_name == primary_icon_name:
found_primary = True
args += ["--app-icon", primary_icon_name]
else:
args += ["--alternate-app-icon", app_icon_name]
if not found_primary:
fail("""
Could not find the primary icon named "{primary_icon_name}" in the list of app_icons provided.

Found the following icon names from those provided: {app_icon_names}.
""".format(
primary_icon_name = primary_icon_name,
app_icon_names = ", ".join(app_icon_name_list),
))
else:
app_icon_name = paths.split_extension(paths.basename(icon_dirs[0]))[0]
args += ["--app-icon", app_icon_name]

# Add arguments for watch extension complication, if there is one.
complication_files = [f for f in asset_files if ".complicationset/" in f.path]
Expand Down Expand Up @@ -193,6 +241,7 @@ def compile_asset_catalog(
output_dir,
output_plist,
platform_prerequisites,
primary_icon_name,
product_type,
rule_label,
xctoolrunner):
Expand All @@ -217,6 +266,8 @@ def compile_asset_catalog(
output_plist: The file reference for the output plist that should be merged
into Info.plist. May be None if the output plist is not desired.
platform_prerequisites: Struct containing information on the platform being targeted.
primary_icon_name: An optional String to identify the name of the primary app icon when
alternate app icons have been provided for the app.
product_type: The product type identifier used to describe the current bundle type.
rule_label: The label of the target being analyzed.
xctoolrunner: A files_to_run for the wrapper around the "xcrun" tool.
Expand All @@ -239,6 +290,7 @@ def compile_asset_catalog(
asset_files = asset_files,
bundle_id = bundle_id,
platform_prerequisites = platform_prerequisites,
primary_icon_name = primary_icon_name,
product_type = product_type,
))
args.extend(collections.before_each(
Expand Down
20 changes: 18 additions & 2 deletions apple/internal/rule_attrs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -651,16 +651,22 @@ hermetic given these inputs to ensure that the result can be safely cached.
),
}

def _app_icon_attrs(*, icon_extension = ".appiconset", icon_parent_extension = ".xcassets"):
def _app_icon_attrs(
*,
icon_extension = ".appiconset",
icon_parent_extension = ".xcassets",
supports_alternate_icons = False):
"""Returns the attribute required to define app icons for the given target.

Args:
icon_extension: A String representing the extension required of the directory containing the
app icon assets. Optional. Defaults to `.appiconset`.
icon_parent_extension: A String representing the extension required of the parent directory
of the directory containing the app icon assets. Optional. Defaults to `.xcassets`.
supports_alternate_icons: Bool representing if the rule supports alternate icons. False by
default.
"""
return {
app_icon_attrs = {
"app_icons": attr.label_list(
allow_files = True,
doc = """
Expand All @@ -672,6 +678,16 @@ named `*.{app_icon_parent_extension}/*.{app_icon_extension}` and there may be on
),
),
}
if supports_alternate_icons:
app_icon_attrs = dicts.add(app_icon_attrs, {
"primary_app_icon": attr.string(
doc = """
An optional String to identify the name of the primary app icon when alternate app icons have been
provided for the app.
""",
),
})
return app_icon_attrs

def _launch_images_attrs():
"""Returns the attribute required to support launch images for a given target."""
Expand Down
5 changes: 4 additions & 1 deletion apple/internal/tvos_rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,7 @@ def _tvos_application_impl(ctx):
launch_storyboard = ctx.file.launch_storyboard,
locales_to_include = ctx.attr.locales_to_include,
platform_prerequisites = platform_prerequisites,
primary_icon_name = ctx.attr.primary_app_icon,
resource_deps = resource_deps,
rule_descriptor = rule_descriptor,
rule_label = label,
Expand Down Expand Up @@ -1449,7 +1450,9 @@ tvos_application = rule_factory.create_apple_rule(
is_executable = True,
predeclared_outputs = {"archive": "%{name}.ipa"},
attrs = [
rule_attrs.app_icon_attrs(),
rule_attrs.app_icon_attrs(
supports_alternate_icons = True,
),
rule_attrs.app_intents_attrs(
deps_cfg = transition_support.apple_platform_split_transition,
),
Expand Down
2 changes: 2 additions & 0 deletions apple/internal/visionos_rules.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ Resolved Xcode is version {xcode_version}.
launch_storyboard = None,
locales_to_include = ctx.attr.locales_to_include,
platform_prerequisites = platform_prerequisites,
primary_icon_name = ctx.attr.primary_app_icon,
resource_deps = resource_deps,
rule_descriptor = rule_descriptor,
rule_label = label,
Expand Down Expand Up @@ -1445,6 +1446,7 @@ visionos_application = rule_factory.create_apple_rule(
rule_attrs.app_icon_attrs(
icon_extension = ".solidimagestack",
icon_parent_extension = ".xcassets",
supports_alternate_icons = True,
),
rule_attrs.app_intents_attrs(
deps_cfg = transition_support.apple_platform_split_transition,
Expand Down
5 changes: 3 additions & 2 deletions doc/rules-ios.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ ios_application(<a href="#ios_application-name">name</a>, <a href="#ios_applicat
<a href="#ios_application-exported_symbols_lists">exported_symbols_lists</a>, <a href="#ios_application-extensions">extensions</a>, <a href="#ios_application-families">families</a>, <a href="#ios_application-frameworks">frameworks</a>, <a href="#ios_application-include_symbols_in_bundle">include_symbols_in_bundle</a>,
<a href="#ios_application-infoplists">infoplists</a>, <a href="#ios_application-ipa_post_processor">ipa_post_processor</a>, <a href="#ios_application-launch_images">launch_images</a>, <a href="#ios_application-launch_storyboard">launch_storyboard</a>, <a href="#ios_application-linkopts">linkopts</a>,
<a href="#ios_application-locales_to_include">locales_to_include</a>, <a href="#ios_application-minimum_deployment_os_version">minimum_deployment_os_version</a>, <a href="#ios_application-minimum_os_version">minimum_os_version</a>, <a href="#ios_application-platform_type">platform_type</a>,
<a href="#ios_application-provisioning_profile">provisioning_profile</a>, <a href="#ios_application-sdk_frameworks">sdk_frameworks</a>, <a href="#ios_application-settings_bundle">settings_bundle</a>, <a href="#ios_application-shared_capabilities">shared_capabilities</a>, <a href="#ios_application-stamp">stamp</a>,
<a href="#ios_application-strings">strings</a>, <a href="#ios_application-version">version</a>, <a href="#ios_application-watch_application">watch_application</a>)
<a href="#ios_application-primary_app_icon">primary_app_icon</a>, <a href="#ios_application-provisioning_profile">provisioning_profile</a>, <a href="#ios_application-sdk_frameworks">sdk_frameworks</a>, <a href="#ios_application-settings_bundle">settings_bundle</a>,
<a href="#ios_application-shared_capabilities">shared_capabilities</a>, <a href="#ios_application-stamp">stamp</a>, <a href="#ios_application-strings">strings</a>, <a href="#ios_application-version">version</a>, <a href="#ios_application-watch_application">watch_application</a>)
</pre>

Builds and bundles an iOS Application.
Expand Down Expand Up @@ -105,6 +105,7 @@ Builds and bundles an iOS Application.
| <a id="ios_application-minimum_deployment_os_version"></a>minimum_deployment_os_version | A required string indicating the minimum deployment OS version supported by the target, represented as a dotted version number (for example, "9.0"). This is different from `minimum_os_version`, which is effective at compile time. Ensure version specific APIs are guarded with `available` clauses. | String | optional | `""` |
| <a id="ios_application-minimum_os_version"></a>minimum_os_version | A required string indicating the minimum OS version supported by the target, represented as a dotted version number (for example, "9.0"). | String | required | |
| <a id="ios_application-platform_type"></a>platform_type | - | String | optional | `"ios"` |
| <a id="ios_application-primary_app_icon"></a>primary_app_icon | An optional String to identify the name of the primary app icon when alternate app icons have been provided for the app. | String | optional | `""` |
| <a id="ios_application-provisioning_profile"></a>provisioning_profile | The provisioning profile (`.mobileprovision` file) to use when creating the bundle. This value is optional for simulator builds as the simulator doesn't fully enforce entitlements, but is required for device builds. | <a href="https://bazel.build/concepts/labels">Label</a> | optional | `None` |
| <a id="ios_application-sdk_frameworks"></a>sdk_frameworks | Names of SDK frameworks to link with (e.g., `AddressBook`, `QuartzCore`). `UIKit` and `Foundation` are always included, even if this attribute is provided and does not list them.<br><br>This attribute is discouraged; in general, targets should list system framework dependencies in the library targets where that framework is used, not in the top-level bundle. | List of strings | optional | `[]` |
| <a id="ios_application-settings_bundle"></a>settings_bundle | A resource bundle (e.g. `apple_bundle_import`) target that contains the files that make up the application's settings bundle. These files will be copied into the root of the final application bundle in a directory named `Settings.bundle`. | <a href="https://bazel.build/concepts/labels">Label</a> | optional | `None` |
Expand Down
Loading