Run apple SDK finders as gclient hooks #50957
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
zanderso
force-pushed
the
run-find-sdk-as-hook
branch
8 times, most recently
from
2e8fe86 to
6b1fa3a
Compare
DEPS
Outdated
| # //build/config/mac/mac_sdk.gni. | ||
| 'mac_sdk_min': '10.14', |
There was a problem hiding this comment.
If the min sdk is also declared here, does it also need to be in buildroot? Does //build/config/mac/mac_sdk.gni need to exist?
There was a problem hiding this comment.
Before this PR and the buildroot PR, this value was in two places. Once in the gn script, and once in mac_sdk.gni. The instance in mac_sdk.gni is unnecessary with some slight rearrangement of the logic in the gn script. The value in the gn script could go away too, if I introduce parsing of the DEPS file into the gn script, but that might be overkill for this PR.
There was a problem hiding this comment.
Should "This must match the setting in //build/config/mac/mac_sdk.gni" comment be updated to reference tools/gn, then?
flutter/buildroot@ebc2748#diff-128083755b38060af3a265f785fd4488d7ac6feaad498a1ee0a8b7cdd8e330c3R10
zanderso
force-pushed
the
run-find-sdk-as-hook
branch
from
6b1fa3a to
a87d189
Compare
zanderso
force-pushed
the
run-find-sdk-as-hook
branch
2 times, most recently
from
cd5b05e to
c24f503
Compare
zanderso
force-pushed
the
run-find-sdk-as-hook
branch
from
c24f503 to
d1406d6
Compare
DEPS
Outdated
| # //build/config/mac/mac_sdk.gni. | ||
| 'mac_sdk_min': '10.14', |
There was a problem hiding this comment.
Should "This must match the setting in //build/config/mac/mac_sdk.gni" comment be updated to reference tools/gn, then?
flutter/buildroot@ebc2748#diff-128083755b38060af3a265f785fd4488d7ac6feaad498a1ee0a8b7cdd8e330c3R10
zanderso
force-pushed
the
run-find-sdk-as-hook
branch
from
d1406d6 to
c72362a
Compare
…144453) flutter/engine@f300ced...76140bc 2024-03-01 [email protected] Run apple SDK finders as gclient hooks (flutter/engine#50957) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll 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 Flutter: 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
3 participants
Currently, these scripts run on each invocation of
gn, and can take many seconds to run.This PR shifts them to run as
gclienthooks instead, so thatgnwill be faster.Needs flutter/buildroot#825.