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

Asset exclusion in android targets is broken when --persistent_android_resource_processor is enabled #310

Open
ThomasCJY opened this issue Nov 5, 2021 · 3 comments
Labels
P2 Priority P2

Comments

@ThomasCJY
Copy link

Description of the problem / feature request:

In android targets, we should be able to exclude certain resources from root directory. Example

assets = glob(
        ["assets/**"],
        exclude = [
              "assets/abc.txt", "**/.DS_Store"
        ]
),
assets_dir = "assets",

However, when --persistent_android_resource_processor is enabled. The exclude will not work.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I created a test branch to repro this issue. You can easily follow the steps in readme to repro it.

What operating system are you running Bazel on?

macos

What's the output of bazel info release?

bazelisk version 
Bazelisk version: v1.3.0
Build label: 4.2.1
Build target: bazel-out/darwin-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Mon Aug 30 15:24:28 2021 (1630337068)
Build timestamp: 1630337068
Build timestamp as int: 1630337068

Investigation

This bug is caused by this line where we only parse the root directory of the assets and send them as data params into resource busy box processor. Considering the fact that resource merger only takes directory as the param, I think a possible fix would be copy and paste valid assets to a temporary folder and use that directory as the input param in the resource merge action.

@nkoroste
Copy link
Contributor

nkoroste commented Nov 5, 2021

cc @ahumesky

@ThomasCJY ThomasCJY changed the title Asset excludsion in android targets is broken when --persistent_android_resource_processor is enabled Asset exclusion in android targets is broken when --persistent_android_resource_processor is enabled Nov 5, 2021
@ahumesky
Copy link
Collaborator

the resource processor relies on sandboxing in a few places like this, because as you saw aapt2 expects a directory rather than a list of files. you could try --worker_sandboxing to see if this avoids the issue

@ahumesky ahumesky added the P2 Priority P2 label Oct 25, 2022
@lukaciko
Copy link
Contributor

From what I understand this goes further; anything in assets_dir can end up affecting several of the resource processing actions, even if it is not listed under assets. Things happen to work in the typical case, which is even described in the docs for android_library.assets:

This is typically a glob of all files under the assets directory

But as soon as you stray from this you can get into trouble.

In our case we saw intermittent failures with @tools_android/tools/crashlytics which lists the whole target directory as the assets_dir here. bazelbuild/bazel#14198 seems to be dealing with another aspect of this issue.

@ahumesky I'm expecting we won't see a proper fix before the Starlark migration? In my opinion it would be more correct if the assets field didn't even exist, it does not have the expected effect.

@ahumesky ahumesky transferred this issue from bazelbuild/bazel Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 Priority P2
Projects
None yet
Development

No branches or pull requests

4 participants