Skip to content
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

Reland "Move native assets to isolated/ directory" #143055

Merged
merged 4 commits into from
Feb 8, 2024

Conversation

dcharkes
Copy link
Contributor

@dcharkes dcharkes commented Feb 7, 2024

Reland of #142709.

The revert of the revert is in the first commit, the fix in the commit on top.

The move of the fakes for packages/flutter_tools/test/general.shard/resident_runner_test.dart was erroneous before, as it was trying to use setters instead of a private field. This PR changes the private _devFS field in the fake to be a public fakeDevFS in line with other fakes.

Original PR description

Native assets in other build systems are not built with package:native_assets_builder invoking build.dart scripts. Instead all packages have their own blaze rules. Therefore we'd like to not depend on package:native_assets_builder from flutter tools in g3 at all.

This PR aims to move the imports of native_assets_builder and native_assets_cli into the isolated/ directory and into the files with a main function that are not used in with other build systems.

In order to be able to remove all imports in files used by other build systems, two new interfaces are added HotRunnerNativeAssetsBuilder and TestCompilerNativeAssetsBuilder. New parameters are then piped all the way through from the entry points:

  • bin/fuchsia_tester.dart
  • lib/executable.dart

The build_system/targets dir is already excluded in other build systems.

So, after this PR only the two above files and build_system/targets import from isolated/native_assets/ and only isolated/native_assets/ import package:native_assets_cli and package:native_assets_builder.

Context:

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

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

@chingjun chingjun left a comment

Choose a reason for hiding this comment

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

LGTM

@dcharkes
Copy link
Contributor Author

dcharkes commented Feb 7, 2024

We might need to wait for the revert to land to be able to apply a g3 fix again.
Or roll the revert and the reland together.

FYI @XilaiZhang

patch cl: cl/604636592

Copy link
Contributor

@vashworth vashworth left a comment

Choose a reason for hiding this comment

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

LGTM!

@dcharkes
Copy link
Contributor Author

dcharkes commented Feb 8, 2024

@XilaiZhang please hit submit on this PR once it's possible on your end.

@XilaiZhang XilaiZhang added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 8, 2024
@auto-submit auto-submit bot merged commit 4e70bfa into master Feb 8, 2024
124 checks passed
@auto-submit auto-submit bot deleted the reland-native-assets-isolated branch February 8, 2024 17:49
@XilaiZhang
Copy link
Contributor

cc @itsjustkevin (current oncall) I have pre attached a g3 fix for this reland and submitted this PR just now.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 9, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 10, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
auto-submit bot pushed a commit to dart-lang/native that referenced this pull request Feb 13, 2024
Now that flutter/flutter#143055 has landed, we should be able to start doing breaking changes.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 14, 2024
auto-submit bot pushed a commit that referenced this pull request Feb 14, 2024
Roll of dart-lang/native#964, which separates the `KernelAsset`s (the asset information embedded in the Dart kernel snapshot) from `Asset`s (the assets in the `build.dart` protocol). See the linked issue for why they ought to be different instead of shared.

This PR does not change any functionality in Flutter.

(Now that #143055 has landed, we can land breaking changes.)

For reference, the same roll in the Dart SDK: https://dart-review.googlesource.com/c/sdk/+/352642
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: desktop Running on desktop autosubmit Merge PR when tree becomes green via auto submit App tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants