diff --git a/README.rst b/README.rst index e1262298..8373f7fc 100644 --- a/README.rst +++ b/README.rst @@ -38,10 +38,12 @@ You probably want to run the above in a CI job, not in your regular development You'll also need to provide the following either in the command line or via environment variables: -* owner: the repo organization/owner -* repo: the repo name -* token: your auth token (encrypt this, don't put this in plaintext in any public configurations!) -* url: the url of your scm host +* owner: the repo organization/owner (for github) +* repo: the repo name (for github) +* password: your auth token for github (encrypt this, don't put this in plaintext in any public configurations!) or p4 ticket for swarm +* url: the url of your scm host (for github) +* username: username to acccess the interface (for swarm) +* host: the host part of the url for the interface api (for swarm) * interface: the type of scm host (such as github) Dependencies: diff --git a/inlineplz/env/jenkins.py b/inlineplz/env/jenkins.py index 9ae2faee..707db852 100644 --- a/inlineplz/env/jenkins.py +++ b/inlineplz/env/jenkins.py @@ -18,12 +18,12 @@ class Jenkins(EnvBase): def __init__(self): if os.environ.get('ghprbPullId') or os.environ.get('ghprbActualCommit'): - self.pull_request = os.environ.get('ghprbPullId') + self.review_id = os.environ.get('ghprbPullId') self.owner = os.environ.get('GITHUB_REPO_OWNER') or os.environ.get('ghprbPullLink').split('/')[-4] self.repo = os.environ.get('GITHUB_REPO_NAME') or os.environ.get('ghprbPullLink').split('/')[-3] self.commit = os.environ.get('ghprbActualCommit') self.interface = 'github' - self.token = os.environ.get('GITHUB_TOKEN') + self.password = os.environ.get('GITHUB_TOKEN') spliturl = urlparse.urlsplit(os.environ.get('ghprbPullLink')) if spliturl.netloc != 'github.com': self.url = '{0}://{1}'.format(spliturl.scheme, spliturl.netloc) diff --git a/inlineplz/env/travis.py b/inlineplz/env/travis.py index 889814b1..ed6ade5e 100644 --- a/inlineplz/env/travis.py +++ b/inlineplz/env/travis.py @@ -12,10 +12,10 @@ class Travis(EnvBase): def __init__(self): - self.pull_request = os.environ.get('TRAVIS_PULL_REQUEST') + self.review_id = os.environ.get('TRAVIS_PULL_REQUEST') self.branch = os.environ.get('TRAVIS_BRANCH') self.repo_slug = os.environ.get('TRAVIS_REPO_SLUG') self.commit = os.environ.get('TRAVIS_COMMIT') self.commit_range = os.environ.get('TRAVIS_COMMIT_RANGE') self.interface = 'github' - self.token = os.environ.get('GITHUB_TOKEN') + self.password = os.environ.get('GITHUB_TOKEN') diff --git a/inlineplz/interfaces/__init__.py b/inlineplz/interfaces/__init__.py index 330d4352..48ca8be0 100644 --- a/inlineplz/interfaces/__init__.py +++ b/inlineplz/interfaces/__init__.py @@ -4,7 +4,9 @@ from __future__ import unicode_literals from inlineplz.interfaces.github import GitHubInterface +from inlineplz.interfaces.swarm import SwarmInterface INTERFACES = { - 'github': GitHubInterface + 'github': GitHubInterface, + 'swarm': SwarmInterface } diff --git a/inlineplz/interfaces/github.py b/inlineplz/interfaces/github.py index a4477674..a13f1ecd 100644 --- a/inlineplz/interfaces/github.py +++ b/inlineplz/interfaces/github.py @@ -9,25 +9,53 @@ import github3 import unidiff +import giturlparse from inlineplz.interfaces.base import InterfaceBase from inlineplz.util import git, system class GitHubInterface(InterfaceBase): - def __init__(self, owner, repo, pr=None, branch=None, token=None, url=None): + def __init__(self, args): """ GitHubInterface lets us post messages to GitHub. - owner and repo are the repository owner/organization and repo name respectively. + args.owner and args.repo are the repository owner/organization and repo name respectively. - pr is the ID number of the pull request. branch is the branch name. either pr OR branch - must be populated. + args.review_id is the ID number of the pull request. + args.branch is the branch name. + either args.review_id OR args.branch must be populated. - token is your GitHub API token. + args.password is your GitHub API token. - url is the base URL of your GitHub instance, such as https://github.com + args.url is the base URL of your GitHub instance, such as https://github.com """ + url = args.url + if args.repo_slug: + owner = args.repo_slug.split('/')[0] + repo = args.repo_slug.split('/')[1] + else: + owner = args.owner + repo = args.repo + if args.url: + try: + url_to_parse = args.url + # giturlparse won't parse URLs that don't end in .git + if not url_to_parse.endswith('.git'): + url_to_parse += '.git' + parsed = giturlparse.parse(url_to_parse) + url = parsed.resource + if not url.startswith('https://'): + url = 'https://' + url + owner = parsed.owner + repo = parsed.name + except giturlparse.parser.ParserError: + pass + + pr = args.review_id + token = args.password + branch = args.branch + self.github = None if not url or url == 'https://github.com': self.github = github3.GitHub(token=token) @@ -89,35 +117,36 @@ def post_messages(self, messages, max_comments): return messages_to_post if not msg.comments: continue - msg_position = self.position(msg) + msg_path = os.path.relpath(msg.path).replace('\\', '/').strip() + msg_position = self.position(msg, msg_path) if msg_position: messages_to_post += 1 - if not self.is_duplicate(msg, msg_position): + if not self.is_duplicate(msg, msg_path, msg_position): # skip this message if we already have too many comments on this file # max comments / 5 is an arbitrary number i totally made up. should maybe be configurable. - if paths.setdefault(msg.path, 0) > max_comments // 5: + if paths.setdefault(msg_path, 0) > max_comments // 5: continue try: print('Creating review comment: {0}'.format(msg)) self.pull_request.create_review_comment( self.format_message(msg), self.last_sha, - msg.path, + msg_path, msg_position ) except github3.GitHubError: pass - paths[msg.path] += 1 + paths[msg_path] += 1 messages_posted += 1 if max_comments >= 0 and messages_posted > max_comments: break print('{} messages posted to Github.'.format(messages_to_post)) return messages_to_post - def is_duplicate(self, message, position): + def is_duplicate(self, message, path, position): for comment in self.pull_request.review_comments(): if (comment.position == position and - comment.path == message.path and + comment.path == path and comment.body.strip() == self.format_message(message).strip()): return True return False @@ -134,14 +163,14 @@ def format_message(message): ) return '`{0}`'.format(list(message.comments)[0].strip()) - def position(self, message): + def position(self, message, path): """Calculate position within the PR, which is not the line number""" if not message.line_number: message.line_number = 1 patch = unidiff.PatchSet(self.diff.split('\n')) for patched_file in patch: target = patched_file.target_file.lstrip('b/') - if target == message.path: + if target == path: offset = 1 for hunk_no, hunk in enumerate(patched_file): for position, hunk_line in enumerate(hunk): diff --git a/inlineplz/interfaces/swarm.py b/inlineplz/interfaces/swarm.py new file mode 100644 index 00000000..1fbd60ee --- /dev/null +++ b/inlineplz/interfaces/swarm.py @@ -0,0 +1,111 @@ +# -*- coding: utf-8 -*- + +import random +import subprocess + +import requests + +from inlineplz.interfaces.base import InterfaceBase + +class SwarmInterface(InterfaceBase): + def __init__(self, args): + """ + SwarmInterface lets us post messages to Swarm (Helix). + + args.username and args.password are the credentials used to access Swarm/Perforce. + args.host is the server (And any additional paths before the api) + args.review_id is the the review number you are commenting on + """ + review_id = args.review_id + try: + review_id = int(review_id) + except (ValueError, TypeError): + print('{0} is not a valid review ID'.format(review_id)) + return + self.username = args.username + self.password = args.password + self.host = args.host + self.topic = "reviews/{}".format(review_id) + # current implementation uses version 8 of the implementation + # https://www.perforce.com/perforce/doc.current/manuals/swarm/index.html#Swarm/swarm-apidoc.html#Swarm_API%3FTocPath%3DSwarm%2520API%7C_____0 + self.version = 'v8' + + def post_messages(self, messages, max_comments): + # randomize message order to more evenly distribute messages across different files + messages = list(messages) + random.shuffle(messages) + + messages_to_post = 0 + messages_posted = 0 + current_comments = self.get_comments(max_comments) + for msg in messages: + if not msg.comments: + continue + messages_to_post += 1 + body = self.format_message(msg) + try: + output = subprocess.check_output(["p4", "fstat", "-T", "depotFile", msg.path]) + except subprocess.CalledProcessError as procError: + print("Process call error: Can't find depotFile for '{}': {}".format(msg.path, procError.output)) + continue + l = output.split() + if len(l) != 3: + print("Invalid output: Can't find depotFile for '{}': {}".format(msg.path, output)) + continue + path = output.split()[2] + if self.is_duplicate(current_comments, body, path, msg.line_number): + print("Duplicate for {}:{}".format(path, msg.line_number)) + continue + # try to send swarm post comment + self.post_comment(body, path, msg.line_number) + messages_posted += 1 + if max_comments >= 0 and messages_posted > max_comments: + break + print('{} messages posted to Swarm.'.format(messages_to_post)) + return messages_to_post + + def post_comment(self, body, path, line_number): + # https://www.perforce.com/perforce/doc.current/manuals/swarm/index.html#Swarm/swarm-apidoc.html#Comments___Swarm_Comments%3FTocPath%3DSwarm%2520API%7CAPI%2520Endpoints%7C_____3 + url = "https://{}/api/{}/comments".format(self.host, self.version) + payload = { + 'topic': self.topic, + 'body': body, + 'context[file]': path, + 'context[rightLine]': line_number + } + #print("{}".format(payload)) + response = requests.post(url, auth=(self.username, self.password), data=payload) + if (response.status_code != requests.codes.ok): + print("Can't post comments, status code: {}".format(response.status_code)) + + def get_comments(self, max_comments=100): + # https://www.perforce.com/perforce/doc.current/manuals/swarm/index.html#Swarm/swarm-apidoc.html#Comments___Swarm_Comments%3FTocPath%3DSwarm%2520API%7CAPI%2520Endpoints%7C_____3 + parameters = "topic={}&max={}".format(self.topic, max_comments) + url = "https://{}/api/{}/comments?{}".format(self.host, self.version, parameters) + response = requests.get(url, auth=(self.username, self.password)) + if (response.status_code != requests.codes.ok): + print("Can't get comments, status code: {}".format(response.status_code)) + return {} + return response.json()["comments"] + + @staticmethod + def is_duplicate(comments, body, path, line_number): + for comment in comments: + try: + if (comment["context"]["rightLine"] == line_number and + comment["context"]["file"] == path and + comment["body"].strip() == body.strip()): + return True + except (KeyError, TypeError): + continue + return False + + @staticmethod + def format_message(message): + if not message.comments: + return '' + return ( + '```\n' + + '\n'.join(sorted(list(message.comments))) + + '\n```' + ) diff --git a/inlineplz/main.py b/inlineplz/main.py index c18cb274..9af6fa57 100644 --- a/inlineplz/main.py +++ b/inlineplz/main.py @@ -10,7 +10,6 @@ import sys import time -import giturlparse import yaml from inlineplz import interfaces @@ -21,14 +20,16 @@ def main(): parser = argparse.ArgumentParser() - parser.add_argument('--pull-request', type=int) - parser.add_argument('--owner', type=str) - parser.add_argument('--repo', type=str) + parser.add_argument('--review-id', type=int, default=None) + parser.add_argument('--owner', type=str, help='the owner of the specified Git repo') + parser.add_argument('--repo', type=str, help='the repo to access through the Git interface') parser.add_argument('--repo-slug', type=str) - parser.add_argument('--branch', type=str) - parser.add_argument('--token', type=str) + parser.add_argument('--branch', type=str, default=None) + parser.add_argument('--password', type=str, default=None, help='credentials for accessing specified interface. This will be the token in Github or ticket/password for P4/Swarm.') parser.add_argument('--interface', type=str, choices=interfaces.INTERFACES) - parser.add_argument('--url', type=str) + parser.add_argument('--url', type=str, default=None) + parser.add_argument('--username', type=str, help='the username of the credentials used to access the specified interface') + parser.add_argument('--host', type=str, help='the hostname of the server that the interface will access. For Perforce, this is the base of the api url for swarm.') parser.add_argument('--enabled-linters', type=str, nargs='+') parser.add_argument('--disabled-linters', type=str, nargs='+') parser.add_argument('--dryrun', action='store_true') @@ -64,8 +65,9 @@ def main(): def update_from_config(args, config): blacklist = [ - 'trusted', 'token', 'interface', 'owner', 'repo', 'config_dir' + 'trusted', 'password', 'interface', 'owner', 'repo', 'config_dir' 'repo_slug', 'pull_request', 'zero_exit', 'dryrun', 'url', 'branch' + 'username' ] for key, value in config.items(): if not key.startswith('_') and key not in blacklist: @@ -107,8 +109,8 @@ def inline(args): interface: How are we going to post comments? owner: Username of repo owner repo: Repository name - pr: Pull request ID - token: Authentication for repository + review_id: Pull request ID + password: Authentication for repository url: Root URL of repository (not your project) Default: https://github.com dryrun: Prints instead of posting comments. zero_exit: If true: always return a 0 exit code. @@ -120,28 +122,6 @@ def inline(args): trusted = args.trusted args = load_config(args) - # TODO: consider moving this git parsing stuff into the github interface - url = args.url - if args.repo_slug: - owner = args.repo_slug.split('/')[0] - repo = args.repo_slug.split('/')[1] - else: - owner = args.owner - repo = args.repo - if args.url: - try: - url_to_parse = args.url - # giturlparse won't parse URLs that don't end in .git - if not url_to_parse.endswith('.git'): - url_to_parse += '.git' - parsed = giturlparse.parse(url_to_parse) - url = parsed.resource - if not url.startswith('https://'): - url = 'https://' + url - owner = parsed.owner - repo = parsed.name - except giturlparse.parser.ParserError: - pass if not args.dryrun and args.interface not in interfaces.INTERFACES: print('Valid inline-plz config not found') return 1 @@ -163,14 +143,7 @@ def inline(args): print_messages(messages) return 0 try: - my_interface = interfaces.INTERFACES[args.interface]( - owner, - repo, - args.pull_request, - args.branch, - args.token, - url - ) + my_interface = interfaces.INTERFACES[args.interface](args) if my_interface.post_messages(messages, args.max_comments) and not args.zero_exit: return 1 except KeyError: diff --git a/inlineplz/message.py b/inlineplz/message.py index 8a8c22d8..43577a3c 100644 --- a/inlineplz/message.py +++ b/inlineplz/message.py @@ -15,7 +15,7 @@ def __init__(self): self.messages = {} def add_message(self, path, line, message): - path = os.path.relpath(path).replace('\\', '/').strip() + path = path.replace('\\', '/').strip() # replace backticks with single quotes to avoid markdown escaping issues message = message.replace('`', '\'').strip() try: @@ -42,7 +42,7 @@ def get_messages(self): class Message(object): def __init__(self, path, line_number): - self.path = os.path.relpath(path).replace('\\', '/') + self.path = path.replace('\\', '/').strip() self.line_number = int(line_number) self.comments = set() diff --git a/setup.py b/setup.py index 16d6d7eb..dd6c0ef3 100644 --- a/setup.py +++ b/setup.py @@ -19,7 +19,8 @@ 'uritemplate.py', 'dirtyjson', 'python-dateutil', - 'git-url-parse' + 'git-url-parse', + 'requests' ] test_requirements = [