This is an archive of the discontinued LLVM Phabricator instance.

github: Fix release automation /branch command with new repo
ClosedPublic

Authored by tstellar on Jun 2 2022, 8:12 PM.

Details

Summary

We started using the llvm/llvm-project-release-prs repo for
backport pull requests, but since this repo is not a fork of
llvm/llvm-project it will reject pull requests from other repos. In
order to fix this, when ever someone uses the /branch command to request
a branch be merged into the release branch, we first copy the branch to
the llvm-project-release-prs repo and then create the pull request.

Diff Detail

Event Timeline

tstellar created this revision.Jun 2 2022, 8:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2022, 8:12 PM
tstellar requested review of this revision.Jun 2 2022, 8:12 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2022, 8:12 PM
thieta accepted this revision.Jun 2 2022, 11:03 PM
This revision is now accepted and ready to land.Jun 2 2022, 11:03 PM

This looks good - only a comment about error handling, but I think we can also address that later if we see it's a large problem.

llvm/utils/git/github-automation.py
233

Maybe there should be some error handling here? this block can be retried if fetch or push fails - network errors happens fairly often.

nikic added inline comments.Jun 3 2022, 12:37 AM
llvm/utils/git/github-automation.py
234

Should we include owner in the branch name, to avoid clashing when multiple repos use the same branch name?

tstellar updated this revision to Diff 443122.Jul 7 2022, 8:00 PM

Address some review comments.

tstellar marked an inline comment as done.Jul 7 2022, 8:02 PM
tstellar added inline comments.
llvm/utils/git/github-automation.py
233

Is there a concise way to do retries in python? I tried implementing this, but I thought it made the script too complicated.

thieta added inline comments.Jul 8 2022, 1:46 AM
llvm/utils/git/github-automation.py
233

maybe:

for i in range(5):
  try:
    local_repo.git.fetch(...)
    local_repo.git.push(...)
  except:
    continue

Or use https://pypi.org/project/retrying/ which is very nice and looks much neater in the code.

tstellar updated this revision to Diff 445151.Jul 15 2022, 3:31 PM

Add retries for copying branches to llvm-project-release-prs repo

tstellar added inline comments.Jul 15 2022, 6:29 PM
llvm/utils/git/github-automation.py
233

That module looks nice, but I didn't want to add another dependency, so I just implemented the retry manually.

thieta accepted this revision.Jul 25 2022, 10:24 PM

LGTM with the caveat that I think we should print the error.

llvm/utils/git/github-automation.py
243

I think maybe we should log the exception here - otherwise we will swallow the error completely and it will be hard to troubleshoot

tstellar updated this revision to Diff 447568.Jul 25 2022, 10:27 PM

Print exception message when any fetch attempt fails.

thieta accepted this revision.Jul 25 2022, 10:54 PM
This revision was landed with ongoing or failed builds.Jul 26 2022, 3:09 PM
This revision was automatically updated to reflect the committed changes.