launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/simplify-mailman-pythonpath into lp:launchpad.

Commit message:
Link Mailman's Python modules into lib/ rather than adding lib/mailman/ to sys.path.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/simplify-mailman-pythonpath/+merge/323912

This allows us to remove a bit more complexity from our buildout configuration, and thus avoid having to carry it over to pip.

Although I removed an XXX comment related to bug 375751 since it no longer had an obvious home, I haven't actually fixed that bug, merely slightly rearranged how the installation is linked into place.

I considered just committing lib/Mailman as a dangling symlink, but decided to create it dynamically as a small concession to people trying to check out the Launchpad tree on case-insensitive file systems (although obviously they still won't be able to build it, but I don't care).
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/simplify-mailman-pythonpath into lp:launchpad.

=== modified file '.bzrignore'
--- .bzrignore	2017-05-08 12:39:30 +0000
+++ .bzrignore	2017-05-11 14:21:08 +0000
@@ -12,6 +12,7 @@
 database/schema/diagrams/*.dot
 thread*.request
 ./TAGS
+lib/Mailman
 lib/mailman
 TAGS
 cover.txt

=== modified file 'Makefile'
--- Makefile	2017-05-08 12:39:30 +0000
+++ Makefile	2017-05-11 14:21:08 +0000
@@ -5,7 +5,7 @@
 
 WD:=$(shell pwd)
 PY=$(WD)/bin/py
-PYTHONPATH:=$(WD)/lib:$(WD)/lib/mailman:${PYTHONPATH}
+PYTHONPATH:=$(WD)/lib:${PYTHONPATH}
 BUILDOUT_CFG=buildout.cfg
 VERBOSITY=-vv
 
@@ -358,6 +358,7 @@
 ifdef LP_MAKE_KEEP_MAILMAN
 	@echo "Keeping previously built mailman."
 else
+	$(RM) lib/Mailman
 	$(RM) -r lib/mailman
 endif
 

=== modified file 'buildmailman.py'
--- buildmailman.py	2013-01-07 03:29:28 +0000
+++ buildmailman.py	2017-05-11 14:21:08 +0000
@@ -3,6 +3,8 @@
 # Copyright 2009, 2010 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from __future__ import print_function
+
 import errno
 import grp
 import os
@@ -37,18 +39,9 @@
     # If we can import the package, we assume Mailman is properly built at
     # the least.  This does not catch re-installs that might be necessary
     # should our copy in sourcecode be updated.  Do that manually.
-    sys.path.append(mailman_path)
     try:
         import Mailman
     except ImportError:
-        # sys.path_importer_cache is a mapping of elements of sys.path to
-        # importer objects used to handle them. In Python2.5+ when an element
-        # of sys.path is found to not exist on disk, a NullImporter is created
-        # and cached - this causes Python to never bother re-inspecting the
-        # disk for that path element. We must clear that cache element so that
-        # our second attempt to import MailMan after building it will actually
-        # check the disk.
-        del sys.path_importer_cache[mailman_path]
         need_build = need_install = True
     else:
         need_build = need_install = False
@@ -69,12 +62,12 @@
     try:
         uid = pwd.getpwnam(user).pw_uid
     except KeyError:
-        print >> sys.stderr, 'No user found:', user
+        print('No user found:', user, file=sys.stderr)
         sys.exit(1)
     try:
         gid = grp.getgrnam(group).gr_gid
     except KeyError:
-        print >> sys.stderr, 'No group found:', group
+        print('No group found:', group, file=sys.stderr)
         sys.exit(1)
 
     # Ensure that the var_dir exists, is owned by the user:group, and has
@@ -90,7 +83,7 @@
         # Just created the var directory, will need to install mailmain bits.
         need_install = True
     os.chown(var_dir, uid, gid)
-    os.chmod(var_dir, 02775)
+    os.chmod(var_dir, 0o2775)
 
     # Skip mailman setup if nothing so far has shown a reinstall needed.
     if not need_install:
@@ -120,22 +113,30 @@
         # Configure.
         retcode = subprocess.call(configure_args, cwd=mailman_source)
         if retcode:
-            print >> sys.stderr, 'Could not configure Mailman:'
+            print('Could not configure Mailman:', file=sys.stderr)
             sys.exit(retcode)
         # Make.
         retcode = subprocess.call(('make', ), cwd=mailman_source)
         if retcode:
-            print >> sys.stderr, 'Could not make Mailman.'
+            print('Could not make Mailman.', file=sys.stderr)
             sys.exit(retcode)
     retcode = subprocess.call(('make', 'install'), cwd=mailman_source)
     if retcode:
-        print >> sys.stderr, 'Could not install Mailman.'
+        print('Could not install Mailman.', file=sys.stderr)
         sys.exit(retcode)
+    # Symlink Mailman's Python modules into the import path.
+    try:
+        os.unlink(os.path.join('lib', 'Mailman'))
+    except OSError as e:
+        if e.errno != errno.ENOENT:
+            raise
+    os.symlink(
+        os.path.join('mailman', 'Mailman'), os.path.join('lib', 'Mailman'))
     # Try again to import the package.
     try:
         import Mailman
     except ImportError:
-        print >> sys.stderr, 'Could not import the Mailman package'
+        print('Could not import the Mailman package', file=sys.stderr)
         return 1
 
     # Check to see if the site list exists.  The output can go to /dev/null
@@ -164,13 +165,13 @@
              addr, password),
             cwd=mailman_bin)
         if retcode:
-            print >> sys.stderr, 'Could not create site list'
+            print('Could not create site list', file=sys.stderr)
             return retcode
 
     retcode = configure_site_list(
         mailman_bin, Mailman.mm_cfg.MAILMAN_SITE_LIST)
     if retcode:
-        print >> sys.stderr, 'Could not configure site list'
+        print('Could not configure site list', file=sys.stderr)
         return retcode
 
     # Create a directory to hold the gzip'd tarballs for the directories of
@@ -195,7 +196,7 @@
         os.close(fd)
         config_file = open(config_file_name, 'w')
         try:
-            print >> config_file, 'advertised = False'
+            print('advertised = False', file=config_file)
         finally:
             config_file.close()
         return subprocess.call(

=== modified file 'buildout.cfg'
--- buildout.cfg	2017-05-08 12:39:30 +0000
+++ buildout.cfg	2017-05-11 14:21:08 +0000
@@ -44,10 +44,6 @@
     lpjsmin
     jsautobuild
     lazr.jobrunner
-# XXX gary 2009-5-12 bug 375751:
-# Make mailman built and installed in a more normal way.
-extra-paths =
-    ${buildout:directory}/lib/mailman
 include-site-packages = true
 allowed-eggs-from-site-packages =
 interpreter = py
@@ -64,7 +60,6 @@
 recipe = z3c.recipe.scripts
 eggs = ${scripts:eggs}
      ipython
-extra-paths = ${scripts:extra-paths}
 include-site-packages = true
 allowed-eggs-from-site-packages =
 initialization = ${scripts:initialization}