Patchwork [accept-scripts,V2] land: fix bug we see when we merge, rather than rebase

login
register
mail settings
Submitter Augie Fackler
Date April 27, 2018, 6:04 p.m.
Message ID <214aea0ce91528a7b22b.1524852260@augie-macbookpro2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/31243/
State Accepted
Headers show

Comments

Augie Fackler - April 27, 2018, 6:04 p.m.
# HG changeset patch
# User Augie Fackler <raf@durin42.com>
# Date 1524713796 14400
#      Wed Apr 25 23:36:36 2018 -0400
# Node ID 214aea0ce91528a7b22ba34ab991c43963872d23
# Parent  66e6638d2a6e78f3af813a274eadbd90679b888e
land: fix bug we see when we merge, rather than rebase

I triggered this when I tagged 4.6rc1 concurrent with 8c8b6d13949a
being landed and accepted. That change was only a doc change, so it
wasn't worth redoing the RC, but then the tagging revision got stuck
because it wasn't a child of a head in the repo.

Unfortunately, I couldn't figure out how to fix this bug without what
amounts to a ground-up rewrite of the land script. Sigh.

Patch

diff --git a/land b/land
--- a/land
+++ b/land
@@ -1,8 +1,11 @@ 
 #!/usr/bin/python
 
+import collections
+import json
 import logging
 import os
 import re
+import subprocess
 
 import accept
 import config
@@ -32,29 +35,71 @@  for l in os.popen("hg log -R %s -r 'defa
 
 take = []
 takelog = ""
-while True:
-    hit = False
-    for n in cq.draft():
-        a = cq.accepted(n)
-        if not a:
-            logging.debug("%s not accepted, skipping", n)
-            continue
-
-        if cq.parent(n) not in heads:
-            logging.debug("last public ancestor of %s not in %s, skipping",
-                          n, dest)
-            continue
-
-        hit = True
-        takelog += "%s %s %s\n" % (n, cq.author(n), " ".join(sorted(a)))
-        logging.debug('taking %s with accepters %r and author %r',
-                      n, a, cq.author(n))
-        take.append(n)
-        heads.remove(cq.parent(n))
-        heads.add(n)
-
-    if not hit:
-        break
+
+def _destcontains(node):
+    try:
+        return subprocess.check_output(
+            ['hg', 'log', '-R', dest, '-r', node, '-T.'],
+            stderr=subprocess.STDOUT) == '.'
+    except subprocess.CalledProcessError:
+        return False
+
+def _thesenodesrevset(nodes):
+    """Given a list of hex nodes, return a revset query that matches them all.
+
+    Just here to make things a bit cleaner.
+    """
+    return ' + '.join(nodes)
+
+# Find all accepted revisions
+all_accepted = [n for n in cq.draft() if cq.accepted(n)]
+info = json.load(os.popen("hg log -R %s -r '%s' -Tjson" % (
+    conf.source, _thesenodesrevset(all_accepted))))
+# dict of node: its parents.
+parents = {i['node']: i['parents'] for i in info}
+
+# Identify heads and roots of accepted revision ranges
+accepted_roots = {r.strip() for r in os.popen(
+    "hg log -R %s -r 'roots(%s)' -T'{node}\\n'" % (
+        conf.source, _thesenodesrevset(all_accepted)))}
+accepted_heads = [h.strip() for h in os.popen(
+    "hg log -R %s -r 'heads(%s)' -T'{node}\\n'" % (
+        conf.source, _thesenodesrevset(all_accepted)))]
+# Condense accepted revisions down into accepted ranges
+acceptrange = collections.namedtuple('acceptrange', 'roots head')
+ranges = []
+for head in accepted_heads:
+    roots = []
+    visit = [head]
+    while visit:
+        current = visit.pop(-1)
+        if _destcontains(current):
+            roots.append(current)
+        elif current not in accepted_roots:
+            visit.extend(parents[current])
+        else:
+            roots.extend(parents[current])
+    ranges.append(acceptrange(roots, head))
+
+logging.debug('accepted roots: %s', ' '.join(accepted_roots))
+logging.debug('accepted heads: %s', ' '.join(accepted_heads))
+# Figure out what range(s) are descended from the repo
+take = []
+for r in ranges:
+    if not all(_destcontains(root) for root in r.roots):
+        logging.debug('not all roots of range %r in dest', r)
+        continue
+    for root in r.roots:
+        if root in heads:
+            heads.remove(root)
+            break
+    else:
+        logging.debug('range %r not descended from a repo head, skipping',
+                      r)
+        continue
+    logging.debug('range %r is valid', r)
+    heads.add(r.head)
+    take.append(r.head)
 
 if take:
     logging.debug("landing changes in %s:", dest)
diff --git a/tests/test-land.t b/tests/test-land.t
--- a/tests/test-land.t
+++ b/tests/test-land.t
@@ -19,3 +19,83 @@  If only r1 is accepted, land pulls it in
   date:        Thu Jan 01 00:00:00 1970 +0000
   summary:     Add alpha.
   
+
+More complicated example built using debugbuilddag. This emulates a bug
+encountered during the hg 4.6 freeze.
+  >>> import os
+  >>> with open(os.environ['HG_ACCEPT_CONFIG']) as f:
+  ...   cfg = f.read()
+  >>> cfglines = cfg.splitlines()
+  >>> cfglines[-2] = cfglines[-2][:-len('source')] + 'complexsrc'
+  >>> cfglines[-1] = cfglines[-1][:-len('target')] + 'complextgt'
+  >>> with open(os.environ['HG_ACCEPT_CONFIG'], 'w') as f:
+  ...   f.write('\n'.join(cfglines))
+
+  $ hg init complexsrc
+  $ hg init complextgt
+  $ cd complexsrc
+  $ hg debugbuilddag '. @stable +2 :pubhead <2 +2 :daggyfix /3 +1 :accepted +3'
+  $ hg phase --public 2
+  $ hg log -G -T'{rev}:{node|shortest} {branch} {tags}\n'
+  o  9:6bb2 stable tip
+  |
+  o  8:0ffd stable
+  |
+  o  7:38e0 stable
+  |
+  o  6:ee2d stable accepted
+  |
+  o    5:4721 stable
+  |\
+  | o  4:6cfb stable daggyfix
+  | |
+  | o  3:0ccc stable
+  | |
+  o |  2:f3cf stable pubhead
+  |/
+  o  1:a079 stable
+  |
+  o  0:1ea7 default
+  
+  $ for rev in 3 4 5 6 7 8 9 ; do
+  >   logpush $rev alice alice
+  > done
+  $ for rev in 3 4 5 6 9 ; do
+  >   echo `hg log -r $rev --template '{node}'` >> ../bob-accepted
+  > done
+  $ cd ../complextgt
+  $ hg pull -r pubhead ../complexsrc
+  pulling from ../complexsrc
+  adding changesets
+  adding manifests
+  adding file changes
+  added 3 changesets with 0 changes to 0 files
+  new changesets 1ea73414a91b:f3cf5bef6844
+  (run 'hg update' to get a working copy)
+  $ hg log -G -T'{rev}:{node|shortest} {branch} {tags}\n'
+  o  2:f3cf stable tip
+  |
+  o  1:a079 stable
+  |
+  o  0:1ea7 default
+  
+We expect to get the hash of 7 as blocker
+  $ blocker
+  38e0ccfa60f109f813eba32713eada1543664dbf alice
+We expect to land 3::6.
+  $ land
+  $ hg log -G -T'{rev}:{node|shortest} {branch} {tags}\n'
+  o  6:ee2d stable tip
+  |
+  o    5:4721 stable
+  |\
+  | o  4:6cfb stable
+  | |
+  | o  3:0ccc stable
+  | |
+  o |  2:f3cf stable
+  |/
+  o  1:a079 stable
+  |
+  o  0:1ea7 default
+