-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
Add support for pluggable transformation of assets at build time using packages #141194
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
tool
Affects the "flutter" command-line tool. See also t: labels.
label
Jan 9, 2024
andrewkolos
force-pushed
the
asset-transformation-2
branch
2 times, most recently
from
January 11, 2024 18:20
c539432
to
9e3252e
Compare
8 tasks
auto-submit bot
pushed a commit
that referenced
this pull request
Jan 23, 2024
…ew type, `AssetBundleEntry` (#142029) Part of work on #141194 The [`AssetBundle`](https://github.com/flutter/flutter/blob/0833929c997c8a9db11c1e0df9a731ab17b77606/packages/flutter_tools/lib/src/asset.dart#L80) class contains two members, `entries` and `entryKinds`. `entries` contains asset data indexed by asset key. `entryKinds` contains the "kinds" of these assets, again indexed by asset key. **Change.** Rather than have two separate maps, this PR proposes combining these maps into one by wrapping the asset data and kind into a single data type `AssetBundleEntry`. **Purpose.** In #141194, I am considering associating more information with an asset. In particular, what transformers are meant to be applied to it when copying it to the build output. Rather than adding another map member onto `AssetBundle` (e.g. `entryTransformers`), I decided to make things neater by introducing the `AssetBundleEntry` type.
andrewkolos
force-pushed
the
asset-transformation-2
branch
from
January 25, 2024 00:47
9e3252e
to
ce5a8ae
Compare
8 tasks
auto-submit bot
pushed a commit
that referenced
this pull request
Jan 25, 2024
Part of work on [#101077](#141194). This is done as a separate PR to avoid a massive diff. ## Context 1. The `FakeCommand` class accepts a list of patterns that's used to match a command given to its `FakeProcessManager`. Since `FakeCommand` can match a list of patterns, not just specifically strings, it can be used to match commands where the exact value of some arguments can't (easily) known ahead of time. For example, a part of the tool may invoke a command with an argument that is the path of a temporarily file that has a randomly-generated basename. 2. The `FakeCommand` class provides on `onRun` parameter, which is a callback that is run when the `FakeProcessManager` runs a command that matches the `FakeCommand` in question. ## Issue In the event that a `FakeCommand` is constructed using patterns, the test code can't know the exact values used for arguments in the command. This PR proposes changing the type of `onRun` from `VoidCallback?` to `void Function(List<String>)?`. When run, the value `List<String>` parameter will be the full command that the `FakeCommand` matched. Example: ```dart FakeCommand( command: <Pattern>[ artifacts.getArtifactPath(Artifact.engineDartBinary), 'run', 'vector_graphics_compiler', RegExp(r'--input=/.*\.temp'), RegExp(r'--output=/.*\.temp'), ], onRun: (List<String> command) { final outputPath = (() { // code to parse `--output` from `command` })(); testFileSystem.file(outputPath).createSync(recursive: true); }, ) ```
8 tasks
andrewkolos
force-pushed
the
asset-transformation-2
branch
from
February 6, 2024 20:41
ce5a8ae
to
1712d6e
Compare
andrewkolos
force-pushed
the
asset-transformation-2
branch
from
February 7, 2024 18:10
1712d6e
to
b71183e
Compare
This was referenced Feb 12, 2024
auto-submit bot
pushed a commit
that referenced
this pull request
Feb 13, 2024
) This is in service of #141194 This will make it easier to get the `flutter run -d <browser>` and `flutter build fuschia` cases easier to get under test.
Closing as superceded by smaller PRs |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WIP
Most recent status update: happy path works appears to work for both
flutter run
andflutter build
for at least web, iOS, and macOS)Closes #101077
You can play with this feature using this project: https://github.com/andrewkolos/asset_transformers_test. Make sure your
flutter
checkout is on this branch and that you have cleaned out your local flutter tool cache (glob<flutter install dir>/bin/cache/flutter_tools*
).Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.