[file_selector] updates build files to use JVM 17 #10097
Conversation
AbdeMohlbi
requested review from
mboetger and
stuartmorgan-g
as code owners
This comment was marked as outdated.
This comment was marked as outdated.
Yeah that sounds good. |
reidbaker
left a comment
reidbaker
left a comment
There was a problem hiding this comment.
Deeper investigation into what would break users.
This plugin (and the other prs) need to set a minimum flutter version to 3.35. see packages/file_selector/file_selector_android/pubspec.yaml
Logic:
AGP 8.0 was the first AGP version to require java 17. https://developer.android.com/build/releases/past-releases/agp-8-0-0-release-notes
3.32 has a warn AGP version set to 8.3 but an error version set to 7.0 https://github.com/flutter/flutter/blob/3.32.8/packages/flutter_tools/gradle/src/main/kotlin/DependencyVersionChecker.kt#L94
3.35 has a error version of AGP set to 8.1.1 https://github.com/flutter/flutter/blob/3.35.4/packages/flutter_tools/gradle/src/main/kotlin/DependencyVersionChecker.kt#L103
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
This comment was marked as outdated.
This comment was marked as outdated.
| environment: | ||
| sdk: ^3.7.0 | ||
| flutter: ">=3.29.0" | ||
| sdk: ">=3.9.0 <4.0.0" |
There was a problem hiding this comment.
Why isn't this using ^ syntax?
There was a problem hiding this comment.
because this is what I saw in all of the other places we have already update 3.35. Would you like me to use the ^ syntax with dart 3.9.0?
There was a problem hiding this comment.
Where are you seeing that? AFAIK we always use ^ for the Dart SDK.
There was a problem hiding this comment.
I assume "root package" means the app-facing package? It is not clear to me why you made this change. |
|
Additional work after 3.35 min flutter version was updated.
|
The PR has still regenerated the Pigeon files, and the change came in a minor Pigeon update. The fix for this PR isn't to re-run Pigeon generation without a major version update, it's to revert all Pigeon-related changes. |
This comment was marked as resolved.
This comment was marked as resolved.
|
#10126 will fix the latent Pigeon definition issue so that nobody else gets bitten by this. |
This comment was marked as resolved.
This comment was marked as resolved.
I don't see any other packages in the repo where someone made the mistake of listing an external class as an interface. |
This comment was marked as resolved.
This comment was marked as resolved.
|
i think this is ready now |
reidbaker
force-pushed
the
file_selector_java_17
branch
from
0abc772 to
b8337b7
Compare
reidbaker
force-pushed
the
file_selector_java_17
branch
from
b8337b7 to
4bee17a
Compare
reidbaker
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/packages/10097, because Pull request flutter/packages/10097 is not in a mergeable state. |
reidbaker
added
the
autosubmit
flutter/packages@287739d...321a584 2025-09-30 [email protected] [local_auth] Adopt structured errors - platform interface (flutter/packages#10023) 2025-09-30 [email protected] Revert hardcoded compilesdk back to `flutter.compileSdkVersion` in `camera_android` (flutter/packages#9668) 2025-09-30 [email protected] [file_selector] updates build files to use JVM 17 (flutter/packages#10097) 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-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
I see a bunch of the other PRs landing. Because of CI issues, this version of |
|
(pub.dev version graphs show that essentially nobody is actually using |
The original intention was to convert them but then after some time i got a lot of conflicts that i decided to close those PRs and then make those changes later on , for the CI failures ,i guess @reidbaker has a theory about it |
|
Checking patch analytics on https://pub.dev/packages/file_selector_android/score and issues filed against the repo https://github.com/search?q=repo%3Aflutter%2Fpackages+file_selector+java&type=issues I think the java 17 packages changes are safe now that we have had a couple of days of soak on the file_selector_android changes. |
…r#176357) flutter/packages@287739d...321a584 2025-09-30 [email protected] [local_auth] Adopt structured errors - platform interface (flutter/packages#10023) 2025-09-30 [email protected] Revert hardcoded compilesdk back to `flutter.compileSdkVersion` in `camera_android` (flutter/packages#9668) 2025-09-30 [email protected] [file_selector] updates build files to use JVM 17 (flutter/packages#10097) 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-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: 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
3 participants
Update minimum flutter version to 3.35 to force a minimum AGP version to something that requires java 17.
part of #176027
All dart code changes were formatter changes.
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3