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

Update AGP to 8.3.0 #1140

Merged
merged 4 commits into from
Mar 5, 2024
Merged

Update AGP to 8.3.0 #1140

merged 4 commits into from
Mar 5, 2024

Conversation

alexvanyo
Copy link
Contributor

@alexvanyo alexvanyo commented Jan 3, 2024

Updates to AGP 8.3.0.

This required a couple of changes due to AGP 8.3.0 being a bit stricter with how dependencies are resolved.

f46e506:

Previously, the Roborazzi ScreenshotHelper utilities were in :core:testing. However, this module is brought in for adding utilities to both unit tests and instrumentation tests.

This change was needed due to the following:

  • ScreenshotHelper calls into Robolectric utilities
  • Therefore, :core:testing had a dependency on Robolectric, which meant that Robolectric was being added into instrumentation tests
  • This brought in a jre guava was being brought into instrumentation tests, which shouldn't be there

0610407:

kotlinx-coroutines-guava depends on jre guava, which meant that :sync:work depended on jre guava in the instrumentation tests. Bumping this to a later android guava avoids a jre guava in instrumentation tests again. I had to add this as a main implementation, otherwise the error from AGP 8.3.0 returned.

1d91231:

A follow-up from investigating more - :sync:work doesn't use the kotlinx-coroutines-guava dependency in tests anyway, so it is cleanest to just remove it.

@lihenggui
Copy link
Contributor

@alexvanyo AGP 8.3 is stable now.

@lihenggui
Copy link
Contributor

lihenggui commented Mar 1, 2024

It will throw this error:

> Task :core:designsystem:checkDemoDebugAndroidTestDuplicateClasses FAILED
ASM Instrumentation process wasn't able to resolve some classes, this means that
the instrumented classes might contain corrupt stack frames. Make sure the
dependencies that contain these classes are on the runtime or the provided
classpath. Otherwise, the jvm might fail to load the corrupt classes at runtime
when running in a jvm environment like unit tests.

Classes that weren't resolved:
> androidx.compose.animation.tooling.ComposeAnimatedProperty
> com.google.common.util.concurrent.ListenableFuture
> androidx.window.extensions.embedding.SplitPlaceholderRule
> androidx.window.extensions.embedding.ActivityRule
> androidx.window.extensions.embedding.SplitPairRule


FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':core:designsystem:checkDemoDebugAndroidTestDuplicateClasses'.
> A failure occurred while executing com.android.build.gradle.internal.tasks.CheckDuplicatesRunnable
   > Duplicate class com.google.common.util.concurrent.ListenableFuture found in modules guava-31.1-jre (com.google.guava:guava:31.1-jre) and listenablefuture-1.0 (com.google.guava:listenablefuture:1.0)
     
     Go to the documentation to learn how to <a href="d.android.com/r/tools/classpath-sync-errors">Fix dependency resolution errors</a>.

This may relate to google/guava#6618

@alexvanyo alexvanyo force-pushed the av/agp-8.3.x branch 2 times, most recently from b80c4a9 to 36c7b30 Compare March 4, 2024 19:47
Change-Id: I6ea5aeaf357dafbae7be0e7f5f0443fbb1a8e68b
@alexvanyo alexvanyo changed the title Update AGP to 8.3 Update AGP to 8.3.0 Mar 4, 2024
@alexvanyo
Copy link
Contributor Author

Thanks for the reminder @lihenggui , updating PR with 8.3.0 to see what failures CI reports.

@alexvanyo alexvanyo marked this pull request as ready for review March 4, 2024 19:55
@alexvanyo alexvanyo requested a review from dturner March 4, 2024 23:03
Change-Id: Icf98a4acc9963656530a1a9fc8b156a553a070af
Change-Id: I3c2c201eb0669165e3594bedfea83bff29c28ce2
Change-Id: Ic5abbd36436a7d8c6382cb02b4ebd7538f8ae1f6
@dturner dturner added the version update This is related to a library, API or SDK update which requires some work. Not just a version bump. label Mar 5, 2024
@dturner
Copy link
Collaborator

dturner commented Mar 5, 2024

Thanks for doing this @alexvanyo. Do you have any more info you can share on "AGP 8.3.0 being a bit stricter with how dependencies are resolved"? I'm guessing that it's stricter about detecting duplicate classes (e.g. ListenableFuture), but are there other changes?

@dturner dturner merged commit 8c88df0 into main Mar 5, 2024
4 checks passed
@dturner dturner deleted the av/agp-8.3.x branch March 5, 2024 14:32
@alexvanyo
Copy link
Contributor Author

AGP 8.3.0 changed behavior to consistently resolve dependency versions from the main classpath and the instrumentation test classpath for both application modules and library modules.

For application modules, the separate loading of the main APK and test APK classpaths means that 8.3.0 causes Gradle's dependencies to reflect actual runtime behavior better (I believe this was the issue causing #1221 (comment). With 8.3.0, this issue should be reflected directly in dependency resolution in Gradle, instead of only at runtime)

However, for libraries, where there is only one test APK, this is a behavior change, as any dependency that is higher in the instrumentation test classpath will be forced to downgrade to the version in the main classpath. As a result, updating to 8.3.0 can cause failures for library modules which previously relied on newer instrumentation test dependencies bumping the version used in the main classpath.

@alexvanyo
Copy link
Contributor Author

The behavior change for library modules is reverting to 8.2 behavior in 8.4.0-alpha12, and this revert will also be cherry-picked to 8.3.x.

SimonMarquis added a commit to SimonMarquis/nowinandroid that referenced this pull request Mar 21, 2024
This is a followup cleanup of android#1163 that was partially addressed by android#1140.

- Remove unused `projects.core.testing` dependencies (or replace with direct dependencies).
- Introduce `androidx.compose.ui.test` bundle.
- Remove `NiaTestRunner` from the default config, forcing consumers to depend on it, even when not used.
SimonMarquis added a commit to SimonMarquis/nowinandroid that referenced this pull request Mar 21, 2024
This is a followup cleanup of android#1163 that was partially addressed by android#1140.

- Remove unused `projects.core.testing` dependencies (or replace with direct dependencies).
- Introduce `androidx.compose.ui.test` bundle.
- Remove `NiaTestRunner` from the default config, forcing consumers to depend on it, even when not used.
SimonMarquis added a commit to SimonMarquis/nowinandroid that referenced this pull request Mar 21, 2024
This is a followup cleanup of android#1163 that was partially addressed by android#1140.

- Remove unused `projects.core.testing` dependencies (or replace with direct dependencies).
- Introduce `androidx.compose.ui.test` bundle.
- Remove `NiaTestRunner` from the default config, forcing consumers to depend on it, even when not used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version update This is related to a library, API or SDK update which requires some work. Not just a version bump.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants