Skip to content
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

dist/tools: Add script for backporting PR's #8968

Merged
merged 2 commits into from
Feb 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions dist/tools/backport_pr/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
# tox envs directory
.tox
77 changes: 77 additions & 0 deletions dist/tools/backport_pr/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
Pull requests backport script
=============================

This script provides functionality to easily backport a merged pull request to
a release branch.

It relies on having a `github` API token stored in `~/.riotgithubtoken` by
default.


The script works by fetching information from the supplied **merged** pull
request. It then looks for the last release branch.
A temporary git `worktree` with a new branch is created based on this release
branch. All commits from the pull request are then cherry-picked into this
branch which is then pushed to `origin`.
It then creates a new pull request on `github` with a reference to the original
pull request. It optionally puts a comment under the original pull request to
the new backport pull request.



Usage
-----

Most common usage would be to run:

backport_pr.py --comment PR_NUMBER
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's the most common use-case, why is '--comment' not activated by default?



If you are executing from a worktree, the script cannot currently detect the
base git directory and must be used with `--gitdir PATH_TO_ORIGINAL_GIT_DIR`


Full help:

usage: backport_pr.py [-h] [-k KEYFILE] [-c] [-n] [-r RELEASE_BRANCH]
[--backport-branch-fmt BACKPORT_BRANCH_FMT] [-d GITDIR]
PR

positional arguments:
PR Pull request number to backport

optional arguments:
-h, --help show this help message and exit
-k KEYFILE, --keyfile KEYFILE
File containing github token
-c, --comment Put a comment with a reference underthe original PR
-n, --noop Limited noop mode, creates branch, but doesn'tpush and
create the PR
-r RELEASE_BRANCH, --release-branch RELEASE_BRANCH
Base the backport on this branch, default is the
latest
--backport-branch-fmt BACKPORT_BRANCH_FMT
Backport branch format. Fields '{release}' and
'{origbranch} will be replaced by the release name and
remote branch name.
-d GITDIR, --gitdir GITDIR
Base git repo to work from


### Create an authentication token

A `Personal access token` must be created on the `github` website and stored in
your home directory at `~/.riotgithubtoken`.

The setting is located at https://github.com/settings/tokens :

Github->Settings->
Developper settings->Personal access tokens->Generate New Token

And it should have the following scope:

repo[public repo]: Access public repositories


**Warning** its value should be saved directly as you cannot see it later
anymore.
225 changes: 225 additions & 0 deletions dist/tools/backport_pr/backport_pr.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
#!/usr/bin/env python3

# Copyright (C) 2018 Freie Universitat Berlin
# Copyright (C) 2018 Inria
#
# This file is subject to the terms and conditions of the GNU Lesser
# General Public License v2.1. See the file LICENSE in the top level
# directory for more details.
#
# @author Koen Zandberg <[email protected]>

import os
import os.path
import sys
import shutil
import argparse

import git
from agithub.GitHub import GitHub

ORG = "RIOT-OS"
REPO = "RIOT"
GITHUBTOKEN_FILE = ".riotgithubtoken"
WORKTREE_SUBDIR = "backport_temp"

RELEASE_PREFIX = ""
RELEASE_SUFFIX = "-branch"

LABELS_REMOVE = ['Process: needs backport']
LABELS_ADD = ['Process: release backport']

BACKPORT_BRANCH = 'backport/{release}/{origbranch}'


def _get_labels(pr):
labels = {label['name'] for label in pr['labels']}
for remove in LABELS_REMOVE:
labels.discard(remove)
labels.update(LABELS_ADD)
return sorted(list(labels))


def _branch_name_strip(branch_name, prefix=RELEASE_PREFIX,
suffix=RELEASE_SUFFIX):
"""Strip suffix and prefix.

>>> _branch_name_strip('2018.10-branch')
'2018.10'
"""
if (branch_name.startswith(prefix) and
branch_name.endswith(suffix)):
if prefix:
branch_name = branch_name.split(prefix, maxsplit=1)[0]
if suffix:
branch_name = branch_name.rsplit(suffix, maxsplit=1)[0]
return branch_name


def _get_latest_release(branches):
version_latest = 0
release_fullname = ''
release_short = ''
for branch in branches:
branch_name = _branch_name_strip(branch['name'])
branch_num = 0
try:
branch_num = int(''.join(branch_name.split('.')))
except ValueError:
pass
if branch_num > version_latest:
version_latest = branch_num
release_short = branch_name
release_fullname = branch['name']
return (release_short, release_fullname)


def _get_upstream(repo):
for remote in repo.remotes:
if (remote.url.endswith("{}/{}.git".format(ORG, REPO)) or
remote.url.endswith("{}/{}".format(ORG, REPO))):
return remote


def _delete_worktree(repo, workdir):
shutil.rmtree(workdir)
repo.git.worktree('prune')


def main():
keyfile = os.path.join(os.environ['HOME'], GITHUBTOKEN_FILE)
parser = argparse.ArgumentParser()
parser.add_argument("-k", "--keyfile", type=argparse.FileType('r'),
default=keyfile,
help="File containing github token")
parser.add_argument("-c", "--comment", action="store_true",
help="Put a comment with a reference under"
"the original PR")
parser.add_argument("-n", "--noop", action="store_true",
help="Limited noop mode, creates branch, but doesn't"
"push and create the PR")
parser.add_argument("-r", "--release-branch", type=str,
help="Base the backport on this branch, "
"default is the latest")
parser.add_argument("--backport-branch-fmt", type=str,
default=BACKPORT_BRANCH,
help="Backport branch format. "
"Fields '{release}' and '{origbranch} will be "
"replaced by the release name and remote branch "
"name.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

default doc missing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could do it with

-    parser = argparse.ArgumentParser()
+    parser = argparse.ArgumentParser(
+        formatter_class=argparse.ArgumentDefaultsHelpFormatter)

But then other default do not look that great :/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about doing it just like the other defaults for now. Other changes can always come later ;-).

parser.add_argument('-d', '--gitdir', type=str, default=os.getcwd(),
help="Base git repo to work from")
parser.add_argument("PR", type=int, help="Pull request number to backport")
args = parser.parse_args()

gittoken = args.keyfile.read().strip()
g = GitHub(token=gittoken)
# TODO: exception handling
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this qualifies the script still to be WIP? ;-)

status, user = g.user.get()
if status != 200:
print("Could not retrieve user: {}".format(user['message']))
exit(1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This

status, obj = endpoint.get()
if status != 200:
   print("error")
   exit(val)

pattern shows up quite often. Any chance to put this into its own function?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should just raise an exception.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function should raise an exception? Yes. Otherwise, I'm not sure what you mean.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that is what I meant indeed.

username = user['login']
status, pulldata = g.repos[ORG][REPO].pulls[args.PR].get()
if status != 200:
print("Commit #{} not found: {}".format(args.PR, pulldata['message']))
sys.exit(2)
if not pulldata['merged']:
print("Original PR not yet merged")
exit(0)
print("Fetching for commit: #{}: {}".format(args.PR, pulldata['title']))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing I noticed here: if I have a multi-line commit message it shows the whole message, not just the summary line.

orig_branch = pulldata['head']['ref']
status, commits = g.repos[ORG][REPO].pulls[args.PR].commits.get()
if status != 200:
print("No commits found for #{}: {}".format(args.PR,
commits['message']))
sys.exit(3)
for commit in commits:
print("found {} : {}".format(commit['sha'],
commit['commit']['message']))

# Find latest release branch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General rule of thumb for me: If you can summarize a large chunk of code with a comment above it: Put it in its own function ;-).

if args.release_branch:
release_fullname = args.release_branch
release_shortname = _branch_name_strip(args.release_branch)
else:
status, branches = g.repos[ORG][REPO].branches.get()
if status != 200:
print("Could not retrieve branches for {}/{}: {}"
.format(ORG,
REPO,
branches['message']))
sys.exit(4)
release_shortname, release_fullname = _get_latest_release(branches)
if not release_fullname:
print("No release branch found, exiting")
sys.exit(5)
print("Backport based on branch {}".format(release_fullname))

repo = git.Repo(args.gitdir)
# Fetch current upstream
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dito

upstream_remote = _get_upstream(repo)
if not upstream_remote:
print("No upstream remote found, can't fetch")
exit(6)
print("Fetching {} remote".format(upstream_remote))

upstream_remote.fetch()
# Build topic branch in temp dir
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dito

new_branch = args.backport_branch_fmt.format(release=release_shortname,
origbranch=orig_branch)
worktree_dir = os.path.join(args.gitdir, WORKTREE_SUBDIR)
repo.git.worktree("add", "-b",
new_branch,
WORKTREE_SUBDIR,
"{}/{}".format(upstream_remote, release_fullname))
bp_repo = git.Repo(worktree_dir)
# Apply commits
for commit in commits:
bp_repo.git.cherry_pick('-x', commit['sha'])
# Push to github
print("Pushing branch {} to origin".format(new_branch))
if not args.noop:
repo.git.push('origin', '{0}:{0}'.format(new_branch))
# Delete worktree
print("Pruning temporary workdir at {}".format(worktree_dir))
_delete_worktree(repo, worktree_dir)

labels = _get_labels(pulldata)
merger = pulldata['merged_by']['login']
if not args.noop:
# Open new PR on github
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And so on...

pr = {
'title': "{} [backport {}]".format(pulldata['title'],
release_shortname),
'head': '{}:{}'.format(username, new_branch),
'base': release_fullname,
'body': "# Backport of #{}\n\n{}".format(args.PR,
pulldata['body']),
'maintainer_can_modify': True,
}
status, new_pr = g.repos[ORG][REPO].pulls.post(body=pr)
if status != 201:
print("Error creating the new pr: \"{}\". Is \"Public Repo\""
" access enabled for the token"
.format(new_pr['message']))
pr_number = new_pr['number']
print("Create PR number #{} for backport".format(pr_number))
g.repos[ORG][REPO].issues[pr_number].labels.post(body=labels)
review_request = {"reviewers": [merger]}
g.repos[ORG][REPO].pulls[pr_number].\
requested_reviewers.post(body=review_request)

# Put commit under old PR
if args.comment and not args.noop:
comment = {"body": "Backport provided in #{}".format(pr_number)}
status, res = g.repos[ORG][REPO].\
issues[args.PR].comments.post(body=comment)
if status != 201:
print("Something went wrong adding the comment: {}"
.format(res['message']))
print("Added comment to #{}".format(args.PR))


if __name__ == "__main__":
main()
4 changes: 4 additions & 0 deletions dist/tools/backport_pr/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
agithub==2.1
gitdb2==2.0.3
GitPython==2.1.9
smmap2==2.0.3
32 changes: 32 additions & 0 deletions dist/tools/backport_pr/tox.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
[tox]
envlist = test,lint,flake8
skipsdist = True

[testenv]
basepython = python3
deps = -r {toxinidir}/requirements.txt
setenv =
script = backport_pr.py
commands =
test: {[testenv:test]commands}
lint: {[testenv:lint]commands}
flake8: {[testenv:flake8]commands}

[testenv:test]
deps =
pytest
{[testenv]deps}
commands =
pytest -v --doctest-modules {env:script}

[testenv:lint]
deps =
pylint
{[testenv]deps}
commands =
pylint {env:script}

[testenv:flake8]
deps = flake8
commands =
flake8 --max-complexity=10 {env:script}