cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Chad Smith has proposed merging ~chad.smith/cloud-init:devtool-check-mps into cloud-init:master.

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/335244

tools: Add check-mps tool for reviewing, voting and merging active MPs

This tool is for the upstream development team members to validate and
merge 'Approved' merge proposals (MPs). On success, it will mark the MP as
merged at the specific upstream revision and mark associated bug_tasks as
'Fix Committed'. On failure to pass commit message lints, it adds a
comment to the MP reporting errors and marks the MP 'Work in progress'.
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:devtool-check-mps into cloud-init:master.

diff --git a/tools/review-mps b/tools/review-mps
new file mode 100755
index 0000000..eb6832a
--- /dev/null
+++ b/tools/review-mps
@@ -0,0 +1,273 @@
+#!/usr/bin/python3
+
+"""Review or merge cloud-init's git branches."""
+
+from argparse import ArgumentParser
+from launchpadlib.launchpad import Launchpad
+import os
+import re
+import sys
+
+if "avoid-pep8-E402-import-not-top-of-file":
+    _tdir = os.path.abspath(os.path.join(os.path.dirname(__file__), ".."))
+    sys.path.insert(0, _tdir)
+    from cloudinit.simpletable import SimpleTable
+    from cloudinit.util import ProcessExecutionError, subp, write_file
+
+
+VALID_MP_STATUSES = [
+    'Work in progress', 'Needs review', 'Approved', 'Rejected', 'Merged',
+    'Code failed to merge', 'Queued', 'Superseded']
+GIT_MAX_WIDTH = 74
+RE_MERGE_URL = r'https://code.launchpad.net/~(?P<lpuser>[-.a-z]+)/cloud-init/\+git/cloud-init/\+merge/\d+'
+RE_GIT_COMMIT_HEADER = r'.{5,74}\n\n'
+RE_LP_BUG_ID = r'LP: #(?P<id>\d+)'
+
+
+### Comment templates
+COMMIT_MESSAGE_LINTS_TMPL = '''
+Thank you for your merge proposal.
+
+Your branch has been set to 'Work in progress'.
+Please set the branch back to 'Needs Review' after resolving the issues below.
+
+Thanks again,
+Your friendly neighborhood cloud-init robot.
+
+
+Your branch {branch_path} needs a final commit message which observes our
+commit message guidelines before it can be reviewed and landed.
+
+1. All lines must be less than {max_line_length} characters.
+2. The commit message listed in launchpad needs to have the format:
+
+A one-liner subject line
+
+More detailed paragraphs describing the functional changes of the
+branch.
+
+LP: #<bug-id>    # if it fixes a specific bug
+
+Please fix the following errors:
+------------------------------
+{errors}
+------------------------------
+
+'''
+
+
+def get_parser():
+    parser = ArgumentParser(description=__doc__)
+    parser.add_argument(
+        '--all', required=False, default=False, action='store_true',
+        help='Whether or not to operate on all merge proposals matching the provied "status". Default:false')
+    parser.add_argument(
+        '--merge', required=False, default=False, action='store_true',
+        help=('Whether to merge the matching branches into --upstream-branch.'
+              ' Default: False'))
+    parser.add_argument(
+        '--status', required=False, default='Approved',
+        choices=VALID_MP_STATUSES,
+        help=('Only review launchpad merge proposals with this status.'
+              ' Default: Approved'))
+    parser.add_argument(
+        '--upstream-branch', required=False, dest='upstream',
+        default='origin/master',
+        help=('The name of remote branch target into which we will merge.'
+              ' Default: origin/master'))
+    parser.add_argument(
+        '--test', required=False, default=False, action='store_true',
+        help=('Whether to run tox before marking merged and fix committed'))
+    return parser
+
+
+def scrub_commit_msg(mp):
+    '''Attempt to scrub commit message.
+
+    @raises: ValueError on invalid format or missing content.
+    '''
+    lines = mp.commit_message.splitlines()
+    errors = []
+    if not re.match(RE_GIT_COMMIT_HEADER, mp.commit_message):
+        errors.append(
+            'Commit messsage summary must be <= {0} chars followed'
+            'by an empty line. Found:\n{1}'.format(
+                 GIT_MAX_WIDTH, '\n'.join(lines[0:2])))
+    commit_msg = lines[0:2]
+    for line_num, line in enumerate(lines[2:], 3):
+        overflow_count = len(line) - GIT_MAX_WIDTH
+        if overflow_count > 0:
+            # TODO actually fix line width and carry overflow to next line.
+            line_beginning =  line[:line.find(' ', 20)]
+            errors.append(
+                'Commit message line #{0} has {1} too many characters.'
+                'Line begins with: "{2}"...'.format(
+                    line_num, overflow_count, line_beginning))
+        commit_msg.append(line)
+    if errors:
+        error_msg = 'Commit message lints:\n - ' + ' - '.join(errors)
+        raise ValueError(error_msg)
+    return '\n'.join(commit_msg)
+
+
+def handle_commit_message_errors(source_mp, error_message):
+    '''Report commit message lints or errors on a merge proposal.
+
+    Add a comment with errors, vote 'Needs Fixing' and mark the branch as
+    'Work in progress'.
+
+    @param source_mp: The MergeProposal object from Launchpad.
+    @param error_message: Specific issue which needs fixing in the
+        commit message.
+    '''
+    source_path = source_mp.source_git_path.replace('refs/heads', '')
+    content = content=COMMIT_MESSAGE_LINTS_TMPL.format(
+        branch_path=source_path, errors=error_message,
+        max_line_length=GIT_MAX_WIDTH)
+    source_mp.createComment(
+        subject='Cloud-init commit message lints on {0}'.format(source_path),
+        content=content, vote='Needs Fixing')
+    source_mp.setStatus(status='Work in progress')
+
+
+def prompt_source_mp(source_mps):
+    '''Prompt user for which source_mp on which they want to operate.
+
+    @returns: The user-selected launchpadlib.MergeProposal object.
+    '''
+    table = SimpleTable(['Choice', 'Merge Proposal', 'Summary'])
+    for idx, mp in enumerate(source_mps):
+        summary = ''
+        if mp.commit_message:
+            summary = mp.commit_message.splitlines()[0][:60]
+        elif mp.description:
+            summary = mp.description.splitlines()[0][:60]
+        table.add_row([idx, mp.web_link, summary])
+    print('Potential merge proposals:\n{0}'.format(table))
+    choice = ''
+    valid_choices = [str(choice) for choice in range(len(source_mps))]
+    valid_choice_str = '/'.join(valid_choices)
+    while choice not in valid_choices:
+        choice = input('Merge which proposal? ({0}) '.format(valid_choice_str))
+    return source_mps[int(choice)]
+
+
+def git_remotes(upstream_remote):
+    '''Return a tuple of remotes and remote-prefix expected for new remotes.
+
+    @param upstream_branch_path: String such as origin/master describing the
+        local branch path representing the upstream remote into which we'll
+        merge.
+    @returns: Tuple containing a list of current remote names and the prefix
+              required when adding git remotes.
+    '''
+    remote_names, _ = subp(['git', 'remote'])
+    out, _ = subp(['git', 'remote', 'get-url', upstream_remote])
+    git_prefix = out.strip().replace('cloud-init', '')
+    return remote_names, git_prefix
+
+
+def create_publish_branch(upstream, publish_branch):
+   '''Create clean publish branch target in the current git repo.'''
+   branches, _ = subp(['git', 'branch'])
+   if publish_branch in branches:
+       subp(['git', 'checkout', 'master'])
+       subp(['git', 'branch', '-D', 'publish_target'])
+   subp(['git', 'checkout', upstream, '-b', publish_branch])
+
+
+def merge_source_mp(lp, source_mp, upstream_branch_path):
+    '''Merge source_mp into the current local branch.
+
+    Also mark the bugs referenced in the commit message as Fix committed.
+    '''
+    source_remote = 'publish_source'
+    upstream_remote, _ = upstream_branch_path.split('/')
+    source_path = source_mp.source_git_path.replace('refs/heads', '')
+    lp_user = re.match(RE_MERGE_URL, source_mp.web_link).group('lpuser')
+    remotes, git_prefix = git_remotes(upstream_remote)
+    if source_remote in remotes:
+        subp(['git', 'remote', 'remove', source_remote])
+    subp(['git', 'remote', 'add', source_remote,
+          '{0}~{1}/cloud-init'.format(git_prefix, lp_user)])
+    subp(['git', 'fetch', source_remote])
+    subp(['git', 'merge', '{0}{1}'.format(source_remote, source_path),
+          '--squash'])
+
+    try:
+        commit_msg = scrub_commit_msg(source_mp)
+    except ValueError as e:
+        handle_commit_message_errors(source_mp, str(e))
+        return 1
+    write_file(os.path.join(os.getcwd(), 'commit.msg'), commit_msg)
+    # Grab author string from most recent commit on the proposed branch
+    author, _ = subp(
+        ['git', 'log', '-n', '1', 'publish_source{0}'.format(source_path),
+         '--pretty="%an <%ae>'])
+    subp(['git', 'commit', '--all', '-F', 'commit.msg', '--author', author])
+    set_bug_status(lp, commit_msg, 'Fix Committed')
+    return 0
+
+
+def set_bug_status(lp, commit_msg, bug_status):
+    '''Close any bugs specifically called out in the commit message.'''
+    bug_ids = re.findall(RE_LP_BUG_ID, commit_msg)
+    print(
+        'Setting status of bugs to {0}:\n{1}'.format(
+            bug_status,
+            '\n'.join('http://pad.lv/{0}'.format(bug_id)
+                      for bug_id in bug_ids))
+    for bug_id in bug_ids:
+        bug = lp.bugs[int(bug_id)]
+        for task in bug.bug_tasks:
+            if task.bug_target_name == 'cloud-init':
+                task.status = bug_status
+                task.lp_save()
+
+
+def main():
+    parser = get_parser()
+    args = parser.parse_args()
+    lp = Launchpad.login_with(
+        "cloud-init git-merge tool", 'production', version='devel')
+    lp_project = lp.projects('cloud-init')
+    source_mps = [
+        mp for mp in lp_project.getMergeProposals(status=args.status)]
+    exit_code = 0
+    if not source_mps:
+        print('No merge proposals in {0} status.'.format(args.status))
+        return exit_code
+
+    create_publish_branch(args.upstream, 'publish_target')
+
+    if not args.all:
+        source_mps = [prompt_source_mp(source_mps)]
+    for source_mp in source_mps:
+        source_path = source_mp.source_git_path.replace('refs/heads', '')
+        if not source_mp.commit_message:
+            error_msg = (
+                'The merge proposal {0} does not have a commit message'.format(
+                    source_path))
+
+            handle_commit_message_errors(source_mp, error_msg)
+            exit_code = 1
+            continue
+        if args.merge:
+            merge_source_mp(lp, source_mp, args.upstream)
+            if args.test:
+                print('Running tox test prior to marking merged')
+                tox_output, _ = subp(['tox'])
+                # Print tox summary
+                print('\n'.join(tox_output.splitlines()[-8:]))
+            commitish, _  = subp(['git', 'log', '-n', '1', '--pretty=%H'])
+            source_mp.setStatus(status='Merged', revid=commitish.strip())
+            source_mp.lp_save()
+    if exit_code != 0:
+        print("Exit in error")
+    else:
+        print("When you're ready:\n  git push {0} publish_target:master")
+    return exit_code
+
+
+if __name__ == '__main__':
+    sys.exit(main())