[web] remove ulimit and add -i to felt #47414
Conversation
Didn't that feature get removed because then |
eyebrowsoffire
left a comment
eyebrowsoffire
left a comment
There was a problem hiding this comment.
LGTM!
FWIW, I dislike the felt snapshot strategy unless we have proper change detection to invalidate the snapshot when necessary, which probably involves parsing a ninja dep file and hashing all the files, which seems like overkill. Or maybe we could flip the script somehow and use ninja to build the felt snapshot... and then felt would attempt to build itself on launch or something? Hmm... I don't know, it's tricky.
|
Yes, it is an issue, but in the past snapshot runs were both the default and broken. In this case I'm going for opt-in (i.e. "I know what I'm doing, and I always know to fallback to non-incremental build if things are going sideways"). |
lib/web_ui/dev/felt
Outdated
| # If set to 1 (via the -i option) will run felt incrementally. | ||
| INCREMENTAL=0 | ||
|
|
||
| # All arguments passed to felt, expect -i, if any. |
lib/web_ui/dev/felt
Outdated
| echo $INCREMENTAL | ||
| echo $ARGS | ||
| exit 0 |
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. |
yjbanov
force-pushed
the
remove-ulimit
branch
from
7b28080 to
4100aca
Compare
|
auto label is removed for flutter/engine/47414, Failed to merge flutter/engine/47414 with FormatException: Unexpected end of input (at character 1) ^ |
auto-submit
bot
removed
the
autosubmit
flutter/engine@27d37db...d441f08 2023-12-02 yjbanov@google.com [web] remove ulimit and add -i to felt (flutter/engine#47414) 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 matanl@google.com,rmistry@google.com,zra@google.com 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
) flutter/engine@27d37db...d441f08 2023-12-02 yjbanov@google.com [web] remove ulimit and add -i to felt (flutter/engine#47414) 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 matanl@google.com,rmistry@google.com,zra@google.com 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
4 participants
The ulimit logic has been failing for me for months now and felt still ran fine. I think we don't need it any more.
Also add the
-ioption for incremental runs offelt. It causes felt to start faster because it doesn't runpub get. In the future, we can also run felt from the snapshot for even faster start-up.