Align actool with upstream (#2804)

Author: adincebic

apple/internal/bundling_support.bzl

@@ -14,6 +14,10 @@
 
 """Low-level bundling name helpers."""
 
+load(
+    "@bazel_skylib//lib:new_sets.bzl",
+    "sets",
+)
 load(
     "//apple:providers.bzl",
     "AppleBaseBundleIdInfo",
@@ -248,24 +252,37 @@ See https://github.com/bazelbuild/rules_apple/blob/master/doc/shared_capabilitie
         suffix_default = suffix_default,
     )
 
+def _ensure_asset_catalog_files_not_in_xcassets(
+        *,
+        extension,
+        files,
+        message = None):
+    """Validates that a subset of asset catalog files are not within an xcassets directory.
+
+    Args:
+      extension: The extension that should be used for the this particular asset that should never
+          be found within the xcassets directory.
+      files: An iterable of files to use.
+      message: A custom error message to use, the list of found files that were found in xcassets
+          directories will be printed afterwards.
+    """
+    _ensure_path_format(
+        files = files,
+        allowed_path_fragments = [],
+        denied_path_fragments = ["xcassets", extension],
+        message = message,
+    )
+
 def _ensure_single_xcassets_type(
         *,
-        additional_path_fragments = [],
-        attr,
         extension,
         files,
         message = None):
-    """Helper for when an xcassets catalog should have a single sub type.
+    """Validates that asset catalog files are nested within an xcassets directory.
 
     Args:
-      additional_path_fragments: Additional path fragments to check for in the path (in order), used
-          to handle actool inputs that don't use the `xcassets` directory, such as Xcode 26 `icon`
-          bundles generated by the Icon Composer tool.
-      attr: The attribute to associate with the build failure if the list of
-          files has an element that is not in a directory with the given
-          extension.
-      extension: The extension that should be used for the different asset
-          type witin the catalog.
+      extension: The extension that should be used for the this particular asset within the xcassets
+          directory.
       files: An iterable of files to use.
       message: A custom error message to use, the list of found files that
           didn't match will be printed afterwards.
@@ -274,9 +291,9 @@ def _ensure_single_xcassets_type(
         message = ("Expected the xcassets directory to only contain files " +
                    "are in sub-directories with the extension %s") % extension
     _ensure_path_format(
-        attr = attr,
         files = files,
-        path_fragments_list = [["xcassets", extension], additional_path_fragments],
+        allowed_path_fragments = ["xcassets", extension],
+        denied_path_fragments = [],
         message = message,
     )
 
@@ -307,63 +324,55 @@ def _path_is_under_fragments(path, path_fragments):
 
     return True
 
-def _ensure_path_format(*, attr, files, path_fragments_list, message = None):
+def _ensure_path_format(
+        *,
+        files,
+        allowed_path_fragments,
+        denied_path_fragments,
+        message = None):
     """Ensure the files match the required path fragments.
 
-    TODO(b/77804841): The places calling this should go away and these types of
-    checks should be done during the resource processing. Right now these checks
-    are being wedged in at the attribute collection steps, and they then get
-    combined into a single list of resources; the bundling then resplits them
-    up in groups to process they by type. So the more validation/splitting done
-    here the slower things get (as double work is done). The bug is to revisit
-    all of this and instead pass through individual things in a structured way
-    so they don't have to be resplit. That would allow the validation to be
-    done while processing (in a single pass) instead.
-
     Args:
-      attr: The attribute to associate with the build failure if the list of
-          files has an element that is not in a directory with the given
-          extension.
       files: An iterable of files to use.
-      path_fragments_list: A list of lists, each inner lists is a sequence of
-          extensions that must be on the paths passed in (to ensure proper
-          nesting).
+      allowed_path_fragments: A list representing a sequence of extensions where each file path
+          passed in MUST MATCH the sequence to ensure proper nesting. If this is provided,
+          denied_path_fragments must be empty.
+      denied_path_fragments: A list representing a sequence of extensions where each file path
+          passed in MUST NOT MATCH the sequence to ensure proper nesting. If this is provided,
+          allowed_path_fragments must be empty.
       message: A custom error message to use, the list of found files that
           didn't match will be printed afterwards.
     """
 
-    formatted_path_fragments_list = []
-    for x in path_fragments_list:
-        formatted_path_fragments_list.append([".%s/" % y for y in x])
+    if allowed_path_fragments and denied_path_fragments:
+        fail("""
+Internal Error: Both allowed_path_fragments and denied_path_fragments were provided, but only one \
+of them should be provided.
 
-    # Just check that the paths include the expected nesting. More complete
-    # checks would likely be the number of outer directories with that suffix,
-    # the number of inner ones, extra directories segments where not expected,
-    # etc.
-    bad_paths = {}
-    for f in files:
-        path = f.path
+Please file an issue on the Apple BUILD Rules.
+""")
 
-        was_good = False
-        for path_fragments in formatted_path_fragments_list:
-            if _path_is_under_fragments(path, path_fragments):
-                was_good = True
-                break  # No need to check other fragments
+    formatted_path_fragments = []
+    for x in allowed_path_fragments + denied_path_fragments:
+        formatted_path_fragments.append(".%s/" % x)
+    allow_path_under_fragments = bool(allowed_path_fragments)
 
-        if not was_good:
-            bad_paths[path] = None
+    bad_paths = sets.make()
+    for f in files:
+        path = f.path
+        if _path_is_under_fragments(path, formatted_path_fragments) != allow_path_under_fragments:
+            sets.insert(bad_paths, path)
 
-    if len(bad_paths):
+    if sets.length(bad_paths):
         if not message:
-            as_paths = [
-                ("*" + "*".join(x) + "...")
-                for x in formatted_path_fragments_list
-            ]
-            message = "Expected only files inside directories named '*.%s'" % (
-                ", ".join(as_paths)
+            message_prefix = (
+                "Expected only " if allow_path_under_fragments else "Did not expect any "
             )
-        formatted_paths = "[\n  %s\n]" % ",\n  ".join(bad_paths.keys())
-        fail("%s, but found the following: %s" % (message, formatted_paths), attr)
+            as_path = "*" + "*".join(formatted_path_fragments) + "..."
+            message = message_prefix + "files inside directories named '*.%s'" % (as_path)
+
+        formatted_paths = "[\n  %s\n]" % ",\n  ".join(sets.to_list(bad_paths))
+        fail("%s, but found the following: %s" % (message, formatted_paths))
 
 def _validate_bundle_id(bundle_id):
     """Ensure the value is a valid bundle it or fail the build.
@@ -393,7 +402,7 @@ def _validate_bundle_id(bundle_id):
 bundling_support = struct(
     bundle_full_name = _bundle_full_name,
     bundle_full_id = _bundle_full_id,
-    ensure_path_format = _ensure_path_format,
+    ensure_asset_catalog_files_not_in_xcassets = _ensure_asset_catalog_files_not_in_xcassets,
     ensure_single_xcassets_type = _ensure_single_xcassets_type,
     validate_bundle_id = _validate_bundle_id,
 )

apple/internal/partials/BUILD

@@ -15,7 +15,6 @@ bzl_library(
     ],
     deps = [
         "//apple/internal:apple_product_type",
-        "//apple/internal:bundling_support",
         "@bazel_skylib//lib:partial",
     ],
 )

apple/internal/partials/app_assets_validation.bzl

@@ -22,12 +22,26 @@ load(
     "//apple/internal:apple_product_type.bzl",
     "apple_product_type",
 )
-load(
-    "//apple/internal:bundling_support.bzl",
-    "bundling_support",
-)
 
-_supports_visionos = hasattr(apple_common.platform_type, "visionos")
+# Standard icon extensions as of Xcode 26 for most Apple platforms (iOS, macOS, watchOS).
+_STANDARD_ICONS = [".appiconset/", ".icon/"]
+
+# Valid icon extensions for specific product types that have exceptional requirements, independent
+# of platform.
+_VALID_ICON_EXTENSIONS_FOR_PRODUCT_TYPE = {
+    apple_product_type.messages_extension: [".stickersiconset/"],
+    apple_product_type.messages_sticker_pack_extension: [".stickersiconset/", ".stickerpack/", ".sticker/", ".stickersequence/"],
+}
+
+# Comprehensive list of all valid icon extensions for each platform. These cover apps, extensions,
+# app clips, and bundle types that can use icons.
+_VALID_ICON_EXTENSIONS_FOR_PLATFORM = {
+    "ios": _STANDARD_ICONS,
+    "macos": _STANDARD_ICONS,
+    "tvos": [".brandassets/"],
+    "watchos": _STANDARD_ICONS,
+    "visionos": [".solidimagestack/"],
+}
 
 def _app_assets_validation_partial_impl(
         *,
@@ -37,74 +51,49 @@ def _app_assets_validation_partial_impl(
         product_type):
     """Implementation for the app assets processing partial."""
 
+    # actool.bzl has the most comprehensive validations since it evaluates the final set of files
+    # before they are sent to actool. We only check here that the user is sending files that look
+    # like they could be app icons via an attribute named `app_icons`, and likewise for launch
+    # images.
+
     if app_icons:
-        if product_type == apple_product_type.messages_extension:
-            message = ("Message extensions must use Messages Extensions Icon Sets " +
-                       "(named .stickersiconset), not traditional App Icon Sets")
-            bundling_support.ensure_single_xcassets_type(
-                attr = "app_icons",
-                extension = "stickersiconset",
-                files = app_icons,
-                message = message,
-            )
-        elif product_type == apple_product_type.messages_sticker_pack_extension:
-            path_fragments = [
-                # Replacement for appiconset.
-                ["xcstickers", "stickersiconset"],
-                # The stickers.
-                ["xcstickers", "stickerpack", "sticker"],
-                ["xcstickers", "stickerpack", "stickersequence"],
-            ]
-            message = (
-                "Message StickerPack extensions use an asset catalog named " +
-                "*.xcstickers. Their main icons use *.stickersiconset; and then " +
-                "under the Sticker Pack (*.stickerpack) goes the Stickers " +
-                "(named *.sticker) and/or Sticker Sequences (named " +
-                "*.stickersequence)"
-            )
-            bundling_support.ensure_path_format(
-                attr = "app_icons",
-                files = app_icons,
-                path_fragments_list = path_fragments,
-                message = message,
-            )
-        elif platform_prerequisites.platform_type == apple_common.platform_type.tvos:
-            bundling_support.ensure_single_xcassets_type(
-                attr = "app_icons",
-                extension = "brandassets",
-                files = app_icons,
-            )
-        elif (_supports_visionos and
-              platform_prerequisites.platform_type == apple_common.platform_type.visionos):
-            message = ("visionOS apps must use visionOS app icon layers grouped in " +
-                       ".solidimagestack bundles, not traditional App Icon Sets")
-            bundling_support.ensure_single_xcassets_type(
-                attr = "app_icons",
-                extension = "solidimagestack",
-                files = app_icons,
-                message = message,
-            )
-        else:
-            # For Xcode 26, appiconset-s can live alongside iOS/macOS/watchOS .icon bundle contents
-            # to ensure rendering is correct for pre-26 Apple OSes, so we need to make that
-            # exception here.
-            additional_path_fragments = []
-            xcode_version = platform_prerequisites.xcode_version_config.xcode_version()
-            if xcode_version >= apple_common.dotted_version("26.0"):
-                additional_path_fragments.append("icon")
-            bundling_support.ensure_single_xcassets_type(
-                additional_path_fragments = additional_path_fragments,
-                attr = "app_icons",
-                extension = "appiconset",
-                files = app_icons,
-            )
+        valid_icon_extensions = (
+            _VALID_ICON_EXTENSIONS_FOR_PRODUCT_TYPE.get(product_type, None) or
+            _VALID_ICON_EXTENSIONS_FOR_PLATFORM[str(platform_prerequisites.platform_type)]
+        )
+        for resource in app_icons:
+            resource_short_path = resource.short_path
+            possible_valid_icon = False
+            for valid_icon_extension in valid_icon_extensions:
+                if valid_icon_extension in resource_short_path:
+                    possible_valid_icon = True
+                    break
+            if (not possible_valid_icon and
+                not resource_short_path.endswith(".xcassets/Contents.json") and
+                not resource_short_path.endswith(".xcstickers/Contents.json")):
+                fail("""
+Found in app_icons a file that cannot be used as an app icon:
+{resource_short_path}
+
+Valid icon bundles for this target have the following extensions: {valid_icon_extensions}
+""".format(
+                    resource_short_path = resource_short_path,
+                    valid_icon_extensions = valid_icon_extensions,
+                ))
 
     if launch_images:
-        bundling_support.ensure_single_xcassets_type(
-            attr = "launch_images",
-            extension = "launchimage",
-            files = launch_images,
-        )
+        for resource in launch_images:
+            resource_short_path = resource.short_path
+            if (not ".launchimage/" in resource_short_path and
+                not resource_short_path.endswith(".xcassets/Contents.json")):
+                fail("""
+Found in launch_images a file that cannot be used as a launch image:
+{resource_short_path}
+
+All launch images must be in a directory named '*.launchimage' within an '*.xcassets' directory.
+""".format(
+                    resource_short_path = resource_short_path,
+                ))
 
     return struct()
 

apple/internal/resource_actions/BUILD

@@ -16,9 +16,9 @@ bzl_library(
     deps = [
         "//apple:utils",
         "//apple/internal:apple_product_type",
+        "//apple/internal:bundling_support",
         "//apple/internal/utils:xctoolrunner",
         "@bazel_skylib//lib:paths",
-        "@bazel_skylib//lib:sets",
         "@build_bazel_apple_support//lib:apple_support",
     ],
 )