hghooks: allow well formed manually crafted servo merge commits to bypass scm_servo_vendor commit hook check (bug 1375092) r?gps draft
authorbyron jones <glob@mozilla.com>
Thu, 22 Jun 2017 16:09:52 +0800
changeset 11300 d869b5a6baf8e31781b8bdcd5b88b376a8f895cb
parent 11297 1f4935bd07062c6854f2bb832ed31662c12d3c9b
push id1714
push userbjones@mozilla.com
push dateFri, 23 Jun 2017 04:46:19 +0000
reviewersgps
bugs1375092
hghooks: allow well formed manually crafted servo merge commits to bypass scm_servo_vendor commit hook check (bug 1375092) r?gps We allow changes to servo created by the prepare-servo-commit hgext. This gives stylo developers the ability to resolve out of sequence landing of servo commits. These commits must meet the following criteria in order to be considered 'well formed': - the commit description must match the formatting that servo-vcs-sync uses; specifically it must start with "servo: Merge#". - the commit must only touch files within the /servo directory. - the commit must contain the extra data that instructs the overlay extension (part of servo-vcs-sync) to skip that revision. MozReview-Commit-ID: CLUIANVRdv5
hghooks/mozhghooks/prevent_vendored_changes.py
hghooks/tests/commitextras.py
hghooks/tests/test-prevent-vendored-changes.t
--- a/hghooks/mozhghooks/prevent_vendored_changes.py
+++ b/hghooks/mozhghooks/prevent_vendored_changes.py
@@ -2,16 +2,21 @@
 # GNU General Public License version 2 or any later version.
 
 import grp
 import os
 
 from mercurial.node import short
 
 
+# From hgext/overlay/__init__.py
+REVISION_KEY = 'subtree_revision'
+SOURCE_KEY = 'subtree_source'
+
+
 def is_servo_vender_member(user):
     try:
         g = grp.getgrnam('scm_servo_vendor')
         if user in g.gr_mem:
             return True
     except KeyError:
         pass
 
@@ -22,16 +27,69 @@ def is_servo_allowed(user):
     if is_servo_vender_member(user):
         return True
 
     return user in {
         'servo-vcs-sync@mozilla.com',
     }
 
 
+""""We allow changes to servo created by the prepare-servo-commit hgext.
+
+This gives stylo developers the ability to resolve out of sequence
+landing of servo commits.  These commits must meet the following criteria
+in order to be considered 'well formed':
+
+- the commit description must match the formatting that servo-vcs-sync
+  uses; specifically it must start with "servo: Merge#".
+- the commit must only touch files within the /servo directory.
+- the commit must contain the extra data that instructs the overlay
+  extension (part of servo-vcs-sync) to skip that revision.
+"""
+
+
+def has_fixup_extra_data(ctx):
+    # REVISION_KEY must be hex.
+    try:
+        int(ctx.extra().get(REVISION_KEY), 16)
+    except (TypeError, ValueError):
+        return False
+
+    # SOURCE_KEY must be URL.
+    try:
+        return ctx.extra().get(SOURCE_KEY).startswith('https://')
+    except AttributeError:
+        return False
+
+
+def valid_fixup_description(ctx):
+    return ctx.description().startswith('servo: Merge #')
+
+
+def valid_fixup_files(ctx):
+    return all(f.startswith('servo/') for f in ctx.files())
+
+
+def is_fixup_commits(repo, nodes):
+    """Returns True when all revisions contain valid fixup commit extra data."""
+    assert nodes is not None
+    for rev in nodes:
+        if not has_fixup_extra_data(repo[rev]):
+            return False
+    return True
+
+
+def write_error(ui, lines):
+    header = '*' * 72
+    ui.write('%s\n' % header)
+    for line in lines:
+        ui.write('%s\n' % line)
+    ui.write('%s\n' % header)
+
+
 def hook(ui, repo, node, source=None, **kwargs):
     if source in ('pull', 'strip'):
         return 0
 
     servonodes = []
 
     for rev in repo.changelog.revs(repo[node].rev()):
         ctx = repo[rev]
@@ -43,27 +101,49 @@ def hook(ui, repo, node, source=None, **
     res = 0
 
     if servonodes:
         ui.write('(%d changesets contain changes to protected servo/ '
                  'directory: %s)\n' % (len(servonodes), ', '.join(servonodes)))
 
         if is_servo_allowed(os.environ['USER']):
             ui.write('(you have permission to change servo/)\n')
+
+        elif is_fixup_commits(repo, servonodes):
+            for node in servonodes:
+                ctx = repo[node]
+
+                if not valid_fixup_description(ctx):
+                    res = 1
+                    write_error(ui, [
+                        'invalid servo fixup commit: %s' % node,
+                        '',
+                        'commit description is malformed',
+                    ])
+
+                if not valid_fixup_files(ctx):
+                    res = 1
+                    write_error(ui, [
+                        'invalid servo fixup commit: %s' % node,
+                        '',
+                        'commit modifies non-servo files',
+                    ])
+
+                if res == 0:
+                    ui.write('(allowing valid fixup commit to servo: %s)\n'
+                             % node)
+
         else:
             res = 1
-            header = '*' * 72
-            ui.write('%s\n' % header)
-            ui.write('you do not have permissions to modify files under '
-                     'servo/\n')
-            ui.write('\n')
-            ui.write('the servo/ directory is kept in sync with the canonical '
-                     'upstream\nrepository at '
-                     'https://github.com/servo/servo\n')
-            ui.write('\n')
-            ui.write('changes to servo/ are only allowed by the syncing tool '
-                     'and by sheriffs\nperforming cross-repository "merges"\n')
-            ui.write('\n')
-            ui.write('to make changes to servo/, submit a Pull Request against '
-                     'the servo/servo\nGitHub project\n')
-            ui.write('%s\n' % header)
+            write_error(ui, [
+                'you do not have permissions to modify files under servo/',
+                '',
+                'the servo/ directory is kept in sync with the canonical upstream',
+                'repository at https://github.com/servo/servo',
+                '',
+                'changes to servo/ are only allowed by the syncing tool and by sheriffs',
+                'performing cross-repository "merges"',
+                '',
+                'to make changes to servo/, submit a Pull Request against the servo/servo',
+                'GitHub project',
+            ])
 
     return res
new file mode 100644
--- /dev/null
+++ b/hghooks/tests/commitextras.py
@@ -0,0 +1,42 @@
+# commitextras.py
+#
+# Copyright 2013 Facebook, Inc.
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+
+from mercurial.i18n import _
+from mercurial import (
+    cmdutil,
+    commands,
+    extensions,
+)
+
+cmdtable = {}
+command = cmdutil.command(cmdtable)
+
+
+def extsetup(ui):
+    entry = extensions.wrapcommand(commands.table, 'commit', _commit)
+    options = entry[1]
+    options.append(('', 'extra', [],
+        _('set a changeset\'s extra values'), _("KEY=VALUE")))
+
+
+def _commit(orig, ui, repo, *pats, **opts):
+    origcommit = repo.commit
+    try:
+        def _wrappedcommit(*innerpats, **inneropts):
+            extras = opts.get('extra')
+            if extras:
+                for raw in extras:
+                    k, v = raw.split('=', 1)
+                    inneropts['extra'][k] = v
+            return origcommit(*innerpats, **inneropts)
+
+        # This __dict__ logic is needed because the normal
+        # extension.wrapfunction doesn't seem to work.
+        repo.__dict__['commit'] = _wrappedcommit
+        return orig(ui, repo, *pats, **opts)
+    finally:
+        del repo.__dict__['commit']
--- a/hghooks/tests/test-prevent-vendored-changes.t
+++ b/hghooks/tests/test-prevent-vendored-changes.t
@@ -89,8 +89,111 @@ Multiple changesets handled properly
   
   to make changes to servo/, submit a Pull Request against the servo/servo
   GitHub project
   ************************************************************************
   transaction abort!
   rollback completed
   abort: pretxnchangegroup.prevent_vendored hook failed
   [255]
+
+Well formed servo fixup commit
+
+  $ cat >> .hg/hgrc << EOF
+  > [extensions]
+  > mq=
+  > commitextras=$TESTDIR/hghooks/tests/commitextras.py
+  > EOF
+
+  $ hg strip 'roots(outgoing())' >/dev/null
+  $ touch servo/file1
+  $ hg -q commit -A -m 'servo: Merge #1234\n\nadd servo/file1' \
+  >   --extra subtree_revision=f7ddd6f8b8c3129966d9b6a832831ef49eefab11 \
+  >   --extra subtree_source=https://hg.mozilla.org/projects/converted-servo-linear
+  $ USER=someone@example.com hg push
+  pushing to $TESTTMP/server
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  (1 changesets contain changes to protected servo/ directory: 8e552ce6e2d1)
+  (allowing valid fixup commit to servo: 8e552ce6e2d1)
+
+Servo fixup commit with bad description
+
+  $ touch servo/file2
+  $ hg -q commit -A -m 'add servo/file2' \
+  >   --extra subtree_revision=f7ddd6f8b8c3129966d9b6a832831ef49eefab11 \
+  >   --extra subtree_source=https://hg.mozilla.org/projects/converted-servo-linear
+  $ USER=someone@example.com hg push
+  pushing to $TESTTMP/server
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  (1 changesets contain changes to protected servo/ directory: 4396af38cc9d)
+  ************************************************************************
+  invalid servo fixup commit: 4396af38cc9d
+  
+  commit description is malformed
+  ************************************************************************
+  transaction abort!
+  rollback completed
+  abort: pretxnchangegroup.prevent_vendored hook failed
+  [255]
+
+Servo fixup commit that touches non-servo files
+
+  $ hg strip 'roots(outgoing())' >/dev/null
+  $ touch file1
+  $ touch servo/file2
+  $ hg -q commit -A -m 'servo: Merge #1234\n\nadd servo/file2' \
+  >   --extra subtree_revision=f7ddd6f8b8c3129966d9b6a832831ef49eefab11 \
+  >   --extra subtree_source=https://hg.mozilla.org/projects/converted-servo-linear
+  $ USER=someone@example.com hg push
+  pushing to $TESTTMP/server
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 2 changes to 2 files
+  (1 changesets contain changes to protected servo/ directory: c1c0d69286cf)
+  ************************************************************************
+  invalid servo fixup commit: c1c0d69286cf
+  
+  commit modifies non-servo files
+  ************************************************************************
+  transaction abort!
+  rollback completed
+  abort: pretxnchangegroup.prevent_vendored hook failed
+  [255]
+
+Servo fixup commit missing extra data
+
+  $ hg strip 'roots(outgoing())' >/dev/null
+  $ touch servo/file2
+  $ hg -q commit -A -m 'servo: Merge #1234\n\nadd servo/file2'
+  $ USER=someone@example.com hg push
+  pushing to $TESTTMP/server
+  searching for changes
+  adding changesets
+  adding manifests
+  adding file changes
+  added 1 changesets with 1 changes to 1 files
+  (1 changesets contain changes to protected servo/ directory: 104a9759c1bc)
+  ************************************************************************
+  you do not have permissions to modify files under servo/
+  
+  the servo/ directory is kept in sync with the canonical upstream
+  repository at https://github.com/servo/servo
+  
+  changes to servo/ are only allowed by the syncing tool and by sheriffs
+  performing cross-repository "merges"
+  
+  to make changes to servo/, submit a Pull Request against the servo/servo
+  GitHub project
+  ************************************************************************
+  transaction abort!
+  rollback completed
+  abort: pretxnchangegroup.prevent_vendored hook failed
+  [255]