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

[DO NOT MERGE] Add integration tests for Android resource processing with aapt and aapt2 #5222

Closed
wants to merge 1,139 commits into from

Conversation

jin
Copy link
Member

@jin jin commented May 20, 2018

This is still a WIP, please do not approve/merge.

Adding tests to see an expected failure for aapt2 on presubmit.

See #5214 for details.

@jin jin self-assigned this May 20, 2018
@jin
Copy link
Member Author

jin commented May 20, 2018

Without --verbose_failures, aapt2 builds fail without any error messages.

https://storage.googleapis.com/bazel-buildkite-artifacts/c5ef945b-93fc-4736-8726-4bcd2a12582f/src/test/shell/bazel/android/aapt_integration_test/attempt_3.log

INFO: Analysed target //java/bazel:bin (35 packages loaded).
INFO: Found 1 target...
[0 / 11] [-----] Creating source manifest for @bazel_tools//src/tools/android/java/com/google/devtools/build/android:ResourceProcessorBusyBox [for host]
[47 / 75] Extracting interface @bazel_tools//src/tools/android/java/com/google/devtools/build/android:all_android_tools [for host]; 1s linux-sandbox ... (2 actions, 1 running)
[71 / 105] Building external/androidsdk/apksigner.jar () [for host]; 1s worker ... (8 actions, 4 running)
[82 / 105] Building external/bazel_tools/src/tools/android/java/com/google/devtools/build/android/dexer/libdexer.jar (15 source files) and running annotation processors (AutoValueProcessor) [for host]; 2s worker ... (4 actions running)
[87 / 105] Building external/bazel_tools/src/tools/android/java/com/google/devtools/build/android/dexer/libdexer.jar (15 source files) and running annotation processors (AutoValueProcessor) [for host]; 7s worker ... (2 actions running)
ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9d63d29e2d8e5af323d72521df3ed25f/sandbox/7147262322196821783/execroot/io_bazel/_tmp/324a2a2d7aecfb244ab208e9c7a22239/workspace.JDr6aIwC/java/bazel/BUILD:11:1: Processing Android resources for //java/bazel:bin failed (Exit 1)
Target //java/bazel:bin failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 36.208s, Critical Path: 15.16s
INFO: 26 processes: 17 linux-sandbox, 9 worker.
FAILED: Build did NOT complete successfully
FAILED: Build did NOT complete successfully
-- Test log: -----------------------------------------------------------
------------------------------------------------------------------------
test_build_with_aapt2 FAILED: aapt2 build failed .
/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/9d63d29e2d8e5af323d72521df3ed25f/sandbox/7147262322196821783/execroot/io_bazel/bazel-out/k8-fastbuild/bin/src/test/shell/bazel/android/aapt_integration_test.runfiles/io_bazel/src/test/shell/bazel/android/aapt_integration_test:49: in call to test_build_with_aapt2
INFO[aapt_integration_test 2018-05-20 02:23:45 (+0000)] Cleaning up workspace
FAILED: test_build_with_aapt2

@jin
Copy link
Member Author

jin commented May 20, 2018

ERROR: /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/406bec4c9f55b1923ffea66c3349501b/sandbox/3469989064504880403/execroot/io_bazel/_tmp/324a2a2d7aecfb244ab208e9c7a22239/workspace.N6U5vgwv/java/bazel/BUILD:11:1: Processing Android resources for //java/bazel:bin failed (Exit 1): ResourceProcessorBusyBox failed: error executing command
  (cd /var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/406bec4c9f55b1923ffea66c3349501b/sandbox/3469989064504880403/execroot/io_bazel/_tmp/324a2a2d7aecfb244ab208e9c7a22239/root/ddbd33075295d49576597fbf26a74600/execroot/__main__ && \
  exec env - \
    PATH=/var/lib/buildkite-agent/.cache/bazel/_bazel_buildkite-agent/406bec4c9f55b1923ffea66c3349501b/sandbox/3469989064504880403/execroot/io_bazel/bazel-out/k8-fastbuild/bin/src/test/shell/bazel/android/aapt_integration_test.runfiles/io_bazel/src/test/shell/bin:.:/usr/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/snap/bin \
  bazel-out/host/bin/external/bazel_tools/src/tools/android/java/com/google/devtools/build/android/ResourceProcessorBusyBox 
--tool AAPT2_PACKAGE 
-- 
--aapt2 bazel-out/host/bin/external/androidsdk/aapt2_binary 
--directData bazel-out/android-armeabi-v7a-fastbuild/bin/java/bazel/_aar/unzipped/resources/aar/res:bazel-out/android-armeabi-v7a-fastbuild/bin/java/bazel/_aar/unzipped/assets/aar/assets:bazel-out/android-armeabi-v7a-fastbuild/bin/java/bazel/aar_processed_manifest/AndroidManifest.xml:::bazel-out/android-armeabi-v7a-fastbuild/bin/java/bazel/aar_symbols/local.bin 
--primaryData ::bazel-out/k8-fastbuild/bin/java/bazel/bin_merged/bin/AndroidManifest.xml 
--buildToolsVersion 27.0.3 
--androidJar external/androidsdk/platforms/android-27/android.jar 
--rOutput bazel-out/k8-fastbuild/bin/java/bazel/bin_symbols/R.txt 
--srcJarOutput bazel-out/k8-fastbuild/bin/java/bazel/bin.srcjar 
--proguardOutput bazel-out/k8-fastbuild/bin/java/bazel/proguard/bin/_bin_proguard.cfg 
--mainDexProguardOutput bazel-out/k8-fastbuild/bin/java/bazel/proguard/bin/main_dex_bin_proguard.cfg 
--manifestOutput bazel-out/k8-fastbuild/bin/java/bazel/bin_processed_manifest/AndroidManifest.xml 
--resourcesOutput bazel-out/k8-fastbuild/bin/java/bazel/bin_files/resource_files.zip 
--packagePath bazel-out/k8-fastbuild/bin/java/bazel/bin.ap_ 
--debug 
--packageForR bazel)
Use --sandbox_debug to see verbose messages from the sandbox

https://source.cloud.google.com/results/invocations/077e2fc8-a19f-4e1d-8acb-ee3aae08ebcd/targets/%2F%2Fsrc%2Ftest%2Fshell%2Fbazel%2Fandroid:aapt_integration_test/tests

@jin jin changed the title [DO NOT MERGE] Add integration tests for Android resource processing with aapt [DO NOT MERGE] Add integration tests for Android resource processing with aapt and aapt2 May 21, 2018
bazel-io pushed a commit that referenced this pull request May 22, 2018
Currently, if there are non-suppressed errors during aapt2 processing, Bazel does not print them.

This helps with debugging #5222

With this, I get:

```
ERROR: /usr/local/google/home/jingwen/code/android_scratch_project/examples/android/java/bazel/BUILD:16:1: Processing Android resources for //examples/android/java/bazel:hello_world failed (Exit 1)
May 21, 2018 5:31:01 PM com.google.devtools.build.android.ResourceProcessorBusyBox main
SEVERE: java.io.IOException: Is a directory
Target //examples/android/java/bazel:hello_world failed to build
```

instead of just:

```
ERROR: /usr/local/google/home/jingwen/code/android_scratch_project/examples/android/java/bazel/BUILD:16:1: Processing Android resources for //examples/android/java/bazel:hello_world failed (Exit 1)
Target //examples/android/java/bazel:hello_world failed to build
```

RELNOTES: None.
PiperOrigin-RevId: 197493881
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@googlebot
Copy link

CLAs look good, thanks!

Googler and others added 17 commits July 23, 2018 08:34
RELNOTES: None.
PiperOrigin-RevId: 205665675
RELNOTES: None
PiperOrigin-RevId: 205668909
On two fronts: First, it should follow standard command line semantics. Second, it should work as intended: --noblock_for_lock means the client will not wait for another command to finish, but will exit eagerly. It can be useful for preventing hanging in applications that are non-interactively calling bazel.

It should have standard startup-option semantics: the default value is accepted as a no-op or can be provided to override a previous value.

The next issue involves 2 different locks - the client lock, and the server-side command lock. This duality exists because we would like, one day, to be able to run certain commands, like info or help, at the same time, so multiple commands would need specialized locks that allow some duality but blocks others. This can only be done at the server level, so as soon as the client gets the "we're connected" grpc message from the server, it releases the client lock and lets the server manage  multiple requests.

There are basically 3 possible states that are relevant to this option:

1) no other client is active, so no one holds the client lock or the command lock - the server can be used, shutdown or started as needed. - no blocking, but no need to block, either, so we're safe
2) another client (client1) holds the lock, but it is currently using a server that we want to reuse. If client1 still holds the client lock, we fail fast. Same thing if client1 is holding the server-side lock: we will exit gracefully when the BlazeCommandDispatcher responds with a failure.
3) client1 holds the lock but its server cannot be reused. (batch clients also fall into this category, as there is no server to reuse - but in this case, the client lock is still in play). However, for server mode, this is broken - the following happens:
   - Server is occupied with client1's request, holds the command lock
   - client2 wants to restart the server, so sends the old server a "shutdown" command
   - the BlazeCommandDispatcher says - nuh-uh, this is busy, and you said you didn't want to wait for the lock
   - client2 absorbs this response
   - waits (blocks...)
   - for a minute
   - then force shuts-down the old server.

So we had 2 problems - we block, and we shutdown a server that we truly intended to keep going. Now, if the server responds saying another action is using it, the client will exit correctly, and leave the old server to do its thing.

RELNOTES: None.
PiperOrigin-RevId: 205671817
--output graph`.

Implementation:
AIUI, currently the "edges' conditions" are lost [1] when the larger graph is initially constructed. It now does a second pass over dependency subgraph to find all the conditional edges and annotates them in dot output. This can be easily extended in other forms of output, but for now it only annotates edges in dot output.

[1]: https://github.com/bazelbuild/bazel/blob/32e9fee4e2192a340d0b1823538bf8e9fdf92b65/src/main/java/com/google/devtools/build/lib/query2/output/OutputFormatter.java#L745-L770

PiperOrigin-RevId: 205685823
…ce_library is available

This way users of the Skylark API don't see this feature expanded.

RELNOTES: None.
PiperOrigin-RevId: 205704719
…ation in a very specific window of time inbetween enqueueing one top-level node for evaluation and checking if another top-level node is done. See the added unit test for details.

RELNOTES: None
PiperOrigin-RevId: 205718683
PiperOrigin-RevId: 205718853
…enerated source are generated.

PiperOrigin-RevId: 205756917
Each FileSystem instance has a digest function, but the getDigest and getHashDigest functions also accepted their own custom parameter functions. We only support 1 hash per filesystem instance, these parameters are redundant.

RELNOTES: None.
PiperOrigin-RevId: 205758571
The conversion approach we were previously using is not stable - the resulting
offsets can differ based on what other things are going on on the same machine
at the same time.

By using an interface and passing it to the relevant places (and only computing
the offset once), we ensure that all conversions are consistent with each other.

PiperOrigin-RevId: 205787309
Fixes bazelbuild#5599

PiperOrigin-RevId: 205794997
DeletePath and CreateJunction are now even more
tolerant with errors, particularly the class of
errors where access is denied.

Also in this change:
- remove DeletePathResult::kParentMissing, as this
  case is handled by CreateFileW's error handling
  later
- do not error-check CreateDirectoryW; if failed,
  just proceed as if the directory already existed
- print more debugging info where possible

Change-Id: I1162dae2c6b7524f14d8892047f9eb51831470dd

Closes bazelbuild#5611.

Change-Id: I78fe6aed6d0b120815339c0923c8a903990921d9
PiperOrigin-RevId: 205796307
… as we

treat java_proto_library.

RELNOTES: None
PiperOrigin-RevId: 205812269
Also simplify LoadingPhaseCompleteEvent, and SkyframeExecutor, and remove
LoadingCallback, which is unnecessary now that we only have a single
implementation (previously LoadingPhaseRunner).

This also removes some of the excessive Skyframe calls introduced by
bazelbuild@1067310, and prepares for interleaving target pattern eval and loading.

PiperOrigin-RevId: 205813197
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and have the pull request author add another comment and the bot will run again. If the bot doesn't comment, it means it doesn't think anything has changed.

@googlebot googlebot added cla: no and removed cla: yes labels Aug 5, 2018
@jin jin restored the fix_aapt2 branch August 5, 2018 21:08
luca-digrazia pushed a commit to luca-digrazia/DatasetCommitsDiffSearch that referenced this pull request Sep 4, 2022
    Currently, if there are non-suppressed errors during aapt2 processing, Bazel does not print them.

    This helps with debugging bazelbuild/bazel#5222

    With this, I get:

    ```
    ERROR: /usr/local/google/home/jingwen/code/android_scratch_project/examples/android/java/bazel/BUILD:16:1: Processing Android resources for //examples/android/java/bazel:hello_world failed (Exit 1)
    May 21, 2018 5:31:01 PM com.google.devtools.build.android.ResourceProcessorBusyBox main
    SEVERE: java.io.IOException: Is a directory
    Target //examples/android/java/bazel:hello_world failed to build
    ```

    instead of just:

    ```
    ERROR: /usr/local/google/home/jingwen/code/android_scratch_project/examples/android/java/bazel/BUILD:16:1: Processing Android resources for //examples/android/java/bazel:hello_world failed (Exit 1)
    Target //examples/android/java/bazel:hello_world failed to build
    ```

    RELNOTES: None.
    PiperOrigin-RevId: 197493881
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet