Skip to content

Conversation

@andrewkolos
Copy link
Contributor

@andrewkolos andrewkolos commented Mar 11, 2024

In service of #143348

When invoking a package to transform an asset, we set FLUTTER_BUILD_MODE to the CLI name of the build mode being used. Inspired by #101077 (comment):

Do transformers know whether they get executed in debug or release mode? I kinda imagine that being useful. Ex: There's a transformer that optimizes the file size of images. Depending on the amount and size of the images, that could take a significant amount of time. Therefore, I might want to only execute it in release builds.

Note for the reviewer: the interesting part of this change can be found in the commit set environment variable to build mode when running asset transformer…. The rest of the change is updating call sites with a new argument.


Boring stuff: Reland of #144752, which had to be reverted because the branch was stale. The original branch branched off master before #144734 landed. That PR introduced a new AssetTransformer call site.

This PR branch is identical to the original but with a new commit that addresses the new call site, update new call sites.

Pre-launch Checklist

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

@andrewkolos andrewkolos added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 11, 2024
@andrewkolos andrewkolos changed the title Add build mode env variable to asset transformer environment Reland "Expose build mode in environment of asset transformer processes" Mar 11, 2024
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.

RSLGTM

@andrewkolos andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 11, 2024
@Hixie
Copy link
Contributor

Hixie commented Mar 12, 2024

Not a big deal but for the future, please bear in mind this part of the Tree Hygiene doc:

Also avoid using "Reland" in the commit message. When you later revert the revert, just land the PR afresh with the original commit message, possibly updated with the information since collected, and include a link to the original PR and to the revert PR so that people can follow the breadcrumbs later.

Otherwise when doing code archeology later (e.g. walking through git blame) it gets quite confusing because the commit that actually stuck doesn't have the actual information that's useful.

@auto-submit
Copy link
Contributor

auto-submit bot commented Mar 12, 2024

auto label is removed for flutter/flutter/144958, due to - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 12, 2024
@andrewkolos andrewkolos 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 Mar 22, 2024
@andrewkolos andrewkolos force-pushed the add-build-mode-env-variable-to-asset-transformer-environment branch from 6bb5296 to 60d2bd3 Compare March 22, 2024 20:42
@christopherfujino
Copy link
Contributor

@andrewkolos is this ready to be submitted?

@andrewkolos andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 22, 2024
@auto-submit auto-submit bot merged commit 0fc08ab into flutter:master Apr 22, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 22, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 22, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 22, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Apr 22, 2024
flutter/flutter@1a905d5...140edb9

2024-04-22 [email protected] Fixed few typos (flutter/flutter#147087)
2024-04-22 [email protected] Add Amir Panahandeh to AUTHORS (flutter/flutter#147052)
2024-04-22 [email protected] print traces when transforming an asset (flutter/flutter#146374)
2024-04-22 [email protected] Roll Packages from 88a3a56 to 01a32c4 (5 revisions) (flutter/flutter#147164)
2024-04-22 [email protected] Reland "Expose build mode in environment of asset transformer processes" (flutter/flutter#144958)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gilnobrega pushed a commit to gilnobrega/flutter that referenced this pull request Apr 22, 2024
…es" (flutter#144958)

Relands flutter#144752, which had to be reverted because the branch was stale. The original branch branched off `master` before flutter#144734 landed. That PR introduced a new `AssetTransformer` call site.

This PR branch is identical to the original but with a new commit that addresses the new call site, [update new call sites](flutter@6bb5296).
@andrewkolos andrewkolos deleted the add-build-mode-env-variable-to-asset-transformer-environment branch April 29, 2024 18:17
TecHaxter pushed a commit to TecHaxter/flutter_packages that referenced this pull request May 22, 2024
flutter/flutter@1a905d5...140edb9

2024-04-22 [email protected] Fixed few typos (flutter/flutter#147087)
2024-04-22 [email protected] Add Amir Panahandeh to AUTHORS (flutter/flutter#147052)
2024-04-22 [email protected] print traces when transforming an asset (flutter/flutter#146374)
2024-04-22 [email protected] Roll Packages from 88a3a56 to 01a32c4 (5 revisions) (flutter/flutter#147164)
2024-04-22 [email protected] Reland "Expose build mode in environment of asset transformer processes" (flutter/flutter#144958)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
JSUYA added a commit to JSUYA/flutter-tizen that referenced this pull request Aug 28, 2024
sfshaza2 pushed a commit to flutter/website that referenced this pull request Dec 17, 2024
…11510)

Documents the feature long-ago implemented by
flutter/flutter#144958.

**Summary**: this documents that asset-transforming Dart packages can
read `FLUTTER_BUILD_MODE` from the environment to determine what mode
the app is being built in. This also tries to fix what I think was a
typo in the original docs.

If you are curious about what asset transformation is, read [Assets and
Images
(flutter.dev)](https://docs.flutter.dev/ui/assets/assets-and-images) to
learn how to bundle assets (arbitrary files, such as images) into your
app and then read [Asset transformation
(flutter.dev)](https://docs.flutter.dev/ui/assets/asset-transformation).

## Presubmit checklist

- [x] This PR is marked as draft with an explanation if not meant to
land until a future stable release.
- [x] This PR doesn’t contain automatically generated corrections
(Grammarly or similar).
- [x] This PR follows the [Google Developer Documentation Style
Guidelines](https://developers.google.com/style) — for example, it
doesn’t use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first person).
- [x] This PR uses [semantic line
breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks)
of 80 characters or fewer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

3 participants