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.

Sign up for GitHub

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

Jump to bottom

incompatible_prohibit_aapt1: Prohibit Bazel usage of aapt1 #10000

Closed
bchang opened this issue Oct 11, 2019 · 11 comments
Closed

incompatible_prohibit_aapt1: Prohibit Bazel usage of aapt1 #10000

bchang opened this issue Oct 11, 2019 · 11 comments
Labels
incompatible-change Incompatible/breaking change team-Android Issues for Android team

Comments

@bchang
Copy link
Contributor

bchang commented Oct 11, 2019

Tracking issue for the incompatible change to prohibit aapt1.

Bazel now uses aapt2 by default (#6907) meaning aapt support is deprecated.

When this change goes in effect, aapt will no longer be available.

To resolve issues when migrating your app to build with aapt2, see https://developer.android.com/studio/command-line/aapt2#aapt2_changes
@jin jin added incompatible-change Incompatible/breaking change team-Android Issues for Android team breaking-change-2.0 labels Oct 11, 2019
bazel-io pushed a commit that referenced this issue Oct 15, 2019
@bchang @copybara-github
Introduce --incompatible_prohibit_aapt1 flag.
f610fd6 
If this flag is active, AAPT2 is the only Android resource packaging tool in use and we prohibit methods of configuring the AAPT tool version.

#10000

PiperOrigin-RevId: 274703802
@dslomov
Copy link
Contributor

dslomov commented Oct 22, 2019

Since the flag was not in 1.0, "migration-1.0" is an inappropriate label. Please follow https://www.bazel.build/breaking-changes-guide.html for introducing breaking changes.

@hlopko
Copy link
Member

hlopko commented Oct 29, 2019

It's also not in 1.1, is it?

@hlopko
Copy link
Member

hlopko commented Oct 29, 2019

cc @meisterT

@jin
Copy link
Member

jin commented Oct 29, 2019

No - the commit isn't tagged with 1.1.

@jin jin removed the migration-1.1 label Oct 29, 2019
@changusmc
Copy link

@jin asked me to comment here instead of filing a new issue. There's still one action that is using aapt1 and when I build my project that uses Jetpack Navigation, it hits the code path for

bazel/src/tools/android/java/com/google/devtools/build/android/AndroidResourceValidatorAction.java

Line 148 in c018f60

resourceProcessor.runAapt(

I included a repro project below.

Description of the problem / feature request:

The source of the problem stemmed building our app which has Jetpack Navigation. Running bazel build //android/... gives me the same error found in https://stackoverflow.com/a/57174533/3338374

	invalid resource directory name: /var/folders/_s/s0td900s19n0m1m1c2l0_p3nb7439n/T/resource_validator_tmp14646118543985275798/tmp-expanded/res navigation

Feature requests: what underlying problem are you trying to solve with this feature?

Building our internal Android sample app that uses Jetpack Navigation.

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

Repro project: https://github.com/changusmc/android-navigation
This repro project is able to hit the same stack trace as my internal project.

Full error logs from building my project: worker-1-AndroidResourceValidator.log

What operating system are you running Bazel on?

MacOSX 10.15.1

What's the output of bazel info release?

release 1.0.0-homebrew

Have you found anything relevant by searching the web?

Need to use aapt2 to build Jetpack Navigation: https://stackoverflow.com/a/57174533/3338374
Bazel should be defaulting aapt2 by now and upcoming PRs will not allow aapt1 #10000

Any other information, logs, or outputs that you want to share?

Replace these lines with your answer.

If the files are large, upload as attachment or provide link.

@changusmc
Copy link

@jin asked me to comment here instead of filing a new issue. There's still one action that is using aapt1 and when I build my project that uses Jetpack Navigation, it hits the code path for

bazel/src/tools/android/java/com/google/devtools/build/android/AndroidResourceValidatorAction.java

Line 148 in c018f60

resourceProcessor.runAapt(

I included a repro project below.

Description of the problem / feature request:

The source of the problem stemmed building our app which has Jetpack Navigation. Running bazel build //android/... gives me the same error found in https://stackoverflow.com/a/57174533/3338374

	invalid resource directory name: /var/folders/_s/s0td900s19n0m1m1c2l0_p3nb7439n/T/resource_validator_tmp14646118543985275798/tmp-expanded/res navigation

Feature requests: what underlying problem are you trying to solve with this feature?

Building our internal Android sample app that uses Jetpack Navigation.

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

Repro project: https://github.com/changusmc/android-navigation
This repro project is able to hit the same stack trace as my internal project.

Full error logs from building my project: worker-1-AndroidResourceValidator.log

What operating system are you running Bazel on?

MacOSX 10.15.1

What's the output of bazel info release?

release 1.0.0-homebrew

Have you found anything relevant by searching the web?

Need to use aapt2 to build Jetpack Navigation: https://stackoverflow.com/a/57174533/3338374
Bazel should be defaulting aapt2 by now and upcoming PRs will not allow aapt1 #10000

Any other information, logs, or outputs that you want to share?

Replace these lines with your answer.
If the files are large, upload as attachment or provide link.

I was able to work around this by moving res/navigation/nav_graph.xml to res/xml/nav_graph.xml

@bchang
Copy link
Contributor Author

bchang commented Nov 20, 2019

This is a known issue, we have someone working on extricating aapt1 from AndroidResourceValidatorAction.

If I understand correctly, this isn't strictly blocking you from preparing your project for the incompatible change.

@changusmc
Copy link

This is a known issue, we have someone working on extricating aapt1 from AndroidResourceValidatorAction.

If I understand correctly, this isn't strictly blocking you from preparing your project for the incompatible change.

I was able to side step this as mentioned above, but I don't know if it means that this project is ready for the incompatible change. I hope that once the validator action is moved to aapt2, I can move nav_graph.xml back to the expected aapt2 path.

@bchang
Copy link
Contributor Author

bchang commented Nov 21, 2019

dbe76ce

The validator action was running both aapt1 and aapt2. With this commit, if you're using --incompatible_prohibit_aapt1, Bazel should not invoke aapt1.

tensorflow-copybara pushed a commit to tensorflow/tensorflow that referenced this issue Nov 21, 2019
@tensorflower-gardener
Remove uses of 'aapt_version' attribute.
c5dce1f 
Bazel 1.0 [1] enables aapt2 by default, so this attribute doesn't do anything.
Bazel 2.0 [2] will prohibit [3] any uses of this attribute.

This also means that building with Bazel versions <1.0 would potentially use aapt1, but this might actually be a good thing.  TF has a minimum version requirement of 0.19, and many aapt2-related fixes have been made in the 0.2x series.

[1] https://blog.bazel.build/2019/10/10/bazel-1.0.html
[2] bazelbuild/bazel#10000
[3] https://github.com/bazelbuild/bazel/blob/b3f54f673259604ea526c560d4abd144bd267295/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java#L262
[4] https://github.com/tensorflow/tensorflow/blob/dc73a9889fc1ebedaf150da2a0c1dfed11da3858/WORKSPACE#L92

PiperOrigin-RevId: 281679131
Change-Id: I47818b8b59eeb78a8c439523ce72e388428f2da9
@aehlig
Copy link
Contributor

aehlig commented Dec 2, 2019

It seems the flag isn't flipped yet, so extending the migration window

bazel-io pushed a commit that referenced this issue Dec 2, 2019
@bchang @copybara-github
Set --incompatible_prohibit_aapt1 default to true.
85e84f7 
#10000

PiperOrigin-RevId: 283339441
@jin jin mentioned this issue Dec 2, 2019
Closed
aehlig pushed a commit that referenced this issue Dec 2, 2019
@bchang @aehlig
Set --incompatible_prohibit_aapt1 default to true.
fd7d5c2 
#10000

PiperOrigin-RevId: 283339441
bazel-io pushed a commit that referenced this issue Dec 6, 2019
@donaldchai @copybara-github
Remove @nullable annotation from aapt2 provider
f31d471 
Per #10000, aapt2 is the only
Android asset packaging tool in use, and allowing the provider to be null just
pushes errors downstream.

PiperOrigin-RevId: 284091553
aehlig pushed a commit that referenced this issue Dec 17, 2019
@bchang @aehlig
Set --incompatible_prohibit_aapt1 default to true.
5c4a26c 
#10000

PiperOrigin-RevId: 283339441
bazel-io pushed a commit that referenced this issue Dec 19, 2019
@aehlig
Release 2.0.0 (2019-12-19)
59e3600 
Baseline: 807ed23

Cherry picks:

   + db0e32c:
     build.sh: Fix bug in build script for RC release
   + 85e84f7:
     Set --incompatible_prohibit_aapt1 default to true.
   + 84eae2f:
     Let shellzelisk fallback to bazel-real if it's the requested
     version.
   + d5ae460:
     Fix a typo in bazel.sh

Incompatible changes:

  - --incompatible_remap_main_repo is enabled by default. Therefore,
    both ways of addressing the main repository, by its name and by
    '@' are now considered referring to the same repository.
    see #7130
  - --incompatible_disallow_dict_lookup_unhashable_keys is enabled by
    default #9184
  - --incompatible_remove_native_maven_jar is now enabled by default
    and the flag removed. See #6799
  - --incompatible_prohibit_aapt1 is enabled by default.
    See #10000

Important changes:

  - --incompatible_proto_output_v2: proto v2 for aquery proto output
    formats, which reduces the output size compared to v1. Note that
    the messages' ids in v2 are in uint64 instead of string like in
    v1.
  - Adds --incompatible_remove_enabled_toolchain_types.
  - Package loading now consistently fails if package loading had a
    glob evaluation that encountered a symlink cycle or symlink
    infinite expansion. Previously, such package loading with such
    glob evaluations would fail only in some cases.
  - The --disk_cache flag can now also be used together
    with the gRPC remote cache.
  - An action's discover inputs runtime metrics is now categorized as
    parse time on the CriticalPathComponent.
  - Make the formatting example more like to the written text by
    adding an initial description.
  - An action's discover inputs runtime metrics is now categorized as
    parse time on the CriticalPathComponent.
  - Bazel's Debian package and the binary installer now include an
    improved wrapper that understands `<WORKSPACE>/.bazelversion`
    files and the `$USE_BAZEL_VERSION` environment variable. This is
    similar to what Bazelisk offers
    (https://github.com/bazelbuild/bazelisk#how-does-bazelisk-know-whi
    ch-bazel-version-to-run-and-where-to-get-it-from), except that it
    works offline and integrates with apt-get.
  - We are planning to deprecate the runfiles manifest files, which
    aren't safe in the presence of whitespace, and also unnecessarily
    require local CPU when remote execution is used. This release
    adds --experimental_skip_runfiles_manifests to disable the
    generation of the input manifests (rule.manifest files) in most
    cases. Note that this flag has no effect on Windows by default or
    if --experimental_enable_runfiles is explicitly set to false.

This release contains contributions from many people at Google, as well as aldersondrive, Benjamin Peterson, Bor Kae Hwang, David Ostrovsky, Jakob Buchgraber, Jin, John Millikin, Keith Smiley, Lauri Peltonen, nikola-sh, Peter Mounce, Tony Hsu.
meteorcloudy pushed a commit to meteorcloudy/bazel that referenced this issue Jan 10, 2020
@aehlig
Release 2.0.0 (2019-12-19)
50514fc 
Baseline: 807ed23

Cherry picks:

   + db0e32c:
     build.sh: Fix bug in build script for RC release
   + 85e84f7:
     Set --incompatible_prohibit_aapt1 default to true.
   + 84eae2f:
     Let shellzelisk fallback to bazel-real if it's the requested
     version.
   + d5ae460:
     Fix a typo in bazel.sh

Incompatible changes:

  - --incompatible_remap_main_repo is enabled by default. Therefore,
    both ways of addressing the main repository, by its name and by
    '@' are now considered referring to the same repository.
    see bazelbuild#7130
  - --incompatible_disallow_dict_lookup_unhashable_keys is enabled by
    default bazelbuild#9184
  - --incompatible_remove_native_maven_jar is now enabled by default
    and the flag removed. See bazelbuild#6799
  - --incompatible_prohibit_aapt1 is enabled by default.
    See bazelbuild#10000

Important changes:

  - --incompatible_proto_output_v2: proto v2 for aquery proto output
    formats, which reduces the output size compared to v1. Note that
    the messages' ids in v2 are in uint64 instead of string like in
    v1.
  - Adds --incompatible_remove_enabled_toolchain_types.
  - Package loading now consistently fails if package loading had a
    glob evaluation that encountered a symlink cycle or symlink
    infinite expansion. Previously, such package loading with such
    glob evaluations would fail only in some cases.
  - The --disk_cache flag can now also be used together
    with the gRPC remote cache.
  - An action's discover inputs runtime metrics is now categorized as
    parse time on the CriticalPathComponent.
  - Make the formatting example more like to the written text by
    adding an initial description.
  - An action's discover inputs runtime metrics is now categorized as
    parse time on the CriticalPathComponent.
  - Bazel's Debian package and the binary installer now include an
    improved wrapper that understands `<WORKSPACE>/.bazelversion`
    files and the `$USE_BAZEL_VERSION` environment variable. This is
    similar to what Bazelisk offers
    (https://github.com/bazelbuild/bazelisk#how-does-bazelisk-know-whi
    ch-bazel-version-to-run-and-where-to-get-it-from), except that it
    works offline and integrates with apt-get.
  - We are planning to deprecate the runfiles manifest files, which
    aren't safe in the presence of whitespace, and also unnecessarily
    require local CPU when remote execution is used. This release
    adds --experimental_skip_runfiles_manifests to disable the
    generation of the input manifests (rule.manifest files) in most
    cases. Note that this flag has no effect on Windows by default or
    if --experimental_enable_runfiles is explicitly set to false.

This release contains contributions from many people at Google, as well as aldersondrive, Benjamin Peterson, Bor Kae Hwang, David Ostrovsky, Jakob Buchgraber, Jin, John Millikin, Keith Smiley, Lauri Peltonen, nikola-sh, Peter Mounce, Tony Hsu.
@laurentlb
Copy link
Contributor

Can you confirm this was released in 2.0?
If so, we can close the issue.

@jin jin closed this as completed Apr 28, 2020
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
Remove @nullable annotation from aapt2 provider
367adcd 
    Per bazelbuild/bazel#10000, aapt2 is the only
    Android asset packaging tool in use, and allowing the provider to be null just
    pushes errors downstream.

    PiperOrigin-RevId: 284091553
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this issue Sep 4, 2022
Introduce --incompatible_prohibit_aapt1 flag.
a6001a7 
    If this flag is active, AAPT2 is the only Android resource packaging tool in use and we prohibit methods of configuring the AAPT tool version.

    bazelbuild/bazel#10000

    PiperOrigin-RevId: 274703802
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
incompatible-change Incompatible/breaking change team-Android Issues for Android team
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@jin @meisterT @laurentlb @bchang @hlopko @dslomov @changusmc @aehlig