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 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 [email protected] Use `ErrorHandlingFileSystem.deleteIfExists` when deleting .plugin_symlinks (flutter/flutter#151073)
2024-07-02 [email protected] ScrollEndNotification example: auto-scroll based on RenderSliver constraints and geometry (flutter/flutter#143538)
2024-07-02 [email protected] Roll Packages from 412ec46 to d2705fb (13 revisions) (flutter/flutter#151169)
2024-07-02 [email protected] docimports for painting (flutter/flutter#151143)
2024-07-02 [email protected] docimports for scheduler (flutter/flutter#151126)
2024-07-02 [email protected] `dismissible.dart` code cleanup (flutter/flutter#150276)
2024-07-02 [email protected] docimports for physics (flutter/flutter#151125)
2024-07-02 [email protected] docimports for services (flutter/flutter#151134)
2024-07-02 [email protected] docimports for cupertino (flutter/flutter#151149)
2024-07-02 [email protected] docimports for gestures (flutter/flutter#151123)
2024-07-02 [email protected] Docimports for foundation (flutter/flutter#151119)
2024-07-02 [email protected] docimports for semantics (flutter/flutter#151132)
2024-07-02 [email protected] [flutter_driver] add allocator mtl to memory event allowlist. (flutter/flutter#151153)
2024-07-02 [email protected] Roll Flutter Engine from 40c087b31515 to 433d360eee11 (7 revisions) (flutter/flutter#151165)
2024-07-02 [email protected] Refactor BuildInfo to always require packageConfigPath (flutter/flutter#150559)
2024-07-02 [email protected] Roll Flutter Engine from d3c5bd66a78f to 40c087b31515 (1 revision) (flutter/flutter#151156)
2024-07-02 [email protected] Roll Flutter Engine from fc5bc14e6091 to d3c5bd66a78f (1 revision) (flutter/flutter#151155)
2024-07-02 [email protected] Fix: `CupertinoActionSheet` should take up max height when actions section is short (flutter/flutter#150708)
2024-07-02 [email protected] Roll Flutter Engine from 3456fee1a6b9 to fc5bc14e6091 (8 revisions) (flutter/flutter#151150)
2024-07-02 [email protected] [tool] remove some temporary printTrace calls (flutter/flutter#151074)
2024-07-01 [email protected] 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 [email protected] Upgrade template Gradle, App AGP, Module AGP, and Kotlin versions, and tests (flutter/flutter#150969)
2024-07-01 [email protected] Roll Flutter Engine from b57a044ed10f to 3456fee1a6b9 (5 revisions) (flutter/flutter#151127)
2024-07-01 [email protected] Read AndroidManifest.xml and emit manifest-aar-impeller-(enabled|disabled) analytics (flutter/flutter#150970)
2024-07-01 [email protected] More docimports for animation library (flutter/flutter#151011)
2024-07-01 [email protected] Bump dartdoc to 8.0.10 (flutter/flutter#151107)
2024-07-01 [email protected] Fix missing `[` in docs (flutter/flutter#151091)
2024-07-01 [email protected] Roll pub packages (flutter/flutter#151028)
2024-07-01 [email protected] 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 [email protected],[email protected],[email protected] 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)

@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