Merge lp:~thumper/pqm/queue-abstraction-2 into lp:pqm

[Tim Penhey]

[Tim Penhey (thumper)]

Due to a fubar in the code, I can't mark as needs review right now.

[Tim Penhey (thumper)]

=== modified file 'bin/pqm'
--- bin/pqm 2008-07-17 21:29:54 +0000
+++ bin/pqm 2008-08-05 06:33:49 +0000
@@ -56,151 +56,23 @@
 from pqm import *
 from pqm.PQMConfigParser import ConfigParser
 from pqm.commandline import parse_command_line
+from pqm.core import get_script_queue, PatchQueueManager
 from pqm.errors import PQMCmdFailure, PQMException
 from pqm.lockfile import LockFile
 from pqm.script import Command, read_email

-def dir_from_option(configp, option, default):
- """calculate a working dir path"""
- return os.path.abspath(os.path.expanduser(configp.get_option('DEFAULT',option, os.path.join(queuedir, default))))
-
 def runtla_internal(sender, cmd, *args):
     return apply(popen_noshell, [arch_path, cmd] + list(args))

+
 def runtla(sender, cmd, *args):
     (status, msg, output) = apply(runtla_internal, [sender, cmd] + list(args))
     if not ((status is None) or (status == 0)):
         raise PQMTlaFailure(sender, ["VCS command %s %s failed (%s): %s" % (cmd, args, status, msg)] + output)
     return output

-def do_run_mode(queuedir, logger, logdir, mail_reply, mail_server,
- from_address, fromaddr, configp, options):
- scripts = find_patches(
- queuedir, logger, rev_optionhandler, configp, options)
- (goodscripts, badscripts) = ([], [])
- for script in scripts:
- if not os.path.isfile("%s/stop.patch" % queuedir):
- run_one_script(logger, script, logdir, goodscripts, badscripts,
- mail_reply, mail_server, from_address, fromaddr, options)
-
- if options.print_report:
- for (patchname, logname) in goodscripts:
- print "Patch: " + patchname
- print "Status: success"
- print "Log: " + logname
- print
- for (patchname, logname) in badscripts:
- print "Patch: " + patchname
- print "Status: failure"
- print "Log: " + logname
- print
-
-def run_one_script(logger, script, logdir, goodscripts, badscripts,
- mail_reply, mail_server, from_address, fromaddr, options):
- # FIXME: This is currently extremely hard to test. move it to the library,
- # and test it!
- try:
- try:
- logger.info('trying script ' + script.filename)
- logname = os.path.join(logdir, os.path.basename(script.filename) + '.log')
- (sender, subject, msg, sig) = read_email(logger, open(script.filename))
- if options.verify_sigs:
- sigid,siguid = verify_sig(
- script.getSender(), msg, sig, 0, logger, options.keyring)
- success = False
- output = []
- failedcmd=None
-
- # ugly transitional code
- pqm.logger = logger
- pqm.workdir = workdir
- pqm.runtla = runtla
- pqm.precommit_hook = precommit_hook
- (successes, unrecognized, output) = script.run()
-
- logger.info('successes: %s' % (successes,))
- logger.info('unrecognized: %s' % (unrecognized,))
- success = True
- goodscripts.append((script.filename, logname))
- except PQMCmdFailure, e:
- ba...

[lp:~thumper/pqm/queue-abstraction-2]

181. By Tim Penhey on 2008-08-05

Merge trunk.

[Dan Watkins (oddbloke)]

Hi Tim,

This is shaping up nicely. I just have a few suggestions which might
improve it.

On Tue, 05 Aug 2008 06:34:31 -0000
Tim Penhey <email address hidden> wrote:
> === modified file 'bin/pqm'
> @@ -371,12 +234,16 @@
> pqm.used_transactions[line[0:-1]] = 1
>
> if options.read_mode:
> - do_read_mode(logger, options)
> + do_read_mode(logger, options, manager.mail_reply,
> manager.mail_server,
> + manager.from_address, manager.nice_from_address)
It feels to me as if do_read_mode shouldn't exist in bin/pqm any more.
I'm not entirely sure where it should move to, but perhaps EmailQueue
would be appropriate?

> === added file 'pqm/core.py'
> @@ -0,0 +1,229 @@
> +def get_script_queue(queuedir, logger, branch_spec_handler, configp,
> + options):
> + """Determine the type of queue from the config."""
> + # Tim Penhey, 2008-05-29
> + # Right now there is only one queue type, but this will change
> RSN.
> + return EmailQueue(
> + queuedir, logger, branch_spec_handler, configp, options)
What other sorts of queue do you envisage there being?

> === modified file 'pqm/script.py'
It seems to me that the Commands should be moved to their own module,
as several different scripts may wish to use them...

> === modified file 'pqm/ui/twistd.py'
> class QueueResource(resource.Resource):
QueueResource doesn't seem to have been updated to use the new
abstraction. For example, it directly checks whether 'stop.patch'
exists, when there is a method on Queue it should be using instead.

Hope this helps,

Dan

--
Daniel Watkins (Odd_Bloke)

[Tim Penhey (thumper)]

On Tuesday 05 August 2008 19:12:19 Daniel Watkins wrote:
> Hi Tim,
>
> This is shaping up nicely. I just have a few suggestions which might
> improve it.

Thanks for looking.

> On Tue, 05 Aug 2008 06:34:31 -0000
> Tim Penhey <email address hidden> wrote:
> > === modified file 'bin/pqm'
> > @@ -371,12 +234,16 @@
> > pqm.used_transactions[line[0:-1]] = 1
> >
> > if options.read_mode:
> > - do_read_mode(logger, options)
> > + do_read_mode(logger, options, manager.mail_reply,
> > manager.mail_server,
> > + manager.from_address, manager.nice_from_address)
> It feels to me as if do_read_mode shouldn't exist in bin/pqm any more.
> I'm not entirely sure where it should move to, but perhaps EmailQueue
> would be appropriate?

Hmm.. I was trying hard not to do too much at once :)

Perhaps moving it to EmailQueue would be the best place for this.

> > === added file 'pqm/core.py'
> > @@ -0,0 +1,229 @@
> > +def get_script_queue(queuedir, logger, branch_spec_handler, configp,
> > + options):
> > + """Determine the type of queue from the config."""
> > + # Tim Penhey, 2008-05-29
> > + # Right now there is only one queue type, but this will change
> > RSN.
> > + return EmailQueue(
> > + queuedir, logger, branch_spec_handler, configp, options)
> What other sorts of queue do you envisage there being?

Well, at least one for Launchpad.

> > === modified file 'pqm/script.py'
> It seems to me that the Commands should be moved to their own module,
> as several different scripts may wish to use them...

Again, I was trying to keep the changes small enough. I'm quite happy for
thinks to be broken up more later, but there were challenges enough with
the mass move that was in this branch.

> > === modified file 'pqm/ui/twistd.py'
> > class QueueResource(resource.Resource):
> QueueResource doesn't seem to have been updated to use the new
> abstraction. For example, it directly checks whether 'stop.patch'
> exists, when there is a method on Queue it should be using instead.

OK, I'll look into this later today.

> Hope this helps,

It does, thanks.

Tim

[lp:~thumper/pqm/queue-abstraction-2]

182. By Tim Penhey on 2008-08-06

Update the QueueResource for the twisted UI to use the queue rather than checking the filesystem for stop.patch.

183. By Tim Penhey on 2008-08-06

Fix the test that was incorrectly using the wrong MergeCommand.

[Tim Penhey (thumper)]

This is the diff from r181 -> r183 which addresses the twisted UI referring to stop.patch, and fixes the broken test.

=== modified file 'pqm/tests/test_pqm.py'
--- pqm/tests/test_pqm.py 2008-08-05 06:31:49 +0000
+++ pqm/tests/test_pqm.py 2008-08-06 03:21:19 +0000
@@ -202,11 +202,11 @@
         self.assertEqual(script.getLines(),
             [("star-merge http://www.example.com/foo/bar "
               "http://www.example.com/bar/baz")])
- self.assertEqual([pqm.MergeCommand(None,
- None,
- None,
- 'http://www.example.com/foo/bar',
- 'http://www.example.com/bar/baz')],
+ self.assertEqual([MergeCommand(None,
+ None,
+ None,
+ 'http://www.example.com/foo/bar',
+ 'http://www.example.com/bar/baz')],
                          script.getCommands())

     def testGPGFields(self):
@@ -231,11 +231,11 @@
             [("star-merge http://www.example.com/argh/blah "
               "http://www.example.com/bing/bong")])
         self.assertEqual(
- [pqm.MergeCommand(None,
- None,
- None,
- 'http://www.example.com/argh/blah',
- 'http://www.example.com/bing/bong')],
+ [MergeCommand(None,
+ None,
+ None,
+ 'http://www.example.com/argh/blah',
+ 'http://www.example.com/bing/bong')],
             script.getCommands())

     def testDate(self):

=== modified file 'pqm/ui/twistd.py'
--- pqm/ui/twistd.py 2008-05-29 02:44:29 +0000
+++ pqm/ui/twistd.py 2008-08-06 03:18:46 +0000
@@ -55,15 +55,10 @@
             return self.getProjectPage(None, request).render(request)

     def getProjectPage(self, selected_project, request):
-
- # Get queuedir
- configp = ConfigParser()
- configp.read(self.queue.filenames)
- handler = pqm.BranchSpecOptionHandler(configp)
- queuedir = pqm.get_queuedir(configp, logging, [])
+ """Render the project page."""

         text = "<h1>PQM Queue: %d scripts</h1>" % len(self.queue.messages)
- if os.path.isfile("%s/stop.patch" % queuedir):
+ if not self.queue.is_processing_requests:
             text += "<h2>PQM is not currently processing additional requests</h2>"
         text += "<p>Current time: %s UTC</p>" % cgi.escape(
             time.strftime("%a, %d %b %Y %H:%M:%S", time.gmtime()))
@@ -157,6 +152,7 @@
     def __init__(self, filenames):
         self.filenames = filenames
         self.messages = []
+ self.is_processing_requests = False

     def refresh(self):
         configp = ConfigParser()
@@ -167,6 +163,7 @@
         script_queue = get_script_queue(
             queuedir, logging, handler, configp, FakeOptions())

+ self.is_processing_requests = script_queue.is_processing_requests(...

[lp:~thumper/pqm/queue-abstraction-2]

184. By Tim Penhey on 2008-12-01

Merge in trunk and fix conflicts.

=== modified file 'bin/pqm'
--- bin/pqm	2008-07-17 11:53:21 +0000
+++ bin/pqm	2009-03-05 05:50:16 +0000
@@ -56,151 +56,23 @@
 from pqm import *
 from pqm.PQMConfigParser import ConfigParser
 from pqm.commandline import parse_command_line
+from pqm.core import get_script_queue, PatchQueueManager
 from pqm.errors import PQMCmdFailure, PQMException
 from pqm.lockfile import LockFile
 from pqm.script import Command, read_email
 
 
-def dir_from_option(configp, option, default):
-    """calculate a working dir path"""
-    return os.path.abspath(os.path.expanduser(configp.get_option('DEFAULT',option, os.path.join(queuedir, default))))
-
 def runtla_internal(sender, cmd, *args):
     return apply(popen_noshell, [arch_path, cmd] + list(args))
 
+
 def runtla(sender, cmd, *args):
     (status, msg, output) = apply(runtla_internal, [sender, cmd] + list(args))
     if not ((status is None) or (status == 0)):
         raise PQMTlaFailure(sender, ["VCS command %s %s failed (%s): %s" % (cmd, args, status, msg)] + output)
     return output
 
-def do_run_mode(queuedir, logger, logdir, mail_reply, mail_server,
-    from_address, fromaddr, configp, options):
-    scripts = find_patches(
-        queuedir, logger, rev_optionhandler, configp, options)
-    (goodscripts, badscripts) = ([], [])
-    for script in scripts:
-        if not os.path.isfile("%s/stop.patch" % queuedir):
-            run_one_script(logger, script, logdir, goodscripts, badscripts,
-                mail_reply, mail_server, from_address, fromaddr, options)
-
-    if options.print_report:
-        for (patchname, logname) in goodscripts:
-            print "Patch: " + patchname
-            print "Status: success"
-            print "Log: " + logname
-            print
-        for (patchname, logname) in badscripts:
-            print "Patch: " + patchname
-            print "Status: failure"
-            print "Log: " + logname
-            print
-
-def run_one_script(logger, script, logdir, goodscripts, badscripts,
-                   mail_reply, mail_server, from_address, fromaddr, options):
-    # FIXME: This is currently extremely hard to test. move it to the library,
-    # and test it!
-    try:
-        success = False
-        try:
-            logger.info('trying script ' + script.filename)
-            logname = os.path.join(logdir, os.path.basename(script.filename) + '.log')
-            (sender, subject, msg, sig) = read_email(logger, open(script.filename))
-            if options.verify_sigs:
-                sigid,siguid = verify_sig(
-                    script.getSender(), msg, sig, 0, logger, options.keyring)
-            output = []
-            failedcmd=None
-    
-            # ugly transitional code
-            pqm.logger = logger
-            pqm.workdir = workdir
-            pqm.runtla = runtla
-            pqm.precommit_hook = precommit_hook
-            (successes, unrecognized, output) = script.run()
-
-            logger.info('successes: %s' % (successes,))
-            logger.info('unrecognized: %s' % (unrecognized,))
-            success = True
-            goodscripts.append((script.filename, logname))
-        except PQMCmdFailure, e:
-            badscripts.append((script.filename, logname))
-            successes = e.goodcmds
-            failedcmd = e.badcmd
-            output = e.output
-            unrecognized=[]
-        except PQMException, e:
-            badscripts.append((script.filename, logname))
-            successes = []
-            failedcmd = []
-            output = [str(e)]
-            unrecognized=[]
-        except Exception, e:
-            # catch all to ensure we get some output in uncaught failures
-            output = [str(e)]
-            raise
-        if mail_reply:
-            send_mail_reply(success, successes, unrecognized,
-                            mail_server, from_address, script.getSender(),
-                            fromaddr, failedcmd, output, script)
-        else:
-            logger.info('not sending mail reply')
-    finally:
-        # ensure we always unlink the script file.
-        log_list(logname, output)
-        os.unlink(script.filename)
-
-def send_mail_reply(success, successes, unrecognized, mail_server, from_address, sender, fromaddr, failedcmd, output, script):
-    if success:
-        retmesg = mail_format_successes(successes, "Command was successful.", unrecognized)
-        if len(successes) > 0:
-            statusmsg='success'
-        else:
-            statusmsg='no valid commands given'
-    else:
-        retmesg = mail_format_successes(successes, "Command passed checks, but was not committed.", unrecognized)
-        retmesg+= "\n%s" % failedcmd
-        retmesg+= '\nCommand failed!'
-        if not script.debug:
-            retmesg += '\nLast 20 lines of log output:'
-            retmesg += ''.join(output[-20:])
-        else:
-            retmesg += '\nAll lines of log output:'
-            retmesg += ''.join(output)
-        statusmsg='failure'
-    server = smtplib.SMTP(mail_server)
-    server.sendmail(from_address, [sender], 'From: %s\r\nTo: %s\r\nSubject: %s\r\n\r\n%s\n' % (fromaddr, sender, statusmsg, retmesg))
-    server.quit()
-
-def mail_format_successes(successes, command_msg, unrecognized):
-    retmesg = []
-    for success in successes:
-        retmesg.append('> ' + success)
-        retmesg.append(command_msg)
-        for line in unrecognized:
-            retmesg.append('> ' + line)
-            retmesg.append('Unrecognized command.')
-    return string.join(retmesg, '\n')
-
-def log_list(logname, list):
-    f = open(logname, 'w')
-    for l in list:
-        f.write(l)
-    f.close()
-
-def run(pqm_subdir, queuedir, logger, logdir, mail_reply, mail_server,
-    from_address, fromaddr, options):
-    lockfile=LockFile(os.path.join(pqm_subdir, 'pqm.lock'), logger,
-        options.no_act, options.cron_mode)
-    lockfile.acquire()
-    try:
-        if options.run_mode:
-            do_run_mode(queuedir, logger, logdir, mail_reply, mail_server,
-                from_address, fromaddr, configp, options)
-    finally:
-        lockfile.release()
-
-def do_read_mode(logger, options):
+def do_read_mode(logger, options, mail_reply, mail_server, from_address, fromaddr):
     sender = None
     try:
         (sender, subject, msg, sig) = read_email(logger)
@@ -230,12 +102,6 @@
 arch_impl = None
 logfile_name = 'pqm.log'
 default_mail_log_level = logging.ERROR
-mail_server = 'localhost'
-queuedir = None
-workdir = None
-logdir = None
-mail_reply = 1
-from_address = None
 precommit_hook = []
 
 (options, args) = parse_command_line(sys.argv[1:])
@@ -290,17 +156,8 @@
 Command.arch_path = arch_path
 
 pqm.gpgv_path = configp.get_option('DEFAULT', 'gpgv_path', 'gpgv')
-myname = configp.get_option('DEFAULT', 'myname', 'Arch Patch Queue Manager')
-
-if configp.has_option('DEFAULT', 'from_address'):
-    from_address = configp.get('DEFAULT', 'from_address')
-else:
-    logger.error("No from_address specified")
-    sys.exit(1)
-fromaddr = '%s <%s>' % (myname, from_address)
-
-mail_reply=configp.get_boolean_option('DEFAULT', 'mail_reply',1)
-# The command line parameter overrides the setting in the config file.
+
+# The command line paramter overrides the setting in the config file.
 if options.verify_sigs:
     options.verify_sigs = configp.get_boolean_option(
         'DEFAULT', 'verify_sigs', True)
@@ -310,15 +167,24 @@
 else:
     queuedir = get_queuedir(configp, logger, args)
 queuedir=os.path.abspath(queuedir)
-pqm_subdir = os.path.join(queuedir, 'pqm')
-pqm.pqm_subdir = pqm_subdir
+
+manager = PatchQueueManager(queuedir, configp, options, logger)
+
+if manager.from_address is None:
+    logger.error("No from_address specified")
+    sys.exit(1)
+
+# Still temporary hack.  Tim Penhey 2008-05-29
+pqm.pqm_subdir = manager.control_dir
+# ugly transitional code
+pqm.logger = logger
+pqm.workdir = manager.work_dir
+pqm.runtla = runtla
+pqm.precommit_hook = precommit_hook
 
 if not configp.has_option('DEFAULT', 'dont_set_home'):
     os.environ['HOME'] = queuedir
 
-workdir=dir_from_option(configp, 'workdir', 'workdir')
-logdir=dir_from_option(configp, 'logdir', 'logs')
-
 if not options.keyring:
     if configp.has_option('DEFAULT', 'keyring'):
         options.keyring = configp.get('DEFAULT', 'keyring')
@@ -329,11 +195,8 @@
     logger.error("Couldn't access keyring %s" % (options.keyring,))
     sys.exit(1)
 
-do_mkdir(queuedir, options.no_act)
+manager.make_directories()
 os.chdir(queuedir)
-do_mkdir(workdir, options.no_act)
-do_mkdir(logdir, options.no_act)
-do_mkdir(pqm_subdir, options.no_act)
 
 rev_optionhandler = pqm.BranchSpecOptionHandler(configp, queuedir=queuedir)
 if len(rev_optionhandler._specs) == 0:
@@ -348,8 +211,8 @@
 
 if not options.no_log:
     if not os.path.isabs(logfile_name):
-        logfile_name = os.path.join(pqm_subdir, logfile_name)
-    logger.debug("Adding log file: %s" % (logfile_name,))
+        logfile_name = os.path.join(manager.control_dir, logfile_name)
+    logger.debug("Adding log file: %s" % logfile_name)
     filehandler = logging.FileHandler(logfile_name)
     if options.loglevel >= logging.WARN:
         filehandler.setLevel(logging.INFO)
@@ -371,12 +234,16 @@
         pqm.used_transactions[line[0:-1]] = 1
 
 if options.read_mode:
-    do_read_mode(logger, options)
+    do_read_mode(logger, options, manager.mail_reply, manager.mail_server,
+                 manager.from_address, manager.nice_from_address)
 
 assert(options.run_mode)
 
-run(pqm_subdir, queuedir, logger, logdir, mail_reply, mail_server,
-    from_address, fromaddr, options)
+script_queue = get_script_queue(
+    queuedir, logger, rev_optionhandler, configp, options)
+
+manager.run(script_queue)
+
 logger.info("main thread exiting...")
 sys.exit(0)
 

=== modified file 'pqm/__init__.py'
--- pqm/__init__.py	2008-07-17 10:09:34 +0000
+++ pqm/__init__.py	2009-03-05 05:50:16 +0000
@@ -27,6 +27,7 @@
 import string
 import sys
 
+from pqm.core import do_mkdir
 from pqm.errors import PQMException, PQMTlaFailure
 from pqm.script import Script
 
@@ -41,6 +42,7 @@
 logger = logging # default value for simple use.
 groups = {}
 
+
 def get_queuedir(config_parser, logger, args):
     """Get the queuedir that should be used from the config"""
     if config_parser.has_option('DEFAULT', 'queuedir'):
@@ -54,23 +56,6 @@
         sys.exit(1)
 
 
-def find_patches(queuedir, logger, branch_spec_handler, configp, options):
-    patches=[]
-    patches_re=re.compile('^patch\.\d+$')
-    for f in os.listdir(queuedir):
-        if patches_re.match(f):
-            fname=os.path.join(queuedir, f)
-            submission_time = os.stat(fname)[stat.ST_MTIME]
-            patches.append((Script(fname, logger, options.verify_sigs,
-                                   submission_time, branch_spec_handler,
-                                   configp, options.keyring),
-                            f))
-    def sortpatches(a, b):
-        return cmp(a[1], b[1])
-    patches.sort(sortpatches)
-    return [patch[0] for patch in patches]
- 
-
 def verify_sig(sender, msg, sig, check_replay, logger, keyring):
     """Verify the GPG signature on a message."""
     verifier = GPGSigVerifier([keyring], gpgv=gpgv_path)
@@ -101,7 +86,6 @@
         logger.error("Replay attack detected, aborting")
         raise PQMException(sender, "Replay attack detected, aborting")
     gpg_key_re = re.compile('^\[GNUPG:\] GOODSIG ([0-9A-F]+) .*<([^>]*)>.*$')
-    sig_from = None
     for line in output:
         match = gpg_key_re.match(line)
         if match:
@@ -476,11 +460,15 @@
 
     def __init__(self, configp, queuedir=None):
         self._configp = configp
+        # TODO: remove the queuedir from this class.  The only reason
+        # it is here is to have the status file.  This should now be
+        # moved into the PatchQueueManager class.
+        # Tim Penhey 2008-05-29
         if self._configp.has_option('DEFAULT', 'queuedir'):
             self.queuedir = os.path.abspath(os.path.expanduser(
                 self._configp.get('DEFAULT', 'queuedir')))
-            do_mkdir(self.queuedir)
-            do_mkdir(os.path.join(self.queuedir, 'pqm'))
+            do_mkdir(self.queuedir, logger)
+            do_mkdir(os.path.join(self.queuedir, 'pqm'), logger)
         else:
             self.queuedir = queuedir
         self._specs = {}
@@ -614,14 +602,3 @@
             self.statusfile.write(line)
         self.statusfile.flush()
         self.statusfile.truncate()
-
-def do_mkdir(name, no_act=False):
-    if os.access(name, os.X_OK):
-        return
-    try:
-        logger.info('Creating directory "%s"' % (name))
-    except:
-        pass
-    if not no_act:
-        os.mkdir(name)
-

=== added file 'pqm/core.py'
--- pqm/core.py	1970-01-01 00:00:00 +0000
+++ pqm/core.py	2009-03-05 05:50:16 +0000
@@ -0,0 +1,229 @@
+# -*- mode: python; coding: utf-8 -*-
+# vim:smartindent cinwords=if,elif,else,for,while,try,except,finally,def,class:ts=4:sts=4:sta:et:ai:shiftwidth=4
+#
+# Copyright © 2004, 2008 Canonical Ltd.
+#	Author: Robert Collins <[e-mail]>
+#	Author: Tim Penhey <[e-mail]>
+
+# 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+import os
+import smtplib
+
+from pqm.emailqueue import EmailQueue
+from pqm.errors import PQMCmdFailure, PQMException
+from pqm.lockfile import LockFile
+
+
+def get_script_queue(queuedir, logger, branch_spec_handler, configp,
+                     options):
+    """Determine the type of queue from the config."""
+    # Tim Penhey, 2008-05-29
+    # Right now there is only one queue type, but this will change RSN.
+    return EmailQueue(
+        queuedir, logger, branch_spec_handler, configp, options)
+
+
+def do_mkdir(name, logger, no_act=False):
+    if os.access(name, os.X_OK):
+        return
+    try:
+        logger.info('Creating directory "%s"' % (name))
+    except:
+        pass
+    if not no_act:
+        os.mkdir(name)
+
+
+class PatchQueueManager(object):
+    """The PatchQueueManager controls the core processing."""
+
+    branch_configuration = None
+
+    # There are four directories that the manager cares about:
+    #  * queue_dir - the primary directory from which the other three
+    #                default to being under.
+    #  * work_dir - where the merging or commands takes place
+    #  * log_dir - where the script execution log files are kept
+    #  * control_dir - where the lockfile, and pqm log file is kept
+    queue_dir = None
+    work_dir = None
+    log_dir = None
+    control_dir = None
+
+    def __init__(self, queue_dir, config_parser, options, logger):
+        self.queue_dir = queue_dir
+        self.config_parser = config_parser
+        self.options = options
+        self.logger = logger
+
+        self.work_dir = self._dir_from_option('workdir', 'workdir')
+        self.log_dir = self._dir_from_option('logdir', 'logs')
+        self.control_dir = os.path.join(self.queue_dir, 'pqm')
+
+        self.mail_reply = config_parser.get_boolean_option(
+            'DEFAULT', 'mail_reply', True)
+        self.mail_server = config_parser.get_option(
+            'DEFAULT', 'mail_server', 'localhost')
+        myname = config_parser.get_option(
+            'DEFAULT', 'myname', 'Arch Patch Queue Manager')
+        self.from_address = config_parser.get(
+            'DEFAULT', 'from_address', None)
+        if self.from_address is not None:
+            self.nice_from_address = '%s <%s>' % (myname, self.from_address)
+
+    def _dir_from_option(self, option, default):
+        """Get the option from the config with a sensible default."""
+        default_dir = os.path.join(self.queue_dir, default)
+        configured_dir = self.config_parser.get_option(
+            'DEFAULT', option, default_dir)
+        return os.path.abspath(os.path.expanduser(configured_dir))
+
+    def make_directories(self):
+        """Make sure the directories exist."""
+        do_mkdir(self.queue_dir, self.logger, self.options.no_act)
+        do_mkdir(self.work_dir, self.logger, self.options.no_act)
+        do_mkdir(self.log_dir, self.logger, self.options.no_act)
+        do_mkdir(self.control_dir, self.logger, self.options.no_act)
+
+    def run(self, script_queue):
+        lockfile = LockFile(
+            os.path.join(self.control_dir, 'pqm.lock'), self.logger,
+            self.options.no_act, self.options.cron_mode)
+        lockfile.acquire()
+        try:
+            self.do_run_mode(script_queue)
+        finally:
+            lockfile.release()
+
+    def do_run_mode(self, script_queue):
+        """Run through the script queue until empty."""
+        (goodscripts, badscripts) = ([], [])
+
+        while script_queue.next_script() is not None:
+            if script_queue.is_processing_requests():
+                self.run_one_script(
+                    script_queue.next_script(), goodscripts, badscripts)
+            script_queue.pop_script()
+
+        if self.options.print_report:
+            for (patchname, logname) in goodscripts:
+                print "Patch: " + patchname
+                print "Status: success"
+                print "Log: " + logname
+                print
+            for (patchname, logname) in badscripts:
+                print "Patch: " + patchname
+                print "Status: failure"
+                print "Log: " + logname
+                print
+
+    def run_one_script(self, script, goodscripts, badscripts):
+        # FIXME: Test it!
+        try:
+            success = False
+            try:
+                self.logger.info('trying script ' + script.filename)
+                logname = os.path.join(self.log_dir, os.path.basename(
+                        script.filename) + '.log')
+
+                script.pre_run_hook()
+
+                output = []
+                failedcmd=None
+
+                (successes, unrecognized, output) = script.run()
+
+                self.logger.info('successes: %s' % (successes,))
+                self.logger.info('unrecognized: %s' % (unrecognized,))
+                success = True
+                goodscripts.append((script.filename, logname))
+            except PQMCmdFailure, e:
+                badscripts.append((script.filename, logname))
+                successes = e.goodcmds
+                failedcmd = e.badcmd
+                output = e.output
+                unrecognized=[]
+            except PQMException, e:
+                badscripts.append((script.filename, logname))
+                successes = []
+                failedcmd = []
+                output = [str(e)]
+                unrecognized=[]
+            except Exception, e:
+                # catch all to ensure we get some output in uncaught failures
+                output = [str(e)]
+                raise
+            if self.mail_reply:
+                self.send_mail_reply(success, successes, unrecognized,
+                                     script.getSender(), failedcmd, output,
+                                     script)
+            else:
+                self.logger.info('not sending mail reply')
+        finally:
+            # ensure we always unlink the script file.
+            log_list(logname, output)
+            os.unlink(script.filename)
+
+    def send_mail_reply(self, success, successes, unrecognized,
+                        to_address, failedcmd, output, script):
+        if success:
+            retmesg = mail_format_successes(
+                successes, "Command was successful.", unrecognized)
+            if len(successes) > 0:
+                statusmsg='success'
+            else:
+                statusmsg='no valid commands given'
+        else:
+            retmesg = mail_format_successes(
+                successes, "Command passed checks, but was not committed.",
+                unrecognized)
+            retmesg += "\n%s" % failedcmd
+            retmesg += '\nCommand failed!'
+            if not script.debug:
+                retmesg += '\nLast 20 lines of log output:'
+                retmesg += ''.join(output[-20:])
+            else:
+                retmesg += '\nAll lines of log output:'
+                retmesg += ''.join(output)
+            statusmsg='failure'
+        server = smtplib.SMTP(self.mail_server)
+        server.sendmail(
+            self.from_address,
+            [to_address],
+            'From: %s\r\nTo: %s\r\nSubject: %s\r\n\r\n%s\n' % (
+                self.nice_from_address, to_address, statusmsg, retmesg))
+        server.quit()
+
+
+def mail_format_successes(successes, command_msg, unrecognized):
+    retmesg = []
+    for success in successes:
+        retmesg.append('> ' + success)
+        # Do we really want the command_msg for every success?
+        retmesg.append(command_msg)
+        # And I'm almost certain that we don't want these for each one.
+        for line in unrecognized:
+            retmesg.append('> ' + line)
+            retmesg.append('Unrecognized command.')
+    return '\n'.join(retmesg)
+
+
+def log_list(logname, list):
+    f = open(logname, 'w')
+    for l in list:
+        f.write(l)
+    f.close()
+

=== added file 'pqm/emailqueue.py'
--- pqm/emailqueue.py	1970-01-01 00:00:00 +0000
+++ pqm/emailqueue.py	2009-03-05 05:50:16 +0000
@@ -0,0 +1,82 @@
+# -*- mode: python; coding: utf-8 -*-
+# vim:smartindent cinwords=if,elif,else,for,while,try,except,finally,def,class:ts=4:sts=4:sta:et:ai:shiftwidth=4
+#
+# Copyright © 2004, 2008 Canonical Ltd.
+#	Author: Robert Collins <[e-mail]>
+#	Author: Tim Penhey <[e-mail]>
+
+# 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+import os
+import re
+import stat
+
+from pqm.queue import Queue
+from pqm.script import EmailScript
+
+
+def find_patches(queuedir, logger, branch_spec_handler, configp, options):
+    patches=[]
+    patches_re=re.compile('^patch\.\d+$')
+    for f in os.listdir(queuedir):
+        if patches_re.match(f):
+            fname=os.path.join(queuedir, f)
+            submission_time = os.stat(fname)[stat.ST_MTIME]
+            script = EmailScript(fname, logger, options.verify_sigs,
+                                 submission_time, branch_spec_handler,
+                                 configp, options.keyring)
+            patches.append((script, f))
+    def sortpatches(a, b):
+        return cmp(a[1], b[1])
+    patches.sort(sortpatches)
+    return [patch[0] for patch in patches]
+
+
+class EmailQueue(Queue):
+    """The email queue gets the scripts from the patch files in the queuedir.
+
+    """
+
+    def __init__(self, queuedir, logger, branch_spec_handler, configp,
+                 options):
+        self.queuedir = queuedir
+        self.configp = configp
+        self.scripts = find_patches(
+            queuedir, logger, branch_spec_handler, configp, options)
+
+    def is_processing_requests(self):
+        """Is the queue currently processing requests?"""
+        return not os.path.isfile("%s/stop.patch" % self.queuedir)
+
+    def next_script(self):
+        """The next script to process.
+
+        :return: A `Script` or None if there is nothing to do.
+        """
+        if len(self.scripts) > 0:
+            return self.scripts[0]
+        else:
+            return None
+
+    def pop_script(self):
+        """Remove the front queue item."""
+        self.scripts.pop(0)
+
+    def items(self):
+        """The scripts currently in the queue.
+
+        This method is used primarily by the UI to show upcoming work.
+        """
+        return self.scripts

=== added file 'pqm/queue.py'
--- pqm/queue.py	1970-01-01 00:00:00 +0000
+++ pqm/queue.py	2009-03-05 05:50:16 +0000
@@ -0,0 +1,49 @@
+# -*- mode: python; coding: utf-8 -*-
+# vim:smartindent cinwords=if,elif,else,for,while,try,except,finally,def,class:ts=4:sts=4:sta:et:ai:shiftwidth=4
+#
+# Copyright © 2008 Canonical Ltd.
+#	Author: Tim Penhey <[e-mail]>
+
+# 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., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+class Queue(object):
+    """The Queue contains scripts for the Patch Queue Manager to do.
+
+    The queue contains instances of the type `Script`.
+    """
+
+    def is_processing_requests(self):
+        """Is the queue currently processing requests?"""
+        raise NotImplementedError()
+
+    def next_script(self):
+        """The next script to process.
+
+        Does not conceptually pop the script from the queue.
+
+        :return: A `Script` or None if there is nothing to do.
+        """
+        raise NotImplementedError()
+
+    def pop_script(self):
+        """Remove the front queue item."""
+        raise NotImplementedError()
+
+    def items(self):
+        """The scripts currently in the queue.
+
+        This method is used primarily by the UI to show upcoming work.
+        """
+        raise NotImplementedError()

=== modified file 'pqm/script.py'
--- pqm/script.py	2008-07-17 02:03:01 +0000
+++ pqm/script.py	2009-03-05 05:50:16 +0000
@@ -73,7 +73,23 @@
 
 
 class Script(object):
-    """A command script."""
+    """A script for the Patch Queue Manager to process.
+
+    A script will contain one or more `Command`s.
+    """
+
+    def pre_run_hook(self):
+        """This hook is called before the script is run."""
+
+    def post_run_hook(self):
+        """This hook is called after the script is run.
+
+        This method is called regardless of whether or not run raised
+        an exception.
+        """
+
+class EmailScript(Script):
+    """A command script generated from an email."""
 
     whitespace_re = re.compile('^\s*$')
     pgp_re = re.compile('^-----BEGIN PGP.*MESSAGE')
@@ -96,6 +112,12 @@
         self._configp = configp
         self._keyring = keyring
 
+    def pre_run_hook(self):
+        """This hook is called before the script is run."""
+        # Make sure that the message has been read, and that the
+        # signature has been verified if needed.
+        self._read()
+
     def _read(self):
         """Read in the script details."""
         #  This is cruft from the binary script. It actually does need to read
@@ -177,8 +199,8 @@
             if not self.isCommand(line):
                 continue
             # identify and construct commands
-            star_match = Script.star_re.match(line)
-            debug_match = Script.debug_re.match(line)
+            star_match = EmailScript.star_re.match(line)
+            debug_match = EmailScript.debug_re.match(line)
             any_match = star_match or debug_match
             if any_match and legacy_lines:
                 result.append(CommandRunner(self,

=== modified file 'pqm/tests/test_pqm.py'
--- pqm/tests/test_pqm.py	2008-07-17 11:53:21 +0000
+++ pqm/tests/test_pqm.py	2009-03-05 05:50:16 +0000
@@ -11,7 +11,9 @@
 import pqm
 from pqm.errors import PQMCmdFailure
 from pqm.PQMConfigParser import ConfigParser
-from pqm.script import Command, CommandRunner, DebugCommand, MergeCommand
+from pqm.script import (
+    Command, CommandRunner, DebugCommand, EmailScript, MergeCommand)
+
 
 sample_message = dedent("""\
     From: [e-mail]
@@ -167,7 +169,7 @@
         self.queue.setUp()
 
     def testName(self):
-        patch = pqm.Script('foo.script', logging, False, 0, None, None)
+        patch = EmailScript('foo.script', logging, False, 0, None, None)
         self.assertEqual(patch.filename, 'foo.script')
         self.scriptname = 'fpp'
 
@@ -188,7 +190,7 @@
         configp = ConfigParser()
         configp.read([self.queue.configFileName])
         handler = pqm.BranchSpecOptionHandler(configp)
-        return pqm.Script(self.scriptname, logging, False, 54, handler, configp)
+        return EmailScript(self.scriptname, logging, False, 54, handler, configp)
 
     def testFields(self):
         script = self.getScript(sample_message)
@@ -201,10 +203,10 @@
             [("star-merge http://www.example.com/foo/bar "
               "http://www.example.com/bar/baz")])
         self.assertEqual([MergeCommand(None,
-                                           None,
-                                           None,
-                                           'http://www.example.com/foo/bar',
-                                           'http://www.example.com/bar/baz')],
+                                       None,
+                                       None,
+                                       'http://www.example.com/foo/bar',
+                                       'http://www.example.com/bar/baz')],
                          script.getCommands())
 
     def testGPGFields(self):
@@ -230,10 +232,10 @@
               "http://www.example.com/bing/bong")])
         self.assertEqual(
             [MergeCommand(None,
-                              None,
-                              None,
-                              'http://www.example.com/argh/blah',
-                              'http://www.example.com/bing/bong')],
+                          None,
+                          None,
+                          'http://www.example.com/argh/blah',
+                          'http://www.example.com/bing/bong')],
             script.getCommands())
 
     def testDate(self):
@@ -298,8 +300,7 @@
 class TestCommandRunner(unittest.TestCase):
 
     def test_star_merge_urls(self):
-        from pqm import Script
-        star_match = Script.star_re.match("star-merge file:///url1 file:///url2")
+        star_match = EmailScript.star_re.match("star-merge file:///url1 file:///url2")
         self.assertEqual(star_match.group(1), 'file:///url1')
         self.assertEqual(star_match.group(2), 'file:///url2')
 

=== modified file 'pqm/ui/tests/test_twisted.py'
--- pqm/ui/tests/test_twisted.py	2008-07-16 15:44:48 +0000
+++ pqm/ui/tests/test_twisted.py	2009-03-05 05:50:16 +0000
@@ -6,6 +6,7 @@
 
 import pqm
 from pqm.PQMConfigParser import ConfigParser
+from pqm.core import get_script_queue
 from pqm.tests.test_pqm import QueueSetup
 
 class TestTwistedUI(unittest.TestCase):
@@ -28,8 +29,11 @@
         configp.read([self.queueSetup.configFileName])
         handler = pqm.BranchSpecOptionHandler(configp)
         queuedir = pqm.get_queuedir(configp, logging, [])
-        patches = pqm.find_patches(
+
+        script_queue = get_script_queue(
             queuedir, logging, handler, configp, FakeOptions())
+
+        patches = script_queue.items()
         self.assertEqual(3, len(patches))
         self.assertEqual(patches[0].filename, self.queueSetup.messageFileName)
         self.assertEqual(set(['project']),

=== modified file 'pqm/ui/twistd.py'
--- pqm/ui/twistd.py	2008-04-16 10:23:52 +0000
+++ pqm/ui/twistd.py	2009-03-05 05:50:16 +0000
@@ -28,6 +28,7 @@
 import pqm
 from pqm.PQMConfigParser import ConfigParser
 from pqm.commandline import default_pqm_config_files
+from pqm.core import get_script_queue
 
 class QueueResource(resource.Resource):
     """A resource that shows a PQM queue."""
@@ -54,15 +55,10 @@
             return self.getProjectPage(None, request).render(request)
 
     def getProjectPage(self, selected_project, request):
-
-        # Get queuedir
-        configp = ConfigParser()
-        configp.read(self.queue.filenames)
-        handler = pqm.BranchSpecOptionHandler(configp)
-        queuedir = pqm.get_queuedir(configp, logging, [])
+        """Render the project page."""
 
         text = "<h1>PQM Queue: %d scripts</h1>" % len(self.queue.messages)
-        if os.path.isfile("%s/stop.patch" % queuedir):
+        if not self.queue.is_processing_requests:
             text += "<h2>PQM is not currently processing additional requests</h2>"
         text += "<p>Current time: %s UTC</p>" % cgi.escape(
             time.strftime("%a, %d %b %Y %H:%M:%S", time.gmtime()))
@@ -156,14 +152,20 @@
     def __init__(self, filenames):
         self.filenames = filenames
         self.messages = []
+        self.is_processing_requests = False
 
     def refresh(self):
         configp = ConfigParser()
         configp.read(self.filenames)
         handler = pqm.BranchSpecOptionHandler(configp)
         queuedir = pqm.get_queuedir(configp, logging, [])
-        self.messages = pqm.find_patches(
+
+        script_queue = get_script_queue(
             queuedir, logging, handler, configp, FakeOptions())
+
+        self.is_processing_requests = script_queue.is_processing_requests()
+        self.messages = script_queue.items()
+
         try:
             [message.getSender() for message in self.messages]
         except: