-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Deferred Components tooling #63773
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
Deferred Components tooling #63773
Conversation
412c3da to
9e4bf2b
Compare
|
This pull request is not suitable for automatic merging in its current state.
|
e176467 to
0777397
Compare
packages/flutter_tools/lib/src/build_system/targets/android.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use platform file separator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be in the output files. Is there anything wrong with always generating it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This option is the switch that determines if gen_snapshot produces split aot or not. WIthout, it produces a regular aot snapshot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whats going on here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copying all app.so files + manifest.json instead of just app.so
packages/flutter_tools/lib/src/build_system/targets/common.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/lib/src/commands/build_setup_split_aot.dart
Outdated
Show resolved
Hide resolved
packages/flutter_tools/lib/src/commands/build_setup_split_aot.dart
Outdated
Show resolved
Hide resolved
|
All of the additional files that are generated need to be tracked somehow so that build invalidations work. There are a few mechanisms for doing so in gradle and in the tool (see input/output files and the build_system doc comment). |
8181b0b to
c4d92b6
Compare
|
This pull request is not suitable for automatic merging in its current state.
|
|
Watching this thread in order to try it ASAP. |
ac732e9 to
5772d39
Compare
|
Something I've noticed is that I don't see where we're tracking the addition .so files output by gen snapshot - we need those to correctly tell gradle how to clean stuff up |
xster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Second pass. Getting closer!
.../flutter_tools/templates/module/android/deferred_component/src/main/AndroidManifest.xml.tmpl
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this inherit from the base apk's build.gradle? i.e. does profile build work and does release build work without signing locally while testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to remove these and let developers opt into it manually if they wish.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Say it does nothing (presumably) if set to true for a project with no deferred components?
Maybe add a link to documentation (at some point) to describe when true really becomes (verifiably) true? When deferred imports are used or when the new tag is added to pubspec.yaml presumably?
packages/flutter_tools/lib/src/build_system/targets/android.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@blasten here we're trying to add entries into the android { } section of a user's build.gradle. We can just ask them to apply from: 'deferred_components.gradle' and then we'll just own that file and freely manipulate it right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to remove the need to add this section manually. It is now added programmatically in flutter.gradle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. It's already in our tools dependencies
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't you make a file for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite. This is strings.xml. The contents here are too dynamic for a template to work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to try to change this to use AndroidManifest.xml + applicationInfo to store the mapping rather than strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
describe the problem statement first. i.e. why is there a golden file and what is it solving
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means if their file is 1000 lines and we're making it 1010 lines, they have to diff it themselves. It's ok but if we can make a .patch file, even better.
|
Tests coming soon. |
xster
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please let @jonahwilliams take another look. This still needs tests but otherwise LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice if we printed a message when we fallback too
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: line length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mini-nit: I assume this can just be a final variable instead of a getter?
packages/flutter_tools/lib/src/android/deferred_components_setup_validator.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: line breaks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll be sure to give this a full formatting pass :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd generally print something here. Since there's a degree of dynamism wrt to the command flag defaulting to true but it being only true when the pubspec tag is also present. So print something when it's really on. On the same line, state whether the verification is also on or off.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm still not sure I understood what the answer to my question was. If the loading units have a list of their libraries, why do you need to keep another list of the libraries?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Describe a bit what the implications are? Maybe it's just an asset component?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: line length
packages/flutter_tools/lib/src/build_system/targets/android.dart
Outdated
Show resolved
Hide resolved
cd8b2c3 to
deb0dba
Compare
eddd96c to
fa97228
Compare
30528bb to
175d4d4
Compare
t # This is a combination of 3 commits. Separate prebuild and build tooling, early working(ish) version Refactor, cleanup Integrate dart library names golden generation refactor REmove extra deps Remove conflicts Sync version numbers with local.properties in modules Remove no-use-bare-instructions Use strings.xml for loading unit -> module mapping Copy assets prototype Begin assets packaging Pack assets into split mdoule Package assets Fix after rebase Add shrinking disable back to support multi-apk builds Rename to deferred_component_config Move commands to proper location Move commands to new locations, cleanup Additional cleanup, rename to deferred d components Begin transition into single command setup Basic one-command verification and setup Build multiple ABIs Assets work Working intermediates system Shift .so multiplexing into dart, cleanup Consolidation and additional cleanup Goldens work Migrate so copy to new target Move setup to prebuild Working prebuild for setup Setup verification work Working setup system Cleanup and begin backwards/debug compatibility Cleanup More cleanup: Refactor verification and setup into calss, split into two parts: pre and post assembly Refactor setup code, address comments Refactoring, begin templates impl, address comments Working template based generation Docs, refactor, rename Refactor, libs copy depfile More depfiles work, debug run assets working Add dynamicFeatures build.gradle programmatically More refactoring, depfile fixes, improvement to copying and loading unit parsing Include unused loading units in base module Crash fixes: Prevent validation on debug builds, fix flutter run behavior Flutter run working again AndroidManifest.xml validation and writing Move check to Target to track dependencies. Resolve various crashes and flutter run issues Fix multi-split assets Generate diff Message formatting, async fixes Fix strings.xml edits and log recommended command to apply diff Improve strings.xml algorithm Improve regex, Add unit tests Begin validator tests, custom loggers Null safety Passing initial tests Add many validator tests Component android dir tests Add tests for AndroidManifest mapping and string.xml names Finish validator tests Test for deferred components .so copying Add validator Target test flutter manifest tests Round 1 analyzer fixes Refactor, passes analyzer Fix casts Merge with upstream Merge Fix abi mismatch when switching targets between builds Pass abi to validator Switch to BufferLogger Begin integration tests for tooling Building deferred components apps works in integration test Passing build appbundle integration tests Fix tests and analyzer, cleanup: Whitespaces Update diff Sync with validator PR Sync with validator PR tests Integrate with new validator Sync with splitbuild Fix null dep Merge changes Add tests for target
|
#76192 contains everything remaining in this PR and it has just landed. Closing. |
Hi Gary, I'm focusing on the aot-split feature of flutter, so I'm trying to run the demo: https://github.com/GaryQian/Flutter-Split-AOT-Sample, but failed, it just shows that no splitaot command found for flutter build,I'm using the newest flutter-sdk and flutter-engine and dart-sdk of the master branch, so what version should I use? or how? thanks |
|
@BottleChain Ahh, I apologize, that demo is very outdated. The actual landed version is completely different. I'll likely update that sample repo at some point. In the meantime, the new WIP instructions/guide is at https://docs.google.com/document/d/10ngyMlLIHiDiD_DueE_NHxzl-obqnfdzvMQj5_WV3Z4/edit?usp=sharing You (and anyone else interested) can go ahead and request access, I'll be happy to grant it. I didn't make it public initially, so it is a bit of a logistical bump to properly publicize, so this will have to do for now. It should be published soon. I'd love any feedback on the feature/process/etc! Quick reference: The new command is just |
Master PR for deferred components build and setup verification tooling.
Primary issue: #57617
go/flutter-aot-split