flutter build aar regenerates tooling between each build-mode step
#162705
Conversation
|
Also /cc @sigurdm, this might have impact with your latest changes for workspaces/assets (probably easy to fix, but worth mentioning). |
camsim99
left a comment
There was a problem hiding this comment.
The premise looks good to me! Leaving to @jmagman for final review.
iOS and macOS are failing with the error now looks like though: https://ci.chromium.org/ui/p/flutter/builders/try/Mac%20tool_integration_tests_5_5/6050/overview. This expected?
Nope 🥹. I'll take a look at the native asset tests, perhaps they have a different pattern for inheritance or have overwritten something or I misunderstand how verify is being called. Likely something small |
| FlutterProject project, { | ||
| bool? releaseMode, | ||
| }) async { | ||
| // Intentionally left blank. We want to rebuild the tooling before every sub-build. |
There was a problem hiding this comment.
uh ... can you explain this to me?
There was a problem hiding this comment.
Yeah. The base class implementation is automatically called in verifyAndRunCommand, but I don't want that to happen in these sub commands. I admit it could probably be done a better way, I was just trying to avoid lots of overridden booleans
There was a problem hiding this comment.
oh, so the base class is the real thing that we use before each build. BUt then there is also a verfy step that we need to .. for lack of a better term, trick?
If my understanding is correct, we could also override/remove the verify step too right?
There was a problem hiding this comment.
That's essentially what this does, at least the part related to meta tooling. I'm not sure it's safe to skip the entire verify step, but I could be talked into it by someone who understands what it does better 😁
There was a problem hiding this comment.
Yeah this seems simpler, I would expand the documentation to explain it more directly.
There was a problem hiding this comment.
Done. I ended up also renaming the function and adding the boolean 🤷🏼
tl;dr, this is because of #162704. I accidentally started evaluating |
|
@jmagman I am going to merge this to unblock and start working on |
Closes #162703.
I still need to do a similar change for
ios|macos-frameworkin #162704.I added two unit tests, as well as opted in an integration test to the flag (it will become the default in #160289).
/cc @reidbaker @gmackall.