Skip to content

Conversation

@mkustermann
Copy link
Member

Almost all of the code is just adopting to changes to the APIs of
package:native_assets_builder, package:native_assets_cli and package:native_toolchain_c

There's only two semantic changes

  • Removes a test that checks for a verification error if a build hook produces a static library if the preferred linking mode is dynamic:
    => The test is written in a very hacky way. By monkey patching the build config.json that flutter build actually made. This monkey patching relies on package:cli_config which is now no longer used.
    => The actual code that checks for this mismatch lives in dart-lang/native repository and is tested there. So there's really no need to duplicate that.

  • The package:native_assets_builder no longer knows about code assets. This is something a user of that package (e.g. flutter tools) adds. Now the dry-run functionality will invoke build hooks who produce code assets without an architecture.
    => The package:native_assets_builder used to expand such a code asset to N different code assets (one for each supported architecture)
    => This logic was now moved to flutter tools. => In the near future we're going to this dry-run complexity, which will then also get rid of this uglyness (of expanding to all archs of an OS).

@github-actions github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. a: desktop Running on desktop labels Nov 5, 2024
@mkustermann mkustermann requested a review from mraleph November 6, 2024 08:21
…ssets_cli,native_toolchain_c}

Almost all of the code is just adopting to changes to the APIs.

There's only two semantic changes

* Removes a test that checks for a verification error if a build hook
  produces a static library if the preferred linking mode is dynamic:
  => The test is written in a very hacky way. By monkey patching the
  buid config.json that flutter build actually made. This monkey
  patching relies on package:cli_config which is now no longer used.
  => The actual code that checks for this mismatch lives in dart-lang/native
  repository and is tested there. So there's really no need to duplicate that.

* The `package:native_assets_builder` no longer knows about code assets.
  This is something a user of that package (e.g. flutter tools) adds.
  Now the dry-run functionality will invoke build hooks who produce code
  asstets without an architecture.
  => The `package:native_assets_builder` used to expand such a code
  asset to N different code assets (one for each supported architecture)
  => This logic was now moved to flutter tools.
  => In the near future we're going to this dry-run complexity, which
  will then also get rid of this uglyness (of expanding to all archs of
  an OS).

Update manually pined package map

Fix tests
@mkustermann mkustermann force-pushed the roll-native-assets-pkgs branch from 2e376a0 to 2a31370 Compare November 6, 2024 10:31
@mkustermann
Copy link
Member Author

Thanks, @mraleph

@mkustermann mkustermann merged commit 3a83e43 into flutter:master Nov 6, 2024
138 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 6, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 6, 2024
flutter/flutter@29d40f7...73546b3

2024-11-06 [email protected] Add test for `image.loading_builder.0.dart` (flutter/flutter#158248)
2024-11-06 [email protected] Make flutter_tools use newest package:{native_assets_builder,native_assets_cli,native_toolchain_c} (flutter/flutter#158214)
2024-11-06 [email protected] Fix RawScrollbar examples and desktop test (flutter/flutter#158237)
2024-11-06 [email protected] Cleanup MenuAnchor and Improve DropdownMenu tests readability (flutter/flutter#158175)
2024-11-06 [email protected] Roll Flutter Engine from a3741d6248b7 to f03f11300a9d (2 revisions) (flutter/flutter#158222)
2024-11-06 [email protected] Update error message for Cocoapods support for synchronized groups/folders (flutter/flutter#158206)
2024-11-06 [email protected] Restore skipped iOS test by looping over `FakeAsync` elapse. (flutter/flutter#158204)
2024-11-06 [email protected] fix: ensure draggable_scrollable_sheet respects shouldCloseOnMinExtenâ�¦ (flutter/flutter#156338)
2024-11-06 [email protected] Roll Flutter Engine from e5e06c97c33c to a3741d6248b7 (14 revisions) (flutter/flutter#158218)
2024-11-06 [email protected] Forward fix `CupertinoDynamicColor` by adding `toARGB32()`. (flutter/flutter#158145)
2024-11-05 [email protected] Remove unused `enableObservatory` flag. (flutter/flutter#158202)
2024-11-05 [email protected] Remove observatory related TODO that is already fixed. (flutter/flutter#158205)
2024-11-05 [email protected] Factor out "shaker" class (flutter/flutter#157748)
2024-11-05 [email protected] Marks Mac_benchmark animated_complex_opacity_perf_macos__e2e_summary to be flaky (flutter/flutter#157424)
2024-11-05 [email protected] Increase subsharding for `Linux tool_integration_tests` (flutter/flutter#158196)
2024-11-05 [email protected] Add test for `raw_scrollbar.2.dart` (flutter/flutter#158161)
2024-11-05 [email protected] use root directory as the default for rootOverride in Cache.test constructor (flutter/flutter#158201)
2024-11-05 [email protected] Kill interactive script job `xcdevice observe` processes on tool/daemon shutdown (flutter/flutter#157646)
2024-11-05 [email protected] Fix: Gap between prefix and suffix icon and input field in input decoâ�¦ (flutter/flutter#152069)
2024-11-05 [email protected] Roll Flutter Engine from f56401062e42 to e5e06c97c33c (1 revision) (flutter/flutter#158194)

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
EArminjon pushed a commit to EArminjon/flutter that referenced this pull request Nov 7, 2024
…ssets_cli,native_toolchain_c} (flutter#158214)

Almost all of the code is just adopting to changes to the APIs of
`package:native_assets_builder`, `package:native_assets_cli` and
`package:native_toolchain_c`

There's only two semantic changes

* Removes a test that checks for a verification error if a build hook
produces a static library if the preferred linking mode is dynamic:
=> The test is written in a very hacky way. By monkey patching the build
config.json that flutter build actually made. This monkey patching
relies on package:cli_config which is now no longer used.
=> The actual code that checks for this mismatch lives in
dart-lang/native repository and is tested there. So there's really no
need to duplicate that.

* The `package:native_assets_builder` no longer knows about code assets.
This is something a user of that package (e.g. flutter tools) adds. Now
the dry-run functionality will invoke build hooks who produce code
assets without an architecture.
=> The `package:native_assets_builder` used to expand such a code asset
to N different code assets (one for each supported architecture)
=> This logic was now moved to flutter tools. => In the near future
we're going to this dry-run complexity, which will then also get rid of
this uglyness (of expanding to all archs of an OS).
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.

2 participants