[flutter_tools] prefactoring for linux, windows release targets #55708
Conversation
fluttergithubbot
added
tool
| /// expected to be in the root output directory. | ||
| /// | ||
| /// All assets and manifests are included from flutter_assets/**. | ||
| abstract class AndroidAssetBundle extends Target { |
There was a problem hiding this comment.
Most apps share the same bundle structure so I refactored this into a common class.
packages/flutter_tools/lib/src/build_system/targets/common.dart
Outdated
Show resolved
Hide resolved
| /// All assets and manifests are included from flutter_assets/**. | ||
| abstract class ApplicationAssetBundle extends Target { | ||
| const ApplicationAssetBundle({ | ||
| this.shouldCopyAssets = true, |
There was a problem hiding this comment.
Document what this means?
| @override | ||
| Future<void> build(Environment environment) async { | ||
| await super.build(environment); | ||
| environment.buildDir.childFile('app.so') |
There was a problem hiding this comment.
Presumably this doesn't apply to iOS and macOS; maybe clarify in the comments the intended platforms?
| List<Source> get inputs => <Source>[ | ||
| ...super.inputs, | ||
| const Source.artifact(Artifact.vmSnapshotData, mode: BuildMode.debug), | ||
| const Source.artifact(Artifact.isolateSnapshotData, mode: BuildMode.debug), |
There was a problem hiding this comment.
Why are the inputs and outputs in the subclass, but the logic that copies from the input to the output location in the superclass? Shouldn't the superclass be describing the inputs and outputs it's managing?
There was a problem hiding this comment.
The vm_snapshot_data , isolate_snapshot_data, and kernel_blob.bin are not part of profile/release builds. This sort of structure allows me to statically describe all of the dependencies while only having a single build implementation.
There was a problem hiding this comment.
I guess I'm not seeing how having one build method is an advantage when it means having the input/output description in a different class—and file—than the code that actually causes it to be an input/output. It seems like this would be incredibly error-prone to maintain because it's not at all obvious that a change here would require changes in a different file, or the other way around.
Why not have a debug subclass that has the debug inputs, outputs, and build logic, and then derive from that here?
…s/flutter into add_desktop_release_targets
| @override | ||
| List<Source> get inputs => <Source>[ | ||
| ...super.inputs, | ||
| const Source.pattern('{BUILD_DIR}/app.so'), |
There was a problem hiding this comment.
Related to the other comment: why aren't these in ReleaseAssetBundle?
| List<Source> get inputs => <Source>[ | ||
| ...super.inputs, | ||
| const Source.artifact(Artifact.vmSnapshotData, mode: BuildMode.debug), | ||
| const Source.artifact(Artifact.isolateSnapshotData, mode: BuildMode.debug), |
There was a problem hiding this comment.
I guess I'm not seeing how having one build method is an advantage when it means having the input/output description in a different class—and file—than the code that actually causes it to be an input/output. It seems like this would be incredibly error-prone to maintain because it's not at all obvious that a change here would require changes in a different file, or the other way around.
Why not have a debug subclass that has the debug inputs, outputs, and build logic, and then derive from that here?
|
Closing as obsolete. |
Description
Initial cleanup to implement Linux, Windows release targets.
Verified that both Linux and Windows still build! Also renames
dart.darttocommon.dartto reduce insanity (or increase sanity).I also added code for the Linux, Windows release bundles. These are fairly trivial since its a single arch build.