Merge #16223: devtools: Fetch and display ACKs at sign-off time in github-merge

0e01e4522e devtools: Fetch and display ACKs at sign-off time in github-merge (Wladimir J. van der Laan)

Pull request description:

  - Fetch the ACKs only at sign-off time. This makes sure that any last-minute ACKs are included (fixes #16200)
  - Show a list of ACKs that will be included and their author before signing off, and warn if there are none

  ![1](https://user-images.githubusercontent.com/126646/59605250-ad070980-910e-11e9-9f9a-d789c7f06ebb.png)
  ![2](https://user-images.githubusercontent.com/126646/59605255-b1332700-910e-11e9-80a5-d1e244f48264.png)

  There's a slight change to the merge commit format—before it was
  ```
      ACKs for commit 88884c:
  (list of ACKs, could be empty)
  ```
  now it is
  ```
  ACKs for top commit:
        jnewbery:
          ACK 5ebc6b0eb
      ... (list of ACKs cannot be empty)
  ```
  or
  ```
  Top commit has no ACKs.
  ```
  I don't think there's a reason to have the abbreviated commit ID there, after all the full commit id is already in the beginning of the merge commit message, and at least the abbreviated one is in every single ACK message.

ACKs for commit 0e01e4:
  fanquake:
    ACK 0e01e4522e0f9e2e994f80fc61f40d61a4a70d79

Tree-SHA512: 8576de016137d71cfc101747e9bb6779c13e0953cf2babee7afc9972bf2bd46f6912be4982b54fa5abf4d91e98e8fdae6b4ca3eef7d6892b7a5f04a7017b6882
This commit is contained in:
MarcoFalke 2019-06-24 14:46:08 -04:00 committed by Pasta
parent 721b38c056
commit ede3de195d
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984

View File

@ -32,10 +32,14 @@ BASH = os.getenv('BASH','bash')
# OS specific configuration for terminal attributes
ATTR_RESET = ''
ATTR_PR = ''
ATTR_NAME = ''
ATTR_WARN = ''
COMMIT_FORMAT = '%H %s (%an)%d'
if os.name == 'posix': # if posix, assume we can use basic terminal escapes
ATTR_RESET = '\033[0m'
ATTR_PR = '\033[1;36m'
ATTR_NAME = '\033[0;36m'
ATTR_WARN = '\033[1;31m'
COMMIT_FORMAT = '%C(bold blue)%H%Creset %s %C(cyan)(%an)%Creset%C(green)%d%Creset'
def git_config_get(option, default=None):
@ -164,18 +168,36 @@ def tree_sha512sum(commit='HEAD'):
return overall.hexdigest()
def get_acks_from_comments(head_commit, comments):
assert len(head_commit) == 6
ack_str ='\n\nACKs for commit {}:\n'.format(head_commit)
# Look for abbreviated commit id, because not everyone wants to type/paste
# the whole thing and the chance of collisions within a PR is small enough
head_abbrev = head_commit[0:6]
acks = []
for c in comments:
review = [l for l in c['body'].split('\r\n') if 'ACK' in l and head_commit in l]
review = [l for l in c['body'].split('\r\n') if 'ACK' in l and head_abbrev in l]
if review:
ack_str += ' {}:\n'.format(c['user']['login'])
ack_str += ' {}\n'.format(review[0])
acks.append((c['user']['login'], review[0]))
return acks
def make_acks_message(head_commit, acks):
if acks:
ack_str ='\n\nACKs for top commit:\n'.format(head_commit)
for name, msg in acks:
ack_str += ' {}:\n'.format(name)
ack_str += ' {}\n'.format(msg)
else:
ack_str ='\n\nTop commit has no ACKs.\n'
return ack_str
def print_merge_details(pull, title, branch, base_branch, head_branch):
def print_merge_details(pull, title, branch, base_branch, head_branch, acks):
print('%s#%s%s %s %sinto %s%s' % (ATTR_RESET+ATTR_PR,pull,ATTR_RESET,title,ATTR_RESET+ATTR_PR,branch,ATTR_RESET))
subprocess.check_call([GIT,'log','--graph','--topo-order','--pretty=format:'+COMMIT_FORMAT,base_branch+'..'+head_branch])
if acks is not None:
if acks:
print('{}ACKs:{}'.format(ATTR_PR, ATTR_RESET))
for (name, message) in acks:
print('* {} {}({}){}'.format(message, ATTR_NAME, name, ATTR_RESET))
else:
print('{}Top commit has no ACKs!{}'.format(ATTR_WARN, ATTR_RESET))
def parse_arguments():
epilog = '''
@ -225,9 +247,6 @@ def main():
info = retrieve_pr_info(repo,pull,ghtoken)
if info is None:
sys.exit(1)
comments = retrieve_pr_comments(repo,pull,ghtoken) + retrieve_pr_reviews(repo,pull,ghtoken)
if comments is None:
sys.exit(1)
title = info['title'].strip()
body = info['body'].strip()
# precedence order for destination branch argument:
@ -257,6 +276,8 @@ def main():
sys.exit(3)
try:
subprocess.check_call([GIT,'log','-q','-1','refs/heads/'+head_branch], stdout=devnull, stderr=stdout)
head_commit = subprocess.check_output([GIT,'log','-1','--pretty=format:%H',head_branch]).decode('utf-8')
assert len(head_commit) == 40
except subprocess.CalledProcessError:
print("ERROR: Cannot find head of pull request #%s on %s." % (pull,host_repo), file=stderr)
sys.exit(3)
@ -281,7 +302,6 @@ def main():
message = firstline + '\n\n'
message += subprocess.check_output([GIT,'log','--no-merges','--topo-order','--pretty=format:%H %s (%an)',base_branch+'..'+head_branch]).decode('utf-8')
message += '\n\nPull request description:\n\n ' + body.replace('\n', '\n ') + '\n'
message += get_acks_from_comments(head_commit=subprocess.check_output([GIT,'log','-1','--pretty=format:%H',head_branch]).decode('utf-8')[:6], comments=comments)
try:
subprocess.check_call([GIT,'merge','-q','--commit','--no-edit','--no-ff','--no-gpg-sign','-m',message.encode('utf-8'),head_branch])
except subprocess.CalledProcessError:
@ -299,20 +319,14 @@ def main():
if len(symlink_files) > 0:
sys.exit(4)
# Put tree SHA512 into the message
# Compute SHA512 of git tree (to be able to detect changes before sign-off)
try:
first_sha512 = tree_sha512sum()
message += '\n\nTree-SHA512: ' + first_sha512
except subprocess.CalledProcessError:
print("ERROR: Unable to compute tree hash")
sys.exit(4)
try:
subprocess.check_call([GIT,'commit','--amend','--no-gpg-sign','-m',message.encode('utf-8')])
except subprocess.CalledProcessError:
print("ERROR: Cannot update message.", file=stderr)
sys.exit(4)
print_merge_details(pull, title, branch, base_branch, head_branch)
print_merge_details(pull, title, branch, base_branch, head_branch, None)
print()
# Run test command if configured.
@ -345,8 +359,24 @@ def main():
print("ERROR: Tree hash changed unexpectedly",file=stderr)
sys.exit(8)
# Retrieve PR comments and ACKs and add to commit message, store ACKs to print them with commit
# description
comments = retrieve_pr_comments(repo,pull,ghtoken) + retrieve_pr_reviews(repo,pull,ghtoken)
if comments is None:
print("ERROR: Could not fetch PR comments and reviews",file=stderr)
sys.exit(1)
acks = get_acks_from_comments(head_commit=head_commit, comments=comments)
message += make_acks_message(head_commit=head_commit, acks=acks)
# end message with SHA512 tree hash, then update message
message += '\n\nTree-SHA512: ' + first_sha512
try:
subprocess.check_call([GIT,'commit','--amend','--no-gpg-sign','-m',message.encode('utf-8')])
except subprocess.CalledProcessError:
print("ERROR: Cannot update message.", file=stderr)
sys.exit(4)
# Sign the merge commit.
print_merge_details(pull, title, branch, base_branch, head_branch)
print_merge_details(pull, title, branch, base_branch, head_branch, acks)
while True:
reply = ask_prompt("Type 's' to sign off on the above merge, or 'x' to reject and exit.").lower()
if reply == 's':