[vector_graphics] Use hooks in addition to asset transformers

[mosuem]

Requires flutter config --enable-dart-data-assets.

This is an example of how replacing asset transformers with build/link hooks could look like. The basic idea is that package vector_graphics_compiler exposes a helper method, here compileSvg, which is then called in the package's build hook. Options can be set there as well.

For now, asset transformers and build hooks can simply co-exist - in the future, when hooks are stable enough, we will want to deprecate asset transformers and move towards this mechanism.

[bkonyi]

LGTM!

[chinmaygarde]

Any progress on the presubmit failures?

[mosuem]

Slow progress :) There are a lot of presubmits..

[mosuem]

@chinmaygarde How to solve the remaining failures?

The legacy N-1 will fail as this is a feature only present on Flutter main branch. For the other failures, I don't understand how to read what is failing. They are quite verbose. Help would be appreciated!

[jtmcdole]

This PR needs to re-target the master branch. It cannot land on main.

[stuartmorgan-g]

This PR needs to re-target the master branch. It cannot land on main.

flutter/packages doesn't have master.

[stuartmorgan-g]

The legacy N-1 will fail as this is a feature only present on Flutter main branch.

N-1 and N-2 tests only run if the package's pubspec.yaml claims to support that version of the SDK. If this PR makes a package not work with older versions of the SDK, then it's an error to land it without changing the SDK requirement, as that would break clients using older versions of Flutter.

[stuartmorgan-g]

For the other failures, I don't understand how to read what is failing. They are quite verbose. Help would be appreciated!

A bunch of them appear to be flake. At least these two appear to be real though:

• https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8702950596237063569/+/u/Run_package_tests/build_examples/stdout

  The output contained unsupported output:
  - Asset with type "data_assets/data" is not a supported asset type (code_assets/code are supported)
  

• https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8702950595889471601/+/u/Run_package_tests/build_examples__Win32_/stdout

  Target dart_build failed : error : Building native assets failed. See the logs for more details.
  

For the second, I'm not sure what "the logs" that flutter error message is referring to are.

[mosuem]

For the other failures, I don't understand how to read what is failing. They are quite verbose. Help would be appreciated!

A bunch of them appear to be flake. At least these two appear to be real though:

• https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8702950596237063569/+/u/Run_package_tests/build_examples/stdout

  The output contained unsupported output:
  - Asset with type "data_assets/data" is not a supported asset type (code_assets/code are supported)
  

• https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8702950595889471601/+/u/Run_package_tests/build_examples__Win32_/stdout

  Target dart_build failed : error : Building native assets failed. See the logs for more details.
  

For the second, I'm not sure what "the logs" that flutter error message is referring to are.

Thanks! This is probably due to the fact that the data-assets feature is not on stable yet. How do I only run the tests against dev?

[mosuem]

Also, I did upgrade the minimum SDK version, but the N-1 and N-2 tests are still running and failing. Or did I misunderstand something?

[stuartmorgan-g]

Thanks! This is probably due to the fact that the data-assets feature is not on stable yet. How do I only run the tests against dev?

Generally speaking, you don't. Is there a specific need for landing this PR prior to stable support that requires overriding our standard policy?

Also, I did upgrade the minimum SDK version, but the N-1 and N-2 tests are still running and failing.

These lines are breaking things. It looks like those are something from the original repo's configuration that I didn't notice and clean up when importing, and they should be removed.

[mosuem]

Thanks! This is probably due to the fact that the data-assets feature is not on stable yet. How do I only run the tests against dev?

Generally speaking, you don't. Is there a specific need for landing this PR prior to stable support that requires overriding our standard policy?

I would be interested in having people try out this feature easily. So a -dev version of vector_graphics_compiler could be released with a dependency on a -dev version of the SDK. Or is that overly complicated? Alternatively, I could release a new package with the functionality which could be used as a drop-in replacement, and deprecate that package sometime in the future in favor of merging into this package.

[stuartmorgan-g]

So a -dev version of vector_graphics_compiler could be released with a dependency on a -dev version of the SDK.

I'm aware of that, but we aren't really set up to allow for that.

Or is that overly complicated?

We do not currently have a release branch system for the packages repo, so doing this would in practice prevent us from shipping any updates of any kind to current clients of vector_graphics for as long as it takes to ship a new version of Flutter stable, which is the reason for the policy.

Alternatively, I could release a new package with the functionality which could be used as a drop-in replacement, and deprecate that package sometime in the future in favor of merging into this package.

Is that really easier than having people use a git dependency pointing to a branch? The discoverability of -dev versions is low (one of the reasons we've never prioritized making shipping -dev versions from this repo a thing), and the discoverability of a new alternate package would be even lower. I would think anyone who could be told to use a new package could be told to use a canned snippet with the correct git dependency?

[stuartmorgan-g]

From triage: What's the status of this PR? Was the decision to wait for the next stable release?

[stuartmorgan-g]

From triage: Is this unblocked now that Flutter 3.38 is out?

[mosuem]

What to do about

[0:03] Running for packages/vector_graphics...
  No version change.
  Found NEXT; validating next version in the CHANGELOG.
No version change found, but the change to this package could not be verified to be exempt
from version changes according to repository policy.
If this is a false positive, please comment in the PR to explain why the PR
is exempt, and add (or ask your reviewer to add) the "override: no versioning needed" label.

I added a line to the changelog already, but this doesn't seem to help.

[mosuem]

Also, is there a way to run these checks locally? Pushing to get the results is tedious.

[stuartmorgan-g]

I added a line to the changelog already, but this doesn't seem to help.

It's flagging the lack of versioning, no the lack of changelog ("No version change found, but the change to this package could not be verified to be exempt from version changes"). It looks like the change can be exempted since it only affects the example.

Also, is there a way to run these checks locally? Pushing to get the results is tedious.

Sure, essentially all of the CI is just driving repo tooling commands. If you're looking for the specific command to run locally for some CI task, you can look at .ci/targets/* to find the corresponding task.

[stuartmorgan-g]

version exemption: The vector_graphics pubspec change only affects local development.

[mosuem]

Are some of the failures flakes? Mac_arm64 custom_package_tests stable reports a failure in package:pigeon, which I did not touch.

Also, I can't reproduce the failure of flutter build linux in packages/vector_graphics_compiler/example locally, using the newest flutter version on master and after running git clean -xfd. Anything special happening in CI here?

[stuartmorgan-g]

Are some of the failures flakes? Mac_arm64 custom_package_tests stable reports a failure in package:pigeon, which I did not touch.

That one looks like some kind of Xcode-level flake, yes.

Also, I can't reproduce the failure of flutter build linux in packages/vector_graphics_compiler/example locally, using the newest flutter version on master and after running git clean -xfd. Anything special happening in CI here?

The specific command CI is running is the repo tooling's build-examples --linux, but I would expect that to pretty much just be flutter build linux.

If it's helpful, the macOS version of the failure seems to have more inline error details:

The output contained unsupported output:
- Asset with type "data_assets/data" is not a supported asset type (code_assets/code are supported)

  Building assets for package:example failed.
  build.dart returned with exit code: 1.
  To reproduce run:
  /Volumes/Work/s/w/ir/x/w/flutter/bin/cache/dart-sdk/bin/dart --packages=/Volumes/Work/s/w/ir/x/w/packages/packages/vector_graphics_compiler/.dart_tool/package_config.json /Volumes/Work/s/w/ir/x/w/packages/packages/vector_graphics_compiler/.dart_tool/hooks_runner/example/181d858fc5/hook.dill --config=/Volumes/Work/s/w/ir/x/w/packages/packages/vector_graphics_compiler/.dart_tool/hooks_runner/example/181d858fc5/input.json
  stderr:
  The output contained unsupported output:
- Asset with type "data_assets/data" is not a supported asset type (code_assets/code are supported)

[stuartmorgan-g]

What's the rollout plan here? We generally do not publish production packages with labs dependencies.