More fixes to the start up procedure. The root cause of the fix is that when

var_dir in the config file is a relative path, the bin/master and bin/qrunner
subprocesses end up creating another hierarchy under $VAR_DIR.  This is
because 'bin/mailman start' chdirs to $VAR_DIR before starting master.

The fix involves putting the absolute path to $VAR_DIR in the $MAILMAN_VAR_DIR
environment variable.  If the configuration file reader sees this, it uses the
value instead of $var_dir from the configuration file.

Another change is to remove qrunner's --subproc/-s option and stick this in
the environment as well, using $MAILMAN_UNDER_MASTER_CONTROL.

Other fixes here include:
* Deciphering the master lock file contents correctly
* Reformatting the error message so that it fits in 79 columns when prefixed
  by argparse error info.

5 files changed, 38 insertions, 26 deletions

diff --git a/src/mailman/bin/master.py b/src/mailman/bin/master.py
index 721cc0091..6ccc5b7bc 100644
--- a/src/mailman/bin/master.py
+++ b/src/mailman/bin/master.py
@@ -55,6 +55,8 @@ class ScriptOptions(Options):
     """Options for the master watcher."""
 
     usage = _("""\
+%prog [options]
+
 Master sub-process watcher.
 
 Start and watch the configured queue runners and ensure that they stay alive
@@ -73,9 +75,7 @@ The master also responds to SIGINT, SIGTERM, SIGUSR1 and SIGHUP, which it
 simply passes on to the qrunners.  Note that the master will close and reopen
 its own log files on receipt of a SIGHUP.  The master also leaves its own
 process id in the file `data/master-qrunner.pid` but you normally don't need
-to use this pid directly.
-
-Usage: %prog [options]""")
+to use this pid directly.""")
 
     def add_options(self):
         """See `Options`."""
@@ -119,8 +119,10 @@ def get_lock_data():
     with open(config.LOCK_FILE) as fp:
         filename = os.path.split(fp.read().strip())[1]
     parts = filename.split('.')
-    hostname = DOT.join(parts[1:-2])
-    pid = int(parts[-2])
+    timestamp = parts.pop()
+    pid = parts.pop()
+    hostname = parts.pop()
+    filename = DOT.join(reversed(parts))
     return hostname, int(pid), filename
 
 
@@ -194,15 +196,15 @@ def acquire_lock(force):
         if status == WatcherState.conflict:
             # Hostname matches and process exists.
             message = _("""\
-The master queue runner lock could not be acquired because it
-appears as though another master is already running.""")
+The master queue runner lock could not be acquired
+because it appears as though another master is already running.""")
         elif status == WatcherState.stale_lock:
             # Hostname matches but the process does not exist.
             program = sys.argv[0]
             message = _("""\
-The master queue runner lock could not be acquired.  It appears
-as though there is a stale master lock.  Try re-running $program
-with the -s flag.""")
+The master queue runner lock could not be acquired.
+It appears as though there is a stale master lock.  Try re-running
+$program with the --force flag.""")
         else:
             # Hostname doesn't even match.
             assert status == WatcherState.host_mismatch, (
@@ -210,10 +212,10 @@ with the -s flag.""")
             # pylint: disable-msg=W0612
             hostname, pid, tempfile = get_lock_data()
             message = _("""\
-The master qrunner lock could not be acquired, because it appears
-as if some process on some other host may have acquired it.  We
-can't test for stale locks across host boundaries, so you'll have
-to clean this up manually.
+The master qrunner lock could not be acquired, because it
+appears as if some process on some other host may have acquired it.  We can't
+test for stale locks across host boundaries, so you'll have to clean this up
+manually.
 
 Lock file: $config.LOCK_FILE
 Lock host: $hostname
@@ -348,13 +350,17 @@ class Loop:
             return pid
         # Child.
         #
+        # Set the environment variable which tells the qrunner that it's
+        # running under bin/master control.  This subtly changes the error
+        # behavior of bin/qrunner.
+        os.environ['MAILMAN_UNDER_MASTER_CONTROL'] = '1'
         # Craft the command line arguments for the exec() call.
         rswitch = '--runner=' + spec
         # Wherever master lives, so too must live the qrunner script.
         exe = os.path.join(config.BIN_DIR, 'qrunner')
         # config.PYTHON, which is the absolute path to the Python interpreter,
         # must be given as argv[0] due to Python's library search algorithm.
-        args = [sys.executable, sys.executable, exe, rswitch, '-s']
+        args = [sys.executable, sys.executable, exe, rswitch]
         if self._config_file is not None:
             args.extend(['-C', self._config_file])
         log = logging.getLogger('mailman.qrunner')
diff --git a/src/mailman/bin/qrunner.py b/src/mailman/bin/qrunner.py
index d75eea4e5..ac8cd7dd3 100644
--- a/src/mailman/bin/qrunner.py
+++ b/src/mailman/bin/qrunner.py
@@ -84,7 +84,9 @@ Usage: %prog [options]
 names displayed by the -l switch.
 
 Normally, this script should be started from 'bin/mailman start'.  Running it
-separately or with -o is generally useful only for debugging.
+separately or with -o is generally useful only for debugging.  When run this
+way, the environment variable $MAILMAN_UNDER_MASTER_CONTROL will be set which
+subtly changes some error handling behavior.
 """)
 
     def add_options(self):
@@ -120,12 +122,6 @@ each queue runner runs indefinitely, until the process receives signal."""))
             '-v', '--verbose',
             default=0, action='count', help=_("""\
 Display more debugging information to the log file."""))
-        self.parser.add_option(
-            '-s', '--subproc',
-            default=False, action='store_true', help=_("""\
-This should only be used when running the queue runner as a subprocess of the
-'bin/mailman start' startup script.  It changes some of the exit-on-error
-behavior to work better with that framework."""))
 
     def sanity_check(self):
         """See `Options`."""
@@ -152,7 +148,7 @@ def make_qrunner(name, slice, range, once=False):
     try:
         qrclass = find_name(class_path)
     except ImportError as error:
-        if config.options.options.subproc:
+        if os.environ.get('MAILMAN_UNDER_MASTER_CONTROL') is not None:
             # Exit with SIGTERM exit code so the master watcher won't try to
             # restart us.
             print >> sys.stderr, _('Cannot import runner module: $class_path')
diff --git a/src/mailman/commands/cli_control.py b/src/mailman/commands/cli_control.py
index e6c9c83b3..83432a6e8 100644
--- a/src/mailman/commands/cli_control.py
+++ b/src/mailman/commands/cli_control.py
@@ -108,6 +108,9 @@ class Start:
         # call.
         os.setsid()
         # Instead of cd'ing to root, cd to the Mailman runtime directory.
+        # However, before we do that, set an environment variable used by the
+        # subprocesses to calculate their path to the $VAR_DIR.
+        os.environ['MAILMAN_VAR_DIR'] = config.VAR_DIR
         os.chdir(config.VAR_DIR)
         # Exec the master watcher.
         execl_args = [
diff --git a/src/mailman/config/config.py b/src/mailman/config/config.py
index 37effc77c..224f2a8ae 100644
--- a/src/mailman/config/config.py
+++ b/src/mailman/config/config.py
@@ -156,7 +156,14 @@ class Configuration:
         else:
             print >> sys.stderr, 'No path configuration found:', layout
             sys.exit(1)
-        # First, collect all variables in a substitution dictionary.
+        # First, collect all variables in a substitution dictionary.  $VAR_DIR
+        # is taken from the environment or from the configuration file if the
+        # environment is not set.  Because the var_dir setting in the config
+        # file could be a relative path, and because 'bin/mailman start'
+        # chdirs to $VAR_DIR, without this subprocesses bin/master and
+        # bin/qrunner will create $VAR_DIR hierarchies under $VAR_DIR when
+        # that path is relative.
+        var_dir = os.environ.get('MAILMAN_VAR_DIR', category.var_dir)
         substitutions = dict(
             argv                    = bin_dir,
             # Directories.
@@ -171,7 +178,7 @@ class Configuration:
             pipermail_private_dir   = category.pipermail_private_dir,
             pipermail_public_dir    = category.pipermail_public_dir,
             queue_dir               = category.queue_dir,
-            var_dir                 = category.var_dir,
+            var_dir                 = var_dir,
             # Files.
             creator_pw_file         = category.creator_pw_file,
             lock_file               = category.lock_file,
diff --git a/src/mailman/tests/test_configfile.py b/src/mailman/tests/test_configfile.py
index 95623a785..decac277b 100644
--- a/src/mailman/tests/test_configfile.py
+++ b/src/mailman/tests/test_configfile.py
@@ -77,7 +77,7 @@ class TestConfigFileBase(unittest.TestCase):
     def setUp(self):
         self._root = tempfile.mkdtemp()
         # Ensure that the environment can't cause test failures.
-        self.mailman_config_file = os.environ('MAILMAN_CONFIG_FILE')
+        self.mailman_config_file = os.environ.get('MAILMAN_CONFIG_FILE')
         if self.mailman_config_file is not None:
             del os.environ['MAILMAN_CONFIG_FILE']