Rev 5929: (vila) Thou should not fill stderr pipe while in a subprocess (Andrew in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5929 [merge]
revision-id: pqm at pqm.ubuntu.com-20110527171203-s50ymd0mviaw4c7k
parent: pqm at pqm.ubuntu.com-20110527155502-5eo2hhxcggumdfrt
parent: v.ladeuil+lp at free.fr-20110527145702-8glz4twxmeer1ygq
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Fri 2011-05-27 17:12:03 +0000
message:
  (vila) Thou should not fill stderr pipe while in a subprocess (Andrew
   Bennetts)
modified:
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_import_tariff.py test_import_tariff.p-20100207155145-ff9infp7goncs7zh-1
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2011-05-26 08:05:45 +0000
+++ b/bzrlib/tests/__init__.py	2011-05-27 02:22:45 +0000
@@ -2013,7 +2013,7 @@
     def start_bzr_subprocess(self, process_args, env_changes=None,
                              skip_if_plan_to_signal=False,
                              working_dir=None,
-                             allow_plugins=False):
+                             allow_plugins=False, stderr=subprocess.PIPE):
         """Start bzr in a subprocess for testing.
 
         This starts a new Python interpreter and runs bzr in there.
@@ -2031,6 +2031,9 @@
         :param skip_if_plan_to_signal: raise TestSkipped when true and system
             doesn't support signalling subprocesses.
         :param allow_plugins: If False (default) pass --no-plugins to bzr.
+        :param stderr: file to use for the subprocess's stderr.  Valid values
+            are those valid for the stderr argument of `subprocess.Popen`.
+            Default value is ``subprocess.PIPE``.
 
         :returns: Popen object for the started process.
         """
@@ -2071,7 +2074,7 @@
             command.extend(process_args)
             process = self._popen(command, stdin=subprocess.PIPE,
                                   stdout=subprocess.PIPE,
-                                  stderr=subprocess.PIPE)
+                                  stderr=stderr)
         finally:
             restore_environment()
             if cwd is not None:

=== modified file 'bzrlib/tests/test_import_tariff.py'
--- a/bzrlib/tests/test_import_tariff.py	2011-05-03 23:16:56 +0000
+++ b/bzrlib/tests/test_import_tariff.py	2011-05-27 14:50:58 +0000
@@ -62,7 +62,7 @@
             self.preserved_env_vars[name] = os.environ.get(name)
         super(TestImportTariffs, self).setUp()
 
-    def start_bzr_subprocess_with_import_check(self, args):
+    def start_bzr_subprocess_with_import_check(self, args, stderr_file=None):
         """Run a bzr process and capture the imports.
 
         This is fairly expensive because we start a subprocess, so we aim to
@@ -79,8 +79,13 @@
         # explicitly do want to test against things installed there, therefore
         # we pass it through.
         env_changes = dict(PYTHONVERBOSE='1', **self.preserved_env_vars)
-        return self.start_bzr_subprocess(args, env_changes=env_changes,
-            allow_plugins=(not are_plugins_disabled()))
+        kwargs = dict(env_changes=env_changes,
+                      allow_plugins=(not are_plugins_disabled()))
+        if stderr_file:
+            # We don't want to update the whole call chain so we insert stderr
+            # *iff* we need to
+            kwargs['stderr'] = stderr_file
+        return self.start_bzr_subprocess(args, **kwargs)
 
     def check_forbidden_modules(self, err, forbidden_imports):
         """Check for forbidden modules in stderr.
@@ -187,8 +192,11 @@
     def test_simple_serve(self):
         # 'serve' in a default format working tree shouldn't need many modules
         tree = self.make_branch_and_tree('.')
+        # Capture the bzr serve process' stderr in a file to avoid deadlocks
+        # while the smart client interacts with it.
+        stderr_file = open('bzr-serve.stderr', 'w')
         process = self.start_bzr_subprocess_with_import_check(['serve',
-            '--inet', '-d', tree.basedir])
+            '--inet', '-d', tree.basedir], stderr_file=stderr_file)
         url = 'bzr://localhost/'
         self.permit_url(url)
         client_medium = medium.SmartSimplePipesClientMedium(
@@ -200,6 +208,9 @@
         process.stdin = None
         (out, err) = self.finish_bzr_subprocess(process,
             universal_newlines=False)
+        stderr_file.close()
+        with open('bzr-serve.stderr', 'r') as stderr_file:
+            err = stderr_file.read()
         self.check_forbidden_modules(err,
             ['bzrlib.annotate',
             'bzrlib.atomicfile',

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-05-27 15:55:02 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-05-27 17:12:03 +0000
@@ -78,6 +78,12 @@
    suite.  This can include new facilities for writing tests, fixes to 
    spurious test failures and changes to the way things should be tested.
 
+* Fix deadlock in `TestImportTariffs.test_simple_serve` when stderr gets
+  more output than fits in the default buffer.  This was happening on the
+  Windows buildslave, and could easily happen in other circumstances where
+  the default OS buffer size for pipes is small or the ``python -v``
+  output is large.  (Andrew Bennetts, #784802)
+
 
 bzr 2.4b3
 #########