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

x.y.z-jre version has -android.jar artifact with missing -jre classes #6904

Closed
1 task done
TWiStErRob opened this issue Jan 7, 2024 · 2 comments
Closed
1 task done
Labels
type=defect Bug, not working as expected

Comments

@TWiStErRob
Copy link

TWiStErRob commented Jan 7, 2024

Description

I don't fully understand the problem, so I'm just going to provide a tiny repro here and some guess-work explanation.

Originally the problem came from an Android project with Paparazzi tests, but I managed to eliminate Paparazzi from the picture, only Android Unit Tests with simple dependencies remain.


com.android.tools.layoutlib:layoutlib-api artifact has an enum called com.android.resources.ResourceType, which in it's static class initializer calls Sets.toImmutableEnumSet(). So far nothing special, this works as expected.

However as soon as there's a more complex dependency graph, more specifically: a dependency on Guava 32.1.1+ -jre variant, the classpath becomes strange. An error can happen where the above mentioned toImmutableEnumSet doesn't exist on runtime classpath with the right visibility modifier.

Guess-work

I think this is because of

As I see, without the experiment it would be a different error only, so my guess is that this comes down to metadata. Mentioning both of you, because I think you're the best make heads or tails of this.

Example

Repro:

  1. Clone https://github.com/TWiStErRob/repros/tree/main/guava/gradle-metadata_capabilities-mixed-up
  2. gradlew test
Relevant code pieces
plugins {
	id("com.android.library") version "8.2.1"
}
dependencies {
	testImplementation("junit:junit:4.13.2")
	testImplementation("com.android.tools.layoutlib:layoutlib-api:31.2.1")
	testImplementation("com.google.guava:guava:32.1.1-jre")
}
android {
	namespace = "com.example"
	compileSdk = 34
	defaultConfig.minSdk = 14
}
public class MyTest {
	@org.junit.Test public void test() {
		com.android.resources.ResourceType.values();
	}
}

Expected Behavior

Test passes.

More specifically: when resolving to version ...-jre, ONLY the -jre.jar is on the classpath.

Actual Behavior

java.lang.IllegalAccessError: class com.android.resources.ResourceType tried to access method 'java.util.stream.Collector com.google.common.collect.Sets.toImmutableEnumSet()' (com.android.resources.ResourceType and com.google.common.collect.Sets are in unnamed module of loader 'app')
	at com.android.resources.ResourceType.<clinit>(ResourceType.java:175)
	at com.example.MyTest.test(MyTest.java:11)
More details

Project Synced in Android Studio shows the culprit (this blew my mind):
image

Here are a few version and their paths:

%GRADLE_USER_HOME%\caches\modules-2\files-2.1\com.google.guava\guava\32.1.1-jre\966b183d0f2639bf83f042bafb22db23caff3d18\guava-32.1.1-android.jar
%GRADLE_USER_HOME%\caches\modules-2\files-2.1\com.google.guava\guava\32.1.3-jre\ea090dd85ca2fa12d42d054369df888665230dd7\guava-32.1.3-android.jar
%GRADLE_USER_HOME%\caches\modules-2\files-2.1\com.google.guava\guava\33.0.0-android\cfbbdc54f232feedb85746aeeea0722f5244bb9a\guava-33.0.0-android.jar

It seems from 33.0.0, the version and variant matches, but to trump this all off, when doing gradlew :dependencies --configuration=debugUnitTestRuntimeClasspath I get (irrelevant lines omitted):

|    |    +--- com.google.guava:guava:31.1-jre -> 33.0.0-jre
+--- com.google.guava:guava:33.0.0-jre (*)
\--- com.google.guava:guava -> 33.0.0-jre (c)

where there's no mention of -android!


Packages

com.google.common.collect

Platforms

Android, Java 17

Checklist

@TWiStErRob TWiStErRob added the type=defect Bug, not working as expected label Jan 7, 2024
TWiStErRob added a commit to TWiStErRob/repros that referenced this issue Jan 7, 2024
@jjohannes
Copy link
Contributor

This is the expected behavior. Although, I know that it is confusing, because the version string with the -jre in it looks like it is the variant selection, but it is not (see explanation further down).

To select the JRE version in Android do what you have commented out:

dependencies.constraints {
  testImplementation("com.google.guava:guava") {
    attributes {
      attribute(
        TargetJvmEnvironment.TARGET_JVM_ENVIRONMENT_ATTRIBUTE,
        objects.named(TargetJvmEnvironment.STANDARD_JVM)
      )
    }
  }
}

It's not a "workaround", but a way to tell Gradle to select a different variant than your JVM environment's default.

Since 32.1.1, The -jre in the version number no longer has a meaning for Gradle.
That is, 33.0.0-jre and 33.0.0-android are the same from for Gradle. This way, there are no longer accidental switches from jre to android (or the other way around) by version conflict resolution. In previous versions, you could declare ...-jre but get ...-android, because of a higher ...-android version somewhere in the dependency graph.

Ideally, there won't be two versions 33.0.0-jre and 33.0.0-android but just 33.0.0. But I guess that schema needs to be kept for Maven consumers. I fully understand why encoding the "variant" in the version was chosen back in the day. But strictly speaking it is "wrong". The version conflict resolution of tools (both Gradle and Maven) is not made for using the version like this and that leads to follow up confusion.

Because of the above, it gets confusing when you look at the resolution result of Gradle. In your example it says that it resolved to -> 33.0.0-jre. But as the -jre here has no meaning, it could have been 33.0.0-android as well and the result would be the same. What matters is the variant it resolves to and the Jar file that's behind that variant. Unfortunately, the dependencies tasks does not show that. It would be nice if the dependencies task would have some kind of "extended mode" that also shows the variants and files.

You can use dependencyInsight to get the full picture (almost, as this also does not show the Jar file):

$ gradle dependencyInsight --configuration releaseUnitTestRuntimeClasspath --dependency com.google.guava:guava

> Task :dependencyInsight
com.google.guava:guava:33.0.0-jre
  Variant androidRuntimeElements:
    | Attribute Name                                  | Provided     | Requested    |
    |-------------------------------------------------|--------------|--------------|
    | org.gradle.dependency.bundling                  | external     |              |
    | org.gradle.jvm.version                          | 8            |              |
    | org.gradle.libraryelements                      | jar          |              |
    | org.gradle.status                               | release      |              |
    | org.gradle.category                             | library      | library      |
    | org.gradle.jvm.environment                      | android      | android      |
    | org.gradle.usage                                | java-runtime | java-runtime |
    | com.android.build.api.attributes.AgpVersionAttr |              | 8.2.1        |
    | com.android.build.api.attributes.BuildTypeAttr  |              | release      |
   Selection reasons:
      - By constraint
      - By conflict resolution: between versions 33.0.0-jre and 31.1-jre

Here we have the clues Variant androidRuntimeElements and org.gradle.jvm.environment = android


With this comment, I only want to explain the situation. I don't want to say that things are perfect as they are. But I am not sure what a concrete next action would be. Things I see:

  1. On the Guava side, it would be good to get rid of the -jre and -android in the version at some point, but I am not sure if and how that could ever be possible.
  2. On the Gradle side, I think there is room for improving what the dependency task reports.
  3. ...and in the DSL for defining attribute values (attributes { attribute(TargetJvmEnvironment.TARGET_JVM_ENVIRONMENT_ATTRIBUTE, objects.named(TargetJvmEnvironment.STANDARD_JVM)) } is very clumsy and the variant selection mechanisms are still quite hard to understand if a build user needs to interact with them directly (as it is the case here).

@cpovirk
Copy link
Member

cpovirk commented Jan 12, 2024

Sorry, @TWiStErRob, for the trouble with this, and thanks again, @jjohannes, for jumping in with an explanation.

I still need to do more about this class of problem. My recent work has been mostly to add APIs like Sets.toImmutableSet to the Android flavor so that it differs from the JRE flavor mostly just in implementation details, rather than in public API. So far, I've added APIs as package-private, and I'm waiting to see if users encounter problems when they adopt 33.0.0. If no one reports problems, then I expect to make the APIs public and add more such APIs, at which point things might work for you even when Gradle selects the Android variant. (Thanks for being among those to test 33.0.0, and sorry again.)

I think you're right that this is fundamentally the same as #6801, so I'll try to consolidate things back there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type=defect Bug, not working as expected
Projects
None yet
Development

No branches or pull requests

3 participants