-
Notifications
You must be signed in to change notification settings - Fork 29.8k
[github actions] refactor and fix cherry pick actions #140499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
.github/workflows/easy-cp.yml
Outdated
| git fetch origin $RELEASE_BRANCH | ||
| git fetch origin master | ||
| git checkout -b $RELEASE_BRANCH --track origin/$RELEASE_BRANCH | ||
| git checkout -b cp-${CHANNEL}-${COMMIT_SHA} --track origin/$RELEASE_BRANCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are being created under a bot account, does origin need to be changed to upstream/$RELEASE_BRANCH?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we use the marketplace checkout package actions/checkout, the origin is set to flutter/flutter.
If we get rid of dependency on market place checkout, and if we use a fork of the repository, then we can git remote set origin to be the fork, and git remote set upstream to be flutter/flutter. Currently we are not using a fork and are using market place action, origin would be flutter/flutter and upstream is unset.
.github/workflows/easy-cp.yml
Outdated
| working-directory: ./flutter | ||
| run: | | ||
| git push -u origin cp-${CHANNEL}-${COMMIT_SHA} | ||
| gh pr create --title "[cp-${CHANNEL}]${PR_TITLE}" --body-file ../PULL_REQUEST_CP_TEMPLATE.md --base ${RELEASE_BRANCH} --label "cp: review" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be explicitly using the CP_TEMPLATE.md from HEAD instead of what's on the branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right currently the PULL_REQUEST.md is still a downloaded file from HEAD instead of a file on the branch.
There is a step before this step that downloads the template from HEAD. Couldn't remove the download logic yet because this file doesn't exist on release branch (e.g. 3.16-candidate.0). If we remove the download step, then when we checkout the release branch, this template becomes unaccessible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case, should the git push be to the action bot's fork?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Umm I thought in design doc we agreed on creating the branch on the flutter/flutter repo directly and push, removing the usage of fork.
To push to action bot's fork, we need a separate workflow to keep the action bot's fork always up to date with upstream.
(Umm i think Casey previously mentioned that creating branches on flutter/flutter are cheap because they are all refs)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to maintain the fork's branch. See my Cocoon branch at https://github.com/CaseyHillers/cocoon, hasn't been updated in 4 years :-)
Instead, make sure to run git fetch on the upstream's RC before creating a tracking branch on the fork.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
umm I can try reverting back to the fork method.
I thought in previous discussions, it was pointed out creating branches on upstream directly is cheaper and easier, and therefore we switched to the current method of creating branches on upstream. What would be the verdict on why we are now deciding to revert back to the fork method?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated PR to use fork.
Tested: #140530 which is opened from fork to upstream. sample workflow
.github/workflows/easy-cp.yml
Outdated
| working-directory: ./flutter | ||
| run: | | ||
| git push -u origin cp-${CHANNEL}-${COMMIT_SHA} | ||
| gh pr create --title "[cp-${CHANNEL}]${PR_TITLE}" --body-file ../PULL_REQUEST_CP_TEMPLATE.md --base ${RELEASE_BRANCH} --label "cp: review" --repo flutter/flutter --head flutteractionsbot:cp-${CHANNEL}-${COMMIT_SHA} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to explicitly define --head? Since the cp branch is the current HEAD, it sounds like it should use it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, removed.
Yeah personally I prefer being more explicit to avoid chances of error, but i am good with either way
.github/workflows/easy-cp.yml
Outdated
| working-directory: ./flutter | ||
| run: | | ||
| git push -u origin cp-${CHANNEL}-${COMMIT_SHA} | ||
| gh pr create --title "[cp-${CHANNEL}]${PR_TITLE}" --body-file ../PULL_REQUEST_CP_TEMPLATE.md --base ${RELEASE_BRANCH} --label "cp: review" --repo flutter/flutter --head flutteractionsbot:cp-${CHANNEL}-${COMMIT_SHA} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the start of the title, [cp- -> [CP-
Acronyms are capitalized to distinguish them from normal words.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL, thanks for pointing this out!
Manual roll Flutter from 11def8e to cc40425 (118 revisions) Manual roll requested by tarrinneal@google.com flutter/flutter@11def8e...cc40425 2024-01-05 engine-flutter-autoroll@skia.org Roll Flutter Engine from 0bbb4d61ce82 to f60d9a9a3395 (1 revision) (flutter/flutter#140993) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from b2a9ce88a19e to 0bbb4d61ce82 (3 revisions) (flutter/flutter#140990) 2024-01-04 yjbanov@google.com [web] Fix and unskip a few more CanvasKit tests (flutter/flutter#140821) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from bd175aa5e0b6 to b2a9ce88a19e (1 revision) (flutter/flutter#140986) 2024-01-04 danny@tuppeny.com Pin package:vm_service (flutter/flutter#140972) 2024-01-04 36861262+QuncCccccc@users.noreply.github.com Add scrollbar for menus (flutter/flutter#140941) 2024-01-04 christopherfujino@gmail.com manual pub roll to pick up dds fixes (flutter/flutter#140979) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from b81023eb71c9 to bd175aa5e0b6 (2 revisions) (flutter/flutter#140980) 2024-01-04 polinach@google.com Temporarily remove env variable for leak tracking bots. (flutter/flutter#140978) 2024-01-04 chillers@google.com Add Flutter CI status to README (flutter/flutter#140513) 2024-01-04 polinach@google.com Reland "integrate testWidgets with leak tracking" (#140521) (flutter/flutter#140928) 2024-01-04 15619084+vashworth@users.noreply.github.com Run half of iOS devicelab tests with Xcode 15 (flutter/flutter#140927) 2024-01-04 39755477+sharabiddin@users.noreply.github.com Fix `SegmentedButton` states update logic (flutter/flutter#140772) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from f539acfb8c5a to b81023eb71c9 (1 revision) (flutter/flutter#140973) 2024-01-04 pateltirth454@gmail.com [Fix] Consistency in ButtonStyleButton related Tests (flutter/flutter#140610) 2024-01-04 engine-flutter-autoroll@skia.org Roll Packages from bbb4134 to 31fc7b5 (6 revisions) (flutter/flutter#140967) 2024-01-04 stuartmorgan@google.com Fix local engine use in macOS plugins (flutter/flutter#140222) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7d5a120a601b to f539acfb8c5a (2 revisions) (flutter/flutter#140959) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 1ff3cb885842 to 7d5a120a601b (1 revision) (flutter/flutter#140946) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from c8bf51f0d4cd to 1ff3cb885842 (1 revision) (flutter/flutter#140943) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from bfd2d8a100ec to c8bf51f0d4cd (2 revisions) (flutter/flutter#140939) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from 28ae9e35c331 to bfd2d8a100ec (1 revision) (flutter/flutter#140937) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from ab4098c742f8 to 28ae9e35c331 (1 revision) (flutter/flutter#140936) 2024-01-04 engine-flutter-autoroll@skia.org Roll Flutter Engine from e169f3677008 to ab4098c742f8 (2 revisions) (flutter/flutter#140933) 2024-01-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7c2adb811059 to e169f3677008 (1 revision) (flutter/flutter#140929) 2024-01-03 magder@google.com Remove deprecated bitcode stripping from tooling (flutter/flutter#140903) 2024-01-03 54558023+keyonghan@users.noreply.github.com Add Windows leak tracking targets (flutter/flutter#140423) 2024-01-03 xilaizhang@google.com [github actions] refactor and fix cherry pick actions (flutter/flutter#140499) 2024-01-03 magder@google.com Migrate Xcode projects last version checks to Xcode 15.1 (flutter/flutter#140256) 2024-01-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from bf232c4da241 to 7c2adb811059 (3 revisions) (flutter/flutter#140920) 2024-01-03 goderbauer@google.com fix typo and reflow (flutter/flutter#140925) 2024-01-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Re-land integrate testWidgets with leak tracking." (flutter/flutter#140926) 2024-01-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from bf979d220283 to bf232c4da241 (1 revision) (flutter/flutter#140915) 2024-01-03 ybz975218925@gmail.com Changes the regular cursor to a floating cursor when a long press occurs. (flutter/flutter#138479) 2024-01-03 65075121+AcarFurkan@users.noreply.github.com Add `SegmentedButton.styleFrom` (flutter/flutter#137542) 2024-01-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from c62bcff5b809 to bf979d220283 (1 revision) (flutter/flutter#140910) 2024-01-03 polinach@google.com Re-land integrate testWidgets with leak tracking. (flutter/flutter#140521) 2024-01-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 98b72c7ffe71 to c62bcff5b809 (2 revisions) (flutter/flutter#140905) 2024-01-03 jonahwilliams@google.com [flutter_tools] add support for --enable-impeller to test device. (flutter/flutter#140899) 2024-01-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from cf7536964a2f to 98b72c7ffe71 (1 revision) (flutter/flutter#140897) 2024-01-03 233583+mossmana@users.noreply.github.com Handle KEYCODE_DPAD_CENTER and KEYCODE_ENTER (flutter/flutter#140808) 2024-01-03 lsaudon@gmail.com Add Lucas Saudon to AUTHORS (flutter/flutter#139965) 2024-01-03 21270878+elliette@users.noreply.github.com Link to wiki page about updating dependencies in each `pubspec.yaml` file (flutter/flutter#140826) 2024-01-03 fluttergithubbot@gmail.com Marks Linux_pixel_7pro native_assets_android to be unflaky (flutter/flutter#140866) ... --------- Co-authored-by: Tarrin Neal <tarrinneal@gmail.com>
This PR makes the following changes:
Remove dependency on peters/evans package
The market place action introduces overheads that don't properly consume tokens. e.g. :failed workflow that says token is not supplied
This PR changes the market place action to git commands that we have full control over, provides better error msg for debug, and properly consumes token.
Align usage of tokens:
All tokens in the workflow now uses flutter actions bot PAT token. From experiments, a mixed usage of different tokens in different steps sometimes cause the workflow to fail on authentication.
Tested: Tested with a similar workflow on my personal repository, and it produces the expected cherry pick PR as end result