Patchwork [3,of,3,v2] patchbomb: add support for a file that configures default To and CC

login
register
mail settings
Submitter Augie Fackler
Date June 15, 2017, 3:16 a.m.
Message ID <5b5570529e237d1fd70e.1497496594@imladris.local>
Download mbox | patch
Permalink /patch/21384/
State Changes Requested
Headers show

Comments

Augie Fackler - June 15, 2017, 3:16 a.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1497387366 14400
#      Tue Jun 13 16:56:06 2017 -0400
# Node ID 5b5570529e237d1fd70ef3e6ba80739bd51a9d52
# Parent  85fa6c9f000f374bebd40c401e4d0945d2194e1f
patchbomb: add support for a file that configures default To and CC

This lets repositories configure a shorthand (I used devel in the
tests, but you could do anything) for their project list, and also
supports fileset-based "subscriptions" so that people can request
being CC'ed on certain changes by default.
Yuya Nishihara - June 15, 2017, 2:35 p.m.
On Wed, 14 Jun 2017 23:16:34 -0400, Augie Fackler wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1497387366 14400
> #      Tue Jun 13 16:56:06 2017 -0400
> # Node ID 5b5570529e237d1fd70ef3e6ba80739bd51a9d52
> # Parent  85fa6c9f000f374bebd40c401e4d0945d2194e1f
> patchbomb: add support for a file that configures default To and CC

The config syntax looks good to me, thanks. A few bugs found.

> -    to = getaddrs('To', ask=True)
> +    todefault = set()
> +    ccdefault = set(opts['cc'])
> +    tocctrustdefault = True
> +    if opts['to'] and repo.wvfs.exists('.hgemaildefaults'):
> +        mailconf = config.config()
> +        confpath = repo.wvfs.join('.hgemaildefaults')
> +        with repo.wvfs('.hgemaildefaults') as fp:
> +            mailconf.parse(confpath, fp.read())
> +        for toflag in opts['to']:
> +            # @ is unsupported in destination configs to avoid
> +            # collisions with routable email addresses.
> +            if '@' in toflag or toflag not in mailconf:
> +                todefault.add(toflag)
> +                continue
> +            todefault.add(mailconf.get(toflag, 'to', None))
> +            ccdefault.add(mailconf.get(toflag, 'cc', None))

Perhaps these should be parsed as lists to deduplicate to/cc addresses.

> +            # Find all the config entries in our current section,
> +            # minus reserved entries...
> +            subents = ((k, v) for k, v in mailconf.items(toflag)
> +                       if k not in ('cc', 'to', 'bcc'))
> +            # Group them by the part before the :
> +            grouped = itertools.groupby(
> +                subents, key=lambda s: s[0].split(':')[0])
> +            # Now we coalesce it all down to a dict of dicts
> +            subs = {subsec:
> +                    {key[len(subsec) + 1:]: value for key, value in values}
> +                    for subsec, values in grouped}
> +            for r in revs:
> +                ctx = repo[r]
> +                mod = set(ctx.getfileset('modified() or added() or removed()'))
> +                for name, sub in subs.iteritems():
> +                    match = False
> +                    if 'files' in sub:
> +                        match = bool(
> +                            mod.intersection(ctx.match(sub['files'])))

Since matcher requires a list of patterns, this would be parsed as
['s', 'e', 't', ':', ...]. Also, list(matcher) does not return matched files,
but a list of explicitly-specified patterns. I don't know the best practice
to match against changed files, but one way is

  [f for f in mod if m(f)]

And matcher is relative to cwd by default. We'll probably want to specify
matchmod.match(root, cwd='', ...) to force path resolution to relative to
repo.root.

> +        todefault = {t for t in todefault if t is not None}
> +        ccdefault = {t for t in ccdefault if t is not None}

Nit: could be todefault.discard(None)

> diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
> --- a/tests/test-patchbomb.t
> +++ b/tests/test-patchbomb.t
> @@ -85,9 +85,9 @@ email.to config setting. Same for --cc:
>    X-Mercurial-Node: 8580ff50825a50c8f716709acdf8de0deddcd6ab
>    X-Mercurial-Series-Index: 1
>    X-Mercurial-Series-Total: 1
> -  Message-Id: <8580ff50825a50c8f716.60@augie-macbookpro2.roam.corp.google.com>
> -  X-Mercurial-Series-Id: <8580ff50825a50c8f716.60@augie-macbookpro2.roam.corp.google.com>
> -  User-Agent: Mercurial-patchbomb/4.2.1+627-72f2cafb81e3
> +  Message-Id: <8580ff50825a50c8f716.*@*> (glob)
> +  X-Mercurial-Series-Id: <8580ff50825a50c8f716.*@*> (glob)
> +  User-Agent: Mercurial-patchbomb/* (glob)

This should belong to the previous patch.

Patch

diff --git a/hgext/patchbomb.py b/hgext/patchbomb.py
--- a/hgext/patchbomb.py
+++ b/hgext/patchbomb.py
@@ -75,6 +75,7 @@  from __future__ import absolute_import
 
 import email as emailmod
 import errno
+import itertools
 import os
 import socket
 import tempfile
@@ -83,6 +84,7 @@  from mercurial.i18n import _
 from mercurial import (
     cmdutil,
     commands,
+    config,
     error,
     formatter,
     hg,
@@ -641,7 +643,7 @@  def email(ui, repo, *revs, **opts):
 
     showaddrs = []
 
-    def getaddrs(header, ask=False, default=None):
+    def getaddrs(header, ask=False, trustdefault=True, default=None):
         configkey = header.lower()
         opt = header.replace('-', '_').lower()
         addrs = opts.get(opt)
@@ -655,7 +657,7 @@  def email(ui, repo, *revs, **opts):
         if not addr:
             specified = (ui.hasconfig('email', configkey) or
                          ui.hasconfig('patchbomb', configkey))
-            if not specified and ask:
+            if (not trustdefault or not specified) and ask:
                 addr = prompt(ui, header, default=default)
         if addr:
             showaddrs.append('%s: %s' % (header, addr))
@@ -665,11 +667,63 @@  def email(ui, repo, *revs, **opts):
                 ui, [default], _charsets, opts.get('test'))
         return []
 
-    to = getaddrs('To', ask=True)
+    todefault = set()
+    ccdefault = set(opts['cc'])
+    tocctrustdefault = True
+    if opts['to'] and repo.wvfs.exists('.hgemaildefaults'):
+        mailconf = config.config()
+        confpath = repo.wvfs.join('.hgemaildefaults')
+        with repo.wvfs('.hgemaildefaults') as fp:
+            mailconf.parse(confpath, fp.read())
+        for toflag in opts['to']:
+            # @ is unsupported in destination configs to avoid
+            # collisions with routable email addresses.
+            if '@' in toflag or toflag not in mailconf:
+                todefault.add(toflag)
+                continue
+            todefault.add(mailconf.get(toflag, 'to', None))
+            ccdefault.add(mailconf.get(toflag, 'cc', None))
+            # Find all the config entries in our current section,
+            # minus reserved entries...
+            subents = ((k, v) for k, v in mailconf.items(toflag)
+                       if k not in ('cc', 'to', 'bcc'))
+            # Group them by the part before the :
+            grouped = itertools.groupby(
+                subents, key=lambda s: s[0].split(':')[0])
+            # Now we coalesce it all down to a dict of dicts
+            subs = {subsec:
+                    {key[len(subsec) + 1:]: value for key, value in values}
+                    for subsec, values in grouped}
+            for r in revs:
+                ctx = repo[r]
+                mod = set(ctx.getfileset('modified() or added() or removed()'))
+                for name, sub in subs.iteritems():
+                    match = False
+                    if 'files' in sub:
+                        match = bool(
+                            mod.intersection(ctx.match(sub['files'])))
+                    if match:
+                        wantcc = sub.get('cc', '')
+                        ui.debug('subscription %s matched, adding cc: %s\n'
+                            % (name, wantcc))
+                        ccdefault.update(config.parselist(wantcc))
+        todefault = {t for t in todefault if t is not None}
+        ccdefault = {t for t in ccdefault if t is not None}
+        # We've already handled the flags, so null the flag out. This
+        # also means the user will be prompted for an address, which
+        # is what we want, since the configuration can change as part
+        # of an `hg pull --update` or similar.
+        opts['to'] = []
+        opts['cc'] = []
+        tocctrustdefault = False
+
+    to = getaddrs('To', ask=True, default=', '.join(sorted(todefault)),
+                  trustdefault=tocctrustdefault)
     if not to:
         # we can get here in non-interactive mode
         raise error.Abort(_('no recipient addresses provided'))
-    cc = getaddrs('Cc', ask=True, default='')
+    cc = getaddrs('Cc', ask=True, default=', '.join(sorted(ccdefault)),
+                  trustdefault=tocctrustdefault)
     bcc = getaddrs('Bcc')
     replyto = getaddrs('Reply-To')
 
diff --git a/tests/test-patchbomb.t b/tests/test-patchbomb.t
--- a/tests/test-patchbomb.t
+++ b/tests/test-patchbomb.t
@@ -85,9 +85,9 @@  email.to config setting. Same for --cc:
   X-Mercurial-Node: 8580ff50825a50c8f716709acdf8de0deddcd6ab
   X-Mercurial-Series-Index: 1
   X-Mercurial-Series-Total: 1
-  Message-Id: <8580ff50825a50c8f716.60@augie-macbookpro2.roam.corp.google.com>
-  X-Mercurial-Series-Id: <8580ff50825a50c8f716.60@augie-macbookpro2.roam.corp.google.com>
-  User-Agent: Mercurial-patchbomb/4.2.1+627-72f2cafb81e3
+  Message-Id: <8580ff50825a50c8f716.*@*> (glob)
+  X-Mercurial-Series-Id: <8580ff50825a50c8f716.*@*> (glob)
+  User-Agent: Mercurial-patchbomb/* (glob)
   Date: Thu, 01 Jan 1970 00:01:00 +0000
   From: quux
   To: foo
@@ -3072,15 +3072,22 @@  Test hg email defaults settings:
   $ cat >> .hgemaildefaults <<EOF
   > [devel]
   > to = mercurial-devel@example.com
-  > [devel.setsubscriptions]
-  > c or d = c-or-d@example.com
-  > a or b = a-or-b@example.com
+  > ab:files = set: a or b
+  > ab:cc = a-or-b@example.com
+  > cd:files = set: c or d
+  > cd:cc = c-or-d@example.com
+  > [foo]
+  > hatebin:files = set: binary()
+  > hatebin:cc = hates-binary-files@example.com
   > EOF
-This change modifies file d
-  $ hg email --date '1970-1-1 0:2' --to=devel --test -r 10 --config patchbomb.publicurl=
+This change modifies file d. Note that the "foo" to disappears from
+the To line because it has a setsubscription defined above.
+  $ hg email --date '1970-1-1 0:2' --to=devel --to=foo --test -r 10 --config patchbomb.publicurl=
   From [test]: test
   this patch series consists of 1 patches.
   
+  To [mercurial-devel@example.com]: mercurial-devel@example.com
+  Cc [c-or-d@example.com]: c-or-d@example.com
   
   displaying [PATCH] dd ...
   Content-Type: text/plain; charset="us-ascii"
@@ -3095,7 +3102,8 @@  This change modifies file d
   User-Agent: Mercurial-patchbomb/* (glob)
   Date: Thu, 01 Jan 1970 00:02:00 +0000
   From: test
-  To: devel
+  To: mercurial-devel@example.com
+  Cc: c-or-d@example.com
   
   # HG changeset patch
   # User test
@@ -3118,6 +3126,8 @@  This change modifies file b
   From [test]: test
   this patch series consists of 1 patches.
   
+  To [mercurial-devel@example.com]: mercurial-devel@example.com
+  Cc [a-or-b@example.com]: a-or-b@example.com
   
   displaying [PATCH] b ...
   Content-Type: text/plain; charset="us-ascii"
@@ -3132,7 +3142,8 @@  This change modifies file b
   User-Agent: Mercurial-patchbomb/* (glob)
   Date: Thu, 01 Jan 1970 00:02:00 +0000
   From: test
-  To: devel
+  To: mercurial-devel@example.com
+  Cc: a-or-b@example.com
   
   # HG changeset patch
   # User test
@@ -3154,6 +3165,8 @@  This change modifies file b
   this patch series consists of 2 patches.
   
   (optional) Subject: [PATCH 0 of 2] 
+  To [mercurial-devel@example.com, specific-person@example.com]: mercurial-devel@example.com, specific-person@example.com
+  Cc [a-or-b@example.com, c-or-d@example.com, this-person-also@example.com]: a-or-b@example.com, c-or-d@example.com, this-person-also@example.com
   
   displaying [PATCH 1 of 2] b ...
   Content-Type: text/plain; charset="us-ascii"
@@ -3168,8 +3181,8 @@  This change modifies file b
   User-Agent: Mercurial-patchbomb/* (glob)
   Date: Thu, 01 Jan 1970 00:02:00 +0000
   From: test
-  To: devel, specific-person@example.com
-  Cc: this-person-also@example.com
+  To: mercurial-devel@example.com, specific-person@example.com
+  Cc: a-or-b@example.com, c-or-d@example.com, this-person-also@example.com
   
   # HG changeset patch
   # User test
@@ -3200,8 +3213,8 @@  This change modifies file b
   User-Agent: Mercurial-patchbomb/* (glob)
   Date: Thu, 01 Jan 1970 00:02:01 +0000
   From: test
-  To: devel, specific-person@example.com
-  Cc: this-person-also@example.com
+  To: mercurial-devel@example.com, specific-person@example.com
+  Cc: a-or-b@example.com, c-or-d@example.com, this-person-also@example.com
   
   # HG changeset patch
   # User test