-
Notifications
You must be signed in to change notification settings - Fork 29.8k
Convert base application name handling to kotlin source (start of FGP kt conversion) #155963
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
Conversation
|
cc @reidbaker I couldn't figure out how to wire up kotlin files to work in the same gradle project, but I was able to get it working if I define them in a sub project like this. WDYT about this approach? (still needs tests/to figure out how to make the module case work, so not ready for review) |
|
Looks like in particular this doesn't work with the legacy script application of the flutter gradle plugin. I wonder if I can somehow hack |
I think it is time to go back to java class based conversions. We ran into the kotlin legacy script issue several times before. Java conversions are net better, net reduction in languages and net easier to test. |
This is unfortunately also a problem with the approach used for the Java conversion, and for the same reason. The new application of the flutter gradle plugin includes the directory that contains the plugin code in the build as a composite build, like this:
But the old application applies it as a script plugin, which means that the existing flutter/packages/flutter_tools/gradle/build.gradle.kts Lines 5 to 26 in f7bcb28
|
|
This is going to be blocked until we turn down the script application of the flutter gradle plugin, which we can start to do after Feb 2025. It would be good to migrate all of our own code in the meantime. |
|
cc @bartekpacia |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-
finally we're getting rid of this hack
-
and we'll get proper support in IntelliJ for developing the gradle plugin
I'm excited!
|
|
||
| class BaseApplicationNameHandlerTest { | ||
| @Test | ||
| fun setBaseName_respectsFlutterToolProperty() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kotlin supports this:
| fun setBaseName_respectsFlutterToolProperty() { | |
| fun `setBaseName respects Flutter tool property`() { |
it's a matter of taste but I think it's cool.
(not sure if there are some android-specific problems with it though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's awesome, changed!
Still need to get these tests hooked up to run in CI, that is the last part of the PR. Working on it now.
FYI that they are runnable (with coverage even) in Android studio now through the ui
| class BaseApplicationNameHandler { | ||
| companion object { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be simply an object (instead of class + companion object)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work, changed! I haven't had the chance to write much Kotlin, so extra scrutiny on things that look like abnormal Kotlin is appreciated
|
Question: I think a better layout would be this (what I have currently in #161352): |
Hmm I can try changing to that structure, I don't remember the reason why at this point if there was one (the PR is pretty old) |
|
cool! I'll just add that |
bkonyi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tool test LGTM, RSLGTM on the Kotlin changes.
…t of FGP kt conversion) (flutter/flutter#155963)
…t of FGP kt conversion) (flutter/flutter#155963)
…t of FGP kt conversion) (flutter/flutter#155963)
…t of FGP kt conversion) (flutter/flutter#155963)
Roll Flutter from 72db8f6 to 40c2b86 (33 revisions) flutter/flutter@72db8f6...40c2b86 2025-01-14 christopherfujino@gmail.com update changelog for 3.27.2 release (flutter/flutter#161569) 2025-01-14 tessertaha@gmail.com Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599) 2025-01-14 30870216+gaaclarke@users.noreply.github.com Added special case for fat width arcs (flutter/flutter#161255) 2025-01-14 matanlurey@users.noreply.github.com Replace `fetch `with `gclient sync`. (flutter/flutter#161565) 2025-01-14 engine-flutter-autoroll@skia.org Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597) 2025-01-14 robert.ancell@canonical.com Remove unused method (flutter/flutter#161572) 2025-01-14 hbatagelo@gmail.com Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375) 2025-01-14 bruno.leroux@gmail.com Update InputDecoration.border documentation (flutter/flutter#161415) 2025-01-14 dkwingsmt@users.noreply.github.com [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917) 2025-01-14 a-siva@users.noreply.github.com Roll Dart to Version 3.7.0-323.0.dev (flutter/flutter#161567) 2025-01-14 goderbauer@google.com Use wildcards (flutter/flutter#161548) 2025-01-14 jmccandless@google.com Autocomplete Options Width (flutter/flutter#143249) 2025-01-13 jason-simmons@users.noreply.github.com Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405) 2025-01-13 matanlurey@users.noreply.github.com Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529) 2025-01-13 flar@google.com Fix paths when running clang-tidy on git diffs (flutter/flutter#161496) 2025-01-13 yjbanov@google.com [web:a11y] treat empty tappables as buttons (flutter/flutter#161360) 2025-01-13 58190796+MitchellGoodwin@users.noreply.github.com Add route settings to CupertinoSheetRoute (flutter/flutter#161528) 2025-01-13 matanlurey@users.noreply.github.com Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532) 2025-01-13 matanlurey@users.noreply.github.com Remove last two references to Cirrus CI. (flutter/flutter#161530) 2025-01-13 codefu@google.com Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550) 2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555) 2025-01-13 bkonyi@google.com Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124) 2025-01-13 matanlurey@users.noreply.github.com Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450) 2025-01-13 dhankechakishan@gmail.com Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223) 2025-01-13 matanlurey@users.noreply.github.com Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527) 2025-01-13 chinmaygarde@google.com [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251) 2025-01-13 flar@google.com Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453) 2025-01-13 jmccandless@google.com Context menu button callback docs clarification (flutter/flutter#161451) 2025-01-13 43089218+chika3742@users.noreply.github.com Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241) 2025-01-13 codefu@google.com Udpate documentation on the third_party directories (flutter/flutter#161407) 2025-01-13 matanlurey@users.noreply.github.com Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452) 2025-01-13 34871572+gmackall@users.noreply.github.com Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963) 2025-01-13 jonahwilliams@google.com [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438) 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 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: ...
… kt conversion) (flutter#155963) Wires up a new gradle subproject defining kotlin classes to be used by the FGP, so that we can incrementally move the entire plugin to be written in kotlin source. Starts by moving a piece of kotlin script. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [ ] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [ ] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md --------- Co-authored-by: Gray Mackall <mackall@google.com>
…t of FGP kt conversion) (flutter/flutter#155963)
…t of FGP kt conversion) (flutter/flutter#155963)
…t of FGP kt conversion) (flutter/flutter#155963)
…t of FGP kt conversion) (flutter/flutter#155963)
…t of FGP kt conversion) (flutter/flutter#155963)
Roll Flutter from 72db8f6 to 40c2b86 (33 revisions) flutter/flutter@72db8f6...40c2b86 2025-01-14 christopherfujino@gmail.com update changelog for 3.27.2 release (flutter/flutter#161569) 2025-01-14 tessertaha@gmail.com Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599) 2025-01-14 30870216+gaaclarke@users.noreply.github.com Added special case for fat width arcs (flutter/flutter#161255) 2025-01-14 matanlurey@users.noreply.github.com Replace `fetch `with `gclient sync`. (flutter/flutter#161565) 2025-01-14 engine-flutter-autoroll@skia.org Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597) 2025-01-14 robert.ancell@canonical.com Remove unused method (flutter/flutter#161572) 2025-01-14 hbatagelo@gmail.com Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375) 2025-01-14 bruno.leroux@gmail.com Update InputDecoration.border documentation (flutter/flutter#161415) 2025-01-14 dkwingsmt@users.noreply.github.com [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917) 2025-01-14 a-siva@users.noreply.github.com Roll Dart to Version 3.7.0-323.0.dev (flutter/flutter#161567) 2025-01-14 goderbauer@google.com Use wildcards (flutter/flutter#161548) 2025-01-14 jmccandless@google.com Autocomplete Options Width (flutter/flutter#143249) 2025-01-13 jason-simmons@users.noreply.github.com Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405) 2025-01-13 matanlurey@users.noreply.github.com Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529) 2025-01-13 flar@google.com Fix paths when running clang-tidy on git diffs (flutter/flutter#161496) 2025-01-13 yjbanov@google.com [web:a11y] treat empty tappables as buttons (flutter/flutter#161360) 2025-01-13 58190796+MitchellGoodwin@users.noreply.github.com Add route settings to CupertinoSheetRoute (flutter/flutter#161528) 2025-01-13 matanlurey@users.noreply.github.com Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532) 2025-01-13 matanlurey@users.noreply.github.com Remove last two references to Cirrus CI. (flutter/flutter#161530) 2025-01-13 codefu@google.com Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550) 2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555) 2025-01-13 bkonyi@google.com Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124) 2025-01-13 matanlurey@users.noreply.github.com Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450) 2025-01-13 dhankechakishan@gmail.com Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223) 2025-01-13 matanlurey@users.noreply.github.com Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527) 2025-01-13 chinmaygarde@google.com [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251) 2025-01-13 flar@google.com Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453) 2025-01-13 jmccandless@google.com Context menu button callback docs clarification (flutter/flutter#161451) 2025-01-13 43089218+chika3742@users.noreply.github.com Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241) 2025-01-13 codefu@google.com Udpate documentation on the third_party directories (flutter/flutter#161407) 2025-01-13 matanlurey@users.noreply.github.com Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452) 2025-01-13 34871572+gmackall@users.noreply.github.com Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963) 2025-01-13 jonahwilliams@google.com [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438) 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 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: ...
Roll Flutter from 72db8f6 to 40c2b86 (33 revisions) flutter/flutter@72db8f6...40c2b86 2025-01-14 christopherfujino@gmail.com update changelog for 3.27.2 release (flutter/flutter#161569) 2025-01-14 tessertaha@gmail.com Fix `showLicensePage` does not inherit ambient `Theme` (flutter/flutter#161599) 2025-01-14 30870216+gaaclarke@users.noreply.github.com Added special case for fat width arcs (flutter/flutter#161255) 2025-01-14 matanlurey@users.noreply.github.com Replace `fetch `with `gclient sync`. (flutter/flutter#161565) 2025-01-14 engine-flutter-autoroll@skia.org Roll Packages from 3c3bc68 to d1fd623 (4 revisions) (flutter/flutter#161597) 2025-01-14 robert.ancell@canonical.com Remove unused method (flutter/flutter#161572) 2025-01-14 hbatagelo@gmail.com Fix crash when closing a window with `Alt+F4` in multi-win Flutter on Windows (flutter/flutter#161375) 2025-01-14 bruno.leroux@gmail.com Update InputDecoration.border documentation (flutter/flutter#161415) 2025-01-14 dkwingsmt@users.noreply.github.com [Web] Allow specifying the strategy on when to use <img> element to display images (flutter/flutter#159917) 2025-01-14 a-siva@users.noreply.github.com Roll Dart to Version 3.7.0-323.0.dev (flutter/flutter#161567) 2025-01-14 goderbauer@google.com Use wildcards (flutter/flutter#161548) 2025-01-14 jmccandless@google.com Autocomplete Options Width (flutter/flutter#143249) 2025-01-13 jason-simmons@users.noreply.github.com Move the analyzer_benchmark to Mac arm64 devicelab bots (flutter/flutter#161405) 2025-01-13 matanlurey@users.noreply.github.com Remove references to `cirrus`, mostly in doc comments. (flutter/flutter#161529) 2025-01-13 flar@google.com Fix paths when running clang-tidy on git diffs (flutter/flutter#161496) 2025-01-13 yjbanov@google.com [web:a11y] treat empty tappables as buttons (flutter/flutter#161360) 2025-01-13 58190796+MitchellGoodwin@users.noreply.github.com Add route settings to CupertinoSheetRoute (flutter/flutter#161528) 2025-01-13 matanlurey@users.noreply.github.com Copy `linux_host_engine` as `linux_host_engine_test`, removing `archives: [...]`. (flutter/flutter#161532) 2025-01-13 matanlurey@users.noreply.github.com Remove last two references to Cirrus CI. (flutter/flutter#161530) 2025-01-13 codefu@google.com Mark `Mac_mokey microbenchmarks` as flakey (flutter/flutter#161550) 2025-01-13 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (#160241)" (flutter/flutter#161555) 2025-01-13 bkonyi@google.com Add validator execution times to `flutter doctor --verbose` (flutter/flutter#158124) 2025-01-13 matanlurey@users.noreply.github.com Explain more specifically how to use `flutter drive`/what it does (flutter/flutter#161450) 2025-01-13 dhankechakishan@gmail.com Fixed repeated strings for incompatible Gradle or AGP version in `create` command (flutter/flutter#161223) 2025-01-13 matanlurey@users.noreply.github.com Remove `WEB_SHARD_COUNT`, which no longer exists post-Cirrus. (flutter/flutter#161527) 2025-01-13 chinmaygarde@google.com [Impeller] Update guidance on prebuilt artifacts. (flutter/flutter#161251) 2025-01-13 flar@google.com Migrate DisplayList unit tests to DL/Impeller geometry classes (flutter/flutter#161453) 2025-01-13 jmccandless@google.com Context menu button callback docs clarification (flutter/flutter#161451) 2025-01-13 43089218+chika3742@users.noreply.github.com Match CupertinoPageTransitionsBuilder animation duration to CupertinoPageRoute (flutter/flutter#160241) 2025-01-13 codefu@google.com Udpate documentation on the third_party directories (flutter/flutter#161407) 2025-01-13 matanlurey@users.noreply.github.com Propagate environment variables when `flutter drive` is invoked. (flutter/flutter#161452) 2025-01-13 34871572+gmackall@users.noreply.github.com Convert base application name handling to kotlin source (start of FGP kt conversion) (flutter/flutter#155963) 2025-01-13 jonahwilliams@google.com [Impeller] remove API 30 restriction for SurfaceControl testing. (flutter/flutter#161438) 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 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: ...
Wires up a new gradle subproject defining kotlin classes to be used by the FGP, so that we can incrementally move the entire plugin to be written in kotlin source.
Starts by moving a piece of kotlin script.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.