Enable asset transformation for flutter build for iOS, Android, Windows, MacOS, Linux, and web (also flutter run without hot reload support)

[andrewkolos]

See title. These are are the platforms that use the CopyAssets Target as part of their build target.

Partial implementation of #143348.

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I signed the CLA.
• I listed at least one issue that this PR fixes in the description above.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making, or this PR is test-exempt.
• All existing and new tests are passing.

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

[andrewkolos]

I suspect caching is likely to not work properly when path-dependencies are used as transformers.

Consider the scenario where I have a path dependency in use that as a transformer. I perform a build, edit the transformer's code, and then run a build again. The CopyAssets target doesn't depend on code files from path dependencies, so the asset won't get retransformed.

edit: I'm content in adding this as a TODO on the tracking issue.

[christopherfujino]

I suspect caching is likely to not work properly when path-dependencies are used as transformers.

Consider the scenario where I have a path dependency in use that as a transformer. I perform a build, edit the transformer's code, and then run a build again. The CopyAssets target doesn't depend on code files from path dependencies, so the asset won't get retransformed.

edit: I'm content in adding this as a TODO on the tracking issue.

Unless you think there's some reason that path dependencies would be used MORE in this particular workflow, I'm ok with leaving this as a TODO. I think path deps in pubspec.yaml invalidates all kinds of caching assumptions in general.

[andrewkolos]

Unless you think there's some reason that path dependencies would be used MORE in this particular workflow, I'm ok with leaving this as a TODO. I think path deps in pubspec.yaml invalidates all kinds of caching assumptions in general.

Indeed, see #141626 as an example.

[christopherfujino]

Overall this LGTM. I weakly feel we should have some integration testing for this, but I'll comment my thoughts on #143348, I don't think it should block landing this, especially as the tests we do have seem to cover interesting API surface well.

[andrewkolos]

Overall this LGTM. I weakly feel we should have some integration testing for this, but I'll comment my thoughts on #143348, I don't think it should block landing this, especially as the tests we do have seem to cover interesting API surface well.

Ack. I added a note about path dependencies to the tracking issue OP. Thanks for the quite fantastic review here—you caught at least one would-be P1.

[auto-submit]

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

[christopherfujino]

Ack. I added a note about path dependencies to the tracking issue OP. Thanks for the quite fantastic review here—you caught at least one would-be P1.

Was that not failing the build if the transformer failed? or something else?

[andrewkolos]

Ack. I added a note about path dependencies to the tracking issue OP. Thanks for the quite fantastic review here—you caught at least one would-be P1.

Was that not failing the build if the transformer failed? or something else?

Yeah, that's what came to mind.

edit: FYI I see Google testing failing, and I have a g3fix going out