Skip to content

Conversation

@mkustermann
Copy link
Member

Currently the NativeAsset target in flutter tools is responsible for two things:

  • performing the dart build (in the app as well as all transitive pub dependencies)
  • taking output (shared libraries) from this build and copying them around

This intermingling of responsibilities leads to more complex code and potentially unnecessary work: If the source code changed (e.g. .c files change) we have to run the dart build again. But doing so may result in the same shared libraries (e.g. adding comments to the .c code). Currently we're going to copy the shared libraries despite them having not changed, which then may cause upstream things to be dirtied (if it's based on timestamp of files) and re-built.

Instead this PR splits this NativeAsset into the two orthogonal pieces

  • DartBuild target that is responsible for the dart build
  • InstallCodeAssets that is responsible for copying shared libraries to the right place and producing a native_assets.yaml.

This decopuling is also prepration for a future where a dart build can produce other kinds of assets (e.g. data assets) and is used in the web build as well. (The web build would use DartBuild but not InstalCodeAssets).

@github-actions github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Nov 14, 2024
Copy link
Member

@mraleph mraleph left a comment

Choose a reason for hiding this comment

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

LGTM

@mkustermann
Copy link
Member Author

Thanks, @bkonyi !

Currently the `NativeAsset` target in flutter tools is responsible for
two things:

  * performing the dart build (in the app as well as all transitive pub dependencies)
  * taking output (shared libraries) from this build and copying them
    around

This intermingling of responsibilities leads to more complex code and
potentially unnecessary work: If the source code changed (e.g. `.c`
files change) we have to run the dart build again. But doing so may
result in the same shared libraries (e.g. adding comments to the `.c`
code). Currently we're going to copy the shared libraries despite them
having not changed, which then may cause upstream things to be dirtied
(if it's based on timestamp of files) and re-built.

Instead this PR splits this `NativeAsset` into the two orthogonal pieces

* `DartBuild` target that is responsible for the dart build
* `InstallCodeAssets` that is responsible for copying shared libraries
  to the right place and producing a `native_assets.yaml`.

This decopuling is also prepration for a future where a dart build can
produce other kinds of assets (e.g. data assets) and is used in the web
build as well. (The web build would use `DartBuild` but not
`InstalCodeAssets`).
@mkustermann mkustermann merged commit 01590aa into flutter:master Nov 15, 2024
144 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 16, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 16, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 18, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 18, 2024
flutter/flutter@0e2d55e...b3818f6

2024-11-18 [email protected] Roll Flutter Engine from a63841d0c64b to f365c9f5dce3 (1 revision) (flutter/flutter#159064)
2024-11-18 [email protected] Roll Flutter Engine from 4878f1c9ed1c to a63841d0c64b (8 revisions) (flutter/flutter#159057)
2024-11-17 [email protected] Roll Flutter Engine from 4c1eb8cacb95 to 4878f1c9ed1c (1 revision) (flutter/flutter#159050)
2024-11-17 [email protected] Roll Flutter Engine from 9d0720a7b0ec to 4c1eb8cacb95 (1 revision) (flutter/flutter#159049)
2024-11-17 [email protected] Roll Flutter Engine from 6f9854ad6ed7 to 9d0720a7b0ec (2 revisions) (flutter/flutter#159047)
2024-11-17 [email protected] Roll Flutter Engine from 40000c742910 to 6f9854ad6ed7 (2 revisions) (flutter/flutter#159041)
2024-11-16 [email protected] Fix NavigationBar example overflow alignment (flutter/flutter#159034)
2024-11-16 [email protected] Roll Flutter Engine from 908061196148 to 40000c742910 (1 revision) (flutter/flutter#159038)
2024-11-16 [email protected] Roll Flutter Engine from 85d445e3cf78 to 908061196148 (3 revisions) (flutter/flutter#159032)
2024-11-16 [email protected] Roll Flutter Engine from 1800193ba961 to 85d445e3cf78 (1 revision) (flutter/flutter#159020)
2024-11-16 [email protected] Roll Flutter Engine from 98c3b7f8d1a7 to 1800193ba961 (1 revision) (flutter/flutter#159019)
2024-11-16 [email protected] Roll Flutter Engine from f23832175b7a to 98c3b7f8d1a7 (1 revision) (flutter/flutter#159017)
2024-11-16 [email protected] Roll Flutter Engine from f649330affa8 to f23832175b7a (1 revision) (flutter/flutter#159016)
2024-11-16 [email protected] Roll Flutter Engine from 619804c0fbb7 to f649330affa8 (47 revisions) (flutter/flutter#159015)
2024-11-16 [email protected] Marks Linux web_benchmarks_skwasm_st to be unflaky (flutter/flutter#158563)
2024-11-16 [email protected] Marks Windows windows_desktop_impeller to be unflaky (flutter/flutter#158565)
2024-11-15 [email protected] [flutter triage] Update list of frequent web contributors (flutter/flutter#159008)
2024-11-15 [email protected] Switch `flutter_build_apk_health_tests` to use a subset of current tests. (flutter/flutter#159004)
2024-11-15 [email protected] Make the focus node on SelectableRegion optional. (flutter/flutter#158994)
2024-11-15 [email protected] Style change in Flutter-Web-Triage.md (flutter/flutter#159006)
2024-11-15 [email protected] Refactor native asset integration into flutter tools (flutter/flutter#158932)
2024-11-15 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Try running historically flaky tests first to make `flutter build apk` health tests time out more often? (#158967)" (flutter/flutter#158993)
2024-11-15 [email protected] [web] Remove the benchmarks of the HTML renderer (flutter/flutter#158520)

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] 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
@reidbaker reidbaker mentioned this pull request Dec 13, 2024
11 tasks
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants