use host_cpu in BUILD.gn to determine where to find gen_snapshot#34870
use host_cpu in BUILD.gn to determine where to find gen_snapshot#34870auto-submit[bot] merged 4 commits intoflutter:mainfrom
Conversation
flar
changed the title
flar
force-pushed
the
gate-gen_snapshot_dir-on-platform
branch
from
f4fc35c to
e6d65c6
Compare
zanderso
left a comment
zanderso
left a comment
There was a problem hiding this comment.
lgtm after fix noted below. I'll follow-up with changes to the GN to compute the value for the flag instead of hardcoding it.
| ) | ||
|
|
||
| parser.add_argument('--dst', type=str, required=True) | ||
| parser.add_argument('--clang-dir', type=str, required=True) |
There was a problem hiding this comment.
The bots call this script, so to make this a soft transition, the new flag would need to be option with a default value of 'clang_x64'.
There was a problem hiding this comment.
Yay! That explains the mystery errors that had no logs.
There was a problem hiding this comment.
But this begs the question - if/when we start having Apple Silicon machines running our CI, won't this just shift the problem from needing a default to needing the create_macos_gen_snapshots.py script be able to figure out the default?
There was a problem hiding this comment.
Or teaching the CI machines to figure this out and provide the answer when they call the script? Maybe that's the best strategy?
There was a problem hiding this comment.
Yeah, we'll have to modify the recipe to get it to work on arm64.
There was a problem hiding this comment.
Is it worth updating the recipes now?
There was a problem hiding this comment.
After this lands, we should update to explicitly pass the new flag, but I think we should wait to add logic to programatically figure out the arch until we're ready to bring those machines online.
2 participants
Fixes: flutter/flutter#108238