Skip to content

Conversation

@gmackall
Copy link
Member

@gmackall gmackall commented Jun 27, 2024

After the land of flutter/engine#53592, there is some log spam:

e: /Users/mackall/.gradle/caches/transforms-3/c1e137371ec1afe9bc9bd7b05823752d/transformed/fragment-1.7.1/jars/classes.jar!/META-INF/fragment_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: /Users/mackall/.gradle/caches/transforms-3/d86c7cb1c556fe1655fa56db671c649c/transformed/jetified-activity-1.8.1/jars/classes.jar!/META-INF/activity_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
...

I think this is harmless, but still annoying. Upgrading the AGP version fixes it. To be honest, I don't know why (I expected the Kotlin version would do it). But after #146307, our tests have been running on AGP/Gradle 8.1/8.3 for a while, so it makes sense to upgrade anyways.

In a follow up PR:

  1. Also upgrade the tests that were left behind in Reland "Bump to AGP 8.1/Gradle 8.3 (almost) everywhere" #146307, as I think removal of discontinued plugins paved the way here.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 27, 2024
@gmackall

This comment was marked as resolved.

@gmackall gmackall marked this pull request as ready for review July 1, 2024 18:10
@gmackall
Copy link
Member Author

gmackall commented Jul 1, 2024

Requesting review from you as well @christopherfujino because this further fragments a subset of the tools tests into yet another shard, and I want to hear if you are opposed or otherwise make you aware.

The justification why another shard is needed is that the dependency checking tests need Java 11, and not the preview sdk. And the preview tests now need Java 17 (unless we hardcode old Gradle versions, right now they use create).

And this set is also different from the main shard, which uses Java 17 and the stable (not preview) android sdk.

@christopherfujino
Copy link
Contributor

Requesting review from you as well @christopherfujino because this further fragments a subset of the tools tests into yet another shard, and I want to hear if you are opposed or otherwise make you aware.

The justification why another shard is needed is that the dependency checking tests need Java 11, and not the preview sdk. And the preview tests now need Java 17 (unless we hardcode old Gradle versions, right now they use create).

And this set is also different from the main shard, which uses Java 17 and the stable (not preview) android sdk.

Yeah, this is ugly, and not efficient with CI resources, but I can't think of a better short-term solution, so I think this is fine. FYI @andrewkolos

Long-term, it might be worth having the tool_integration_tests install multiple JDKs and such in the same build, and then allowing individual tests to select the one they want to test. This would require additional design, however.

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine

@gmackall gmackall added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 1, 2024
@auto-submit auto-submit bot merged commit bd5ab96 into flutter:master Jul 1, 2024
@gmackall
Copy link
Member Author

gmackall commented Jul 1, 2024

Agreed on the wish to have multiple JDKs, and then configure per test

@goderbauer
Copy link
Member

@goderbauer goderbauer added the revert Autorevert PR (with "Reason for revert:" comment) label Jul 1, 2024
auto-submit bot pushed a commit that referenced this pull request Jul 1, 2024
@auto-submit auto-submit bot removed the revert Autorevert PR (with "Reason for revert:" comment) label Jul 1, 2024
auto-submit bot added a commit that referenced this pull request Jul 1, 2024
…sions, and tests (#150969)" (#151147)

Reverts: #150969
Initiated by: goderbauer
Reason for reverting: Failing test in https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8743574743030691569/+/u/run_android_obfuscate_test/stdout
Original PR Author: gmackall

Reviewed By: {christopherfujino, reidbaker}

This change reverts the following previous change:
After the land of flutter/engine#53592, there is some log spam:
```
e: /Users/mackall/.gradle/caches/transforms-3/c1e137371ec1afe9bc9bd7b05823752d/transformed/fragment-1.7.1/jars/classes.jar!/META-INF/fragment_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: /Users/mackall/.gradle/caches/transforms-3/d86c7cb1c556fe1655fa56db671c649c/transformed/jetified-activity-1.8.1/jars/classes.jar!/META-INF/activity_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
...
```

I think this is harmless, but still annoying. Upgrading the AGP version fixes it. To be honest, I don't know why (I expected the Kotlin version would do it). But after #146307, our tests have been running on AGP/Gradle 8.1/8.3 for a while, so it makes sense to upgrade anyways.

In a follow up PR:
1. Also upgrade the tests that were left behind in #146307, as I think removal of discontinued plugins paved the way here.
@gmackall
Copy link
Member Author

gmackall commented Jul 1, 2024

Hmm, looks like this breaks building modules as AARs.

TODO:

  1. Figure out why
  2. The test that we can build a module as an AAR should probably run in presubmit if we change the templates for modules, so add that to the reland.

sigurdm pushed a commit to sigurdm/flutter that referenced this pull request Jul 2, 2024
…d tests (flutter#150969)

After the land of flutter/engine#53592, there is some log spam:
```
e: /Users/mackall/.gradle/caches/transforms-3/c1e137371ec1afe9bc9bd7b05823752d/transformed/fragment-1.7.1/jars/classes.jar!/META-INF/fragment_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: /Users/mackall/.gradle/caches/transforms-3/d86c7cb1c556fe1655fa56db671c649c/transformed/jetified-activity-1.8.1/jars/classes.jar!/META-INF/activity_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
...
```

I think this is harmless, but still annoying. Upgrading the AGP version fixes it. To be honest, I don't know why (I expected the Kotlin version would do it). But after flutter#146307, our tests have been running on AGP/Gradle 8.1/8.3 for a while, so it makes sense to upgrade anyways.

In a follow up PR:
1. Also upgrade the tests that were left behind in flutter#146307, as I think removal of discontinued plugins paved the way here.
sigurdm pushed a commit to sigurdm/flutter that referenced this pull request Jul 2, 2024
…sions, and tests (flutter#150969)" (flutter#151147)

Reverts: flutter#150969
Initiated by: goderbauer
Reason for reverting: Failing test in https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8743574743030691569/+/u/run_android_obfuscate_test/stdout
Original PR Author: gmackall

Reviewed By: {christopherfujino, reidbaker}

This change reverts the following previous change:
After the land of flutter/engine#53592, there is some log spam:
```
e: /Users/mackall/.gradle/caches/transforms-3/c1e137371ec1afe9bc9bd7b05823752d/transformed/fragment-1.7.1/jars/classes.jar!/META-INF/fragment_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: /Users/mackall/.gradle/caches/transforms-3/d86c7cb1c556fe1655fa56db671c649c/transformed/jetified-activity-1.8.1/jars/classes.jar!/META-INF/activity_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
...
```

I think this is harmless, but still annoying. Upgrading the AGP version fixes it. To be honest, I don't know why (I expected the Kotlin version would do it). But after flutter#146307, our tests have been running on AGP/Gradle 8.1/8.3 for a while, so it makes sense to upgrade anyways.

In a follow up PR:
1. Also upgrade the tests that were left behind in flutter#146307, as I think removal of discontinued plugins paved the way here.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 2, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 3, 2024
flutter/flutter@99bb2ff...af913a7

2024-07-02 andrewrkolos@gmail.com Use `ErrorHandlingFileSystem.deleteIfExists` when deleting .plugin_symlinks (flutter/flutter#151073)
2024-07-02 hans.muller@gmail.com ScrollEndNotification example: auto-scroll based on RenderSliver constraints and geometry (flutter/flutter#143538)
2024-07-02 engine-flutter-autoroll@skia.org Roll Packages from 412ec46 to d2705fb (13 revisions) (flutter/flutter#151169)
2024-07-02 goderbauer@google.com docimports for painting (flutter/flutter#151143)
2024-07-02 goderbauer@google.com docimports for scheduler (flutter/flutter#151126)
2024-07-02 nate.w5687@gmail.com `dismissible.dart` code cleanup (flutter/flutter#150276)
2024-07-02 goderbauer@google.com docimports for physics (flutter/flutter#151125)
2024-07-02 goderbauer@google.com docimports for services (flutter/flutter#151134)
2024-07-02 goderbauer@google.com docimports for cupertino (flutter/flutter#151149)
2024-07-02 goderbauer@google.com docimports for gestures (flutter/flutter#151123)
2024-07-02 goderbauer@google.com Docimports for foundation (flutter/flutter#151119)
2024-07-02 goderbauer@google.com docimports for semantics (flutter/flutter#151132)
2024-07-02 jonahwilliams@google.com [flutter_driver] add allocator mtl to memory event allowlist. (flutter/flutter#151153)
2024-07-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 40c087b31515 to 433d360eee11 (7 revisions) (flutter/flutter#151165)
2024-07-02 sigurdm@google.com Refactor BuildInfo to always require packageConfigPath (flutter/flutter#150559)
2024-07-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from d3c5bd66a78f to 40c087b31515 (1 revision) (flutter/flutter#151156)
2024-07-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from fc5bc14e6091 to d3c5bd66a78f (1 revision) (flutter/flutter#151155)
2024-07-02 dkwingsmt@users.noreply.github.com Fix: `CupertinoActionSheet` should take up max height when actions section is short (flutter/flutter#150708)
2024-07-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3456fee1a6b9 to fc5bc14e6091 (8 revisions) (flutter/flutter#151150)
2024-07-02 andrewrkolos@gmail.com [tool] remove some temporary printTrace calls (flutter/flutter#151074)
2024-07-01 nate.w5687@gmail.com Implementing a few switch statements (flutter/flutter#150946)
2024-07-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Upgrade template Gradle, App AGP, Module AGP, and Kotlin versions, and tests (#150969)" (flutter/flutter#151147)
2024-07-01 34871572+gmackall@users.noreply.github.com Upgrade template Gradle, App AGP, Module AGP, and Kotlin versions, and tests (flutter/flutter#150969)
2024-07-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from b57a044ed10f to 3456fee1a6b9 (5 revisions) (flutter/flutter#151127)
2024-07-01 codefu@google.com Read AndroidManifest.xml and emit manifest-aar-impeller-(enabled|disabled) analytics (flutter/flutter#150970)
2024-07-01 goderbauer@google.com More docimports for animation library (flutter/flutter#151011)
2024-07-01 goderbauer@google.com Bump dartdoc to 8.0.10 (flutter/flutter#151107)
2024-07-01 matthiasngeozhongsim@yahoo.com Fix missing `[` in docs (flutter/flutter#151091)
2024-07-01 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#151028)
2024-07-01 jason-simmons@users.noreply.github.com Fix teardown of a FocusScopeNode in material/app_test (flutter/flutter#151115)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC camillesimon@google.com,rmistry@google.com,stuartmorgan@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@gmackall
Copy link
Member Author

gmackall commented Jul 3, 2024

Hmm, looks like this breaks building modules as AARs.

TODO:

  1. Figure out why
  2. The test that we can build a module as an AAR should probably run in presubmit if we change the templates for modules, so add that to the reland.

Looks like using AGP 7.3.0
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/gradle/aar_init_script.gradle#L29
project.components contains [all, debug, profile, release], but is empty in AGP 8.1.0. Need to see if I can find some docs explaining why that might have changed, but that means that we never successfully add the corresponding aar task (e.g. assembleAarDebug)

@reidbaker
Copy link
Contributor

@gmackall
Copy link
Member Author

gmackall commented Jul 3, 2024

I think it is because of
https://developer.android.com/build/releases/past-releases/agp-8-0-0-release-notes

AGP 8.0 creates no SoftwareComponent by default. Instead AGP creates SoftwareComponents only for variants that are configured to be published using the publishing DSL.

And we need to follow https://developer.android.com/build/publish-library/configure-pub-variants, or otherwise change how we are initializing the Aar task.

gmackall pushed a commit to gmackall/flutter that referenced this pull request Jul 8, 2024
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024
…d tests (flutter#150969)

After the land of flutter/engine#53592, there is some log spam:
```
e: /Users/mackall/.gradle/caches/transforms-3/c1e137371ec1afe9bc9bd7b05823752d/transformed/fragment-1.7.1/jars/classes.jar!/META-INF/fragment_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: /Users/mackall/.gradle/caches/transforms-3/d86c7cb1c556fe1655fa56db671c649c/transformed/jetified-activity-1.8.1/jars/classes.jar!/META-INF/activity_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
...
```

I think this is harmless, but still annoying. Upgrading the AGP version fixes it. To be honest, I don't know why (I expected the Kotlin version would do it). But after flutter#146307, our tests have been running on AGP/Gradle 8.1/8.3 for a while, so it makes sense to upgrade anyways.

In a follow up PR:
1. Also upgrade the tests that were left behind in flutter#146307, as I think removal of discontinued plugins paved the way here.
victorsanni pushed a commit to victorsanni/flutter that referenced this pull request Jul 8, 2024
…sions, and tests (flutter#150969)" (flutter#151147)

Reverts: flutter#150969
Initiated by: goderbauer
Reason for reverting: Failing test in https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8743574743030691569/+/u/run_android_obfuscate_test/stdout
Original PR Author: gmackall

Reviewed By: {christopherfujino, reidbaker}

This change reverts the following previous change:
After the land of flutter/engine#53592, there is some log spam:
```
e: /Users/mackall/.gradle/caches/transforms-3/c1e137371ec1afe9bc9bd7b05823752d/transformed/fragment-1.7.1/jars/classes.jar!/META-INF/fragment_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
e: /Users/mackall/.gradle/caches/transforms-3/d86c7cb1c556fe1655fa56db671c649c/transformed/jetified-activity-1.8.1/jars/classes.jar!/META-INF/activity_release.kotlin_module: Module was compiled with an incompatible version of Kotlin. The binary version of its metadata is 1.8.0, expected version is 1.6.0.
...
```

I think this is harmless, but still annoying. Upgrading the AGP version fixes it. To be honest, I don't know why (I expected the Kotlin version would do it). But after flutter#146307, our tests have been running on AGP/Gradle 8.1/8.3 for a while, so it makes sense to upgrade anyways.

In a follow up PR:
1. Also upgrade the tests that were left behind in flutter#146307, as I think removal of discontinued plugins paved the way here.
auto-submit bot pushed a commit that referenced this pull request Jul 10, 2024
…ions, and tests"... but no longer upgrade module AGP version (#151433)

Relands #150969, but removes the upgrade to the module AGP version.

The reason is that a more complicated change is required because in AGP 8.0 software components are no longer generated by default, but rather only generated for variants that are configured to be published using the publishing DSL (see the `android.disableAutomaticComponentCreation` section of https://developer.android.com/build/releases/past-releases/agp-8-0-0-release-notes).

That broke our aar initialization script, because the components didn't exist so the `aar` tasks never got [created here](https://github.com/flutter/flutter/blob/9ff9c67272f7f95c7d53ac3f89e0b659df9ddf3e/packages/flutter_tools/gradle/aar_init_script.gradle#L29).

Verified that the one postsubmit that failed now passes (`android_obfuscate_test`)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024
flutteractionsbot pushed a commit to flutteractionsbot/flutter that referenced this pull request Aug 27, 2024
…ions, and tests"... but no longer upgrade module AGP version (flutter#151433)

Relands flutter#150969, but removes the upgrade to the module AGP version.

The reason is that a more complicated change is required because in AGP 8.0 software components are no longer generated by default, but rather only generated for variants that are configured to be published using the publishing DSL (see the `android.disableAutomaticComponentCreation` section of https://developer.android.com/build/releases/past-releases/agp-8-0-0-release-notes).

That broke our aar initialization script, because the components didn't exist so the `aar` tasks never got [created here](https://github.com/flutter/flutter/blob/9ff9c67272f7f95c7d53ac3f89e0b659df9ddf3e/packages/flutter_tools/gradle/aar_init_script.gradle#L29).

Verified that the one postsubmit that failed now passes (`android_obfuscate_test`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants