diff --git a/llvm/docs/GettingStarted.rst b/llvm/docs/GettingStarted.rst --- a/llvm/docs/GettingStarted.rst +++ b/llvm/docs/GettingStarted.rst @@ -528,6 +528,17 @@ Please ask for help if you're having trouble with your particular git workflow. +Git pre-push hook +^^^^^^^^^^^^^^^^^ + +We include an optional pre-push hook that run some sanity checks on the revisions +you are about to push and ask confirmation if you push multiple commits at once. +You can set it up (on Unix systems) by running from the repository root: + +.. code-block:: console + + % ln -sf ../../llvm/utils/git/pre-push.py .git/hooks/pre-push + Bisecting commits ^^^^^^^^^^^^^^^^^ diff --git a/llvm/docs/Phabricator.rst b/llvm/docs/Phabricator.rst --- a/llvm/docs/Phabricator.rst +++ b/llvm/docs/Phabricator.rst @@ -183,6 +183,13 @@ the git revision number in the Comment, set the Action to "Close Revision" and click Submit. Note the review must have been Accepted first. +Arcanist also adds extra tags that are mostly noise in the commit message, for +this reason avoid using `arc land` and push commits to master directly with git +after removing tags other than "Reviewed by" and "Differential Revision". +You can run `llvm/utils/git/arcfilter.sh` to clean the commit message of the +current "HEAD" commit automatically. You can also setup a git hook to catch this +for you (see `Getting Started `). + Committing someone's change from Phabricator ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -217,12 +224,6 @@ ninja check-$whatever # Rerun the appropriate tests if needed. git push -Or - -:: - - arc land D - Abandoning a change ------------------- diff --git a/llvm/utils/git/arcfilter.sh b/llvm/utils/git/arcfilter.sh new file mode 100755 --- /dev/null +++ b/llvm/utils/git/arcfilter.sh @@ -0,0 +1,8 @@ +#!/bin/sh + +# Remove the phabricator tags from the commit at HEAD +git log -1 --pretty=%B | \ + sed 's/^Summary://' | \ + awk '/Reviewers: /{p=1; sub(/Reviewers: .*Differential Revision: /, "")}; /Differential Revision: /{p=0;}; !p' | \ + git commit --amend -F - ; } + diff --git a/llvm/utils/git/pre-push.py b/llvm/utils/git/pre-push.py new file mode 100755 --- /dev/null +++ b/llvm/utils/git/pre-push.py @@ -0,0 +1,250 @@ +#!/usr/bin/env python +# +# ======- git-llvm - LLVM Git Help Integration ---------*- python -*--========# +# +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +# +# ==------------------------------------------------------------------------==# + +""" +git-llvm integration +==================== + +This file provides integration for git. + +The git llvm push sub-command can be used to push changes to GitHub. It is +designed to be a thin wrapper around git, and its main purpose is to +detect and prevent merge commits from being pushed to the main repository. + +Usage: + +git-llvm push + +This will push changes from the current HEAD to the branch . + +""" + +from __future__ import print_function +import argparse +import collections +import os +import re +import shutil +import subprocess +import sys +import time +import getpass +assert sys.version_info >= (2, 7) + +try: + dict.iteritems +except AttributeError: + # Python 3 + def iteritems(d): + return iter(d.items()) +else: + # Python 2 + def iteritems(d): + return d.iteritems() + +try: + # Python 3 + from shlex import quote +except ImportError: + # Python 2 + from pipes import quote + +VERBOSE = False +QUIET = False +dev_null_fd = None +z40="0000000000000000000000000000000000000000" + + +def eprint(*args, **kwargs): + print(*args, file=sys.stderr, **kwargs) + + +def log(*args, **kwargs): + if QUIET: + return + print(*args, **kwargs) + + +def log_verbose(*args, **kwargs): + if not VERBOSE: + return + print(*args, **kwargs) + + +def die(msg): + eprint(msg) + sys.exit(1) + + +def ask_confirm(prompt): + # Python 2/3 compatibility + try: + read_input = raw_input + except NameError: + read_input = input + + while True: + query = read_input('%s (y/N): ' % (prompt)) + if query.lower() not in ['y','n', '']: + print('Expect y or n!') + continue + return query.lower() == 'y' + + +def get_dev_null(): + """Lazily create a /dev/null fd for use in shell()""" + global dev_null_fd + if dev_null_fd is None: + dev_null_fd = open(os.devnull, 'w') + return dev_null_fd + + +def shell(cmd, strip=True, cwd=None, stdin=None, die_on_failure=True, + ignore_errors=False, text=True, print_raw_stderr=False): + # Escape args when logging for easy repro. + quoted_cmd = [quote(arg) for arg in cmd] + log_verbose('Running in %s: %s' % (cwd, ' '.join(quoted_cmd))) + + err_pipe = subprocess.PIPE + if ignore_errors: + # Silence errors if requested. + err_pipe = get_dev_null() + + start = time.time() + p = subprocess.Popen(cmd, cwd=cwd, stdout=subprocess.PIPE, stderr=err_pipe, + stdin=subprocess.PIPE, + universal_newlines=text) + stdout, stderr = p.communicate(input=stdin) + elapsed = time.time() - start + + log_verbose('Command took %0.1fs' % elapsed) + + if p.returncode == 0 or ignore_errors: + if stderr and not ignore_errors: + if not print_raw_stderr: + eprint('`%s` printed to stderr:' % ' '.join(quoted_cmd)) + eprint(stderr.rstrip()) + if strip: + if text: + stdout = stdout.rstrip('\r\n') + else: + stdout = stdout.rstrip(b'\r\n') + if VERBOSE: + for l in stdout.splitlines(): + log_verbose("STDOUT: %s" % l) + return stdout + err_msg = '`%s` returned %s' % (' '.join(quoted_cmd), p.returncode) + eprint(err_msg) + if stderr: + eprint(stderr.rstrip()) + if die_on_failure: + sys.exit(2) + raise RuntimeError(err_msg) + + +def git(*cmd, **kwargs): + return shell(['git'] + list(cmd), **kwargs) + + +def program_exists(cmd): + if sys.platform == 'win32' and not cmd.endswith('.exe'): + cmd += '.exe' + for path in os.environ["PATH"].split(os.pathsep): + if os.access(os.path.join(path, cmd), os.X_OK): + return True + return False + +def get_revs_to_push(range): + commits = git('rev-list', range).splitlines() + # Reverse the order so we print the oldest commit first + commits.reverse() + return commits + +def handle_push(args, local_ref, local_sha, remote_ref, remote_sha): + '''Check a single push request (which can include multiple revisions)''' + # Handle request to delete + if local_sha == z40: + if not ask_confirm("Are you sure you want to delete '%s' on remote '%s'?" % (remote_ref, args.url)): + die("Aborting") + return + + # Push a new branch + if remote_sha == z40: + if not ask_confirm("Are you sure you want to push a new branch/tag '%s' on remote '%s'?" % (remote_ref, args.url)): + die("Aborting") + range=local_sha + return + else: + # Update to existing branch, examine new commits + range="%s..%s" % (remote_sha, local_sha) + + revs = get_revs_to_push(range) + if not revs: + # This can happen if someone is force pushing an older revision to a branch + return + + # Print the revision about to be pushed commits + print("Pushing to '%s' on remote '%s'" % (remote_ref, args.url)) + for sha in revs: + print(" - " + git("show", "--oneline", "--quiet", sha)) + + if len(revs) > 1: + if not ask_confirm("Are you sure you want to push %d commits'?" % len(revs)): + die("Aborting") + + + for sha in revs: + msg = git("log","--format=%B","-n1", sha) + if "Differential Revision" not in msg: + continue + for line in msg.splitlines(): + for tag in ["Summary", "Reviewers", "Subscribers", "Tags"]: + if line.startswith(tag + ":"): + eprint("Please remove arcanist tags from the commit message (found '%s' tag in %s)" % (tag, sha[:12])) + if len(revs) == 1: + eprint("Try running: llvm/utils/git/arcfilter.sh") + die("Aborting (force push by adding '--no-verify')") + + return + + +if __name__ == '__main__': + if not program_exists('git'): + die('error: cannot fine git command') + + argv = sys.argv[1:] + p = argparse.ArgumentParser( + prog='pre-push', formatter_class=argparse.RawDescriptionHelpFormatter, + description=__doc__) + verbosity_group = p.add_mutually_exclusive_group() + verbosity_group.add_argument('-q', '--quiet', action='store_true', + help='print less information') + verbosity_group.add_argument('-v', '--verbose', action='store_true', + help='print more information') + + p.add_argument( + 'remote', + type=str, + help="Name of the remote") + p.add_argument( + 'url', + type=str, + help="URL for the remote") + + args = p.parse_args(argv) + VERBOSE = args.verbose + QUIET = args.quiet + + lines=sys.stdin.readlines() + sys.stdin = open("/dev/tty", "r") + for line in lines: + local_ref, local_sha, remote_ref, remote_sha = line.split() + handle_push(args, local_ref, local_sha, remote_ref, remote_sha) +