pullcreatemarkers: query Phabricator to get "landed as" information

Summary:
The current code scans all *new public* commits to figure out "landed as"
information. It has problems:
- Sometimes it missed commits somehow and there is no way to "rescan" and
  re-add the "landed as" relationships. ([example](https://fb.workplace.com/groups/scm/permalink/2397939190255687/))
- In the future we might make `pull` smarter to not send all new public
  commits, this extension is a blocker for that.

This diff implements an alternative approach - ask Phabricator for *all draft*
commits' landed commits. It solves the above problems.

A new command `debugmarklanded` was added so the cleanup logic can be triggered
manually without pull.

Reviewed By: singhsrb

Differential Revision: D18229535

fbshipit-source-id: 6fe453a42332dfa8aaa99f389f1903e81f07d665

edenscm/hgext/extlib/phabricator/graphql.py

@@ -14,6 +14,7 @@ import json
 import operator
 
 from edenscm.mercurial import encoding, pycompat, util
+from edenscm.mercurial.node import bin
 
 from . import arcconfig, phabricator_graphql_client, phabricator_graphql_client_urllib
 
@@ -29,6 +30,11 @@ class ClientError(Exception):
 
 class Client(object):
     def __init__(self, repodir=None, ca_bundle=None, repo=None):
+        if repo is not None:
+            if repodir is None:
+                repodir = repo.root
+            if ca_bundle is None:
+                ca_bundle = repo.ui.configpath("web", "cacerts")
         if not repodir:
             repodir = pycompat.getcwd()
         self._mock = "HG_ARC_CONDUIT_MOCK" in encoding.environ
@@ -157,6 +163,49 @@ class Client(object):
             "latest_phabricator_version"
         ]
 
+    def getlandednodes(self, diffids, timeout=10):
+        """Get landed nodes for diffids. Return {diffid: node}"""
+        query = """
+            query DiffToCommitQuery($diffids: [String!]!){
+                phabricator_diff_query(query_params: {
+                    numbers: $diffids
+                }) {
+                    results {
+                        nodes {
+                            number
+                            phabricator_diff_commit {
+                                nodes {
+                                    commit_identifier
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+            """
+        params = {"diffids": diffids}
+        ret = self._client.query(timeout, query, params)
+        # Example result:
+        # { "data": {
+        #     "phabricator_diff_query": [
+        #       { "results": {
+        #           "nodes": [
+        #             { "phabricator_diff_commit": {
+        #                 "nodes": [
+        #                   { "commit_identifier": "9396e4a63208eb034b8b9cca909f9914cb2fbe85" } ] } } ] } } ] } }
+        difftonode = {}
+        for result in ret["data"]["phabricator_diff_query"][0]["results"]["nodes"]:
+            try:
+                diffid = "%s" % result["number"]
+                nodes = result["phabricator_diff_commit"]["nodes"]
+                if nodes:
+                    difftonode[diffid] = bin(nodes[0]["commit_identifier"])
+            except (KeyError, IndexError):
+                # Not fatal.
+                continue
+
+        return difftonode
+
     def getrevisioninfo(self, timeout, signalstatus, *revision_numbers):
         rev_numbers = self._normalizerevisionnumbers(revision_numbers)
         if self._mock:

edenscm/hgext/pullcreatemarkers.py

@@ -1,29 +1,100 @@
 # Copyright (c) Facebook, Inc. and its affiliates.
 #
 # This software may be used and distributed according to the terms of the
-# GNU General Public License version 2.
 
-# pullcreatemarkers.py - create obsolescence markers on pull for better rebases
+"""
-#
+mark commits as "Landed" on pull
-# The goal of this extensions is to create obsolescence markers locally for
+
-# commits previously landed.
+Config::
-# It uses the phabricator revision number in the commit message to detect the
+
-# relationship between a draft commit and its landed counterpart.
+    [pullcreatemarkers]
-# Thanks to these markers, less information is displayed and rebases can have
+    # Use graphql to query what diffs are landed, instead of scanning
-# less irrelevant conflicts.
+    # through pulled commits.
-from edenscm.mercurial import (
+    use-graphql = true
+"""
+from ..mercurial import (
     commands,
     extensions,
     mutation,
     obsolete,
     phases,
+    registrar,
     visibility,
 )
-
+from ..mercurial.i18n import _
-from .extlib.phabricator import diffprops
+from ..mercurial.node import short
+from .extlib.phabricator import diffprops, graphql
 from .phabstatus import COMMITTEDSTATUS, getdiffstatus
 
 
+cmdtable = {}
+command = registrar.command(cmdtable)
+
+
+def _cleanuplanded(repo, dryrun=False, skipnodes=None):
+    """Query Phabricator about states of draft commits and optionally mark them
+    as landed.
+
+    This uses mutation and visibility directly.
+    """
+    if skipnodes is None:
+        skipnodes = set()
+    difftodraft = {}  # {str: node}
+    for ctx in repo.set("draft() - obsolete()"):
+        if ctx.node() in skipnodes:
+            continue
+        diffid = diffprops.parserevfromcommitmsg(ctx.description())  # str or None
+        if diffid:
+            difftodraft.setdefault(diffid, []).append(ctx.node())
+
+    client = graphql.Client(repo=repo)
+    difftopublic = client.getlandednodes(list(difftodraft.keys()))
+    ui = repo.ui
+    unfi = repo.unfiltered()
+    mutationentries = []
+    tohide = set()
+    for diffid, draftnodes in sorted(difftodraft.items()):
+        publicnode = difftopublic.get(diffid)
+        if publicnode is None or publicnode not in unfi:
+            continue
+        # sanity check - the public commit should have a sane commit message.
+        if diffprops.parserevfromcommitmsg(unfi[publicnode].description()) != diffid:
+            continue
+        draftnodestr = ", ".join(short(d) for d in draftnodes)
+        ui.status(
+            _("marking D%s (%s) as landed as %s\n")
+            % (diffid, draftnodestr, short(publicnode))
+        )
+        for draftnode in draftnodes:
+            tohide.add(draftnode)
+            skipnodes.add(draftnode)
+            mutationentries.append(
+                mutation.createsyntheticentry(
+                    unfi, mutation.ORIGIN_SYNTHETIC, [draftnode], publicnode, "land"
+                )
+            )
+    if not tohide:
+        return
+    if not dryrun:
+        with unfi.lock(), unfi.transaction("pullcreatemarkers"):
+            if mutation.recording(unfi):
+                mutation.recordentries(unfi, mutationentries, skipexisting=False)
+            if visibility.tracking(unfi):
+                visibility.remove(unfi, tohide)
+    # In case the graphql result is paginated, query again to fetch the
+    # remaining results.
+    _cleanuplanded(repo, dryrun=dryrun, skipnodes=skipnodes)
+
+
+@command("debugmarklanded", commands.dryrunopts)
+def debugmarklanded(ui, repo, **opts):
+    """query Phabricator and mark landed commits"""
+    dryrun = opts.get("dry_run")
+    _cleanuplanded(repo, dryrun=dryrun)
+    if dryrun:
+        ui.status(_("(this is a dry-run, nothing was actually done)\n"))
+
+
 def getdiff(rev):
     phabrev = diffprops.parserevfromcommitmsg(rev.description())
     return int(phabrev) if phabrev else None
@@ -44,7 +115,11 @@ def _pull(orig, ui, repo, *args, **opts):
     maxrevbeforepull = len(repo.changelog)
     r = orig(ui, repo, *args, **opts)
     maxrevafterpull = len(repo.changelog)
-    createmarkers(r, repo, maxrevbeforepull, maxrevafterpull)
+
+    if ui.configbool("pullcreatemarkers", "use-graphql"):
+        _cleanuplanded(repo)
+    else:
+        createmarkers(r, repo, maxrevbeforepull, maxrevafterpull)
     return r
 
 

edenscm/mercurial/commands/eden.py

@@ -3,6 +3,8 @@
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2.
 
+# no-check-code
+
 import argparse
 import binascii
 import collections

tests/test-check-code.t

@@ -43,6 +43,7 @@ New errors are not allowed. Warnings are strongly discouraged.
   Skipping edenscm/hgext/extlib/ctreemanifest/treemanifest.h it has no-che?k-code (glob)
   Skipping edenscm/hgext/globalrevs.py it has no-che?k-code (glob)
   Skipping edenscm/hgext/hgsql.py it has no-che?k-code (glob)
+  Skipping edenscm/mercurial/commands/eden.py it has no-che?k-code (glob)
   Skipping edenscm/mercurial/httpclient/__init__.py it has no-che?k-code (glob)
   Skipping edenscm/mercurial/httpclient/_readers.py it has no-che?k-code (glob)
   Skipping edenscm/mercurial/statprof.py it has no-che?k-code (glob)
@@ -63,7 +64,7 @@ New errors are not allowed. Warnings are strongly discouraged.
   Skipping tests/test-hgsql-encoding.t it has no-che?k-code (glob)
   Skipping tests/test-hgsql-race-conditions.t it has no-che?k-code (glob)
   Skipping tests/test-rustthreading.py it has no-che?k-code (glob)
-  edenscm/mercurial/commands/eden.py:407: use foobar, not foo_bar naming --> def cmd_get_file_size(self, request):
+  edenscm/hgext/extlib/phabricator/graphql.py:37: use foobar, not foo_bar naming --> ca_bundle = repo.ui.configpath("web", "cacerts")
   tests/run-tests.py:*: don't use camelcase in identifiers --> self.testsSkipped = 0 (glob)
 
 @commands in debugcommands.py should be in alphabetical order.

tests/test-help.t

@@ -351,7 +351,6 @@ Test extension help:
        phabstatus    (no help text available)
        phrevset      provides support for Phabricator revsets
        pullcreatemarkers
-                     (no help text available)
        purge         command to delete untracked files from the working
                      directory
        pushrebase    rebases commits during push