From 9c4f44f70acadd70fcdb0a0d5d66cefbc9da20bf Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 15 Mar 2011 14:56:39 -0400 Subject: [PATCH 01/11] Fix issue #11432. if the stdin pipe is the same file descriptor as either stdout or stderr in the _posixsubprocess C extension module it would unintentionally close the fds and raise an error. --- Modules/_posixsubprocess.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 0f85da97cea..bf10cbb43e1 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -99,10 +99,10 @@ static void child_exec(char *const exec_array[], if (p2cread > 2) { POSIX_CALL(close(p2cread)); } - if (c2pwrite > 2) { + if (c2pwrite > 2 && c2pwrite != p2cread) { POSIX_CALL(close(c2pwrite)); } - if (errwrite != c2pwrite && errwrite > 2) { + if (errwrite != c2pwrite && errwrite != p2cread && errwrite > 2) { POSIX_CALL(close(errwrite)); } From e14e9c2218dc449a374a31a5bc4f9e1a82ef61fa Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 15 Mar 2011 14:55:17 -0400 Subject: [PATCH 02/11] Add unittests demonstrating issue #11432. --- Lib/test/test_subprocess.py | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 73f44ad93af..46e50c350bf 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -3,6 +3,7 @@ import subprocess import sys import signal +import io import os import errno import tempfile @@ -1186,6 +1187,24 @@ def test_pass_fds(self): close_fds=False, pass_fds=(fd, ))) self.assertIn('overriding close_fds', str(context.warning)) + def test_stdout_stdin_are_single_inout_fd(self): + with io.open(os.devnull, "r+") as inout: + p = subprocess.Popen([sys.executable, "-c", "import sys; sys.exit(0)"], + stdout=inout, stdin=inout) + p.wait() + + def test_stdout_stderr_are_single_inout_fd(self): + with io.open(os.devnull, "r+") as inout: + p = subprocess.Popen([sys.executable, "-c", "import sys; sys.exit(0)"], + stdout=inout, stderr=inout) + p.wait() + + def test_stderr_stdin_are_single_inout_fd(self): + with io.open(os.devnull, "r+") as inout: + p = subprocess.Popen([sys.executable, "-c", "import sys; sys.exit(0)"], + stderr=inout, stdin=inout) + p.wait() + def test_wait_when_sigchild_ignored(self): # NOTE: sigchild_ignore.py may not be an effective test on all OSes. sigchild_ignore = support.findfile("sigchild_ignore.py", @@ -1458,19 +1477,6 @@ def test_invalid_args(self): raise c.exception -def test_main(): - unit_tests = (ProcessTestCase, - POSIXProcessTestCase, - Win32ProcessTestCase, - ProcessTestCasePOSIXPurePython, - CommandTests, - ProcessTestCaseNoPoll, - HelperFunctionTests, - CommandsWithSpaces, - ContextManagerTests) - - support.run_unittest(*unit_tests) - support.reap_children() - if __name__ == "__main__": - test_main() + unittest.main() + support.reap_children() From 8f7724f9a4ec57d689a436a064e2e047b2ad0d97 Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 15 Mar 2011 15:24:43 -0400 Subject: [PATCH 03/11] merge d71476b9a55d from tip, use start_new_session instead of os.setsid. --- Lib/webbrowser.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/Lib/webbrowser.py b/Lib/webbrowser.py index e369acb1df6..415f12ac5f3 100644 --- a/Lib/webbrowser.py +++ b/Lib/webbrowser.py @@ -228,15 +228,9 @@ def _invoke(self, args, remote, autoraise): else: # for TTY browsers, we need stdin/out inout = None - # if possible, put browser in separate process group, so - # keyboard interrupts don't affect browser as well as Python - setsid = getattr(os, 'setsid', None) - if not setsid: - setsid = getattr(os, 'setpgrp', None) - p = subprocess.Popen(cmdline, close_fds=True, stdin=inout, stdout=(self.redirect_stdout and inout or None), - stderr=inout, preexec_fn=setsid) + stderr=inout, start_new_session=True) if remote: # wait five secons. If the subprocess is not finished, the # remote invocation has (hopefully) started a new instance. From 961e0e85c007677ba4406382d6951a39487d440c Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 15 Mar 2011 15:43:39 -0400 Subject: [PATCH 04/11] revert the test_main() change from 08daf3ef6509 so that regrtest continues to run this properly. --- Lib/test/test_subprocess.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 46e50c350bf..3cc387b6a3d 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1477,6 +1477,19 @@ def test_invalid_args(self): raise c.exception +def test_main(): + unit_tests = (ProcessTestCase, + POSIXProcessTestCase, + Win32ProcessTestCase, + ProcessTestCasePOSIXPurePython, + CommandTests, + ProcessTestCaseNoPoll, + HelperFunctionTests, + CommandsWithSpaces, + ContextManagerTests) + + support.run_unittest(*unit_tests) + support.reap_children() + if __name__ == "__main__": unittest.main() - support.reap_children() From 28edfeedb5fc4fc64190cfcc0d32494a8a84c4cb Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 15 Mar 2011 16:02:10 -0400 Subject: [PATCH 05/11] issue 11432 news entry. --- Misc/NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Misc/NEWS b/Misc/NEWS index adc719bf2f5..c1100650352 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,10 @@ What's New in Python 3.2.1? Core and Builtins ----------------- +- Issue #11432: A bug was introduced in subprocess.Popen on posix systems with + 3.2.0 where the stdout or stderr file descriptor being the same as the stdin + file descriptor would raise an exception. webbrowser.open would fail. fixed. + - Issue #11450: Don't truncate hg version info in Py_GetBuildInfo() when there are many tags (e.g. when using mq). Patch by Nadeem Vawda. From 2c50a09ac47e701768a4e20f8a03d17263e8db8f Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 15 Mar 2011 21:02:59 +0100 Subject: [PATCH 06/11] On behalf of Tarek: Issue #11501: disutils.archive_utils.make_zipfile no longer fails if zlib is not installed. Instead, the zipfile.ZIP_STORED compression is used to create the ZipFile. Patch by Natalia B. Bidart. --- Lib/distutils/archive_util.py | 19 +++++++---- Lib/distutils/tests/test_archive_util.py | 42 +++++++++++++++++++++--- Lib/distutils/tests/test_bdist_dumb.py | 8 +++++ Lib/distutils/tests/test_sdist.py | 15 ++++++++- Lib/test/support.py | 33 +++++++++++++++++++ Misc/ACKS | 1 + Misc/NEWS | 4 +++ 7 files changed, 110 insertions(+), 12 deletions(-) diff --git a/Lib/distutils/archive_util.py b/Lib/distutils/archive_util.py index 6dd0445dbe6..c06eba351d8 100644 --- a/Lib/distutils/archive_util.py +++ b/Lib/distutils/archive_util.py @@ -9,6 +9,12 @@ from warnings import warn import sys +try: + import zipfile +except ImportError: + zipfile = None + + from distutils.errors import DistutilsExecError from distutils.spawn import spawn from distutils.dir_util import mkpath @@ -74,11 +80,6 @@ def make_zipfile(base_name, base_dir, verbose=0, dry_run=0): available, raises DistutilsExecError. Returns the name of the output zip file. """ - try: - import zipfile - except ImportError: - zipfile = None - zip_filename = base_name + ".zip" mkpath(os.path.dirname(zip_filename), dry_run=dry_run) @@ -105,8 +106,12 @@ def make_zipfile(base_name, base_dir, verbose=0, dry_run=0): zip_filename, base_dir) if not dry_run: - zip = zipfile.ZipFile(zip_filename, "w", - compression=zipfile.ZIP_DEFLATED) + try: + zip = zipfile.ZipFile(zip_filename, "w", + compression=zipfile.ZIP_DEFLATED) + except RuntimeError: + zip = zipfile.ZipFile(zip_filename, "w", + compression=zipfile.ZIP_STORED) for dirpath, dirnames, filenames in os.walk(base_dir): for name in filenames: diff --git a/Lib/distutils/tests/test_archive_util.py b/Lib/distutils/tests/test_archive_util.py index aff426521d9..f9698495903 100644 --- a/Lib/distutils/tests/test_archive_util.py +++ b/Lib/distutils/tests/test_archive_util.py @@ -7,12 +7,13 @@ from os.path import splitdrive import warnings +from distutils import archive_util from distutils.archive_util import (check_archive_formats, make_tarball, make_zipfile, make_archive, ARCHIVE_FORMATS) from distutils.spawn import find_executable, spawn from distutils.tests import support -from test.support import check_warnings, run_unittest +from test.support import check_warnings, run_unittest, patch try: import zipfile @@ -20,10 +21,18 @@ except ImportError: ZIP_SUPPORT = find_executable('zip') +try: + import zlib + ZLIB_SUPPORT = True +except ImportError: + ZLIB_SUPPORT = False + + class ArchiveUtilTestCase(support.TempdirManager, support.LoggingSilencer, unittest.TestCase): + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_make_tarball(self): # creating something to tar tmpdir = self.mkdtemp() @@ -84,8 +93,9 @@ def _create_files(self): base_name = os.path.join(tmpdir2, 'archive') return tmpdir, tmpdir2, base_name - @unittest.skipUnless(find_executable('tar') and find_executable('gzip'), - 'Need the tar command to run') + @unittest.skipUnless(find_executable('tar') and find_executable('gzip') + and ZLIB_SUPPORT, + 'Need the tar, gzip and zlib command to run') def test_tarfile_vs_tar(self): tmpdir, tmpdir2, base_name = self._create_files() old_dir = os.getcwd() @@ -169,7 +179,8 @@ def test_compress_deprecated(self): self.assertTrue(not os.path.exists(tarball)) self.assertEqual(len(w.warnings), 1) - @unittest.skipUnless(ZIP_SUPPORT, 'Need zip support to run') + @unittest.skipUnless(ZIP_SUPPORT and ZLIB_SUPPORT, + 'Need zip and zlib support to run') def test_make_zipfile(self): # creating something to tar tmpdir = self.mkdtemp() @@ -182,6 +193,29 @@ def test_make_zipfile(self): # check if the compressed tarball was created tarball = base_name + '.zip' + self.assertTrue(os.path.exists(tarball)) + + @unittest.skipUnless(ZIP_SUPPORT, 'Need zip support to run') + def test_make_zipfile_no_zlib(self): + patch(self, archive_util.zipfile, 'zlib', None) # force zlib ImportError + + called = [] + zipfile_class = zipfile.ZipFile + def fake_zipfile(*a, **kw): + if kw.get('compression', None) == zipfile.ZIP_STORED: + called.append((a, kw)) + return zipfile_class(*a, **kw) + + patch(self, archive_util.zipfile, 'ZipFile', fake_zipfile) + + # create something to tar and compress + tmpdir, tmpdir2, base_name = self._create_files() + make_zipfile(base_name, tmpdir) + + tarball = base_name + '.zip' + self.assertEqual(called, + [((tarball, "w"), {'compression': zipfile.ZIP_STORED})]) + self.assertTrue(os.path.exists(tarball)) def test_check_archive_formats(self): self.assertEqual(check_archive_formats(['gztar', 'xxx', 'zip']), diff --git a/Lib/distutils/tests/test_bdist_dumb.py b/Lib/distutils/tests/test_bdist_dumb.py index cc37fef8b01..55ba58d14fa 100644 --- a/Lib/distutils/tests/test_bdist_dumb.py +++ b/Lib/distutils/tests/test_bdist_dumb.py @@ -18,6 +18,13 @@ """ +try: + import zlib + ZLIB_SUPPORT = True +except ImportError: + ZLIB_SUPPORT = False + + class BuildDumbTestCase(support.TempdirManager, support.LoggingSilencer, support.EnvironGuard, @@ -34,6 +41,7 @@ def tearDown(self): sys.argv[:] = self.old_sys_argv[1] super(BuildDumbTestCase, self).tearDown() + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_simple_built(self): # let's create a simple package diff --git a/Lib/distutils/tests/test_sdist.py b/Lib/distutils/tests/test_sdist.py index 655e50dcfc2..eaf39a45bae 100644 --- a/Lib/distutils/tests/test_sdist.py +++ b/Lib/distutils/tests/test_sdist.py @@ -40,6 +40,13 @@ somecode%(sep)sdoc.txt """ +try: + import zlib + ZLIB_SUPPORT = True +except ImportError: + ZLIB_SUPPORT = False + + class SDistTestCase(PyPIRCCommandTestCase): def setUp(self): @@ -78,6 +85,7 @@ def _warn(*args): cmd.warn = _warn return dist, cmd + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_prune_file_list(self): # this test creates a package with some vcs dirs in it # and launch sdist to make sure they get pruned @@ -119,6 +127,7 @@ def test_prune_file_list(self): # making sure everything has been pruned correctly self.assertEqual(len(content), 4) + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_make_distribution(self): # check if tar and gzip are installed @@ -153,6 +162,7 @@ def test_make_distribution(self): result.sort() self.assertEqual(result, ['fake-1.0.tar', 'fake-1.0.tar.gz']) + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_add_defaults(self): # http://bugs.python.org/issue2279 @@ -218,6 +228,7 @@ def test_add_defaults(self): finally: f.close() + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_metadata_check_option(self): # testing the `medata-check` option dist, cmd = self.get_cmd(metadata={}) @@ -277,7 +288,7 @@ def test_finalize_options(self): cmd.formats = 'supazipa' self.assertRaises(DistutilsOptionError, cmd.finalize_options) - + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_get_file_list(self): # make sure MANIFEST is recalculated dist, cmd = self.get_cmd() @@ -318,6 +329,7 @@ def test_get_file_list(self): self.assertEqual(len(manifest2), 6) self.assertIn('doc2.txt', manifest2[-1]) + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_manifest_marker(self): # check that autogenerated MANIFESTs have a marker dist, cmd = self.get_cmd() @@ -334,6 +346,7 @@ def test_manifest_marker(self): self.assertEqual(manifest[0], '# file GENERATED by distutils, do NOT edit') + @unittest.skipUnless(ZLIB_SUPPORT, 'Need zlib support to run') def test_manual_manifest(self): # check that a MANIFEST without a marker is left alone dist, cmd = self.get_cmd() diff --git a/Lib/test/support.py b/Lib/test/support.py index 9fb3ee00c10..bef7161e9a1 100644 --- a/Lib/test/support.py +++ b/Lib/test/support.py @@ -1096,3 +1096,36 @@ def strip_python_stderr(stderr): """ stderr = re.sub(br"\[\d+ refs\]\r?\n?$", b"", stderr).strip() return stderr + +def patch(test_instance, object_to_patch, attr_name, new_value): + """Override 'object_to_patch'.'attr_name' with 'new_value'. + + Also, add a cleanup procedure to 'test_instance' to restore + 'object_to_patch' value for 'attr_name'. + The 'attr_name' should be a valid attribute for 'object_to_patch'. + + """ + # check that 'attr_name' is a real attribute for 'object_to_patch' + # will raise AttributeError if it does not exist + getattr(object_to_patch, attr_name) + + # keep a copy of the old value + attr_is_local = False + try: + old_value = object_to_patch.__dict__[attr_name] + except (AttributeError, KeyError): + old_value = getattr(object_to_patch, attr_name, None) + else: + attr_is_local = True + + # restore the value when the test is done + def cleanup(): + if attr_is_local: + setattr(object_to_patch, attr_name, old_value) + else: + delattr(object_to_patch, attr_name) + + test_instance.addCleanup(cleanup) + + # actually override the attribute + setattr(object_to_patch, attr_name, new_value) diff --git a/Misc/ACKS b/Misc/ACKS index fe80c8bbdfe..c7fd2ff9a03 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -72,6 +72,7 @@ Eric Beser Steven Bethard Stephen Bevan Ron Bickers +Natalia B. Bidart David Binger Dominic Binks Philippe Biondi diff --git a/Misc/NEWS b/Misc/NEWS index d4ce939119f..c4627fac66a 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -40,6 +40,10 @@ Core and Builtins Library ------- +- Issue #11501: disutils.archive_utils.make_zipfile no longer fails if zlib is + not installed. Instead, the zipfile.ZIP_STORED compression is used to create + the ZipFile. Patch by Natalia B. Bidart. + - Issue #11491: dbm.error is no longer raised when dbm.open is called with the "n" as the flag argument and the file exists. The behavior matches the documentation and general logic. From ec4b44b2dac099bd3b3806be3f5d5b550c4356dc Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 15 Mar 2011 15:03:13 -0500 Subject: [PATCH 07/11] make this subversion artifact empty --- Include/patchlevel.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Include/patchlevel.h b/Include/patchlevel.h index 61d898cc5a6..58a79fef7fd 100644 --- a/Include/patchlevel.h +++ b/Include/patchlevel.h @@ -26,8 +26,9 @@ #define PY_VERSION "3.1.3+" /*--end constants--*/ -/* Subversion Revision number of this file (not of the repository) */ -#define PY_PATCHLEVEL_REVISION "$Revision$" +/* Subversion Revision number of this file (not of the repository). Empty + since Mercurial migration. */ +#define PY_PATCHLEVEL_REVISION "" /* Version as a single 4-byte hex number, e.g. 0x010502B2 == 1.5.2b2. Use this for numeric comparisons, e.g. #if PY_VERSION_HEX >= ... */ From 485119eb1e351f1f142a06275c6b3b3182c38bf4 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 15 Mar 2011 15:07:20 -0500 Subject: [PATCH 08/11] kill PY_PATCHLEVEL_REVISION --- Include/patchlevel.h | 4 ---- Misc/NEWS | 5 +++++ 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Include/patchlevel.h b/Include/patchlevel.h index 116be891d86..d68443e0a8a 100644 --- a/Include/patchlevel.h +++ b/Include/patchlevel.h @@ -26,10 +26,6 @@ #define PY_VERSION "3.3a0" /*--end constants--*/ -/* Subversion Revision number of this file (not of the repository). Empty - since Mercurial migration. */ -#define PY_PATCHLEVEL_REVISION "" - /* Version as a single 4-byte hex number, e.g. 0x010502B2 == 1.5.2b2. Use this for numeric comparisons, e.g. #if PY_VERSION_HEX >= ... */ #define PY_VERSION_HEX ((PY_MAJOR_VERSION << 24) | \ diff --git a/Misc/NEWS b/Misc/NEWS index 018c62d8c8b..168aed8635a 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -237,6 +237,11 @@ Tests - Issue #10990: Prevent tests from clobbering a set trace function. +C-API +----- + +- PY_PATCHLEVEL_REVISION has been removed, since it's meaningless with Mercurial. + What's New in Python 3.2? ========================= From 126848a2d8e7b22401d9883549005d6b46339d91 Mon Sep 17 00:00:00 2001 From: Antoine Pitrou Date: Tue, 15 Mar 2011 21:17:10 +0100 Subject: [PATCH 09/11] Fix whitespace in test_subprocess --- Lib/test/test_subprocess.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index c2a808c004b..c781904ed9c 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -558,7 +558,7 @@ def test_leaking_fds_on_error(self): # Windows raises IOError except (IOError, OSError) as err: # ignore errors that indicate the command was not found - if err.errno not in (errno.ENOENT, errno.EACCES): + if err.errno not in (errno.ENOENT, errno.EACCES): raise def test_issue8780(self): From c217a4b71a847326ce4e6e47f1d52fe45ad005fb Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 15 Mar 2011 15:18:47 -0500 Subject: [PATCH 10/11] should use 'is' here --- Lib/test/test_dis.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index c3b2a4fc72b..7cda0dced9a 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -276,7 +276,7 @@ def test_dis_traceback(self): old = getattr(sys, 'last_traceback', not_defined) def cleanup(): - if old != not_defined: + if old is not not_defined: sys.last_traceback = old else: del sys.last_traceback From 47afc2af790478dd0cdee628e9c9035766e5d4d4 Mon Sep 17 00:00:00 2001 From: Benjamin Peterson Date: Tue, 15 Mar 2011 15:54:50 -0500 Subject: [PATCH 11/11] PyErr_Print can leave sys.last_traceback hanging around; kill it --- Lib/test/test_dis.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_dis.py b/Lib/test/test_dis.py index 7cda0dced9a..643e2e66ebe 100644 --- a/Lib/test/test_dis.py +++ b/Lib/test/test_dis.py @@ -265,28 +265,26 @@ def test_disassemble_method_bytes(self): self.do_disassembly_test(method_bytecode, dis_c_instance_method_bytes) def test_dis_none(self): + try: + del sys.last_traceback + except AttributeError: + pass self.assertRaises(RuntimeError, dis.dis, None) def test_dis_object(self): self.assertRaises(TypeError, dis.dis, object()) def test_dis_traceback(self): - not_defined = object() - tb = None - old = getattr(sys, 'last_traceback', not_defined) - - def cleanup(): - if old is not not_defined: - sys.last_traceback = old - else: - del sys.last_traceback + try: + del sys.last_traceback + except AttributeError: + pass try: 1/0 except Exception as e: tb = e.__traceback__ sys.last_traceback = tb - self.addCleanup(cleanup) tb_dis = self.get_disassemble_as_string(tb.tb_frame.f_code, tb.tb_lasti) self.do_disassembly_test(None, tb_dis)