Bump goldctl #47391
Bump goldctl #47391
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 Hixie or stuartmorgan on 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. |
| <String, dynamic>{ | ||
| 'dependency': 'goldctl', | ||
| 'version': 'git_revision:dddc0623e63150cbbafdcb273d4048f329e1dd09', | ||
| 'version': 'git_revision:720a542f6fe4f92922c3b8f0fdcc4d2ac6bb83cd', |
There was a problem hiding this comment.
@eyebrowsoffire can we get this value from .ci.yaml instead of hard-coding it here?
There was a problem hiding this comment.
Yeah I think it was better when we kept this version in one single source of truth in the recipes. Between here and the ci.yaml in the framework (updated in flutter/flutter#137441), there were more than 7 different version strings. 😕
Tracking all these down feels like it was better when there one place this was updated.
There was a problem hiding this comment.
Answering the specific question, no, there is not a way to specify these from .ci.yaml currently. Only the orchestrator CI step is actually specified in the .ci.yaml file. This generates the builder JSON file that specifies the engine_v2 sub-builds, so we have to specify that here. I do understand the distaste for the deep layering of our CI processes, but it's just the way things are at the moment.
There was a problem hiding this comment.
My recommendation for the engine repo would be to pull the CIPD package with gclient sync by listing it in the DEPS file. Then at least for the engine repo, the hash would be in only one place. The tricky bit would be that the scripts that use goldctl would need to be updated to refer to the destination of the CIPD package in the engine tree.
There was a problem hiding this comment.
I think that sounds good. I am not at all familiar with the engine implementation of Gold, maybe @mdebbar can advise on changing it to use gclient sync? I'd like to land this for now since it is trying to resolve a high number of flakes we are seeing from the service.
| config_name: mac_host_engine | ||
| dependencies: >- | ||
| [ | ||
| {"dependency": "goldctl", "version": "git_revision:f808dcff91b221ae313e540c09d79696cd08b8de"} |
There was a problem hiding this comment.
Actually this one might not be necessary at all. This task is just the engine_v2 orchestrator task (the parent task that spawns the sub-builds). It seems unlikely that any actual tests will run in the orchestrator that use skia gold.
| [ | ||
| {"dependency": "chrome_and_driver", "version": "version:107.0"}, | ||
| {"dependency": "firefox", "version": "version:83.0"}, | ||
| {"dependency": "goldctl", "version": "git_revision:3a77d0b12c697a840ca0c7705208e8622dc94603"} |
There was a problem hiding this comment.
Yeah and this readme is woefully out of date, from before the engine_v2 migration. I should update it.
Piinks
added
autosubmit
…137632) flutter/engine@47465eb...4da3e5b 2023-10-31 katelovett@google.com Bump goldctl (flutter/engine#47391) 2023-10-31 skia-flutter-autoroll@skia.org Roll Skia from 065f0d2302ca to b3360479118f (4 revisions) (flutter/engine#47510) 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 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
…lutter#137632) flutter/engine@47465eb...4da3e5b 2023-10-31 katelovett@google.com Bump goldctl (flutter/engine#47391) 2023-10-31 skia-flutter-autoroll@skia.org Roll Skia from 065f0d2302ca to b3360479118f (4 revisions) (flutter/engine#47510) 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 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
Related to flutter/flutter#137177
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.