From 18f50ed9206deec7d67459e47e4a12c3f1be81ae Mon Sep 17 00:00:00 2001 From: Orgad Shaneh Date: Wed, 10 Apr 2013 05:53:15 +0300 Subject: TrivialRebase: Refactor as a class And improve input validation Change-Id: I379b457ac72e13259f73610b199dc8db68e9b369 --- contrib/trivial_rebase.py | 367 +++++++++++++++++++++++----------------------- 1 file changed, 186 insertions(+), 181 deletions(-) (limited to 'contrib') diff --git a/contrib/trivial_rebase.py b/contrib/trivial_rebase.py index cb67b53a46..804f2b951a 100755 --- a/contrib/trivial_rebase.py +++ b/contrib/trivial_rebase.py @@ -39,186 +39,191 @@ import json import re import subprocess -class CheckCallError(OSError): - """CheckCall() returned non-0.""" - def __init__(self, command, cwd, retcode, stdout, stderr=None): - OSError.__init__(self, command, cwd, retcode, stdout, stderr) - self.command = command - self.cwd = cwd - self.retcode = retcode - self.stdout = stdout - self.stderr = stderr - -def CheckCall(command, cwd=None): - """Like subprocess.check_call() but returns stdout. - - Works on python 2.4 - """ - try: - process = subprocess.Popen(command, cwd=cwd, stdout=subprocess.PIPE) - std_out, std_err = process.communicate() - except OSError, e: - raise CheckCallError(command, cwd, e.errno, None) - if process.returncode: - raise CheckCallError(command, cwd, process.returncode, std_out, std_err) - return std_out, std_err - -def GsqlQuery(sql_query, server, port): - """Runs a gerrit gsql query and returns the result""" - gsql_cmd = ['ssh', '-p', port, server, 'gerrit', 'gsql', '--format', - 'JSON', '-c', sql_query] - try: - (gsql_out, _gsql_stderr) = CheckCall(gsql_cmd) - except CheckCallError, e: - print "return code is %s" % e.retcode - print "stdout and stderr is\n%s%s" % (e.stdout, e.stderr) - raise - - new_out = gsql_out.replace('}}\n', '}}\nsplit here\n') - return new_out.split('split here\n') - -def FindPrevRev(changeId, patchset, server, port): - """Finds the revision of the previous patch set on the change""" - sql_query = ("\"SELECT revision FROM patch_sets WHERE " - "change_id = %s AND patch_set_id = %s\"" % (changeId, (patchset - 1))) - revisions = GsqlQuery(sql_query, server, port) - - json_dict = json.loads(revisions[0], strict=False) - return json_dict["columns"]["revision"] - -def GetApprovals(changeId, patchset, server, port): - """Get all the approvals on a specific patch set - - Returns a list of approval dicts""" - sql_query = ("\"SELECT value,account_id,category_id FROM patch_set_approvals " - "WHERE change_id = %s AND patch_set_id = %s AND value != 0\"" - % (changeId, (patchset - 1))) - gsql_out = GsqlQuery(sql_query, server, port) - approvals = [] - for json_str in gsql_out: - data = json.loads(json_str, strict=False) - if data["type"] == "row": - approvals.append(data["columns"]) - return approvals - -def GetEmailFromAcctId(account_id, server, port): - """Returns the preferred email address associated with the account_id""" - sql_query = ("\"SELECT preferred_email FROM accounts WHERE account_id = %s\"" - % account_id) - email_addr = GsqlQuery(sql_query, server, port) - - json_dict = json.loads(email_addr[0], strict=False) - return json_dict["columns"]["preferred_email"] - -def GetPatchId(revision): - git_show_cmd = ['git', 'show', revision] - patch_id_cmd = ['git', 'patch-id'] - patch_id_process = subprocess.Popen(patch_id_cmd, stdout=subprocess.PIPE, - stdin=subprocess.PIPE) - git_show_process = subprocess.Popen(git_show_cmd, stdout=subprocess.PIPE) - return patch_id_process.communicate(git_show_process.communicate()[0])[0] - -def SuExec(server, port, private_key, as_user, cmd): - suexec_cmd = ['ssh', '-l', "Gerrit Code Review", '-p', port, server, '-i', - private_key, 'suexec', '--as', as_user, '--', cmd] - CheckCall(suexec_cmd) - -def DiffCommitMessages(commit1, commit2): - log_cmd1 = ['git', 'log', '--pretty=format:"%an %ae%n%s%n%b"', - commit1 + '^!'] - commit1_log = CheckCall(log_cmd1) - log_cmd2 = ['git', 'log', '--pretty=format:"%an %ae%n%s%n%b"', - commit2 + '^!'] - commit2_log = CheckCall(log_cmd2) - if commit1_log != commit2_log: - return True - return False - -def Main(): - server = 'localhost' - usage = "%(prog)s [--server-port=PORT]" - parser = argparse.ArgumentParser(usage=usage) - parser.add_argument("--change-url", dest="changeUrl", help="Change URL") - parser.add_argument("--project", help="Project path in Gerrit") - parser.add_argument("--commit", help="Git commit-ish for this patchset") - parser.add_argument("--patchset", type=int, help="The patchset number") - parser.add_argument("--private-key-path", dest="private_key_path", - help="Full path to Gerrit SSH daemon's private host key") - parser.add_argument("--server-port", dest="port", default='29418', - help="Port to connect to Gerrit's SSH daemon " - "[default: %(default)s]") - - args = parser.parse_known_args()[0] - try: - changeId = re.search(r'\d+', args.changeUrl).group() - except AttributeError: - parser.print_help() - exit(0) - - if args.patchset == 1: - # Nothing to detect on first patchset - exit(0) - prev_revision = None - prev_revision = FindPrevRev(changeId, args.patchset, server, args.port) - if not prev_revision: - # Couldn't find a previous revision - exit(0) - prev_patch_id = GetPatchId(prev_revision) - cur_patch_id = GetPatchId(args.commit) - if not (prev_patch_id and cur_patch_id): - if not prev_patch_id: - print "GetPatchId failed for commit %s" % (prev_revision) - if not cur_patch_id: - print "GetPatchId failed for commit %s" % (args.commit) - exit(0) - if cur_patch_id.split()[0] != prev_patch_id.split()[0]: - # patch-ids don't match - exit(0) - # Patch ids match. This is a trivial rebase. - # In addition to patch-id we should check if the commit message changed. Most - # approvers would want to re-review changes when the commit message changes. - changed = DiffCommitMessages(prev_revision, args.commit) - if changed: - # Insert a comment into the change letting the approvers know only the - # commit message changed - comment_msg = ("\'--message=New patchset patch-id matches previous patchset" - ", but commit message has changed.'") - comment_cmd = ['ssh', '-p', args.port, server, 'gerrit', 'approve', - '--project', args.project, comment_msg, args.commit] - CheckCall(comment_cmd) - exit(0) - - # Need to get all approvals on prior patch set, then suexec them onto - # this patchset. - approvals = GetApprovals(changeId, args.patchset, server, args.port) - gerrit_approve_msg = ("\'Automatically re-added by Gerrit trivial rebase " - "detection script.\'") - for approval in approvals: - # Note: Sites with different 'copy_min_score' values in the - # approval_categories DB table might want different behavior here. - # Additional categories should also be added if desired. - if approval["category_id"] == "CRVW": - approve_category = '--code-review' - elif approval["category_id"] == "VRIF": - # Don't re-add verifies - #approve_category = '--verified' - continue - elif approval["category_id"] == "SUBM": - # We don't care about previous submit attempts - continue - else: - print "Unsupported category: %s" % approval - exit(0) - - score = approval["value"] - gerrit_approve_cmd = ['gerrit', 'approve', '--project', args.project, - '--message', gerrit_approve_msg, approve_category, - score, args.commit] - email_addr = GetEmailFromAcctId(approval["account_id"], server, - args.port) - SuExec(server, args.port, args.private_key_path, email_addr, - ' '.join(gerrit_approve_cmd)) - exit(0) +class TrivialRebase: + def __init__(self): + self.server = 'localhost' + usage = "%(prog)s [--server-port=PORT]" + parser = argparse.ArgumentParser(usage=usage) + parser.add_argument("--change-url", dest="changeUrl", help="Change URL") + parser.add_argument("--project", help="Project path in Gerrit") + parser.add_argument("--commit", help="Git commit-ish for this patchset") + parser.add_argument("--patchset", type=int, help="The patchset number") + parser.add_argument("--private-key-path", dest="private_key_path", + help="Full path to Gerrit SSH daemon's private host key") + parser.add_argument("--server-port", dest="port", default='29418', + help="Port to connect to Gerrit's SSH daemon " + "[default: %(default)s]") + args = parser.parse_known_args()[0] + if None in [args.changeUrl, args.project, args.commit, args.patchset]: + parser.error("Incomplete arguments") + try: + self.changeId = re.search(r'\d+$', args.changeUrl).group() + except AttributeError: + parser.error("Invalid changeId") + self.project = args.project + self.commit = args.commit + self.patchset = args.patchset + self.private_key_path = args.private_key_path + self.port = args.port + + class CheckCallError(OSError): + """CheckCall() returned non-0.""" + def __init__(self, command, cwd, retcode, stdout, stderr=None): + OSError.__init__(self, command, cwd, retcode, stdout, stderr) + self.command = command + self.cwd = cwd + self.retcode = retcode + self.stdout = stdout + self.stderr = stderr + + def CheckCall(self, command, cwd=None): + """Like subprocess.check_call() but returns stdout. + + Works on python 2.4 + """ + try: + process = subprocess.Popen(command, cwd=cwd, stdout=subprocess.PIPE) + std_out, std_err = process.communicate() + except OSError, e: + raise self.CheckCallError(command, cwd, e.errno, None) + if process.returncode: + raise self.CheckCallError(command, cwd, process.returncode, std_out, std_err) + return std_out, std_err + + def GsqlQuery(self, sql_query): + """Runs a gerrit gsql query and returns the result""" + gsql_cmd = ['ssh', '-p', self.port, self.server, 'gerrit', 'gsql', + '--format', 'JSON', '-c', sql_query] + try: + (gsql_out, _gsql_stderr) = self.CheckCall(gsql_cmd) + except self.CheckCallError, e: + print "return code is %s" % e.retcode + print "stdout and stderr is\n%s%s" % (e.stdout, e.stderr) + raise + + new_out = gsql_out.replace('}}\n', '}}\nsplit here\n') + return new_out.split('split here\n') + + def FindPrevRev(self): + """Finds the revision of the previous patch set on the change""" + sql_query = ("\"SELECT revision FROM patch_sets WHERE " + "change_id = %s AND patch_set_id = %s\"" % + (self.changeId, (self.patchset - 1))) + revisions = self.GsqlQuery(sql_query) + + json_dict = json.loads(revisions[0], strict=False) + return json_dict["columns"]["revision"] + + def GetApprovals(self): + """Get all the approvals on a specific patch set + + Returns a list of approval dicts""" + sql_query = ("\"SELECT value,account_id,category_id FROM patch_set_approvals " + "WHERE change_id = %s AND patch_set_id = %s AND value != 0\"" + % (self.changeId, (self.patchset - 1))) + gsql_out = self.GsqlQuery(sql_query) + approvals = [] + for json_str in gsql_out: + data = json.loads(json_str, strict=False) + if data["type"] == "row": + approvals.append(data["columns"]) + return approvals + + def GetEmailFromAcctId(self, account_id): + """Returns the preferred email address associated with the account_id""" + sql_query = ("\"SELECT preferred_email FROM accounts WHERE account_id = %s\"" + % account_id) + email_addr = self.GsqlQuery(sql_query) + + json_dict = json.loads(email_addr[0], strict=False) + return json_dict["columns"]["preferred_email"] + + def GetPatchId(self, revision): + git_show_cmd = ['git', 'show', revision] + patch_id_cmd = ['git', 'patch-id'] + patch_id_process = subprocess.Popen(patch_id_cmd, stdout=subprocess.PIPE, + stdin=subprocess.PIPE) + git_show_process = subprocess.Popen(git_show_cmd, stdout=subprocess.PIPE) + return patch_id_process.communicate(git_show_process.communicate()[0])[0] + + def SuExec(self, as_user, cmd): + suexec_cmd = ['ssh', '-l', "Gerrit Code Review", '-p', self.port, self.server, '-i', + self.private_key_path, 'suexec', '--as', as_user, '--', cmd] + self.CheckCall(suexec_cmd) + + def DiffCommitMessages(self, prev_commit): + log_cmd1 = ['git', 'log', '--pretty=format:"%an %ae%n%s%n%b"', + prev_commit + '^!'] + commit1_log = self.CheckCall(log_cmd1) + log_cmd2 = ['git', 'log', '--pretty=format:"%an %ae%n%s%n%b"', + self.commit + '^!'] + commit2_log = self.CheckCall(log_cmd2) + if commit1_log != commit2_log: + return True + return False + + def Run(self): + if self.patchset == 1: + # Nothing to detect on first patchset + return + prev_revision = self.FindPrevRev() + if not prev_revision: + # Couldn't find a previous revision + return + prev_patch_id = self.GetPatchId(prev_revision) + cur_patch_id = self.GetPatchId(self.commit) + if not (prev_patch_id and cur_patch_id): + # Merge commit + if not prev_patch_id: + print "GetPatchId failed for commit %s" % (prev_revision) + if not cur_patch_id: + print "GetPatchId failed for commit %s" % (self.commit) + return + if cur_patch_id.split()[0] != prev_patch_id.split()[0]: + # patch-ids don't match + return + # Patch ids match. This is a trivial rebase. + # In addition to patch-id we should check if the commit message changed. Most + # approvers would want to re-review changes when the commit message changes. + changed = self.DiffCommitMessages(prev_revision) + if changed: + # Insert a comment into the change letting the approvers know only the + # commit message changed + comment_msg = ("\'--message=New patchset patch-id matches previous patchset" + ", but commit message has changed.'") + comment_cmd = ['ssh', '-p', self.port, self.server, 'gerrit', 'approve', + '--project', self.project, comment_msg, self.commit] + self.CheckCall(comment_cmd) + return + + # Need to get all approvals on prior patch set, then suexec them onto + # this patchset. + approvals = self.GetApprovals() + gerrit_approve_msg = ("\'Automatically re-added by Gerrit trivial rebase " + "detection script.\'") + for approval in approvals: + # Note: Sites with different 'copy_min_score' values in the + # approval_categories DB table might want different behavior here. + # Additional categories should also be added if desired. + if approval["category_id"] == "CRVW": + approve_category = '--code-review' + elif approval["category_id"] == "VRIF": + # Don't re-add verifies + #approve_category = '--verified' + continue + elif approval["category_id"] == "SUBM": + # We don't care about previous submit attempts + continue + else: + print "Unsupported category: %s" % approval + return + + score = approval["value"] + gerrit_approve_cmd = ['gerrit', 'approve', '--project', self.project, + '--message', gerrit_approve_msg, approve_category, + score, self.commit] + email_addr = self.GetEmailFromAcctId(approval["account_id"]) + self.SuExec(email_addr, ' '.join(gerrit_approve_cmd)) if __name__ == "__main__": - Main() + TrivialRebase().Run() -- cgit v1.2.3