[PATCH 1 of 4 V2] histedit: make verification configurable

Mateusz Kwapich mitrandir at fb.com
Wed Nov 18 01:55:49 UTC 2015


# HG changeset patch
# User Mateusz Kwapich <mitrandir at fb.com>
# Date 1447807046 28800
#      Tue Nov 17 16:37:26 2015 -0800
# Node ID 2aa95a3b1dcb9921c65a74228306fc1693dbf1d2
# Parent  2da6a2dbfc42bdec4bcaf47da947c64ff959a59c
histedit: make verification configurable

Before we can add a 'base' action to histedit need to change verification
so that action can specify which steps of verification should run for it.

Also it's everything we need for the exec and stop actions implementation.

I thought about baking verification into each histedit action (so each
of them is responsible for verifying its constraints) but it felt wrong
because:
 - every action would need to know its context (eg. the list of all other
   actions)
 - a lot of duplicated work will be added - each action will iterate through
   all others
 - the steps of the verification would need to be extracted and named anyway
   in order to be reused

The verifyrules function grows too big now. I plan to refator it in one of
the next series.

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -334,6 +334,24 @@
             raise error.Abort(_('unknown changeset %s listed') % rulehash[:12])
         return cls(state, node)
 
+    def constraints(self):
+        """Return a set of constrains that this action should be verified for
+
+           Available constraints:
+               noduplicates - aborts if there are multiple rules for one node
+               noother - abort if the node doesn't belong to edited stack
+        """
+
+        return set(['noduplicates', 'noother'])
+
+    def nodetoverify(self):
+        """Returns a node associated with the action that will be used for
+        verification purposes.
+
+        If the action doesn't correspond to node it should return None
+        """
+        return self.node
+
     def run(self):
         """Runs the action. The default behavior is simply apply the action's
         rulectx onto the current parentctx."""
@@ -807,7 +825,7 @@
             f.close()
         rules = [l for l in (r.strip() for r in rules.splitlines())
                  if l and not l.startswith('#')]
-        rules = verifyrules(rules, repo, [repo[c] for [_a, c] in state.rules])
+        rules = verifyrules(rules, state, [repo[c] for [_a, c] in state.rules])
         state.rules = rules
         state.write()
         return
@@ -888,7 +906,7 @@
             f.close()
         rules = [l for l in (r.strip() for r in rules.splitlines())
                  if l and not l.startswith('#')]
-        rules = verifyrules(rules, repo, ctxs)
+        rules = verifyrules(rules, state, ctxs)
 
         parentctxnode = repo[root].parents()[0].node()
 
@@ -1038,34 +1056,42 @@
 
     return rules
 
-def verifyrules(rules, repo, ctxs):
+def verifyrules(rules, state, ctxs):
     """Verify that there exists exactly one edit rule per given changeset.
 
     Will abort if there are to many or too few rules, a malformed rule,
     or a rule on a changeset outside of the user-given range.
     """
+    known_constraints = ['noother', 'noduplicates']
     parsed = []
     expected = set(c.hex() for c in ctxs)
     seen = set()
     for r in rules:
         if ' ' not in r:
             raise error.Abort(_('malformed line "%s"') % r)
-        action, rest = r.split(' ', 1)
-        ha = rest.strip().split(' ', 1)[0]
-        try:
-            ha = repo[ha].hex()
-        except error.RepoError:
-            raise error.Abort(_('unknown changeset %s listed') % ha[:12])
-        if ha not in expected:
-            raise error.Abort(
-                _('may not use changesets other than the ones listed'))
-        if ha in seen:
-            raise error.Abort(_('duplicated command for changeset %s') %
-                    ha[:12])
-        seen.add(ha)
-        if action not in actiontable or action.startswith('_'):
-            raise error.Abort(_('unknown action "%s"') % action)
-        parsed.append([action, ha])
+        verb, rest = r.split(' ', 1)
+
+        if verb not in actiontable or verb.startswith('_'):
+            raise error.Abort(_('unknown action "%s"') % verb)
+        action = actiontable[verb].fromrule(state, rest)
+        constraints = action.constraints()
+        for constraint in constraints:
+            if constraint not in known_constraints:
+                error.Abort(_('uknown constraint "%s"') % constraint)
+
+        nodetoverify = action.nodetoverify()
+        if nodetoverify is not None:
+            ha = node.hex(nodetoverify)
+            if 'noother' in constraints and ha not in expected:
+                raise error.Abort(
+                    _('may not use "%s" with changesets '
+                      'other than the ones listed') % verb)
+            if 'noduplicates' in constraints and ha in seen:
+                raise error.Abort(_('duplicated command for changeset %s') %
+                        ha[:12])
+            seen.add(ha)
+            rest = ha
+        parsed.append([verb, rest])
     missing = sorted(expected - seen)  # sort to stabilize output
     if missing:
         raise error.Abort(_('missing rules for changeset %s') %
diff --git a/tests/test-histedit-arguments.t b/tests/test-histedit-arguments.t
--- a/tests/test-histedit-arguments.t
+++ b/tests/test-histedit-arguments.t
@@ -175,7 +175,7 @@
   > pick c8e68270e35a 3 four
   > pick 08d98a8350f3 4 five
   > EOF
-  abort: may not use changesets other than the ones listed
+  abort: may not use "pick" with changesets other than the ones listed
   [255]
 
 Test malformed line
diff --git a/tests/test-histedit-commute.t b/tests/test-histedit-commute.t
--- a/tests/test-histedit-commute.t
+++ b/tests/test-histedit-commute.t
@@ -281,7 +281,7 @@
   > pick de71b079d9ce e
   > pick 38b92f448761 c
   > EOF
-  abort: may not use changesets other than the ones listed
+  abort: may not use "pick" with changesets other than the ones listed
   $ hg log --graph
   @  changeset:   7:803ef1c6fcfd
   |  tag:         tip


More information about the Mercurial-devel mailing list