Merge remote-tracking branch 'ansgar/pu/multiarchive-1' into merge

* ansgar/pu/multiarchive-1:
  rewrite code for sending mails about processed uploads
  dak/process_upload.py: improve error message for ArchiveExceptions and Rejects
Signed-off-by: NJoerg Jaspert <joerg@debian.org>

dak/process_policy.py

@@ -45,7 +45,8 @@ from daklib.dak_exceptions import CantOpenError, AlreadyLockedError, CantGetLock
 from daklib.config import Config
 from daklib.archive import ArchiveTransaction
 from daklib.urgencylog import UrgencyLog
-from daklib.textutils import fix_maintainer
+
+import daklib.announce
 
 # Globals
 Options = None
@@ -155,25 +156,8 @@ def comment_accept(upload, srcqueue, comments, transaction):
     if not Options['No-Action']:
         Logger.log(["Policy Queue ACCEPT", srcqueue.queue_name, changesname])
 
-    # Send announcement
-    if upload.source is not None:
-        subst = subst_for_upload(upload)
-        announce = ", ".join(upload.target_suite.announce or [])
-        tracking = cnf.get('Dinstall::TrackingServer')
-        if tracking and upload.source is not None:
-            announce = '{0}\nBcc: {1}@{2}'.format(announce, upload.changes.source, tracking)
-        subst['__ANNOUNCE_LIST_ADDRESS__'] = announce
-        message = utils.TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.announce'))
-        utils.send_mail(message)
-
-    # TODO: code duplication. Similar code is in process-upload.
-    if cnf.find_b('Dinstall::CloseBugs') and upload.changes.closes is not None and upload.source is not None:
-        for bugnum in upload.changes.closes:
-            subst['__BUG_NUMBER__'] = bugnum
-            message = utils.TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.bug-close'))
-            utils.send_mail(message)
-
-            del subst['__BUG_NUMBER__']
+    pu = get_processed_upload(upload)
+    daklib.announce.announce_accept(upload)
 
     # TODO: code duplication. Similar code is in process-upload.
     # Move .changes to done
@@ -190,9 +174,9 @@ def comment_accept(upload, srcqueue, comments, transaction):
 
 @try_or_reject
 def comment_reject(*args):
-    real_comment_reject(*args)
+    real_comment_reject(*args, manual=True)
 
-def real_comment_reject(upload, srcqueue, comments, transaction, notify=True):
+def real_comment_reject(upload, srcqueue, comments, transaction, notify=True, manual=False):
     cnf = Config()
 
     fs = transaction.fs
@@ -231,19 +215,18 @@ def real_comment_reject(upload, srcqueue, comments, transaction, notify=True):
     ### Send mail notification
 
     if notify:
-        subst = subst_for_upload(upload)
-        subst['__MANUAL_REJECT_MESSAGE__'] = ''
-        subst['__REJECT_MESSAGE__'] = comments
+        rejected_by = None
+        reason = comments
 
         # Try to use From: from comment file if there is one.
         # This is not very elegant...
         match = re.match(r"\AFrom: ([^\n]+)\n\n", comments)
         if match:
-            subst['__REJECTOR_ADDRESS__'] = match.group(1)
-            subst['__REJECT_MESSAGE__'] = '\n'.join(comments.splitlines()[2:])
+            rejected_by = match.group(1)
+            reason = '\n'.join(comments.splitlines()[2:])
 
-        message = utils.TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'queue.rejected'))
-        utils.send_mail(message)
+        pu = get_processed_upload(upload)
+        daklib.announce.announce_reject(pu, reason, rejected_by)
 
     print "  REJECT"
     if not Options["No-Action"]:
@@ -274,46 +257,28 @@ def remove_upload(upload, transaction):
 
 ################################################################################
 
-def subst_for_upload(upload):
-    # TODO: similar code in process-upload
-    cnf = Config()
+def get_processed_upload(upload):
+    pu = daklib.announce.ProcessedUpload()
 
-    maintainer_field = upload.changes.changedby or upload.changes.maintainer
-    if upload.source is not None:
-        addresses = utils.mail_addresses_for_upload(upload.changes.maintainer, maintainer_field, upload.changes.fingerprint)
-    else:
-        addresses = utils.mail_addresses_for_upload(upload.changes.maintainer, upload.changes.maintainer, upload.changes.fingerprint)
+    pu.maintainer = upload.changes.maintainer
+    pu.changed_by = upload.changes.changedby
+    pu.fingerprint = upload.changes.fingerprint
+
+    pu.suites = []
+    pu.from_policy_suites = [ upload.target_suite ]
 
     changes_path = os.path.join(upload.policy_queue.path, upload.changes.changesname)
-    changes_contents = open(changes_path, 'r').read()
-
-    bcc = 'X-DAK: dak process-policy'
-    if 'Dinstall::Bcc' in cnf:
-        bcc = '{0}\nBcc: {1}'.format(bcc, cnf['Dinstall::Bcc'])
-
-    subst = {
-        '__DISTRO__': cnf['Dinstall::MyDistribution'],
-        '__ADMIN_ADDRESS__': cnf['Dinstall::MyAdminAddress'],
-
-        '__CHANGES_FILENAME__': upload.changes.changesname,
-        '__SOURCE__': upload.changes.source,
-        '__VERSION__': upload.changes.version,
-        '__ARCHITECTURE__': upload.changes.architecture,
-        '__MAINTAINER__': maintainer_field,
-        '__MAINTAINER_FROM__': fix_maintainer(maintainer_field)[1],
-        '__MAINTAINER_TO__': ", ".join(addresses),
-        '__CC__': 'X-DAK-Rejection: manual or automatic',
-        '__REJECTOR_ADDRESS__': cnf['Dinstall::MyEmailAddress'],
-        '__BCC__': bcc,
-        '__BUG_SERVER__': cnf.get('Dinstall::BugServer'),
-        '__FILE_CONTENTS__': changes_contents,
-    }
-
-    override_maintainer = cnf.get('Dinstall::OverrideMaintainer')
-    if override_maintainer:
-        subst['__MAINTAINER_TO__'] = override_maintainer
-
-    return subst
+    pu.changes = open(changes_path, 'r').read()
+    pu.changes_filename = upload.changes.changesname
+    pu.sourceful = upload.source is not None
+    pu.source = upload.changes.source
+    pu.version = upload.changes.version
+    pu.architecture = upload.changes.architecture
+    pu.bugs = upload.changes.closes
+
+    pu.program = "process-policy"
+
+    return pu
 
 ################################################################################
 

dak/process_upload.py

@@ -176,10 +176,11 @@ from daklib.urgencylog import UrgencyLog
 from daklib.summarystats import SummaryStats
 from daklib.config import Config
 import daklib.utils as utils
-from daklib.textutils import fix_maintainer
 from daklib.regexes import *
 
+import daklib.announce
 import daklib.archive
+import daklib.checks
 import daklib.upload
 
 ###############################################################################
@@ -206,78 +207,51 @@ def try_or_reject(function):
     """Try to call function or reject the upload if that fails
     """
     def wrapper(directory, upload, *args, **kwargs):
+        reason = 'No exception caught. This should not happen.'
+
         try:
             return function(directory, upload, *args, **kwargs)
+        except (daklib.archive.ArchiveException, daklib.checks.Reject) as e:
+            reason = e
         except Exception as e:
-            try:
-                reason = "There was an uncaught exception when processing your upload:\n{0}\nAny original reject reason follows below.".format(traceback.format_exc())
-                upload.rollback()
-                return real_reject(directory, upload, reason=reason)
-            except Exception as e:
-                reason = "In addition there was an exception when rejecting the package:\n{0}\nPrevious reasons:\n{1}".format(traceback.format_exc(), reason)
-                upload.rollback()
-                return real_reject(directory, upload, reason=reason, notify=False)
+            reason = "There was an uncaught exception when processing your upload:\n{0}\nAny original reject reason follows below.".format(traceback.format_exc())
 
-    return wrapper
+        try:
+            upload.rollback()
+            return real_reject(directory, upload, reason=reason)
+        except Exception as e:
+            reason = "In addition there was an exception when rejecting the package:\n{0}\nPrevious reasons:\n{1}".format(traceback.format_exc(), reason)
+            upload.rollback()
+            return real_reject(directory, upload, reason=reason, notify=False)
 
-def subst_for_upload(upload):
-    cnf = Config()
+        raise Exception('Rejecting upload failed after multiple tries. Giving up. Last reason:\n{0}'.format(reason))
 
+    return wrapper
+
+def get_processed_upload(upload):
     changes = upload.changes
     control = upload.changes.changes
 
-    if upload.final_suites is None or len(upload.final_suites) == 0:
-        suite_name = '(unknown)'
-    else:
-        suite_names = []
-        for suite in upload.final_suites:
-            if suite.policy_queue:
-                suite_names.append("{0}->{1}".format(suite.suite_name, suite.policy_queue.queue_name))
-            else:
-                suite_names.append(suite.suite_name)
-        suite_name = ','.join(suite_names)
-
-    maintainer_field = control.get('Maintainer', cnf['Dinstall::MyEmailAddress'])
-    changed_by_field = control.get('Changed-By', maintainer_field)
-    maintainer = fix_maintainer(changed_by_field)
-    if upload.changes.source is not None:
-        addresses = utils.mail_addresses_for_upload(maintainer_field, changed_by_field, changes.primary_fingerprint)
-    else:
-        addresses = utils.mail_addresses_for_upload(maintainer_field, maintainer_field, changes.primary_fingerprint)
-
-    # debian-{devel-,}-changes@lists.debian.org toggles writes access based on this header
-    bcc = 'X-DAK: dak process-upload'
-    if 'Dinstall::Bcc' in cnf:
-        bcc = '{0}\nBcc: {1}'.format(bcc, cnf['Dinstall::Bcc'])
+    pu = daklib.announce.ProcessedUpload()
 
-    subst = {
-        '__DISTRO__': cnf['Dinstall::MyDistribution'],
-        '__ADMIN_ADDRESS__': cnf['Dinstall::MyAdminAddress'],
+    pu.maintainer = control.get('Maintainer')
+    pu.changed_by = control.get('Changed-By')
+    pu.fingerprint = changes.primary_fingerprint
 
-        '__CHANGES_FILENAME__': upload.changes.filename,
+    pu.suites = upload.final_suites or []
+    pu.from_policy_suites = []
 
-        '__SOURCE__': control.get('Source', '(unknown)'),
-        '__ARCHITECTURE__': control.get('Architecture', '(unknown)'),
-        '__VERSION__': control.get('Version', '(unknown)'),
+    pu.changes = open(upload.changes.path, 'r').read()
+    pu.changes_filename = upload.changes.filename
+    pu.sourceful = upload.changes.source is not None
+    pu.source = control.get('Source')
+    pu.version = control.get('Version')
+    pu.architecture = control.get('Architecture')
+    pu.bugs = changes.closed_bugs
 
-        '__SUITE__': suite_name,
+    pu.program = "process-upload"
 
-        '__DAK_ADDRESS__': cnf['Dinstall::MyEmailAddress'],
-        '__MAINTAINER_FROM__': maintainer[1],
-        '__MAINTAINER_TO__': ", ".join(addresses),
-        '__MAINTAINER__': changed_by_field,
-        '__BCC__': bcc,
-
-        '__BUG_SERVER__': cnf.get('Dinstall::BugServer'),
-
-        '__FILE_CONTENTS__': open(upload.changes.path, 'r').read(),
-        }
-
-    override_maintainer = cnf.get('Dinstall::OverrideMaintainer')
-    if override_maintainer:
-        subst['__MAINTAINER_TO__'] = subst['__MAINTAINER_FROM__'] = override_maintainer
-
-    return subst
+    return pu
 
 @try_or_reject
 def accept(directory, upload):
@@ -300,41 +274,8 @@ def accept(directory, upload):
             urgency = cnf['Urgency::Default']
         UrgencyLog().log(control['Source'], control['Version'], urgency)
 
-    # send mail to maintainer
-    subst = subst_for_upload(upload)
-    message = utils.TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.accepted'))
-    utils.send_mail(message)
-
-    # send mail to announce lists and tracking server
-    if accepted_to_real_suite and sourceful_upload:
-        subst = subst_for_upload(upload)
-        announce = set()
-        for suite in upload.final_suites:
-            if suite.policy_queue is not None:
-                continue
-            announce.update(suite.announce or [])
-        announce_address = ", ".join(announce)
-
-        tracking = cnf.get('Dinstall::TrackingServer')
-        if tracking and 'source' in upload.changes.architectures:
-            announce_address = '{0}\nBcc: {1}@{2}'.format(announce_address, control['Source'], tracking)
-
-        subst['__ANNOUNCE_LIST_ADDRESS__'] = announce_address
-
-        message = utils.TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.announce'))
-        utils.send_mail(message)
-
-    # Only close bugs for uploads that were not redirected to a policy queue.
-    # process-policy will close bugs for those once they are accepted.
-    subst = subst_for_upload(upload)
-    if accepted_to_real_suite and cnf.find_b('Dinstall::CloseBugs') and sourceful_upload:
-        for bugnum in upload.changes.closed_bugs:
-            subst['__BUG_NUMBER__'] = str(bugnum)
-
-            message = utils.TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.bug-close'))
-            utils.send_mail(message)
-
-            del subst['__BUG_NUMBER__']
+    pu = get_processed_upload(upload)
+    daklib.announce.announce_accept(pu)
 
     # Move .changes to done, but only for uploads that were accepted to a
     # real suite.  process-policy will handle this for uploads to queues.
@@ -360,9 +301,8 @@ def accept_to_new(directory, upload):
     upload.install_to_new()
     # TODO: tag bugs pending
 
-    subst = subst_for_upload(upload)
-    message = utils.TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.new'))
-    utils.send_mail(message)
+    pu = get_processed_upload(upload)
+    daklib.announce.announce_new(pu)
 
     SummaryStats().accept_count += 1
     SummaryStats().accept_bytes += upload.changes.bytes
@@ -404,14 +344,8 @@ def real_reject(directory, upload, reason=None, notify=True):
     fh.close()
 
     if notify:
-        subst = subst_for_upload(upload)
-        subst['__REJECTOR_ADDRESS__'] = cnf['Dinstall::MyEmailAddress']
-        subst['__MANUAL_REJECT_MESSAGE__'] = ''
-        subst['__REJECT_MESSAGE__'] = reason
-        subst['__CC__'] = 'X-DAK-Rejection: automatic (moo)'
-
-        message = utils.TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'queue.rejected'))
-        utils.send_mail(message)
+        pu = get_processed_upload(upload)
+        daklib.announce.announce_reject(pu, reason)
 
     SummaryStats().reject_count += 1
 

daklib/announce.py

+"""module to send announcements for processed packages
+
+@contact: Debian FTP Master <ftpmaster@debian.org>
+@copyright: 2012, Ansgar Burchardt <ansgar@debian.org>
+@license: GPL-2+
+"""
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+import os
+
+from daklib.config import Config
+from daklib.textutils import fix_maintainer
+from daklib.utils import mail_addresses_for_upload, TemplateSubst, send_mail
+
+class ProcessedUpload(object):
+    # people
+    maintainer = None
+    changed_by = None
+    fingerprint = None
+
+    # suites
+    suites = []
+    from_policy_suites = []
+
+    # package
+    changes = None
+    changes_filename = None
+    sourceful = None
+    source = None
+    architecture = None
+    version = None
+    bugs = None
+
+    # program
+    program = "unknown-program"
+
+def _subst_for_upload(upload):
+    cnf = Config()
+
+    maintainer = upload.maintainer or cnf['Dinstall::MyEmailAddress']
+    changed_by = upload.changed_by or maintainer
+    if upload.sourceful:
+        maintainer_to = mail_addresses_for_upload(maintainer, changed_by, upload.fingerprint)
+    else:
+        maintainer_to = mail_addresses_for_upload(maintainer, maintainer, upload.fingerprint)
+
+    bcc = 'X-DAK: dak {0}'.format(upload.program)
+    if 'Dinstall::Bcc' in cnf:
+        bcc = '{0}\nBcc: {1}'.format(bcc, cnf['Dinstall::Bcc'])
+
+    subst = {
+        '__DISTRO__': cnf['Dinstall::MyDistribution'],
+        '__BUG_SERVER__': cnf.get('Dinstall::BugServer'),
+        '__ADMIN_ADDRESS__': cnf['Dinstall::MyAdminAddress'],
+        '__DAK_ADDRESS__': cnf['Dinstall::MyEmailAddress'],
+        '__REJECTOR_ADDRESS__': cnf['Dinstall::MyEmailAddress'],
+        '__MANUAL_REJECT_MESSAGE__': '',
+
+        '__BCC__': bcc,
+
+        '__MAINTAINER__': changed_by,
+        '__MAINTAINER_FROM__': fix_maintainer(changed_by)[1],
+        '__MAINTAINER_TO__': ', '.join(maintainer_to),
+        '__CHANGES_FILENAME__': upload.changes_filename,
+        '__FILE_CONTENTS__': upload.changes,
+        '__SOURCE__': upload.source,
+        '__VERSION__': upload.version,
+        '__ARCHITECTURE__': upload.architecture,
+        }
+
+    override_maintainer = cnf.get('Dinstall::OverrideMaintainer')
+    if override_maintainer:
+        subst['__MAINTAINER_FROM__'] = subst['__MAINTAINER_TO__'] = override_maintainer
+
+    return subst
+
+def announce_reject(upload, reason, rejected_by=None):
+    cnf = Config()
+    subst = _subst_for_upload(upload)
+
+    automatic = rejected_by is None
+
+    subst['__CC__'] = 'X-DAK-Rejection: {0}'.format('automatic' if automatic else 'manual')
+    subst['__REJECT_MESSAGE__'] = reason
+
+    if rejected_by:
+        subst['__REJECTOR_ADDRESS__'] = rejected_by
+
+    if not automatic:
+        subst['__BCC__'] = '{0}\nBcc: {1}'.format(subst['__BCC__'], cnf['Dinstall::MyEmailAddress'])
+
+    message = TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'queue.rejected'))
+    send_mail(message)
+
+def announce_accept(upload):
+    cnf = Config()
+    subst = _subst_for_upload(upload)
+
+    accepted_to_real_suite = any(suite.policy_queue in None for suite in upload.suites)
+
+    suite_names = []
+    for suite in upload.suites:
+        if suite.policy_queue:
+            suite_names.append("{0}->{1}".format(suite.suite_name, suite.policy_queue.queue_name))
+        else:
+            suite_names.append(suite.suite_name)
+    suite_names.extend(suite.suite_name for suite in upload.from_policy_suites)
+    subst['__SUITE__'] = ', '.join(suite_names) or '(none)'
+
+    message = TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.accepted'))
+    send_mail(message)
+
+    if accepted_to_real_suite and upload.sourceful:
+        # senf mail to announce lists and tracking server
+        announce = set(suite.announce or [] for suite in upload.suites if suite.policy_queue is None)
+        announce_list_address = ", ".join(announce)
+
+        tracking = cnf.get('Dinstall::TrackingServer')
+        if tracking:
+            announce_list_address = "{0}\n{1}@{2}".format(announce_list_address, upload.source, tracking)
+
+        my_subst = subst.copy()
+        my_subst['__ANNOUNCE_LIST_ADDRESS__'] = announce_list_address
+
+        message = TemplateSubst(my_subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.announce'))
+        utils.send_mail(message)
+
+    if accepted_to_real_suite and upload.sourceful and cnf.find_b('Dinstall::CloseBugs'):
+        for bug in upload.bugs:
+            my_subst = subst.copy()
+            my_subst['__BUG_NUMBER__'] = str(bug)
+
+            message = TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.bug-close'))
+            send_mail(message)
+
+def announce_new(upload):
+    cnf = Config()
+    subst = _subst_for_upload(upload)
+
+    message = TemplateSubst(subst, os.path.join(cnf['Dir::Templates'], 'process-unchecked.new'))
+    send_mail(message)