Skip to content

Avoid depending on files from build_system/targets other than from top level entrypoints in flutter_tools. #142760

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.

Sign up for GitHub

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

Jump to bottom
Merged
auto-submit merged 4 commits into from
Feb 2, 2024
Merged

Avoid depending on files from build_system/targets other than from top level entrypoints in flutter_tools. #142760

auto-submit merged 4 commits into from
Feb 2, 2024

Conversation

@chingjun
Copy link
Contributor

@chingjun chingjun commented Feb 1, 2024

Add a new BuildTargets class that provides commonly used build targets. And avoid importing files from build_system/targets except from the top level entrypoints or from top level commands.

Also move scene_importer.dart and shader_compiler.dart into build_system/tools because they are not Target classes, but wrapper for certain tools.

With this change, we can ignore all files in build_system/targets internally and make PR #142709 easier to land internally. See cl/603434066 for the corresponding internal change.

Related to:
#142709
#142041

Also note that I have opted to add a new variable in globals.dart for BuildTargets in this PR, but I know that we are trying to get rid of globals. Several alternatives that I was considering:

  1. Add a new field in BuildSystem that returns a BuildTargets instance. Since BuildSystem is already in globals, we can access build targets using globals.buildSystem.buildTargets without adding a new global variable.
  2. Properly inject the BuildTargetsImpl instance from the top level executable.dart and top level commands.

Let me know if you want me to do one of the above instead. Thanks!

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems tool Affects the "flutter" command-line tool. See also t: labels. labels Feb 1, 2024
@christopherfujino
Copy link
Contributor

christopherfujino commented Feb 2, 2024

With this change, we can ignore all files in build_system/targets internally and make PR #142709 easier to land internally.

So to be clear, everything under build_system/targets will NOT be rolled internally, right?

@chingjun
Copy link
Contributor Author

chingjun commented Feb 2, 2024 •
edited
Loading

Right, they will not be built internally.

@chingjun chingjun force-pushed the isolate_build_system branch from 3c30afa to 00549fe Compare February 2, 2024 01:01
@christopherfujino
Copy link
Contributor

christopherfujino commented Feb 2, 2024

Right, they will not be built internally.

ahh, right. 👍

christopherfujino
christopherfujino approved these changes Feb 2, 2024
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@chingjun chingjun added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Feb 2, 2024
@chingjun chingjun force-pushed the isolate_build_system branch from df1a4a0 to a423fd0 Compare February 2, 2024 05:22
@chingjun
Avoid depending on files from build_system/targets other than from to…
7d3b4d0 
…p level entrypoints in flutter_tools.

Add a new `BuildTargets` class that provides commonly used build
targets. And avoid importing files from `build_system/targets` except
from the top level entrypoints or from top level commands.

Also move `scene_importer.dart` and `shader_compiler.dart` into
`build_system/tools` because they are not `Target` classes, but wrapper
for certain tools.
@chingjun
Fix analyzer issues
f24b625
@chingjun
Update function signature after resolving conflict.
61ca76d
@chingjun
More fix to function signature
4b1e353
@chingjun chingjun force-pushed the isolate_build_system branch from ce96860 to 4b1e353 Compare February 2, 2024 06:31
@chingjun chingjun added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 2, 2024
@auto-submit auto-submit bot merged commit ac7879e into flutter:master Feb 2, 2024
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 2, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 2, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
cacf46a 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 2, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 2, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
ed8f7be 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@XilaiZhang XilaiZhang added the cp: beta cherry pick this pull request to beta release candidate branch label Feb 2, 2024
@flutteractionsbot
Copy link

flutteractionsbot commented Feb 2, 2024

Failed to create CP due to merge conflicts.
You will need to create the PR manually. See the cherrypick wiki for more info.

@XilaiZhang XilaiZhang removed the cp: beta cherry pick this pull request to beta release candidate branch label Feb 2, 2024
@chingjun chingjun mentioned this pull request Feb 2, 2024
Merged
8 tasks
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 3, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 3, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
36817bc 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 3, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 3, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
42aceba 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 4, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 4, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
bb60ee1 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 4, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 4, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
e269ae9 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 5, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 5, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
e481d8c 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 5, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 5, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
9e744fb 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 5, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 5, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
73ad8cb 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Feb 5, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 5, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
79c7611 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
tarrinneal pushed a commit to flutter/packages that referenced this pull request Feb 5, 2024
@engine-flutter-autoroll
Manual roll Flutter from e02e207 to 0b5cd50 (46 revisions) (#6053)
d63629d 
Manual roll Flutter from e02e207 to 0b5cd50 (46 revisions)

Manual roll requested by tarrinneal@google.com

flutter/flutter@e02e207...0b5cd50

2024-02-05 124896814+BiskupMaik@users.noreply.github.com fix AppBar docs
for backgroundColor & foregroundColor (flutter/flutter#142430)
2024-02-04 98614782+auto-submit[bot]@users.noreply.github.com Reverts
"Update gradle lockfiles template" (flutter/flutter#142889)
2024-02-04 barpac02@gmail.com Update gradle lockfiles template
(flutter/flutter#140115)
2024-02-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from
20742e37e54e to f34c658b9600 (1 revision) (flutter/flutter#142876)
2024-02-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from
23763db72272 to 20742e37e54e (1 revision) (flutter/flutter#142850)
2024-02-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from
fee02145da8c to 23763db72272 (3 revisions) (flutter/flutter#142848)
2024-02-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from
9869d47a2736 to fee02145da8c (2 revisions) (flutter/flutter#142847)
2024-02-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from
78c63d3c2c68 to 9869d47a2736 (1 revision) (flutter/flutter#142842)
2024-02-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from
266d5d0b5588 to 78c63d3c2c68 (1 revision) (flutter/flutter#142836)
2024-02-02 49699333+dependabot[bot]@users.noreply.github.com Bump
github/codeql-action from 3.23.2 to 3.24.0 (flutter/flutter#142839)
2024-02-02 49699333+dependabot[bot]@users.noreply.github.com Bump
codecov/codecov-action from 3.1.6 to 4.0.1 (flutter/flutter#142838)
2024-02-02 jmccandless@google.com Update TextSelectionOverlay
(flutter/flutter#142463)
2024-02-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from
e29263212bfd to 266d5d0b5588 (5 revisions) (flutter/flutter#142832)
2024-02-02 luccas.clezar@gmail.com Fix CupertinoTextSelectionToolbar
clipping (flutter/flutter#138195)
2024-02-02 barpac02@gmail.com Reland "Add support for Gradle Kotlin DSL
(#140744)" (flutter/flutter#142752)
2024-02-02 jmccandless@google.com Support navigation during a Cupertino
back gesture (flutter/flutter#142248)
2024-02-02 chingjun@google.com Avoid depending on files from
build_system/targets other than from top level entrypoints in
flutter_tools. (flutter/flutter#142760)
2024-02-02 engine-flutter-autoroll@skia.org Roll Packages from
5b48c44 to d37fb0a (14 revisions) (flutter/flutter#142812)
2024-02-02 32242716+ricardoamador@users.noreply.github.com Add a link
the different possible Android virtual device configs
(flutter/flutter#142765)
2024-02-02 15619084+vashworth@users.noreply.github.com Allow all iOS
tests to use either iOS 16 or 17 (flutter/flutter#142714)
2024-02-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from
b35153d00b2e to e29263212bfd (2 revisions) (flutter/flutter#142799)
2024-02-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from
dd4c79a6c864 to b35153d00b2e (10 revisions) (flutter/flutter#142783)
2024-02-02 jacksongardner@google.com Wasm/JS Dual Compile with the
flutter tool (flutter/flutter#141396)
2024-02-02 hans.muller@gmail.com Reland: Added
ButtonStyle.foregroundBuilder and ButtonStyle.backgroundBuilder
(flutter/flutter#142762)
2024-02-01 32242716+ricardoamador@users.noreply.github.com Use proto
name for emulator version and show cipd package version
(flutter/flutter#142262)
2024-02-01 xilaizhang@google.com [github actions] ping actor of workflow
on cherry pick pr creation (flutter/flutter#142676)
2024-02-01 fluttergithubbot@gmail.com Marks Linux_android_emu android
views to be unflaky (flutter/flutter#142590)
2024-02-01 nathan.wilson1232@gmail.com Implement `switch` expressions in
`lib/src/material/` (flutter/flutter#142634)
2024-02-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
9beb7e82e081 to dd4c79a6c864 (1 revision) (flutter/flutter#142749)
2024-02-01 pateltirth454@gmail.com Write Tests for API Example of
`form.0.dart` (flutter/flutter#142635)
2024-02-01 polinach@google.com Make leak_tracking bots sticked to the
left even if bot thinks they are non-flacky. (flutter/flutter#142744)
2024-02-01 15619084+vashworth@users.noreply.github.com Upload
DerivedData logs in CI (flutter/flutter#142643)
2024-02-01 magder@google.com Test codesigning xcframeworks in artifacts
(flutter/flutter#142666)
2024-02-01 davidmartos96@gmail.com Fix gen_defaults test randomness
(flutter/flutter#142743)
2024-02-01 98614782+auto-submit[bot]@users.noreply.github.com Reverts
"Added ButtonStyle.foregroundBuilder and ButtonStyle.backgroundBuilder"
(flutter/flutter#142748)
2024-02-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
39415c3eed42 to 9beb7e82e081 (5 revisions) (flutter/flutter#142745)
2024-02-01 magder@google.com Remove unused deprecated autoroll
mirror-remote flag (flutter/flutter#142738)
2024-02-01 polinach@google.com Fix leaks in tests.
(flutter/flutter#142677)
2024-02-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
8c43332c6ffc to 39415c3eed42 (1 revision) (flutter/flutter#142740)
2024-02-01 magder@google.com Remove verbose-system-logs on iOS perf
tests (flutter/flutter#142739)
2024-02-01 magder@google.com Remove outdated arm64_armv7 check
(flutter/flutter#142737)
2024-02-01 62812903+sstasi95@users.noreply.github.com fix
CupertinoTabView's Android back button handling with PopScope
(flutter/flutter#141604)
2024-02-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from
68943afd62d1 to 8c43332c6ffc (8 revisions) (flutter/flutter#142726)
2024-02-01 christopherfujino@gmail.com Unpin test
(flutter/flutter#141427)
...
dumazy added a commit to dumazy/flutter that referenced this pull request Feb 7, 2024
@dumazy
Merge branch 'master' into fix/form_autovalidatemode_always
1d5f999 
* master: (45 commits)
  Reverts "Update gradle lockfiles template" (flutter#142889)
  Update gradle lockfiles template (flutter#140115)
  Roll Flutter Engine from 20742e3 to f34c658 (1 revision) (flutter#142876)
  Roll Flutter Engine from 23763db to 20742e3 (1 revision) (flutter#142850)
  Roll Flutter Engine from fee0214 to 23763db (3 revisions) (flutter#142848)
  Roll Flutter Engine from 9869d47 to fee0214 (2 revisions) (flutter#142847)
  Roll Flutter Engine from 78c63d3 to 9869d47 (1 revision) (flutter#142842)
  Roll Flutter Engine from 266d5d0 to 78c63d3 (1 revision) (flutter#142836)
  Bump github/codeql-action from 3.23.2 to 3.24.0 (flutter#142839)
  Bump codecov/codecov-action from 3.1.6 to 4.0.1 (flutter#142838)
  Update TextSelectionOverlay (flutter#142463)
  Roll Flutter Engine from e292632 to 266d5d0 (5 revisions) (flutter#142832)
  Fix CupertinoTextSelectionToolbar clipping (flutter#138195)
  Reland "Add support for Gradle Kotlin DSL (flutter#140744)" (flutter#142752)
  Support navigation during a Cupertino back gesture (flutter#142248)
  Avoid depending on files from build_system/targets other than from top level entrypoints in flutter_tools. (flutter#142760)
  Roll Packages from 5b48c44 to d37fb0a (14 revisions) (flutter#142812)
  Add a link the different possible Android virtual device configs (flutter#142765)
  Allow all iOS tests to use either iOS 16 or 17 (flutter#142714)
  Roll Flutter Engine from b35153d to e292632 (2 revisions) (flutter#142799)
  ...
@eyebrowsoffire eyebrowsoffire mentioned this pull request Feb 8, 2024
Merged
auto-submit bot pushed a commit that referenced this pull request Feb 8, 2024
@eyebrowsoffire
Revert Dual Web Compile changes (#143175)
2efeeb4 
Dual Web Compile has had some issues where `flutter test` is not respecting the `--web-renderer` flag for some reason. I haven't gotten entirely to the bottom of the issue, but for now we need to rever these changes while I investigate. This reverts the following PRs:

#143128
#141396

While doing this revert, I had a few merge conflicts with #142760, and I tried to resolve the merge conflicts within the spirit of that PR's change, but @chingjun I might need your input on whether the imports I have modified are okay with regards to the change you were making.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
@engine-flutter-autoroll
ac7879e Avoid depending on files from build_system/targets other than…
526aa21 
… from top level entrypoints in flutter_tools. (flutter/flutter#142760)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

a: text input Entering text in a text field or keyboard related problems 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@chingjun @christopherfujino @flutteractionsbot @XilaiZhang