.DS_Store inside *AppIcons.xcassets causes validation error

[jerrymarino]

If a user opens a *AppIcons.xcassets in the finder ( or creates a .DS_Store in some other way ), it will cause a validation error.

ERROR: /Users/jerrymarino/Projects/ios-review/Pinterest/iOS/App/BUILD:81:1: in ios_application rule //Pinterest/iOS/App:Pinterest: Traceback (most recent call last):
        File "/Users/jerrymarino/Projects/ios-review/Pinterest/iOS/App/BUILD", line 81
                ios_application(name = 'Pinterest')
        File "/private/var/tmp/_bazel_jerrymarino/dab264afbccc7ab80d8809aaba91b923/external/build_bazel_rules_apple/apple/bundling/ios_rules.bzl", line 78, in _ios_application_impl
                bundling_support.ensure_single_xcassets_type("app_icons", app_icons, "appiconset")
        File "/private/var/tmp/_bazel_jerrymarino/dab264afbccc7ab80d8809aaba91b923/external/build_bazel_rules_apple/apple/bundling/bundling_support.bzl", line 322, in bundling_support.ensure_single_xcassets_type
                _ensure_path_format(attr, files, [["xcassets", extension...], ...)
        File "/private/var/tmp/_bazel_jerrymarino/dab264afbccc7ab80d8809aaba91b923/external/build_bazel_rules_apple/apple/bundling/bundling_support.bzl", line 407, in _ensure_path_format
                fail(("%s, but found the following: %...)), ...)
attribute app_icons: Expected the xcassets directory to only contain files are in sub-directories with the extension appiconset, but found the following: [
  Pinterest/iOS/App/Resources/ProdAppIcons.xcassets/.DS_Store
]

If we don't think this is a good idea to add a special case for DS_Store in the validation logic, I can add it to my rule:

ios_application(
    name = "Pinterest",
    app_icons = glob(["Resources/ProdAppIcons.xcassets/**"], exclude=["*.DS_Store"]),

but it seems suboptimal for everyone to have to add this.

[thomasvl]

If we silently drop the files, it does mean bazel will rerun the action when just the .DS_Store file changes. And since lots of Apple tools produce files with timestamps, it might cause more things to run when nothing really changed. 🙁

The other question should we do this for any other types of resources? ios_application calls out a few specific types on some attributes, but what about when things are passed on the *_library rules just as data or structured_resources do we strip them there also?

If someone is bundling some sorta templates (like Xcode project templates or say stuff to build diskimages, etc.) they might want the .DS_Store files to control how the directory being created will first appear for the user. So there would likely need to still be a way to force them to be included.

One option might be to use print() to issue a warning about them being dropped (so it isn't unnoticed), and document some tag that can go on a target to suppress the dropping for the folks that need them. The confusing part will be does the tag go on rule listing the resources or the final bundling rule, the later likely having a much larger scope of impact than desired.

Aside: I've seen a lot of projects suggest https://github.com/github/gitignore/blob/master/Global/macOS.gitignore, which doesn't fully help because a local build can fail after the developer opens the directory, but it atleast avoids these getting checked in to version control.

[thii]

We're hitting this as well. Once in awhile the build failed which made developers have doubts about Bazel's reproducibility.

It looks like the validation only checks if the .xcassets directory only includes a single xcassets type. The non-existence of a .DS_Store file in the .xcassets directory just gets validated by accident. It doesn't prevent Bazel to rerun the action if a .DS_Store file leaked under a sub-directory of .xcassets (for instance, a .appiconset directory).

I think it's fine to ignore .DS_Store files in the validation logic, as there would never be an xcassets type with the .DS_Store extension.

[keith]

It doesn't prevent Bazel to rerun the action if a .DS_Store file leaked under a sub-directory of .xcassets (for instance, a .appiconset directory).

So you're saying that bazel will re-run the action if a .DS_Store is added / changed even if it doesn't fail this validation logic?

If that's the case I think we would prefer this fail fast, in order to encourage users to exclude this from their globs, otherwise it could be a really hard to track down source of non-reproducibility

[thii]

So you're saying that bazel will re-run the action if a .DS_Store is added / changed even if it doesn't fail this validation logic?

That's right; it also applies to the build of other resource types as well, not just xcassets.

If that's the case I think we would prefer this fail fast, in order to encourage users to exclude this from their globs, otherwise it could be a really hard to track down source of non-reproducibility

The problem here is that this can fail because of an unrelated validation. It's a bit strange where it would fail when a .DS_Store file exists in an xcassets directory but not the other.

Technically we could drop all the .DS_Store files from actool actions, and warn users about that. I'm not aware of any use case that would need them in here, and actool just ignore them anyway.