[U-Boot] patman: add support for omitting bouncing addresses

[Philipp Tomsich <philipp.tomsich@theobroma-systems.com>]

On Mon, 28 Aug 2017, Chris Packham wrote:

> Add a --bounce-file option which can be used to omit addresses that are
> known to bounce. The option specifies a file which lists addresses (one
> per line) that are stripped from the Cc list.

Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>

Absolutely love this (in fact, I had something like this on my to-do list 
as the number of bounces seemed to grow over time).

A few comments below.

>
> Signed-off-by: Chris Packham <judge.packham@gmail.com>
> ---
> Here's a quick proof of concept. Right now the .bounces file has to
> match exactly the addresses that are extracted by patman. This makes it
> easy to do set(a) - set(b) to remove all the bounces in one hit.
>
> tools/patman/patman.py | 6 +++++-
> tools/patman/series.py | 5 ++++-
> tools/patman/tools.py  | 8 ++++++++
> 3 files changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/tools/patman/patman.py b/tools/patman/patman.py
> index 4b3bc787453e..567226a4d3ce 100755
> --- a/tools/patman/patman.py
> +++ b/tools/patman/patman.py
> @@ -56,6 +56,9 @@ parser.add_option('-v', '--verbose', action='store_true', dest='verbose',
>        default=False, help='Verbose output of errors and warnings')
> parser.add_option('--cc-cmd', dest='cc_cmd', type='string', action='store',
>        default=None, help='Output cc list for patch file (used by git)')
> +parser.add_option('--bounce-file', dest='bounce_fname', type='string',
> +                  default='.bounces',

The '.bounces' default doesn't really fit with how such settings are 
handled throughout patman.

My gut feeling is that
  (a) we want to have a project-wide bounces config (doc/git-bounces ???)
      that complements doc/git-mailrc
  (b) that any overrides/user-local additional settings should be picked up
      via ~/.patman (either by referencing a local overrides file or by
      having some additional overrides directly there)
  (c) this entire mechanism needs to be tied into Setup(...) in settings.py

> +                  help='File containing addresses to skip [default: %default]')
> parser.add_option('--no-check', action='store_false', dest='check_patch',
>                   default=True,
>                   help="Don't check for patch compliance")
> @@ -158,7 +161,8 @@ else:
>
>     cc_file = series.MakeCcFile(options.process_tags, cover_fname,
>                                 not options.ignore_bad_tags,
> -                                options.add_maintainers)
> +                                options.add_maintainers,
> +                                options.bounce_fname)
>
>     # Email the patches out (giving the user time to check / cancel)
>     cmd = ''
> diff --git a/tools/patman/series.py b/tools/patman/series.py
> index d3947a7c2ac5..b6533ab5ee58 100644
> --- a/tools/patman/series.py
> +++ b/tools/patman/series.py
> @@ -11,6 +11,7 @@ import os
> import get_maintainer
> import gitutil
> import terminal
> +from tools import FindBounces
>
> # Series-xxx tags that we understand
> valid_series = ['to', 'cc', 'version', 'changes', 'prefix', 'notes', 'name',
> @@ -202,7 +203,7 @@ class Series(dict):
>             print(col.Color(col.RED, str))
>
>     def MakeCcFile(self, process_tags, cover_fname, raise_on_error,
> -                   add_maintainers):
> +                   add_maintainers, bounce_fname):
>         """Make a cc file for us to use for per-commit Cc automation
>
>         Also stores in self._generated_cc to make ShowActions() faster.
> @@ -222,6 +223,7 @@ class Series(dict):
>         fname = '/tmp/patman.%d' % os.getpid()
>         fd = open(fname, 'w')
>         all_ccs = []
> +        bounces = FindBounces(bounce_fname)
>         for commit in self.commits:
>             cc = []
>             if process_tags:
> @@ -234,6 +236,7 @@ class Series(dict):
>             elif add_maintainers:
>                 cc += get_maintainer.GetMaintainer(commit.patch)
>             cc = [m.encode('utf-8') if type(m) != str else m for m in cc]
> +            cc = set(cc) - set(bounces)
>             all_ccs += cc
>             print(commit.patch, ', '.join(set(cc)), file=fd)
>             self._generated_cc[commit.patch] = cc
> diff --git a/tools/patman/tools.py b/tools/patman/tools.py
> index ba2485303037..9d206a390b02 100644
> --- a/tools/patman/tools.py
> +++ b/tools/patman/tools.py
> @@ -118,3 +118,11 @@ def Align(pos, align):
>
> def NotPowerOfTwo(num):
>     return num and (num & (num - 1))
> +
> +def FindBounces(bounce_fname):
> +    """generate a list of bouncing addresses from a file"""
> +    try:
> +        with open(bounce_fname) as fd:
> +            return [line.strip() for line in fd]
> +    except IOError:
> +        return []
>

Shouldn't this be merged into gitutil.BuildEmailList or into 
gitutil.LookupEmail? It would also be nice if the bounces would be 
handled in analogy to aliases and referenced via settings.alias

Regards,
Philipp.