Revise maintainers module

The data ordered by package name is the one we use the most often, so
make that the primary form.

Also use mtime_cache for the result of parsing cygwin-pkg-maint.

v2:
Fix mkgitolite and reports

Keep maintainers in a list, not a set, because we need "first named
maintainer".

Don't write permissions lines with an empty list of names, as that's
syntactically incorrect.

8 files changed, 107 insertions, 90 deletions

diff --git a/calm/calm.py b/calm/calm.py
index 54d0155..5d1753b 100755
--- a/calm/calm.py
+++ b/calm/calm.py
@@ -150,10 +150,10 @@ def process_relarea(args, state):
 
 def process_uploads(args, state):
     # read maintainer list
-    mlist = maintainers.read(args)
+    mlist = maintainers.maintainer_list(args)
 
     # make the list of all packages
-    all_packages = maintainers.all_packages(mlist)
+    all_packages = maintainers.all_packages(args.pkglist)
 
     # for each maintainer
     for name in sorted(mlist.keys()):
@@ -662,13 +662,13 @@ def mail_cb(state, loghandler):
 
     # send each maintainer mail containing log entries caused by their actions,
     # or pertaining to their packages
-    #
-    # XXX: prev_maint=False here is a kind of wrong: it prevents the previous
-    # maintainer of an orphaned package from getting mails about it being
-    # altered by a trusted maintainer, but also stops them getting mails if the
-    # do something themselves...
-    mlist = maintainers.read(state.args, prev_maint=False)
+    mlist = maintainers.maintainer_list(state.args)
     for m in mlist.values():
+        # may happen for previous maintainers who orphaned all their packages
+        # before an email became mandatory
+        if not m.email:
+            continue
+
         email = m.email
         if m.name == 'ORPHANED':
             email = common_constants.EMAILS.split(',')
diff --git a/calm/maintainers.py b/calm/maintainers.py
index 7c1fc7d..cc75040 100644
--- a/calm/maintainers.py
+++ b/calm/maintainers.py
@@ -35,19 +35,33 @@
 # - the timestamp when 'ignoring' warnings were last emitted
 #
 
-# XXX: Rather than this implementing an object which reads cygwin-pkg-maint when
-# constructed at specific places in the code, perhaps this needs to contain the
-# list (and it's inversion) and accessors, and invalidate that stored list when
-# cygwin-pkg-maint changes...
-
-import itertools
 import logging
 import os
 import re
-from collections import defaultdict
+from collections import UserString
 
 from . import utils
 
+
+#
+# MaintainerPackage object behaves like a string of the package name, but also
+# supports is_orphaned() and maintainers() methods
+#
+class MaintainerPackage(UserString):
+    def __init__(self, name, maintainers, orphaned):
+        super().__init__(name)
+        self._maintainers = maintainers
+        self._orphaned = orphaned
+
+    # XXX: for historical reasons, 'ORPHANED' still appears in the maintainer
+    # list, when this is True.  Probably should fix that...
+    def is_orphaned(self):
+        return self._orphaned
+
+    def maintainers(self):
+        return self._maintainers
+
+
 #
 #
 #
@@ -128,17 +142,19 @@ def add_directories(mlist, homedirs):
                             m.quiet = True
                         elif l:
                             m.email.append(l)
-        if not m.email:
-            logging.error("no email address known for maintainer '%s'" % (m.name))
 
     return mlist
 
 
 # add maintainers from the package maintainers list, with the packages they
 # maintain
-def add_packages(mlist, pkglist, prev_maint=True):
+@utils.mtime_cache
+def _read_pkglist(pkglist):
+    mpkgs = {}
+
     with open(pkglist) as f:
         for (i, l) in enumerate(f):
+            orphaned = False
             l = l.rstrip()
 
             # match lines of the form '<package> <maintainer(s)|status>'
@@ -159,12 +175,12 @@ def add_packages(mlist, pkglist, prev_maint=True):
                     # orphaned packages are assigned to 'ORPHANED'
                     elif status == 'ORPHANED':
                         m = status
+                        orphaned = True
 
-                        if prev_maint:
-                            # also add any previous maintainer(s) listed
-                            prevm = re.match(r'^ORPHANED\s\((.*)\)', rest)
-                            if prevm:
-                                m = m + '/' + prevm.group(1)
+                        # also add any previous maintainer(s) listed
+                        prevm = re.match(r'^ORPHANED\s\((.*)\)', rest)
+                        if prevm:
+                            m = m + '/' + prevm.group(1)
                     else:
                         logging.error("unknown package status '%s' in line %s:%d: '%s'" % (status, pkglist, i, l))
                         continue
@@ -172,6 +188,7 @@ def add_packages(mlist, pkglist, prev_maint=True):
                     m = rest
 
                 # joint maintainers are separated by '/'
+                maintainers = list()
                 for name in m.split('/'):
                     name = name.strip()
 
@@ -185,35 +202,46 @@ def add_packages(mlist, pkglist, prev_maint=True):
                         logging.error("non-ascii maintainer name '%s' in line %s:%d, skipped" % (rest, pkglist, i))
                         continue
 
-                    m = Maintainer._find(mlist, name)
-                    m.pkgs.append(pkg)
+                    maintainers.append(name)
+
+                mpkgs[pkg] = MaintainerPackage(pkg, maintainers, orphaned)
 
             else:
                 logging.error("unrecognized line in %s:%d: '%s'" % (pkglist, i, l))
 
-    return mlist
+    return mpkgs
+
+
+#
+def pkg_list(pkglist):
+    return _read_pkglist(pkglist)
 
 
 # create maintainer list
-def read(args, prev_maint=True):
+def maintainer_list(args):
     mlist = {}
-    mlist = add_directories(mlist, args.homedir)
-    mlist = add_packages(mlist, args.pkglist, prev_maint)
 
-    return mlist
+    # add all maintainers for all packages
+    for p in pkg_list(args.pkglist).values():
+        for m in p.maintainers():
+            Maintainer._find(mlist, m).pkgs.append(p)
 
+    # read information from homedirs
+    mlist = add_directories(mlist, args.homedir)
 
-# invert to a per-package list of maintainers
-def invert(mlist):
-    _pkgs = defaultdict(list)
-    # for each maintainer
+    # check all maintainers have an email
     for m in mlist.values():
-        # for each package
-        for p in m.pkgs:
-            # add the maintainer name
-            _pkgs[p].append(m.name)
+        if m.name == 'ORPHANED':
+            continue
 
-    return _pkgs
+        # not required if only a previous maintainer for some orphaned packages
+        if all(p.is_orphaned() for p in m.pkgs):
+            continue
+
+        if not m.email:
+            logging.error("no email address known for maintainer '%s'" % (m.name))
+
+    return mlist
 
 
 def update_reminder_times(mlist):
@@ -222,5 +250,5 @@ def update_reminder_times(mlist):
 
 
 # a list of all packages
-def all_packages(mlist):
-    return list(itertools.chain.from_iterable(mlist[m].pkgs for m in mlist))
+def all_packages(pkglist):
+    return pkg_list(pkglist).keys()
diff --git a/calm/mkgitoliteconf.py b/calm/mkgitoliteconf.py
index c67277e..a7d7cf6 100755
--- a/calm/mkgitoliteconf.py
+++ b/calm/mkgitoliteconf.py
@@ -27,7 +27,6 @@
 
 import argparse
 import sys
-from collections import defaultdict
 
 from . import common_constants
 from . import maintainers
@@ -49,21 +48,8 @@ def transform_username(name):
 #
 
 def do_main(args):
-    # read maintainer list
-    mlist = {}
-    mlist = maintainers.add_packages(mlist, args.pkglist)
-
-    # make the list of all packages
-    maintainers.all_packages(mlist)
-
-    # invert to a per-package list of maintainers
-    pkgs = defaultdict(list)
-    # for each maintainer
-    for m in mlist.values():
-        # for each package
-        for p in m.pkgs:
-            # add the maintainer name
-            pkgs[p].append(m.name)
+    # Get per-package list of maintainers
+    pkgs = maintainers.pkg_list(args.pkglist)
 
     # header
     print("# automatically generated by mkgitoliteconf")
@@ -89,14 +75,24 @@ def do_main(args):
 
     # for each package
     for p in sorted(pkgs):
-        users = ' '.join(map(transform_username, pkgs[p]))
-        owner = pkgs[p][0]  # first named maintainer
+        pkg = pkgs[p]
+
+        users = ' '.join(map(transform_username, [m for m in pkg.maintainers() if m != 'ORPHANED']))
+
+        owner = ''
+        if pkg.maintainers():
+            owner = pkg.maintainers()[0]  # first named maintainer
+
+        if pkg.is_orphaned():
+            owner = 'ORPHANED'
 
         print("repo git/cygwin-packages/%s" % (p))
         print("C  = %s @leads" % (users))
-        print("RW master$ = %s" % (users))
-        print("RW refs/tags/ = %s" % (users))
-        print("owner = %s" % (owner))
+        if users:
+            print("RW master$ = %s" % (users))
+            print("RW refs/tags/ = %s" % (users))
+        if owner:
+            print("owner = %s" % (owner))
         print("")
 
 
diff --git a/calm/package.py b/calm/package.py
index 81d07fe..21639cb 100755
--- a/calm/package.py
+++ b/calm/package.py
@@ -1086,13 +1086,11 @@ def validate_package_maintainers(args, packages):
     if not args.pkglist:
         return error
 
-    # read maintainer list
-    mlist = {}
-    mlist = maintainers.add_packages(mlist, args.pkglist)
-    pkg_maintainers = maintainers.invert(mlist)
+    # read package maintainer list
+    pkg_maintainers = maintainers.pkg_list(args.pkglist)
 
     # make the list of all packages
-    all_packages = maintainers.all_packages(mlist)
+    all_packages = pkg_maintainers.keys()
 
     # validate that all packages are in the package list
     for p in sorted(packages):
@@ -1120,7 +1118,7 @@ def validate_package_maintainers(args, packages):
                         logging.error("package '%s' is not obsolete, but has no maintainer" % (p))
                         error = True
 
-                if 'ORPHANED' in pkg_maintainers[es_pn]:
+                if (es_pn in pkg_maintainers) and (pkg_maintainers[es_pn].is_orphaned()):
                     # note orphaned packages
                     packages[p].orphaned = True
 
@@ -1353,8 +1351,7 @@ def write_repo_json(args, packages, f):
     for arch in packages:
         package_list.update(packages[arch])
 
-    mlist = maintainers.read(args, None)
-    pkg_maintainers = maintainers.invert(mlist)
+    pkg_maintainers = maintainers.pkg_list(args.pkglist)
 
     pl = []
     for pn in sorted(package_list):
@@ -1397,8 +1394,8 @@ def write_repo_json(args, packages, f):
         if 'license' in po.version_hints[bv]:
             d['license'] = po.version_hints[bv]['license']
 
-        if pkg_maintainers[po.orig_name] and ('ORPHANED' not in pkg_maintainers[po.orig_name]):
-            d['maintainers'] = sorted(pkg_maintainers[po.orig_name])
+        if (po.orig_name in pkg_maintainers) and (not pkg_maintainers[po.orig_name].is_orphaned()):
+            d['maintainers'] = sorted(pkg_maintainers[po.orig_name].maintainers())
 
         pl.append(d)
 
diff --git a/calm/pkg2html.py b/calm/pkg2html.py
index 2a0d25d..0e7778b 100755
--- a/calm/pkg2html.py
+++ b/calm/pkg2html.py
@@ -142,8 +142,7 @@ def update_package_listings(args, packages):
     summaries = os.path.join(args.htdocs, 'summary')
     ensure_dir_exists(args, summaries)
 
-    mlist = maintainers.read(args, None)
-    pkg_maintainers = maintainers.invert(mlist)
+    pkg_maintainers = maintainers.pkg_list(args.pkglist)
 
     toremove = glob.glob(os.path.join(summaries, '*'))
 
@@ -262,11 +261,14 @@ def update_package_listings(args, packages):
                     es_po = arch_package(packages, es)
                     if not es_po:
                         es_po = po
+
                     m_pn = es_po.orig_name
-                    if 'ORPHANED' in pkg_maintainers[m_pn]:
+                    if m_pn not in pkg_maintainers:
+                        m = None
+                    elif pkg_maintainers[m_pn].is_orphaned():
                         m = 'ORPHANED'
                     else:
-                        m = ', '.join(sorted(pkg_maintainers[m_pn]))
+                        m = ', '.join(sorted(pkg_maintainers[m_pn].maintainers()))
 
                     if m:
                         print('<span class="detail">maintainer(s)</span>: %s ' % m, file=f)
diff --git a/calm/reports.py b/calm/reports.py
index 3932d78..43b9030 100644
--- a/calm/reports.py
+++ b/calm/reports.py
@@ -65,8 +65,7 @@ def linkify(pn, po):
 # produce a report of unmaintained packages
 #
 def unmaintained(args, packages, reportsdir):
-    mlist = maintainers.read(args, None)
-    pkg_maintainers = maintainers.invert(mlist)
+    pkg_maintainers = maintainers.pkg_list(args.pkglist)
 
     um_list = []
 
@@ -78,7 +77,7 @@ def unmaintained(args, packages, reportsdir):
         if po.kind != package.Kind.source:
             continue
 
-        if 'ORPHANED' not in pkg_maintainers[po.orig_name]:
+        if (po.orig_name not in pkg_maintainers) or (not pkg_maintainers[po.orig_name].is_orphaned()):
             continue
 
         # the highest version we have
diff --git a/calm/tool_util.py b/calm/tool_util.py
index 712b0e2..f677f86 100644
--- a/calm/tool_util.py
+++ b/calm/tool_util.py
@@ -43,26 +43,19 @@ def split(pvr):
 
 
 def permitted(p):
-    # check CYGNAME is a maintainer for package
     cygname = os.environ.get('CYGNAME', None)
     if not cygname:
         logging.error("who are you?")
         return False
 
-    mlist = {}
-    mlist = maintainers.add_packages(mlist, common_constants.PKGMAINT)
-
     # CYGNAME is a maintainer for package
-    if p in mlist[cygname].pkgs:
+    pkg_list = maintainers.pkg_list(common_constants.PKGMAINT)
+    if cygname in pkg_list[p].maintainers():
         return True
 
     # CYGNAME is a trusted maintainer
     if cygname in common_constants.TRUSTEDMAINT.split('/'):
         return True
 
-    if cygname not in mlist:
-        logging.error("'%s' is not a package maintainer" % (cygname))
-        return False
-
     logging.error("package '%s' is not in the package list for maintainer '%s'" % (p, cygname))
     return False
diff --git a/test/test_calm.py b/test/test_calm.py
index bd8390d..bc39fdc 100755
--- a/test/test_calm.py
+++ b/test/test_calm.py
@@ -317,9 +317,11 @@ class CalmTest(unittest.TestCase):
     def test_maint_pkglist(self):
         self.maxDiff = None
 
+        args = types.SimpleNamespace()
+        args.homedir = 'testdata/homes'
+        args.pkglist = 'testdata/pkglist/cygwin-pkg-maint'
         mlist = {}
-        mlist = maintainers.add_directories(mlist, 'testdata/homes')
-        mlist = maintainers.add_packages(mlist, 'testdata/pkglist/cygwin-pkg-maint')
+        mlist = maintainers.maintainer_list(args)
 
         compare_with_expected_file(self, 'testdata/pkglist', mlist)