launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/build-twisted-plugin-cache into lp:launchpad.

Commit message:
Build dropin.cache for all installed Twisted plugins in "make compile", and merge txlongpoll and txpkgupload back into the scripts recipe.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/build-twisted-plugin-cache/+merge/331240

If an ampoule subprocess finds itself needing to build dropin.cache for a Twisted plugin, and that plugin imports twisted.internet.reactor at some point and thus installs a default reactor, then twisted.application.reactors.installReactor (called by the ampoule bootstrap code) will crash with ReactorAlreadyInstalledError in twisted.internet.main.installReactor.  This will then proceed to ruin everyone's day with incomprehensible buildbot failures.

https://twistedmatrix.com/documents/current/core/howto/plugin.html#plugin-caching recommends that we build the cache files when installing packages, and it certainly seems like a good idea not to rely on writing files into eggs at run-time.

With this, I believe we can go back to the merged recipes, and thus remove a blocker to conversion to pip.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/build-twisted-plugin-cache into lp:launchpad.

=== modified file 'Makefile'
--- Makefile	2017-09-04 12:35:40 +0000
+++ Makefile	2017-09-23 03:20:32 +0000
@@ -47,7 +47,8 @@
 # NB: It's important BUILDOUT_BIN only mentions things genuinely produced by
 # buildout.
 BUILDOUT_BIN = \
-    $(PY) bin/apiindex bin/bzr bin/combine-css bin/fl-build-report \
+    $(PY) bin/apiindex bin/build-twisted-plugin-cache bin/bzr \
+    bin/combine-css bin/fl-build-report \
     bin/fl-credential-ctl bin/fl-install-demo bin/fl-monitor-ctl \
     bin/fl-record bin/fl-run-bench bin/fl-run-test bin/googletestservice \
     bin/harness bin/iharness bin/ipy bin/jsbuild bin/lpjsmin\
@@ -236,6 +237,7 @@
 compile: $(PY) $(VERSION_INFO)
 	${SHHH} $(MAKE) -C sourcecode build PYTHON=${PYTHON} \
 	    LPCONFIG=${LPCONFIG}
+	${SHHH} bin/build-twisted-plugin-cache
 	${SHHH} LPCONFIG=${LPCONFIG} ${PY} -t buildmailman.py
 
 test_build: build

=== modified file 'buildout.cfg'
--- buildout.cfg	2017-09-18 10:08:40 +0000
+++ buildout.cfg	2017-09-23 03:20:32 +0000
@@ -5,8 +5,6 @@
 parts =
     scripts
     tags
-    txlongpoll
-    txpkgupload
 unzip = true
 eggs-directory = eggs
 download-cache = download-cache
@@ -53,21 +51,3 @@
 [tags]
 recipe = z3c.recipe.tag:tags
 eggs = lp
-
-[txlongpoll]
-recipe = z3c.recipe.scripts
-eggs = ${scripts:eggs}
-    txlongpoll
-include-site-packages = false
-initialization = ${scripts:initialization}
-entry-points = twistd-for-txlongpoll=twisted.scripts.twistd:run
-scripts = twistd-for-txlongpoll
-
-[txpkgupload]
-recipe = z3c.recipe.scripts
-eggs = ${scripts:eggs}
-    txpkgupload
-include-site-packages = false
-initialization = ${scripts:initialization}
-entry-points = twistd-for-txpkgupload=twisted.scripts.twistd:run
-scripts = twistd-for-txpkgupload

=== modified file 'lib/lp/scripts/runlaunchpad.py'
--- lib/lp/scripts/runlaunchpad.py	2017-09-18 09:17:21 +0000
+++ lib/lp/scripts/runlaunchpad.py	2017-09-23 03:20:32 +0000
@@ -246,8 +246,7 @@
         return config.txlongpoll.launch
 
     def launch(self):
-        twistd_bin = os.path.join(
-            config.root, 'bin', 'twistd-for-txlongpoll')
+        twistd_bin = os.path.join(config.root, 'bin', 'twistd')
         broker_hostname, broker_port = as_host_port(
             config.rabbitmq.host, None, None)
         self.server = TxLongPollServer(

=== added file 'lib/lp/services/twistedsupport/plugincache.py'
--- lib/lp/services/twistedsupport/plugincache.py	1970-01-01 00:00:00 +0000
+++ lib/lp/services/twistedsupport/plugincache.py	2017-09-23 03:20:32 +0000
@@ -0,0 +1,24 @@
+# Copyright 2017 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Build dropin.cache for all installed Twisted plugins.
+
+This would be built on the fly if we didn't do it here, but we want to make
+sure to build it in a predictable environment.  In particular, if a plugin's
+cache is first built as a result of being run via ampoule, then ampoule will
+fail if any part of the process of importing the plugin installs a default
+reactor.
+"""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+
+from twisted.plugin import (
+    getPlugins,
+    IPlugin,
+    )
+
+
+def main():
+    list(getPlugins(IPlugin))

=== modified file 'lib/lp/services/txlongpoll/tests/test_server.py'
--- lib/lp/services/txlongpoll/tests/test_server.py	2017-09-18 09:17:21 +0000
+++ lib/lp/services/txlongpoll/tests/test_server.py	2017-09-23 03:20:32 +0000
@@ -22,8 +22,7 @@
     def test_service_config(self):
         # TxLongPollServer pokes some .ini configuration into its
         # service_config attributes.
-        twistd_bin = os.path.join(
-            config.root, 'bin', 'twistd-for-txlongpoll')
+        twistd_bin = os.path.join(config.root, 'bin', 'twistd')
         fixture = self.useFixture(TxLongPollServer(
             broker_user='guest', broker_password='guest', broker_vhost='/',
             broker_port=123, frontend_port=None,

=== modified file 'setup.py'
--- setup.py	2017-09-18 10:08:40 +0000
+++ setup.py	2017-09-23 03:20:32 +0000
@@ -104,7 +104,9 @@
         'transaction',
         'Twisted',
         'txfixtures',
+        'txlongpoll',
         'txlongpollfixture',
+        'txpkgupload',
         'wadllib',
         'z3c.pt',
         'z3c.ptcompat',
@@ -167,6 +169,8 @@
     entry_points=dict(
         console_scripts=[  # `console_scripts` is a magic name to setuptools
             'apiindex = lp.scripts.utilities.apiindex:main',
+            'build-twisted-plugin-cache = '
+                'lp.services.twistedsupport.plugincache:main',
             'bzr = lp.scripts.utilities.bzr:main',
             'combine-css = lp.scripts.utilities.js.combinecss:main',
             'googletestservice = '

=== modified file 'utilities/start-dev-soyuz.sh'
--- utilities/start-dev-soyuz.sh	2017-09-18 10:08:40 +0000
+++ utilities/start-dev-soyuz.sh	2017-09-23 03:20:32 +0000
@@ -20,7 +20,7 @@
     plugin=$2
     shift 2
     echo "Starting $name."
-    "bin/twistd-for-$name" \
+    bin/twistd \
         --logfile "/var/tmp/development-$name.log" \
         --pidfile "/var/tmp/development-$name.pid" \
         "$plugin" "$@"