8000 "flutter run" will unconditionally rewrite plugin registrants · Issue #134899 · flutter/flutter · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

"flutter run" will unconditionally rewrite plugin registrants #134899

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

Closed
2 tasks done
iamsergio opened this issue Sep 17, 2023 · 5 comments · Fixed by #167262
Closed
2 tasks done

"flutter run" will unconditionally rewrite plugin registrants #134899

iamsergio opened this issue Sep 17, 2023 · 5 comments · Fixed by #167262
Labels
a: build Building flutter applications with the tool a: desktop Running on desktop a: plugins Support for writing, building, and running plugin packages found in release: 3.13 Found to occur in 3.13 found in release: 3.14 Found to occur in 3.14 good first issue Relatively approachable for first-time contributors has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team

Comments

@iamsergio
Copy link

Is there an existing issue for this?

Steps to reproduce

Consecutive runs of flutter run will regenerate and rebuild the C++ boilerplate, which makes the build slower.

On Desktop (at least Linux, but probably others as well), flutter run generates some C++ files such as:

linux/flutter/generated_plugin_registrant.cc
windows/flutter/generated_plugin_registrant.cc

The problem is that it will regenerate them even if they already exist. Flutter should 1st compare the contents before regenerating them, otherwise cmake will notice the new timestamp and trigger a rebuild.

Expected results

flutter run -v should not show any time spent running cmake and ninja if source files haven't changed since the last good build.

Actual results

unneeded rebuilds

Code sample

flutter create foo
cd foo
flutter run
flutter run -v

Screenshots or Video

No response

Logs

[ +414 ms] [2/5] Building CXX object CMakeFiles/pointless.dir/flutter/generated_plugin_registrant.cc.o
[  +20 ms] [3/5] Building CXX object CMakeFiles/pointless.dir/my_application.cc.o
[  +99 ms] [4/5] Linking CXX executable intermediates_do_not_run/pointless
[        ] [4/5] Install the project...

Flutter Doctor output

Flutter 3.13.0 • channel stable • https://github.com/flutter/flutter.git
Framework • revision efbf63d9c6 (5 weeks ago) • 2023-08-15 21:05:06 -0500
Engine • revision 1ac611c64e
Tools • Dart 3.1.0 • DevTools 2.25.0
@huycozy huycozy added the in triage Presently being triaged by the triage team label Sep 18, 2023
@huycozy
Copy link
Member
huycozy commented Sep 18, 2023

Reproduced this issue on Linux. This looks like a proposal instead. Labeling this for more opinions.

flutter doctor -v (stable & master)
[✓] Flutter (Channel stable, 3.13.4, on Ubuntu 22.04.3 LTS 6.2.0-32-generic, locale en_US.UTF-8)
    • Flutter version 3.13.4 on channel stable at /home/huynq/Documents/Working/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 367f9ea16b (5 days ago), 2023-09-12 23:27:53 -0500
    • Engine revision 9064459a8b
    • Dart version 3.1.2
    • DevTools version 2.25.0

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0-rc3)
    • Android SDK at /home/huynq/And
8000
roid/Sdk/
    • Platform android-31, build-tools 34.0.0-rc3
    • Java binary at: /home/huynq/Documents/android-studio/jre/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7590822)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • Chrome at google-chrome

[✓] Linux toolchain - develop for Linux desktop
    • clang version 10.0.0-4ubuntu1
    • cmake version 3.22.1
    • ninja version 1.10.1
    • pkg-config version 0.29.2

[✓] Android Studio (version 2021.1)
    • Android Studio at /home/huynq/Documents/android-studio
    • Flutter plugin version 67.0.1
    • Dart plugin version 211.7817
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7590822)

[✓] VS Code (version 1.82.2)
    • VS Code at /usr/share/code
    • Flutter extension version 3.72.0

[✓] Connected device (2 available)
    • Linux (desktop) • linux  • linux-x64      • Ubuntu 22.04.3 LTS 6.2.0-32-generic
    • Chrome (web)    • chrome • web-javascript • Google Chrome 116.0.5845.187

[✓] Network resources
    • All expected network resources are available.

• No issues found!
[!] Flutter (Channel master, 3.14.0-14.0.pre.309, on Ubuntu 22.04.3 LTS 6.2.0-32-generic, locale en_US.UTF-8)
    • Flutter version 3.14.0-14.0.pre.309 on channel master at /home/huynq/Documents/Working/flutter_master
    ! Warning: `flutter` on your path resolves to /home/huynq/Documents/Working/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /home/huynq/Documents/Working/flutter_master.
      Consider adding /home/huynq/Documents/Working/flutter_master/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /home/huynq/Documents/Working/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /home/huynq/Documents/Working/flutter_master. Consider
      adding /home/huynq/Documents/Working/flutter_master/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 0b540a87f1 (3 days ago), 2023-09-15 13:39:30 +0900
    • Engine revision 45bc4307cd
    • Dart version 3.2.0 (build 3.2.0-162.0.dev)
    • DevTools version 2.28.0-dev.0
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0-rc3)
    • Android SDK at /home/huynq/Android/Sdk/
    • Platform android-31, build-tools 34.0.0-rc3
    • Java binary at: /home/huynq/Documents/android-studio/jre/bin/java
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7590822)
    • All Android licenses accepted.

[✓] Chrome - develop for the web
    • Chrome at google-chrome

[✓] Linux toolchain - develop for Linux desktop
    • clang version 10.0.0-4ubuntu1
    • cmake version 3.22.1
    • ninja version 1.10.1
    • pkg-config version 0.29.2

[✓] Android Studio (version 2021.1)
    • Android Studio at /home/huynq/Documents/android-studio
    • Flutter plugin version 67.0.1
    • Dart plugin version 211.7817
    • Java version OpenJDK Runtime Environment (build 11.0.11+0-b60-7590822)

[✓] VS Code (version 1.82.2)
    • VS Code at /usr/share/code
    • Flutter extension version 3.72.0

[✓] Connected device (2 available)
    • Linux (desktop) • linux  • linux-x64      • Ubuntu 22.04.3 LTS 6.2.0-32-generic
    • Chrome (web)    • chrome • web-javascript • Google Chrome 116.0.5845.187

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.

@huycozy huycozy added tool Affects the "flutter" command-line tool. See also t: labels. platform-windows Building on or for Windows specifically platform-linux Building on or for Linux specifically a: desktop Running on desktop a: build Building flutter applications with the tool has reproducible steps The issue has been confirmed reproducible and is ready to work on team-desktop found in release: 3.13 Found to occur in 3.13 found in release: 3.14 Found to occur in 3.14 and removed in triage Presently being triaged by the triage team labels Sep 18, 2023
@gspencergoog gspencergoog added P3 Issues that are less important to the Flutter project triaged-desktop labels Sep 21, 2023
@cbracken cbracken added team-linux Owned by the Linux platform team fyi-linux For the attention of the Linux platform team and removed team-desktop labels Jun 6, 2024
@flutter-triage-bot flutter-triage-bot bot removed fyi-linux For the attention of the Linux platform team triaged-desktop labels Jun 7, 2024
@flutter-triage-bot
Copy link

The fyi-linux label is redundant with the team-linux label.
The triaged-desktop label is irrelevant if there is no team-desktop label or fyi-desktop label.

@jmagman jmagman added the triaged-linux Triaged by the Linux platform team label Jun 7, 2024
@stuartmorgan-g stuartmorgan-g added a: plugins Support for writing, building, and running plugin packages team-tool Owned by Flutter Tool team triaged-tool Triaged by Flutter Tool team and removed team-linux Owned by the Linux platform team triaged-linux Triaged by the Linux platform team labels Jun 7, 2024
@stuartmorgan-g
Copy link
Contributor
stuartmorgan-g commented Jun 7, 2024

This is true on every platform, adjusting labels and title accordingly. We should just make _renderTemplateToFile read the file if it already exists, and check the strings. That will add some time, but not as much as changing the file timestamp is likely to add to the build.

(Upping priority, since this is low-hanging fruit to improve build times.)

@stuartmorgan-g stuartmorgan-g added good first issue Relatively approachable for first-time contributors and removed P3 Issues that are less important to the Flutter project platform-windows Building on or for Windows specifically labels Jun 7, 2024
@stuartmorgan-g stuartmorgan-g removed the platform-linux Building on or for Linux specifically label Jun 7, 2024
@stuartmorgan-g stuartmorgan-g changed the title "flutter run" will unconditionally generate and rebuild generated_plugin_registrant.cc "flutter run" will unconditionally rewrite plugin registrants Jun 7, 2024
@stuartmorgan-g stuartmorgan-g added the P2 Important issues not at the top of the work list label Jun 7, 2024
@kkdlau
Copy link
kkdlau commented Jun 8, 2024

@stuartmorgan I would like to work this issue, appreciate it if you can assign this issue to me!

8000

@stuartmorgan-g
Copy link
Contributor

@kkdlau Please feel free to send a PR; we don't generally assign issues to people who don't have edit permissions, since they can't unassign themselves if they decide not to work on it.

kkdlau added a commit to kkdlau/flutter that referenced this issue Jun 9, 2024
github-merge-queue bot pushed a commit that referenced this issue Apr 28, 2025
<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->

This PR adds logic to skip overwriting plugin registrant files if their
contents are unchanged.

Fixes #134899

## 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].
- [X] 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.
- [x] 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: build Building flutter applications with the tool a: desktop Running on desktop a: plugins Support for writing, building, and running plugin packages found in release: 3.13 Found to occur in 3.13 found in release: 3.14 Found to occur in 3.14 good first issue Relatively approachable for first-time contributors has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list team-tool Owned by Flutter Tool team tool Affects the "flutter" command-line tool. See also t: labels. triaged-tool Triaged by Flutter Tool team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants
0