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

Turbine breaks ASWB IDE sync for projects using Robolectric (android_local_test) #13144

Closed
jongerrish opened this issue Mar 2, 2021 · 7 comments

Comments

@jongerrish
Copy link
Contributor

Description of the problem / feature request:

Turbine breaks ASWB IDE sync for projects with Robolectric tests.

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

Using Android Studio w/ Blaze v4.0+ sync a project with an android_local_test rule.

What operating system are you running Bazel on?

Mac

What's the output of bazel info release?

development version

If bazel info release returns "development version" or "(@non-git)", tell us how you built Bazel.

Our internal fork based on v.4.0.0

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

git@github.sc-corp.net:Snapchat/bazel.git
2e15fa7e372a3118a09438624494b570e84c44d3
2e15fa7e372a3118a09438624494b570e84c44d3

Have you found anything relevant by searching the web?

I created this issue on the Robolectric tracker:
robolectric/robolectric#6242

Dagger seems to have stumbled on this issue and worked around it by pegging their Bazel version at v3.7.x:
google/dagger@1b5b075

This is the actual Turbine error check that fails:
google/turbine@8bbf4cb

This is the annotation that causes the problem:-
https://github.com/robolectric/robolectric/blob/master/annotations/src/main/java/org/robolectric/annotation/Config.java

I'm unsure why its reporting clinit as the missing attribute however.

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

To reproduce from the command line you can just trigger the ASWB IDE aspect, this command is the smallest stripped down command that the IDE issues. Note that intellij-resolve-java is the key output group since this is what triggers Turbine. Note, that it is possible to build and run tests because Turbine isn't invoked for an android_local_test target in these cases. It's the aspect that triggers Turbine.

bazel build --aspects=@intellij_aspect//:intellij_info_bundled.bzl%intellij_info_aspect "--override_repository=intellij_aspect=/Users/jgerrish/Library/Application Support/Google/AndroidStudio4.1/plugins/aswb/aspect" --output_groups=intellij-resolve-java -- //foo/bar/baz:test
@jongerrish
Copy link
Contributor Author

@cushon - Any idea why Turbine would fail when processing https://github.com/robolectric/robolectric/blob/master/annotations/src/main/java/org/robolectric/annotation/Config.java - Its an interesting annotation (with an implementation and a bunch of complexity) but I'm not sure why Turbine would fail with clinit as a missing attribute. Any insights?

I can work around this in our repo by removing any redundant annotations and the rest migrate to Kotlin whose source files obviously don't trigger Turbine.

jongerrish referenced this issue in google/dagger Mar 2, 2021
This CL fixes the version of Java to JDK8 and Bazel to 3.7.1

For Java, ubuntu-latest will soon be switching from Java 8 to Java 11 as the default (actions/runner-images#1816), and our Bazel builds currently don't work with Java 11 since compile_testing depends on @local_jdk//:lib/tools.jar which doesn't exist in Java 11.

https://github.com/google/bazel-common/blob/master/third_party/java/compile_testing/BUILD#L32

For Bazel, ubuntu-latest currently uses Bazel 4.0.0 by default, which fails with Robolectric tests @config annotation:

```
error: missing required annotation argument: <clinit> @config
```
RELNOTES=N/A
PiperOrigin-RevId: 358062883
@Bencodes
Copy link
Contributor

Bencodes commented Mar 3, 2021

@jongerrish have you tried testing with this commit pulled into your fork? bef4bbb

@cushon
Copy link
Contributor

cushon commented Mar 3, 2021

Thanks for the heads up, this is a bug in turbine that I was able to reproduce at head, I'm working on a fix

@cushon
Copy link
Contributor

cushon commented Mar 3, 2021

The bug affects any annotation declaration with a class initializer. That's somewhat uncommon, because default values for annotation element have to be compile-time constants, so stuff like int ALL_SDKS = -2; is initialized from the constant pool without needing a <clinit> method. The exception here is Class<? extends Application> DEFAULT_APPLICATION = DefaultApplication.class;, since class literals aren't compile-time constants in the JLS 15.28 sense, so there's <clinit> for that:

  static {};
    descriptor: ()V
    flags: (0x0008) ACC_STATIC
    Code:
      stack=1, locals=0, args_size=0
         0: ldc           #1                  // class org/robolectric/annotation/DefaultApplication
         2: putstatic     #2                  // Field DEFAULT_APPLICATION:Ljava/lang/Class;
         5: return
      LineNumberTable:
        line 39: 0

If you want a work-around, inlining DEFAULT_APPLICATION or moving the declaration into another non-annotation class (like Implementation) should avoid the bug.

copybara-service bot pushed a commit to google/turbine that referenced this issue Mar 3, 2021
This was causing an error when checking required annotation elements for
annotations with `<clinit>`s read from bytecode:

```
error: missing required annotation argument: <clinit>
@foo
```

bazelbuild/bazel#13144

PiperOrigin-RevId: 360525202
copybara-service bot pushed a commit to google/turbine that referenced this issue Mar 3, 2021
This was causing an error when checking required annotation elements for
annotations with `<clinit>`s read from bytecode:

```
error: missing required annotation argument: <clinit>
@foo
```

bazelbuild/bazel#13144

PiperOrigin-RevId: 360553027
@cushon cushon mentioned this issue Mar 3, 2021
@jongerrish
Copy link
Contributor Author

Amazing! Thanks for the fast turnaround Liam!

@cushon
Copy link
Contributor

cushon commented Mar 5, 2021

The fix was merged in 57672ac

It won't be visible until it makes it into java_tools release. If you're able to do a cherry-pick I'm not sure what the best way to do that for tools is, @comius may have suggestions.

@cushon cushon closed this as completed Mar 5, 2021
@comius
Copy link
Contributor

comius commented Mar 9, 2021

Making a java_tools release: bazelbuild/java_tools#45

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants