From 1a703082fe9c3c4d5cae26a36ac5648ee1e67ece Mon Sep 17 00:00:00 2001 From: Andrew Shadura Date: Wed, 26 Oct 2016 15:37:19 +0200 Subject: Merge tag 'upstream/1.0.0+git20160928' into debian/experimental --- .arcconfig | 2 +- .pre-commit-config.yaml | 26 + README.md | 27 +- git-phab | 1240 +++++++++++++++++++++++++++++++++++++---------- git-phab.txt | 1 - requirements.txt | 5 + setup.py | 19 +- 7 files changed, 1045 insertions(+), 275 deletions(-) create mode 100644 .pre-commit-config.yaml create mode 100644 requirements.txt diff --git a/.arcconfig b/.arcconfig index 499077f..dbbb1ea 100644 --- a/.arcconfig +++ b/.arcconfig @@ -3,6 +3,6 @@ "lint.engine": "ArcanistConfigurationDrivenLintEngine", "project": "git-phab", "repository.callsign": "GITPHAB", - "default-reviewers": "xclaesse" + "default-reviewers": "xclaesse,thiblahute" } diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..0c30038 --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,26 @@ +- repo: https://github.com/pre-commit/pre-commit-hooks.git + sha: ff65d01841ad012d0a9aa1dc451fc4539d8b7baf + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer + - id: autopep8-wrapper + files: ^git-phab$ + - id: check-docstring-first + - id: check-json + - id: check-yaml + - id: debug-statements + - id: name-tests-test + - id: requirements-txt-fixer + - id: flake8 + files: ^git-phab$ + args: ["--max-complexity=40"] +- repo: https://github.com/pre-commit/pre-commit.git + sha: 495e21b24dfc73624c8c7a16bf974da54e3217e7 + hooks: + - id: validate_config + - id: validate_manifest +- repo: https://github.com/asottile/reorder_python_imports.git + sha: 017e2f64306853ec7f000db52b8280da27eb3b96 + hooks: + - id: reorder-python-imports + language_version: python2.7 diff --git a/README.md b/README.md index 60c5177..a50b4bc 100644 --- a/README.md +++ b/README.md @@ -1,10 +1,11 @@ INSTALL ======= +Install dependencies and copy or symlink executables into your $PATH + ``` - $ pip3 install git-phab - $ pip3 install --upgrade git-phab - $ pip3 uninstall git-phab + $ pip3 install -r requirements.txt + $ ln -s $PWD/git-phab ~/.local/bin/ ``` Optionaly generate and copy or symlink manpage into your $MANPATH @@ -35,10 +36,7 @@ And add this in your ~/.bash_completion: REQUIREMENTS ============ - - pip3 install GitPython - - pip3 install appdirs - - pip3 install argcomplete - - arcanist +See requirements.txt DESCRIPTION =========== @@ -48,12 +46,25 @@ Git subcommand to integrate with phabricator. WORKFLOW EXAMPLE ================ -First, configure where to push WIP branches: +First, specify a personal remote repository where to push WIP branches: ``` $ git config phab.remote xclaesse ``` +Make sure the fetch URL of the repository can be accessed by the reviewers. For example if your remote is called `github`: + +``` + $ git remote show github | grep URL + Fetch URL: git@github.com:NICK/PROJECT.git + Push URL: git@github.com:NICK/PROJECT.git + $ git remote set-url github https://github.com/NICK/PROJECT.git + $ git remote set-url --push github git@github.com:NICK/PROJECT.git + $ git remote show github | grep URL + Fetch URL: https://github.com/NICK/PROJECT.git + Push URL: git@github.com:NICK/PROJECT.git +``` + Before starting your work, create a branch: ``` diff --git a/git-phab b/git-phab index 75108b5..e1eab5b 100755 --- a/git-phab +++ b/git-phab @@ -21,22 +21,32 @@ # along with this program; if not, If not, see # http://www.gnu.org/licenses/. +import base64 +import logging +import socket import tempfile import subprocess import argparse import argcomplete from datetime import datetime import git +import gitdb import os import re import sys import json + import appdirs +import phabricator import shutil + from urllib.parse import urlsplit, urlunsplit -class Colors(object): +ON_WINDOWS = os.name == 'nt' + + +class Colors: HEADER = '\033[95m' OKBLUE = '\033[94m' OKGREEN = '\033[92m' @@ -44,20 +54,121 @@ class Colors(object): FAIL = '\033[91m' ENDC = '\033[0m' + force_disable = False + + @classmethod + def disable(cls): + cls.HEADER = '' + cls.OKBLUE = '' + cls.OKGREEN = '' + cls.WARNING = '' + cls.FAIL = '' + cls.ENDC = '' + + @classmethod + def enable(cls): + if cls.force_disable: + return + + cls.HEADER = '\033[95m' + cls.OKBLUE = '\033[94m' + cls.OKGREEN = '\033[92m' + cls.WARNING = '\033[93m' + cls.FAIL = '\033[91m' + cls.ENDC = '\033[0m' + class GitPhab: + def __init__(self): self.task = None + self.differential = None + self.task_or_revision = None self.remote = None self.assume_yes = False self.reviewers = None self.cc = None self.projects = None self.output_directory = None + self.phab_repo = None + self.staging_url = None + + self.repo = git.Repo(os.getcwd(), search_parent_directories=True) + self.read_arcconfig() + + self._phabricator = None + self._phab_user = None + + @property + def phabricator(self): + if self._phabricator: + return self._phabricator + + needs_credential = False + try: + host = self.phabricator_uri + "/api/" + self._phabricator = phabricator.Phabricator(timeout=120, host=host) + + if not self.phabricator.token and not self.phabricator.certificate: + needs_credential = True + + # FIXME, workaround + # https://github.com/disqus/python-phabricator/issues/37 + self._phabricator.differential.creatediff.api.interface[ + "differential"]["creatediff"]["required"]["changes"] = dict + except phabricator.ConfigurationError: + needs_credential = True + + if needs_credential: + if self.setup_login_certificate(): + self.die("Try again now that the login certificate has been" + " added") + else: + self.die("Please setup login certificate before trying again") + + return self._phabricator + + @property + def phab_user(self): + if self._phab_user: + return self._phab_user + + self._phab_user = self.phabricator.user.whoami() + + return self._phab_user + + def setup_login_certificate(self): + token = input("""LOGIN TO PHABRICATOR +Open this page in your browser and login to Phabricator if necessary: + +%s/conduit/login/ - def check_conduit_reply(self, reply): - if reply['errorMessage'] is not None: - self.die(reply['errorMessage']) +Then paste the API Token on that page below. + +Paste API Token from that page and press : """ % self.phabricator_uri) + path = os.path.join(os.environ['AppData'] if ON_WINDOWS + else os.path.expanduser('~'), '.arcrc') + + host = self.phabricator_uri + "/api/" + host_token = {"token": token} + try: + with open(path) as f: + arcrc = json.load(f) + + if arcrc.get("hosts"): + arcrc["hosts"][host] = host_token + else: + arcrc = { + "hosts": {host: host_token}} + + except (FileNotFoundError, ValueError): + arcrc = {"hosts": {host: host_token}} + + with open(path, "w") as f: + print("Writing %s" % path) + json.dump(arcrc, f, indent=2) + + return True # Copied from git-bz def die(self, message): @@ -104,25 +215,28 @@ class GitPhab: with open(filename, 'r') as f: return [l for l in f.readlines() if not l.startswith("#")] - def create_task(self): + def create_task(self, commits): task_infos = None while not task_infos: - task_infos = self.edit_template( - "\n" - "# Merge branch: %s\n" - "# Please enter a task title and description for the merge " - "request" % self.repo.active_branch.name) + template = "\n# Please enter a task title and description " \ + "for the merge request.\n" \ + "# Commits from branch: %s:" % self.repo.active_branch.name + + Colors.disable() + for c in commits: + template += "\n# - %s" % self.format_commit(c) + Colors.enable() + + task_infos = self.edit_template(template) description = "" title = task_infos[0] if len(task_infos) > 1: description = '\n'.join(task_infos[1:]) - reply = self.conduit("maniphest.createtask", { - "title": title, - "description": description, - "projectPHIDs": self.project_phids, - }) + reply = self.phabricator.maniphest.createtask( + title=title, description=description, + projectPHIDs=self.project_phids) return reply @@ -131,6 +245,11 @@ class GitPhab: m = re.fullmatch('(.+/)?(T[0-9]+)(-.*)?', bname) return m.group(2) if m else None + def revision_from_branchname(self, bname): + # Match 'foo/bar/D123-description' + m = re.fullmatch('(.+/)?(D[0-9]+)(-.*)?', bname) + return m.group(2) if m else None + def get_commits(self, revision_range): try: # See if the argument identifies a single revision @@ -182,14 +301,11 @@ class GitPhab: if diffid: statuses[int(diffid)] = "Unknown" - reply = self.conduit('differential.query', { - "ids": list(statuses.keys()) - }) - - if reply["error"]: + reply = self.phabricator.differential.query(ids=list(statuses.keys())) + if reply.response is None: print("Could not get informations about differentials status") else: - for diff in reply["response"]: + for diff in reply: statuses[int(diff["id"])] = diff["statusName"] for c in commits: @@ -197,17 +313,6 @@ class GitPhab: status = statuses.get(int(diffid)) if diffid else None print(self.format_commit(c, status)) - def conduit(self, cmd, params): - data = bytes(json.dumps(params), 'utf-8') - arc_cmd = ['arc'] - if self.arcrc: - arc_cmd += ['--arcrc-file', self.arcrc] - - arc_cmd += ['call-conduit', cmd] - output = subprocess.check_output(arc_cmd, - input=data) - return json.loads(output.decode('utf-8')) - def in_feature_branch(self): # If current branch is "master" it's obviously not a feature branch. if self.branch_name in ['master']: @@ -264,7 +369,7 @@ class GitPhab: # Fetch what has already been proposed on the task if we don't have # it locally yet. if not remote_commit: - remote_commit = self.fetch()[0] + remote_commit = self.fetch_from_task()[0] # Get the index in commits and all_commits lists of the common # ancestor between HEAD and what has already been proposed. @@ -310,7 +415,7 @@ class GitPhab: try: with open(path) as f: self.config = json.load(f) - except FileNotFoundError as e: + except FileNotFoundError: self.config = {} if 'emails' not in self.config: @@ -328,14 +433,32 @@ class GitPhab: separators=(',', ': ')) def ensure_project_phids(self): - reply = self.conduit("project.query", {"names": self.projects}) - self.project_phids = list(reply["response"]["data"].keys()) + by_names = self.phabricator.project.query(names=self.projects) + by_slugs = self.phabricator.project.query(slugs=self.projects) - names = [p["name"].lower() for p in reply["response"]["data"].values()] - for p in self.projects: - if p not in names: - self.die("%sProject `%s` doesn't seem to exist%s" % - (Colors.FAIL, p, Colors.ENDC)) + if not by_names and not by_slugs: + self.die("%sProjects `%s` doesn't seem to exist%s" % + (Colors.FAIL, self.projects, Colors.ENDC)) + + self.project_phids = [] + project_map = {} + for reply in (by_names, by_slugs): + if not reply.data: + continue + for (phid, data) in reply.data.items(): + project_map[data["name"].lower()] = phid + for s in data["slugs"]: + project_map[s.lower()] = phid + + try: + for p in self.projects: + if p not in project_map: + print("%sProject `%s` doesn't seem to exist%s" % + (Colors.FAIL, p, Colors.ENDC)) + raise + self.project_phids.append(project_map[p]) + except: + self.die("Failed to look up projects in Phabricator") def validate_remote(self): # If a remote is setup ensure that it's valid @@ -343,7 +466,10 @@ class GitPhab: try: self.repo.remote(self.remote) except: - self.die("%s not a valid remote. Aborting." % self.remote) + print("%s%s not a valid remote, can't use it%s." % ( + Colors.HEADER, self.remote, Colors.ENDC)) + self.remote = None + return # Get remote's fetch URL. Unfortunately we can't get it from config # using remote.config_reader.get('url') otherwise it won't rewrite the @@ -386,7 +512,6 @@ class GitPhab: .format(self.remote, fetchurl, pushurl)) def validate_args(self): - self.repo = git.Repo(os.getcwd(), search_parent_directories=True) self.read_arcconfig() self.read_config() @@ -413,6 +538,15 @@ class GitPhab: self.die("Task '%s' is not in the correct format. " "Expecting 'T123'." % self.task) + if self.task_or_revision: + if re.fullmatch('T[0-9]*', self.task_or_revision): + self.task = self.task_or_revision + elif re.fullmatch('D[0-9]*', self.task_or_revision): + self.differential = self.task_or_revision + else: + self.die("Task or revision '%s' is not in the correct format. " + "Expecting 'T123' or 'D123'." % self.task_or_revision) + if hasattr(self, 'revision_range') and not self.revision_range: tracking = self.repo.head.reference.tracking_branch() if not tracking: @@ -444,6 +578,36 @@ class GitPhab: "You can add 'projects': 'p1, p2' in your .arcconfig\n" "Aborting.") + if "repository.callsign" in self.arcconfig: + reply = self.phabricator.repository.query( + callsigns=[self.arcconfig["repository.callsign"]]) + + if len(reply) > 1: + self.die("Multiple repositories returned for callsign ‘{}’.\n" + "You should check your Phabricator " + "configuration.".format( + self.arcconfig["repository.callsign"])) + else: + uris = [remote.url for remote in self.repo.remotes] + reply = self.phabricator.repository.query( + remoteURIs=uris) + + if len(reply) > 1: + self.die("Multiple repositories returned for remote URIs " + "({}).\nYou should check your Phabricator " + "configuration.".format(', '.join(uris))) + + try: + self.phab_repo = reply[0] + except IndexError: + self.die("Could not determine Phabricator repository\n" + "You should check your git remote URIs match those " + "in Phabricator, or set 'repository.callsign' in " + "'.arcconfig'") + + if self.phab_repo.get("staging"): + self.staging_url = self.phab_repo.get("staging").get("uri") + def line_in_headers(self, line, headers): for header in headers: if re.match('^' + re.escape(header), line, flags=re.I): @@ -455,6 +619,7 @@ class GitPhab: body = [] git_fields = [] phab_fields = [] + updates = None # Those are common one-line git field headers git_headers = ['Signed-off-by:', 'Acked-by:', 'Reported-by:', @@ -463,23 +628,51 @@ class GitPhab: phab_headers = ['Cc:', 'differential revision:'] for line in msg.splitlines(): + if updates is not None: + updates.append(line) + continue + if not subject: subject = line continue if self.line_in_headers(line, git_headers): - git_fields.append(line) + if line not in git_fields: + git_fields.append(line) continue if self.line_in_headers(line, phab_headers): - phab_fields.append(line) + if line not in phab_fields: + phab_fields.append(line) + continue + + if line == '---': + updates = [] continue body.append(line) - return subject, body, git_fields + phab_fields + return subject, body, git_fields, phab_fields, updates + + def strip_updates(self, msg): + """ + Return msg with the part after a line containing only "---" removed. + This is a convention used in tools like git-am and Patchwork to + separate the real commit message from meta-discussion, like so: + + From: Mickey Mouse + Subject: Fix alignment + + Previously, the text was 6px too far to the left. + + Bug: http://example.com/bugs/123 + Cc: donald@example.com + --- + v2: don't change vertical alignment, spotted in Donald's review + """ + return msg.split('\n---\n', 1)[0] - def format_commit_msg(self, subject, body, fields, ask=False): + def format_field(self, field, ask=False): # This is the list of fields phabricator will search by default in # commit message, case insensitive. It will confuse phabricator's # parser if they appear in the subject or body of the commit message. @@ -491,21 +684,27 @@ class GitPhab: 'differential revision:', 'conflicts:', 'git-svn-id:', 'auditors:'] - subject = subject.strip() - body = '\n'.join(body).strip() - fields = '\n'.join(fields).strip() - for header in blacklist: header_ = header[:-1] + '_:' - s = re.sub(re.escape(header), header_, subject, flags=re.I) - b = re.sub(re.escape(header), header_, body, flags=re.I) - if (s != subject or b != body) and (not ask or - self.prompt("Commit message contains '%s'.\n" - "It could confuse phabricator's parser.\n" - "Do you want to prefix is with an underscore?" % - header)): - subject = s - body = b + f = re.sub('^' + re.escape(header), header_, field, flags=re.I) + if (f != field) and ( + not ask or self.prompt( + "Commit message contains '%s'.\n" + "It could confuse Phabricator's parser.\n" + "Do you want to suffix it with an underscore?" % + header)): + field = f + + return field + + def format_commit_msg(self, subject, body, git_fields, phab_fields, + ask=False): + subject = subject.strip() + body = '\n'.join(body).strip('\r\n') + fields = '\n'.join(git_fields + phab_fields).strip() + + subject = self.format_field(subject, ask) + body = self.format_field(body, ask) return '\n\n'.join([subject, body, fields]) @@ -531,7 +730,7 @@ class GitPhab: return None def get_reviewers_and_tasks(self, commit): - reviewers = [] + reviewers = set() tasks = [] diffid = self.get_differential_id(commit) @@ -540,10 +739,9 @@ class GitPhab: # This seems to be the only way to get the Maniphest and # reviewers of a differential. - reply = self.conduit('differential.getcommitmessage', { - 'revision_id': diffid - }) - msg = reply['response'] + reply = self.phabricator.differential.getcommitmessage( + revision_id=diffid) + msg = reply.response # Get tasks bound to this differential m = re.search('^Maniphest Tasks: (.*)$', msg, re.MULTILINE) @@ -553,25 +751,354 @@ class GitPhab: m = re.search('^Reviewed By: (.*)$', msg, re.MULTILINE) usernames = [r.strip() for r in m.group(1).split(',')] if m else [] if usernames: - reply = self.conduit('user.query', { - 'usernames': usernames - }) - for user in reply['response']: + reply = self.phabricator.user.query(usernames=usernames) + for user in reply: person = self.format_user(user['realName']) if person: - reviewers.append(person) + reviewers.add(person) return reviewers, tasks def remove_ourself_from_reviewers(self): if self.reviewers is None: return - reply = self.conduit("user.whoami", {}) - username = reply['response']['userName'] + username = self.phab_user.userName reviewers = [r.strip() for r in self.reviewers.split(',')] reviewers = list(filter(lambda r: r != username, reviewers)) self.reviewers = ','.join(reviewers) + def run_linter(self): + if not os.path.exists(".pre-commit-config.yaml"): + if os.path.exists(".arclint"): + subprocess.check_call("arc lint --never-apply-patches", + shell=True) + return None + else: + return None + command = ["pre-commit", "run", "--files"] + + for f in reversed(self.repo.git.show( + "--name-only", "--diff-filter=ACMR", "HEAD").split("\n")): + if not f: + break + command.append(f) + + return subprocess.check_output(command).decode("utf-8") + + def blob_is_binary(self, blob): + if not blob: + return False + + bytes = blob.data_stream[-1].read() + # The mime_type field of a gitpython blob is based only on its filename + # which means that files like 'configure.ac' will return weird MIME + # types, unsuitable for working out whether they are text. Instead, + # check whether any of the bytes in the blob are non-ASCII. + textchars = bytearray({7, 8, 9, 10, 12, 13, 27} | + set(range(0x20, 0x100)) - {0x7f}) + return bool(bytes.translate(None, textchars)) + + def get_changes_for_diff(self, diff): + def file_len(fname): + i = 0 + try: + with open(fname) as f: + for i, l in enumerate(f): + pass + except (FileNotFoundError, IsADirectoryError, UnicodeDecodeError): + return 0 + + return i + 1 + + def set_mode(properties, mode): + if mode is None: + return + if mode == 57344: + # Special case for submodules! + m = 160000 + else: + m = str(oct(mode))[2:] + + properties["unix:filemode"] = m + + change_filename = None + _type = 0 + oldpath = diff.a_path + patch_lines = str(diff.diff.decode("utf-8")).split("\n") + currentpath = diff.b_path + old_properties = {} + new_properties = {} + + change_filename = diff.b_path + if diff.new_file: + _type = 1 + oldpath = None + elif diff.deleted_file: + _type = 3 + change_filename = diff.a_path + currentpath = diff.a_path + elif diff.renamed: + _type = 6 + + set_mode(old_properties, diff.a_mode) + set_mode(new_properties, diff.b_mode) + added_lines = 0 + removed_lines = 0 + for l in patch_lines: + if l.startswith("+"): + added_lines += 1 + elif l.startswith("-"): + removed_lines += 1 + + is_text = (not self.blob_is_binary(diff.a_blob) and + not self.blob_is_binary(diff.b_blob)) + if is_text: + if diff.deleted_file: + file_length = 0 + old_length = len([l for l in patch_lines if + l.startswith('-')]) + else: + file_length = file_len(os.path.join( + self.repo.working_dir, diff.b_path)) + old_length = max(0, file_length - added_lines + removed_lines) + + metadata = {"line:first": 0} + hunks = [{ + "newOffset": "0" if diff.deleted_file else "1", + "oldOffset": "0" if diff.new_file else "1", + "oldLength": old_length, + "newLength": file_length, + "addLines": added_lines, + "delLines": removed_lines, + "corpus": "\n".join(patch_lines[1:]) + }] + filetype = "1" + else: + hunks = [] + phab_file = self.phabricator.file.upload( + data_base64=base64.standard_b64encode( + diff.b_blob.data_stream[-1].read()).decode("utf-8")) + + filetype = "3" + metadata = { + "old:file:size": diff.a_blob.size if diff.a_blob else 0, + "new:file:size": diff.b_blob.size if diff.b_blob else 0, + "new:file:mime-type": diff.b_blob.mime_type, + "new:binary-phid": phab_file.response + } + + return change_filename, {"metadata": metadata, + "oldProperties": old_properties, + "newProperties": new_properties, + "oldPath": oldpath, + "currentPath": currentpath, + "type": _type, + "fileType": filetype, + "hunks": hunks + } + + def get_git_diffs(self, commit): + if commit.parents: + diffs = commit.parents[0].diff( + create_patch=True, unified=999999999) + else: + diffs = commit.diff(git.diff.NULL_TREE if + hasattr(git.diff, "NULL_TREE") else "root", + create_patch=True, + unified=999999999) + return diffs + + def create_diff(self, commit, linter_status): + changes = {} + parent_commit = "" + diffs = self.get_git_diffs(commit) + if commit.parents: + parent_commit = self.repo.head.object.parents[0].hexsha + + for diff in diffs: + changed_file, change = self.get_changes_for_diff(diff) + changes[changed_file] = change + + print(" * Pushing new diff... ", end='') + diff = self.phabricator.differential.creatediff( + changes=changes, + sourceMachine=socket.gethostname(), + sourcePath=self.repo.working_dir, + sourceControlSystem="git", + sourceControlPath="", + sourceControlBaseRevision=parent_commit, + creationMethod="git-phab", + lintStatus=linter_status, + unitStatus="none", + parentRevisionID="", + authorPHID=self.phab_user.phid, + repositoryUUID="", + branch=self.branch_name, + repositoryPHID=self.phab_repo["phid"]) + print("%sOK%s" % (Colors.OKGREEN, Colors.ENDC)) + + return diff + + def push_diff_to_staging(self, diff, commit): + if not self.staging_url: + print(" * %sNo staging repo set, not pushing diff %s%s" % ( + Colors.FAIL, diff.diffid, Colors.ENDC)) + return + + print(" * Pushing diff %d on the staging repo... " % + diff.diffid, end='') + try: + self.repo.git.push( + self.staging_url, "%s:refs/tags/phabricator/diff/%s" % ( + commit.hexsha, diff.diffid)) + print("%sOK%s" % (Colors.OKGREEN, Colors.ENDC)) + except git.exc.GitCommandError as e: + print("%sERROR %s(%s)" % (Colors.FAIL, + Colors.ENDC, + e.stderr.decode("utf-8"))) + + def update_local_commit_info(self, diff, commit): + commit_infos = { + commit.hexsha: { + "commit": commit.hexsha, + "time": commit.authored_date, + "tree": commit.tree.hexsha, + "parents": [p.hexsha for p in commit.parents], + "author": commit.author.name, + "authorEmail": commit.author.email, + "message": commit.message, + } + } + + diffs = self.get_git_diffs(commit) + has_binary = False + for d in diffs: + if d.b_blob and \ + self.blob_is_binary(d.b_blob): + has_binary = True + break + + if not has_binary and not self.staging_url: + commit_infos[commit.hexsha]["raw_commit"] = \ + self.repo.git.format_patch("-1", "--stdout", + commit.hexsha) + + self.phabricator.differential.setdiffproperty( + diff_id=diff.diffid, + name="local:commits", + data=json.dumps(commit_infos)) + + def attach_commit(self, commit): + linter_message = None + print(" * Running linters...", end="") + linter_status = "none" + try: + self.run_linter() + print("%s OK%s" % (Colors.OKGREEN, Colors.ENDC)) + linter_status = "okay" + except BaseException as e: + linter_status = "fail" + if isinstance(e, subprocess.CalledProcessError) and e.stdout: + linter_result = e.stdout.decode("utf-8") + else: + linter_result = str(e) + + if not self.prompt("%s FAILED:\n\n%s\n\n%sAttach anyway?" + % (Colors.FAIL, linter_result, Colors.ENDC)): + raise e + + linter_message = "**LINTER FAILURE:**\n\n```\n%s\n```" % ( + linter_result) + + diff = self.create_diff(commit, linter_status) + phab = self.phabricator + subject, body, git_fields, phab_fields, updates = \ + self.parse_commit_msg(commit.message) + + try: + last_revision_id = self.get_differential_id( + self.repo.head.commit.parents[0]) + except IndexError: + last_revision_id = None + + if last_revision_id: + body.append("Depends on D%s" % last_revision_id) + + phab_fields.append("Projects: %s" % ','.join(self.project_phids)) + + summary = ('\n'.join(body) + '\n' + + '\n'.join(git_fields)).strip('\r\n') + + revision_id = self.get_differential_id(self.repo.head.commit) + if revision_id: + arc_message = phab.differential.getcommitmessage( + revision_id=revision_id, edit="update", + fields=phab_fields).response + else: + arc_message = phab.differential.getcommitmessage( + edit="create", fields=phab_fields).response + + arc_message = arc_message.replace( + "<>", + self.format_field(subject, True)) + if summary != '': + arc_message = arc_message.replace( + "Summary: ", + "Summary:\n" + self.format_field(summary, True)) + + if self.reviewers: + arc_message = arc_message.replace( + "Reviewers: ", "Reviewers: " + self.reviewers) + + if self.cc: + arc_message = arc_message.replace( + "Subscribers: ", "Subscribers: " + self.cc) + + arc_message = '\n'.join([ + l for l in arc_message.split("\n") + if not l.startswith("#")]) + + if self.task: + arc_message += "\n\nManiphest Tasks: %s" % ( + self.task) + + parsed_message = phab.differential.parsecommitmessage( + corpus=arc_message) + + fields = parsed_message["fields"] + if not revision_id: + revision = phab.differential.createrevision(fields=fields, + diffid=diff.diffid) + + if linter_message: + self.phabricator.differential.createcomment( + revision_id=int(revision.revisionid), + message=linter_message, action="none") + + return True, revision, diff + else: + message = None + if updates: + message = "\n".join([u for u in updates if u]) + if not message: + message = self.message + if not message: + message = self.edit_template( + "\n# Explain the changes you made since last " + " commit proposal\n# Last commit:\n#------\n#\n# %s" % + subject) + message = "\n".join(message) + + fields["summary"] = summary + fields["title"] = subject + if linter_message: + message += "\n\n%s" % linter_message + + return False, phab.differential.updaterevision( + id=revision_id, fields=fields, + diffid=diff.diffid, + message=message), diff + def do_attach(self): if self.repo.is_dirty(): self.die("Repository is dirty. Aborting.") @@ -588,10 +1115,17 @@ class GitPhab: summary = "" - # Oldest commit is last in the list + # Oldest commit is last in the list; if there is only one commit, we + # are trying to attach the first commit in the repository, so avoid + # trying to get its parent. commits = self.get_commits(self.revision_range) - s = commits[-1].hexsha + "^..HEAD" - all_commits = list(self.repo.iter_commits(s)) + if len(commits[-1].parents) > 0: + s = commits[-1].hexsha + "^..HEAD" + all_commits = list(self.repo.iter_commits(s)) + else: + s = commits[-1].hexsha + ".." + all_commits = list(self.repo.iter_commits(s)) + all_commits.append(commits[-1]) # Sanity checks for c in commits: @@ -602,7 +1136,7 @@ class GitPhab: self.filter_already_proposed_commits(commits, all_commits) if not commits: - print("Everything has already been proposed") + print("-> Everything has already been proposed") return # Ask confirmation before doing any harm @@ -621,111 +1155,78 @@ class GitPhab: agreed = self.prompt("Attach above commits?") if not agreed: - print("Aborting") + print("-> Aborting") sys.exit(0) if self.task == "T": try: - self.task = self.create_task()["response"]["objectName"] - summary += "New: task %s\n" % self.task + self.task = self.create_task(commits)["objectName"] + summary += " * New: task %s\n" % self.task except KeyError: self.die("Could not create task.") orig_commit = self.repo.head.commit orig_branch = self.repo.head.reference - arc_cmd = ['arc'] - if self.arcrc: - arc_cmd += ['--arcrc-file', self.arcrc] - - arc_cmd += ['diff', - '--allow-untracked', - '--config', 'history.immutable=false', - '--verbatim'] - if self.reviewers: - arc_cmd.append('--reviewers=' + self.reviewers) - if self.cc: - arc_cmd.append('--cc=' + self.cc) - if self.message: - arc_cmd.append('--message=' + self.message) - arc_cmd.append('HEAD~1') - - arc_failed = False + patch_attachement_failure = False try: # Detach HEAD from the branch; this gives a cleaner reflog for the # branch - print("Moving to starting point") - self.repo.head.reference = commits[-1].parents[0] + if len(commits[-1].parents) > 0: + self.repo.head.reference = commits[-1].parents[0] + else: + self.repo.head.reference = commits[-1] self.repo.head.reset(index=True, working_tree=True) for commit in reversed(all_commits): - self.repo.git.cherry_pick(commit.hexsha) + if len(commit.parents) > 0: + self.repo.git.cherry_pick(commit.hexsha) - if not arc_failed and commit in commits: - # Add extra info in the commit msg. It is important that - # phabricator fields are last, after all common git fields - # like 'Reviewed-by:', etc. Note that "Depends on" is not a - # field and is parsed from the body part. - subject, body, fields = self.parse_commit_msg( - commit.message) - last_revision_id = self.get_differential_id( - self.repo.head.commit.parents[0]) - if last_revision_id: - body.append("Depends on D%s" % last_revision_id) - if self.task: - fields.append("Maniphest Tasks: %s" % self.task) - fields.append("Projects: %s" % - ','.join(self.project_phids)) - msg = self.format_commit_msg(subject, body, fields) - - cur_orig_commit = self.repo.head.commit - self.repo.git.commit(amend=True, message=msg) - - print("attach " + commit.hexsha) + if not patch_attachement_failure and commit in commits: + print("-> Attaching %s:" % self.format_commit(commit)) try: - subprocess.check_call(arc_cmd) - except: - print("Command '%s' failed. Finnish rebuilding branch " - "without proposing further patches" % arc_cmd) - arc_failed = True - self.repo.head.commit = cur_orig_commit - summary += "Failed proposing: %s -- " \ + new, revision, diff = self.attach_commit(commit) + except Exception as e: + logging.exception("Failed proposing patch. " + "Finnish rebuilding branch " + "without proposing further patches") + sys.stdout.flush() + patch_attachement_failure = True + summary += " * Failed proposing: %s -- " \ "NO MORE PATCH PROPOSED\n" % self.format_commit( self.repo.head.commit) continue - # arc diff modified our commit message. Re-commit it with - # the original message, adding only the - # "Differential Revision:" line. - msg = commit.message - orig_link = self.get_differential_link(commit) - new_link = self.get_differential_link( - self.repo.head.commit) - if orig_link is None and new_link is not None: - msg = msg + '\nDifferential Revision: ' + new_link - summary += "New: " + msg = self.strip_updates(commit.message) + + # Add the "Differential Revision:" line. + if new: + msg = msg + '\nDifferential Revision: ' + revision.uri + summary += " * New: " else: - summary += "Updated: " + summary += " * Updated: %s " % revision.uri - self.repo.head.commit = cur_orig_commit - self.repo.git.commit(amend=True, message=msg) + self.repo.git.commit("-n", amend=True, message=msg) + self.update_local_commit_info(diff, self.repo.head.object) + self.push_diff_to_staging(diff, self.repo.head.object) + print("%s-> OK%s" % (Colors.OKGREEN, Colors.ENDC)) summary += self.format_commit(self.repo.head.commit) + "\n" else: - print("pick " + commit.hexsha) - summary += "Picked: %s\n" % self.format_commit(commit) + print("-> pick " + commit.hexsha) + summary += " * Picked: %s\n" % self.format_commit(commit) orig_branch.commit = self.repo.head.commit self.repo.head.reference = orig_branch except: - print("Cleaning up back to original state on error") + print("-> Cleaning up back to original state on error") self.repo.head.commit = orig_commit orig_branch.commit = orig_commit self.repo.head.reference = orig_branch self.repo.head.reset(index=True, working_tree=True) raise - if self.remote and self.task and not arc_failed: + if self.remote and self.task and not patch_attachement_failure: try: branch = self.get_wip_branch() remote = self.repo.remote(self.remote) @@ -733,26 +1234,23 @@ class GitPhab: info = remote.push('HEAD:refs/heads/' + branch, force=True)[0] if not info.flags & info.ERROR: - summary += "Branch pushed to %s/%s\n" % (remote, - branch) + summary += " * Branch pushed to %s/%s\n" % (remote, + branch) else: - print("Could not push branch %s/%s: %s" % ( + print("-> Could not push branch %s/%s: %s" % ( remote, branch, info.summary)) uri = "%s#%s" % (self.remote_url, branch) try: - self.conduit('maniphest.update', { - "id": int(self.task[1:]), - "auxiliary": { - "std:maniphest:git:uri-branch": uri - } - }) + self.phabricator.maniphest.update( + id=int(self.task[1:]), + auxiliary={"std:maniphest:git:uri-branch": uri}) except: - print("Failed to set std:maniphest:git:uri-branch to %s" + print("-> Failed to set std:maniphest:git:uri-branch to %s" % uri) except Exception as e: - summary += "Failed: push wip branch: %s\n" % e + summary += " * Failed: push wip branch: %s\n" % e if self.task and not self.branch_task: # Check if we already have a branch for this task @@ -768,7 +1266,7 @@ class GitPhab: if self.prompt('Reset branch %s to what has just been sent ' 'to phabricator?' % branch.name): branch.commit = self.repo.head.commit - summary += "Branch %s reset to %s\n" % \ + summary += " * Branch %s reset to %s\n" % \ (branch.name, branch.commit) else: new_bname = self.branch_name_with_task() @@ -776,7 +1274,7 @@ class GitPhab: if self.prompt("Rename current branch to '%s'?" % new_bname): self.repo.head.reference.rename(new_bname) - summary += "Branch renamed to %s\n" % new_bname + summary += " * Branch renamed to %s\n" % new_bname else: # Current branch is probably something like 'master' or # 'gnome-3-18', better create a new branch than renaming. @@ -788,8 +1286,8 @@ class GitPhab: new_branch.set_tracking_branch(tracking) new_branch.checkout() - summary += "Branch %s created and checked out\n" % \ - new_bname + summary += " * Branch %s created and checked out\n" \ + % new_bname print("\n\nSummary:") print(summary) @@ -815,39 +1313,31 @@ class GitPhab: def get_diff_phid(self, phid): # Convert diff phid to a name - reply = self.conduit("phid.query", {"phids": [phid]}) + reply = self.phabricator.phid.query(phids=[phid]) - self.check_conduit_reply(reply) - assert(len(reply['response']) == 1) + assert(len(reply) == 1) # Convert name to a diff json object - response = reply['response'][phid] + response = reply[phid] assert(response['type'] == "DIFF") d = response['name'].strip("Diff ") - reply = self.conduit("differential.querydiffs", { - "ids": [d] - }) - self.check_conduit_reply(reply) - assert(len(reply['response']) == 1) + reply = self.phabricator.differential.querydiffs(ids=[d]) + assert(len(reply) == 1) - response = reply['response'][d] - assert(response['sourceControlSystem'] == "git") + response = reply[d] return response def get_revision_and_diff(self, diff=None, phid=None): if diff is not None: - query = {"ids": [diff]} + reply = self.phabricator.differential.query(ids=[diff]) else: - query = {"phids": [phid]} - - reply = self.conduit("differential.query", query) - self.check_conduit_reply(reply) - assert(len(reply['response']) == 1) + reply = self.phabricator.differential.query(phids=[phid]) - revision = reply['response'][0] + assert(len(reply) == 1) + revision = reply[0] diff = self.get_diff_phid(revision['activeDiffPHID']) return revision, diff @@ -858,49 +1348,135 @@ class GitPhab: handle, filename = tempfile.mkstemp(".patch", "git-phab-") f = os.fdopen(handle, "w") + commit_hash = None + local_commits = {} + if isinstance(diff["properties"], dict): + local_commits = diff["properties"]["local:commits"] + try: + keys = [k for k in local_commits.keys()] + except TypeError: + keys = [] + + if len(keys) > 1: + self.die("%sRevision %s names several commits, " + "in git-phab workflow, 1 revision == 1 commit." + " We can't cherry-pick that revision.%s" + % (Colors.FAIL, revision.id, Colors.ENDC)) + + if keys: + local_infos = local_commits[keys[0]] + + raw_commit = local_infos.get("raw_commit") + # Use the raw_commit as set by git-phab when providing the patch + if raw_commit: + f.write(raw_commit) + f.close() + return filename + + # Try to rebuild the commit + commit_hash = local_infos.get("commit") + if commit_hash: + f.write("From: %s Mon Sep 17 00:00:00 2001\n" % commit_hash) + + authorname = diff.get("authorName") + email = diff.get("authorEmail") + if not authorname: + # getting author name from phabricator itself + authorname = self.phabricator.user.query( + phids=[revision['authorPHID']])[0]["realName"] + + author = self.format_user(authorname) + if not author: + self.die("%sNo author email for %s%s" + % (Colors.FAIL, authorname, Colors.ENDC)) + else: + author = "%s <%s>" % (authorname, email) + + f.write("From: %s\n" % author) f.write("Date: {} +0000\n".format(date)) - f.write("From: {} <{}>\n".format( - diff['authorName'], - diff['authorEmail'])) f.write("Subject: {}\n\n".format(revision['title'])) # Drop the arc insert Depends on Dxxxx line if needed summary = re.sub(re.compile("^\s*Depends on D\d+\n?", re.M), "", revision['summary']) f.write("{}\n".format(summary)) - f.write("Differential Revision: {}/D{}\n".format( + f.write("Differential Revision: {}/D{}\n\n".format( self.phabricator_uri, revision['id'])) - arc_cmd = ['arc'] - if self.arcrc: - arc_cmd += ['--arcrc-file', self.arcrc] - arc_cmd += ["export", "--git", "--revision", revision['id']] + diffid = self.get_diff_phid(revision['activeDiffPHID'])["id"] + output = self.phabricator.differential.getrawdiff(diffID=diffid) - output = subprocess.check_output(arc_cmd, universal_newlines=True) + f.write(output.response) - f.write(output) f.close() return filename - def am_patch(self, filename): + def am_patch(self, filename, base_commit): try: self.repo.git.am(filename) + return except git.exc.GitCommandError as e: - print(e) self.repo.git.am("--abort") - self.die("{}git am failed, aborting{}".format( - Colors.FAIL, Colors.ENDC)) + if not base_commit: + print(e) + self.die("{}git am failed, aborting{}".format( + Colors.FAIL, Colors.ENDC)) + + cbranch = self.repo.head.reference + + # Checkout base commit to apply patch on + try: + self.repo.head.reference = self.repo.commit(base_commit) + except (gitdb.exc.BadObject, ValueError): + self.die("%sCould not apply patch %s from %s (even on base commit" + " %s), aborting%s" % ( + Colors.FAIL, filename, self.differential, base_commit, + Colors.ENDC)) + self.repo.head.reset(index=True, working_tree=True) + + # Apply the patch on it + self.repo.git.am(filename) + new_commit = self.repo.head.commit + + # Go back to previous branch + self.repo.head.reference = cbranch + self.repo.head.reset(index=True, working_tree=True) + + # And try to cherry pick on patch + self.repo.git.cherry_pick(new_commit.hexsha) + + def fetch_staging_commits(self, diff): + if not self.staging_url: + print("No staging URL") + return False - def do_cherry_pick(self): + try: + self.repo.git.fetch( + self.staging_url, "refs/tags/phabricator/diff/%s" % + diff["id"]) + except git.exc.GitCommandError as e: + print(e) + return False + + return True + + def cherry_pick(self): if self.repo.is_dirty(): self.die("Repository is dirty. Aborting.") - print("Checking revision: ", self.differential) + print("Checking revision:", self.differential) did = self.differential.strip("D") + if not did.isdigit(): + self.die("Invalid diff ID ‘{}’".format(self.differential)) + revision, diff = self.get_revision_and_diff(diff=did) + if self.fetch_staging_commits(diff): + self.repo.git.cherry_pick("FETCH_HEAD") + return + if self.has_been_applied(revision): self.die("{} was already applied\n".format(self.differential)) @@ -908,14 +1484,11 @@ class GitPhab: if self.output_directory: self.move_to_output_directory(revision, diff, filename) else: - self.am_patch(filename) + self.am_patch(filename, diff.get("sourceControlBaseRevision")) os.unlink(filename) - def do_merge(self): - if self.repo.is_dirty(): - self.die("Repository is dirty. Aborting.") - - print("Checking revision: ", self.differential) + def get_differentials_to_apply_for_revision(self): + print("Checking revision:", self.differential) did = self.differential.strip("D") revision, diff = self.get_revision_and_diff(diff=did) @@ -930,12 +1503,13 @@ class GitPhab: revision, diff = self.get_revision_and_diff(phid=p) if self.has_been_applied(revision): - print("Already applied revision: ", revision['id']) continue - print("Getting Depend: D{}".format(revision['id'])) dq.append((revision, diff)) + return pq + def apply_differential_with_dependencies(self): + pq = self.get_differentials_to_apply_for_revision() n = 0 while pq != []: (r, d) = pq.pop() @@ -945,47 +1519,188 @@ class GitPhab: self.move_to_output_directory(r, d, filename, n) else: print("Applying D{}".format(r['id'])) - self.am_patch(filename) + self.am_patch(filename, d.get("sourceControlBaseRevision")) os.unlink(filename) n += 1 + def do_apply(self): + if self.repo.is_dirty(): + self.die("Repository is dirty. Aborting.") + elif not self.differential and not self.task: + self.die("No task or revision provided. Aborting.") + + if self.differential: + if self.no_dependencies: + self.cherry_pick() + else: + self.apply_differential_with_dependencies() + + return + + commit_info = self.fetch_from_task() + if self.no_dependencies: + if commit_info[0]: + self.repo.git.cherry_pick(commit_info[0].hexsha) + return + else: + self.die("Can not apply revisions from a task" + " without its dependencies as the task" + " might refer to several revisions.") + + starting_commit = self.repo.head.commit + try: + common_ancestor = self.repo.merge_base(commit_info[0], + starting_commit) + except git.exc.GitCommandError: + self.die("No common ancestor found between Task commit" + " and the current repository.") + + for commit in reversed(list(self.repo.iter_commits( + common_ancestor[0].hexsha + '^..' + commit_info[0].hexsha))): + try: + self.repo.git.cherry_pick(commit.hexsha) + except git.exc.GitCommandError as e: + stderr = e.stderr.decode("utf-8") + if "The previous cherry-pick is now empty," \ + " possibly due to conflict resolution." \ + in stderr: + self.repo.git.reset() + elif stderr.startswith("error: could not apply"): + self.die("%s\\nnWhen the conflict are fixed run" + " `git phab apply %s` again." % ( + stderr, self.task)) + else: + raise e + def do_log(self): commits = self.get_commits(self.revision_range) self.print_commits(commits) - def fetch(self): - reply = self.conduit('maniphest.query', { - "ids": [int(self.task[1:])] - }) - props = list(reply['response'].values())[0] - uri = props['auxiliary']['std:maniphest:git:uri-branch'] + def fetch_from_task(self): + reply = self.phabricator.maniphest.query(ids=[int(self.task[1:])]) + if not reply: + self.die("Not task found for ID: %s" % self.task) + + props = list(reply.values())[0] + auxiliary = props['auxiliary'] + if not auxiliary or not auxiliary.get('std:maniphest:git:uri-branch'): + # FIXME: There is currently no way to retrieve revisions + # associated with a task from the conduit API + self.die("%sCan not apply revisions from a task" + " if no 'remote branch' has been set for it.%s\n" + "INFO: You need to find what revisions are" + " associated with the tasks and apply them." + % (Colors.FAIL, Colors.ENDC)) + + uri = auxiliary['std:maniphest:git:uri-branch'] remote, branch = uri.split('#') - print("Git URI: %s, branch: %s" % (remote, branch)) self.repo.git.fetch(remote, "%s" % branch) + commit = self.repo.commit('FETCH_HEAD') + + return (commit, remote, branch) + + def create_fake_fetch(self, revision, diff): + current_branch = self.repo.active_branch + base_commit = diff.get("sourceControlBaseRevision") + if base_commit: + try: + self.repo.git.checkout(base_commit) + except git.exc.GitCommandError: + base_commit = None + + if not base_commit: + print("%sWARNING: Building `fake fetch` from" + " current commit (%s)\nas we do not have" + " information or access to the base commit" + " the revision has been proposed from%s" % ( + Colors.WARNING, self.repo.head.commit.hexsha, + Colors.ENDC)) + self.repo.git.checkout(self.repo.head.commit.hexsha) + + pq = self.get_differentials_to_apply_for_revision() + if pq: + n = 0 + while pq != []: + (r, d) = pq.pop() + + filename = self.write_patch_file(r, d) + print("Applying D{}".format(r['id'])) + + self.am_patch(filename, None) + os.unlink(filename) + n += 1 + branch_name = self.clean_phab_branch_name(revision.get('branch'), + self.differential) + + remote = "file://" + self.repo.working_dir + with open(os.path.join(self.repo.working_dir, ".git", + "FETCH_HEAD"), + "w") as fetch_head_file: + fetch_head_file.write("%s branch '%s' of %s" % ( + self.repo.head.commit.hexsha, branch_name, remote)) + + current_branch.checkout() commit = self.repo.commit('FETCH_HEAD') - print("Commit '%s' from remote branch '%s' has been fetched" % - (commit.hexsha, branch)) - return (commit, branch) + return commit, remote, branch_name def do_fetch(self): - if not self.task: - self.die("No task provided. Aborting.") + if not self.differential and not self.task: + self.die("No task or revision provided. Aborting.") - self.fetch() + if self.differential: + commit, remote, branch_name = self.fetch_from_revision() + else: + commit, remote, branch_name = self.fetch_from_task() - def do_checkout(self): - if not self.task: - self.die("No task provided. Aborting.") + if not self.checkout: + print("From %s\n" + " * branch %s -> FETCH_HEAD" % ( + remote, branch_name)) + return + + self.checkout_branch(commit, remote, branch_name) + + def clean_phab_branch_name(self, branch_name, default): + if not branch_name or branch_name in ['master']: + return default + + revision = self.revision_from_branchname(branch_name) + if revision: + return branch_name[len(revision + '-'):] + + task = self.task_from_branchname(branch_name) + if task: + return branch_name[len(task + '-'):] + + return branch_name - commit, remote_branch_name = self.fetch() + def fetch_from_revision(self): + did = self.differential.strip("D") + + revision, diff = self.get_revision_and_diff(diff=did) + + if not self.fetch_staging_commits(diff): + return self.create_fake_fetch(revision, diff) + + return (self.repo.rev_parse("FETCH_HEAD"), self.staging_url, + self.clean_phab_branch_name(revision['branch'], + self.differential)) + + def checkout_branch(self, commit, remote, remote_branch_name): + if self.differential: + branchname_match_method = self.revision_from_branchname + branch_name = self.differential + else: + branchname_match_method = self.task_from_branchname + branch_name = self.task # Lookup for an existing branch for this task branch = None for b in self.repo.branches: - if self.task_from_branchname(b.name) == self.task: + if branchname_match_method(b.name) == branch_name: branch = b break @@ -1033,6 +1748,7 @@ class GitPhab: def do_clean(self): branch_task = [] + self.repo.git.prune() for r in self.repo.references: if r.is_remote() and r.remote_name != self.remote: continue @@ -1042,9 +1758,9 @@ class GitPhab: branch_task.append((r, task)) task_ids = [t[1:] for b, t in branch_task] - reply = self.conduit('maniphest.query', {"ids": task_ids}) + reply = self.phabricator.maniphest.query(ids=task_ids) - for tphid, task in reply["response"].items(): + for tphid, task in reply.items(): if not task["isClosed"]: continue @@ -1055,8 +1771,11 @@ class GitPhab: if self.prompt("Task '%s' has been closed, do you want to " "delete branch '%s'?" % (task_name, branch)): if branch.is_remote(): - self.repo.git.push(self.remote, - ":" + branch.remote_head) + try: + self.repo.git.push(self.remote, + ":" + branch.remote_head) + except git.exc.GitCommandError: + pass else: self.repo.delete_head(branch, force=True) @@ -1067,7 +1786,7 @@ class GitPhab: self.die("Repository is dirty. Aborting.") if self.task: - commit, remote_branch_name = self.fetch() + commit, remote, remote_branch_name = self.fetch_from_task() branch = self.repo.active_branch if not self.prompt("Do you want to reset branch %s to %s?" % (branch.name, commit.hexsha)): @@ -1107,15 +1826,17 @@ class GitPhab: # - Add "Reviewed-by:" line # - Ensure body doesn't contain blacklisted words # - Ensure phabricator fields are last to make its parser happy - subject, body, fields = self.parse_commit_msg( - self.repo.head.commit.message) + # - Discard updates/discussion of previous patch revisions + subject, body, git_fields, phab_fields, updates = \ + self.parse_commit_msg(self.repo.head.commit.message) for r in reviewers: - # Note that we prepend here to make sur phab fields - # stay last. - fields.insert(0, "Reviewed-by: " + r) + field = "Reviewed-by: " + r + if field not in git_fields: + git_fields.append(field) - msg = self.format_commit_msg(subject, body, fields, True) + msg = self.format_commit_msg(subject, body, git_fields, + phab_fields, True) self.repo.git.commit(amend=True, message=msg) orig_branch.commit = self.repo.head.commit @@ -1143,10 +1864,8 @@ class GitPhab: # Propose to close tasks for task in set(all_tasks): if self.prompt("Do you want to close '%s'?" % task): - self.conduit("maniphest.update", { - 'id': int(task[1:]), - 'status': 'resolved' - }) + self.phabricator.maniphest.update(id=int(task[1:]), + status='resolved') def run(self): self.validate_args() @@ -1157,7 +1876,17 @@ class GitPhab: def DisabledCompleter(prefix, **kwargs): return [] + +def check_dependencies_versions(): + required_pygit_version = '2.0' + if git.__version__ < required_pygit_version: + print("%sPythonGit >= %s required %s found%s" + % (Colors.FAIL, required_pygit_version, + git.__version__, Colors.ENDC)) + exit(1) + if __name__ == '__main__': + check_dependencies_versions() parser = argparse.ArgumentParser(description='Phabricator integration.') subparsers = parser.add_subparsers(dest='subparser_name') subparsers.required = True @@ -1206,28 +1935,20 @@ if __name__ == '__main__': "the tracking branch is used") \ .completer = DisabledCompleter - cherrypick_parser = subparsers.add_parser( - 'cherry-pick', help="Cherrypick a patch from differential") - cherrypick_parser.add_argument( + apply_parser = subparsers.add_parser( + 'apply', help="Apply a revision and its dependencies" + " on the current tree") + apply_parser.add_argument( '--output-directory', '-o', metavar='', help="Directory to put patches in") - cherrypick_parser.add_argument( - 'differential', metavar='Differential ID', - default=None, - help="help Differential ID to cherrypick") \ + apply_parser.add_argument( + 'task_or_revision', metavar='<(T|D)123>', nargs='?', + help="The task or revision to fetch") \ .completer = DisabledCompleter - - merge_parser = subparsers.add_parser( - 'merge', help="Merge a revision and its dependencies") - merge_parser.add_argument( - '--output-directory', '-o', - metavar='', - help="Directory to put patches in") - merge_parser.add_argument( - 'differential', metavar='Differential ID', - default=None, - help="help Differential ID to merge") \ + apply_parser.add_argument( + '--no-dependencies', "-n", action="store_true", + help="Do not apply dependencies of a revision.") \ .completer = DisabledCompleter log_parser = subparsers.add_parser( @@ -1242,15 +1963,12 @@ if __name__ == '__main__': fetch_parser = subparsers.add_parser( 'fetch', help="Fetch a task's branch") fetch_parser.add_argument( - 'task', metavar='', nargs='?', - help="The task to fetch") \ + 'task_or_revision', metavar='<(T|D)123>', nargs='?', + help="The task or revision to fetch") \ .completer = DisabledCompleter - - checkout_parser = subparsers.add_parser( - 'checkout', help="Checkout a task's branch") - checkout_parser.add_argument( - 'task', metavar='', nargs='?', - help="The task to checkout") \ + fetch_parser.add_argument( + '--checkout', "-c", action="store_true", + help="Also checks out the commits in a branch.") \ .completer = DisabledCompleter browse_parser = subparsers.add_parser( diff --git a/git-phab.txt b/git-phab.txt index 3e6530a..5c5bd52 100644 --- a/git-phab.txt +++ b/git-phab.txt @@ -135,4 +135,3 @@ Push current branch to origin/wip/phab/T123 Fetch a branch associated with the task T123 [verse] $ git phab fetch T123 - diff --git a/requirements.txt b/requirements.txt new file mode 100644 index 0000000..6ca3d39 --- /dev/null +++ b/requirements.txt @@ -0,0 +1,5 @@ +appdirs +argcomplete +GitPython>=2.0.0 # Do not forget to update git-phab::check_dependencies_versions +phabricator +pre-commit diff --git a/setup.py b/setup.py index 6616010..22cc7a0 100644 --- a/setup.py +++ b/setup.py @@ -1,11 +1,17 @@ -import os +import subprocess +import sys from setuptools import setup +setup_requires = [] +if "upload" in sys.argv: + setup_requires=['setuptools-markdown'], +else: + setup_requires=['pre-commit'] setup( name="git-phab", - version="1.0", + version="1.9.0", author="Xavier Claessens", author_email="xavier.claessens@collabora.com", description=("Git subcommand to integrate with phabricator"), @@ -13,11 +19,16 @@ setup( keywords="phabricator tool git", url="http://packages.python.org/git-phab", long_description_markdown_filename='README.md', - setup_requires=['setuptools-markdown'], + setup_requires=setup_requires, classifiers=[ "Topic :: Utilities", "License :: OSI Approved :: GNU General Public License (GPL)" ], - install_requires=['GitPython', 'appdirs', 'argcomplete'], + install_requires=['GitPython>=2.0.0', 'appdirs', 'argcomplete', 'phabricator'], scripts=['git-phab'], ) + +try: + subprocess.check_call(["pre-commit", "install"]) +except (FileNotFoundError, subprocess.CalledProcessError): + print("Could not install `pre-commit` hook") -- cgit v1.2.3