From 6c58b28f2f5f8b3ee47db26cfd9011f9ab537e0f Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Mon, 17 Oct 2011 11:05:57 +1100 Subject: [PATCH] Issue #7833: Ext. modules built using distutils on Windows no longer get a manifest --- Lib/distutils/msvc9compiler.py | 78 +++++++++++++++++------ Lib/distutils/tests/test_msvc9compiler.py | 47 +++++++++++++- Misc/NEWS | 4 ++ 3 files changed, 106 insertions(+), 23 deletions(-) diff --git a/Lib/distutils/msvc9compiler.py b/Lib/distutils/msvc9compiler.py index 0cddb5c8e0e..0807a360956 100644 --- a/Lib/distutils/msvc9compiler.py +++ b/Lib/distutils/msvc9compiler.py @@ -627,15 +627,7 @@ class MSVCCompiler(CCompiler) : self.library_filename(dll_name)) ld_args.append ('/IMPLIB:' + implib_file) - # Embedded manifests are recommended - see MSDN article titled - # "How to: Embed a Manifest Inside a C/C++ Application" - # (currently at http://msdn2.microsoft.com/en-us/library/ms235591(VS.80).aspx) - # Ask the linker to generate the manifest in the temp dir, so - # we can embed it later. - temp_manifest = os.path.join( - build_temp, - os.path.basename(output_filename) + ".manifest") - ld_args.append('/MANIFESTFILE:' + temp_manifest) + self.manifest_setup_ldargs(output_filename, build_temp, ld_args) if extra_preargs: ld_args[:0] = extra_preargs @@ -653,21 +645,54 @@ class MSVCCompiler(CCompiler) : # will still consider the DLL up-to-date, but it will not have a # manifest. Maybe we should link to a temp file? OTOH, that # implies a build environment error that shouldn't go undetected. - if target_desc == CCompiler.EXECUTABLE: - mfid = 1 - else: - mfid = 2 - # Remove references to the Visual C runtime - self._remove_visual_c_ref(temp_manifest) - out_arg = '-outputresource:%s;%s' % (output_filename, mfid) - try: - self.spawn(['mt.exe', '-nologo', '-manifest', - temp_manifest, out_arg]) - except DistutilsExecError as msg: - raise LinkError(msg) + mfinfo = self.manifest_get_embed_info(target_desc, ld_args) + if mfinfo is not None: + mffilename, mfid = mfinfo + out_arg = '-outputresource:%s;%s' % (output_filename, mfid) + try: + self.spawn(['mt.exe', '-nologo', '-manifest', + mffilename, out_arg]) + except DistutilsExecError as msg: + raise LinkError(msg) else: log.debug("skipping %s (up-to-date)", output_filename) + def manifest_setup_ldargs(self, output_filename, build_temp, ld_args): + # If we need a manifest at all, an embedded manifest is recommended. + # See MSDN article titled + # "How to: Embed a Manifest Inside a C/C++ Application" + # (currently at http://msdn2.microsoft.com/en-us/library/ms235591(VS.80).aspx) + # Ask the linker to generate the manifest in the temp dir, so + # we can check it, and possibly embed it, later. + temp_manifest = os.path.join( + build_temp, + os.path.basename(output_filename) + ".manifest") + ld_args.append('/MANIFESTFILE:' + temp_manifest) + + def manifest_get_embed_info(self, target_desc, ld_args): + # If a manifest should be embedded, return a tuple of + # (manifest_filename, resource_id). Returns None if no manifest + # should be embedded. See http://bugs.python.org/issue7833 for why + # we want to avoid any manifest for extension modules if we can) + for arg in ld_args: + if arg.startswith("/MANIFESTFILE:"): + temp_manifest = arg.split(":", 1)[1] + break + else: + # no /MANIFESTFILE so nothing to do. + return None + if target_desc == CCompiler.EXECUTABLE: + # by default, executables always get the manifest with the + # CRT referenced. + mfid = 1 + else: + # Extension modules try and avoid any manifest if possible. + mfid = 2 + temp_manifest = self._remove_visual_c_ref(temp_manifest) + if temp_manifest is None: + return None + return temp_manifest, mfid + def _remove_visual_c_ref(self, manifest_file): try: # Remove references to the Visual C runtime, so they will @@ -676,6 +701,8 @@ class MSVCCompiler(CCompiler) : # runtimes are not in WinSxS folder, but in Python's own # folder), the runtimes do not need to be in every folder # with .pyd's. + # Returns either the filename of the modified manifest or + # None if no manifest should be embedded. manifest_f = open(manifest_file) try: manifest_buf = manifest_f.read() @@ -688,9 +715,18 @@ class MSVCCompiler(CCompiler) : manifest_buf = re.sub(pattern, "", manifest_buf) pattern = "\s*" manifest_buf = re.sub(pattern, "", manifest_buf) + # Now see if any other assemblies are referenced - if not, we + # don't want a manifest embedded. + pattern = re.compile( + r"""|)""", re.DOTALL) + if re.search(pattern, manifest_buf) is None: + return None + manifest_f = open(manifest_file, 'w') try: manifest_f.write(manifest_buf) + return manifest_file finally: manifest_f.close() except IOError: diff --git a/Lib/distutils/tests/test_msvc9compiler.py b/Lib/distutils/tests/test_msvc9compiler.py index a0d62efcfde..5fa1ca10d47 100644 --- a/Lib/distutils/tests/test_msvc9compiler.py +++ b/Lib/distutils/tests/test_msvc9compiler.py @@ -7,7 +7,36 @@ from distutils.errors import DistutilsPlatformError from distutils.tests import support from test.support import run_unittest -_MANIFEST = """\ +# A manifest with the only assembly reference being the msvcrt assembly, so +# should have the assembly completely stripped. Note that although the +# assembly has a reference the assembly is removed - that is +# currently a "feature", not a bug :) +_MANIFEST_WITH_ONLY_MSVC_REFERENCE = """\ + + + + + + + + + + + + + + + + + +""" + +# A manifest with references to assemblies other than msvcrt. When processed, +# this assembly should be returned with just the msvcrt part removed. +_MANIFEST_WITH_MULTIPLE_REFERENCES = """\ @@ -115,7 +144,7 @@ class msvc9compilerTestCase(support.TempdirManager, manifest = os.path.join(tempdir, 'manifest') f = open(manifest, 'w') try: - f.write(_MANIFEST) + f.write(_MANIFEST_WITH_MULTIPLE_REFERENCES) finally: f.close() @@ -133,6 +162,20 @@ class msvc9compilerTestCase(support.TempdirManager, # makes sure the manifest was properly cleaned self.assertEqual(content, _CLEANED_MANIFEST) + def test_remove_entire_manifest(self): + from distutils.msvc9compiler import MSVCCompiler + tempdir = self.mkdtemp() + manifest = os.path.join(tempdir, 'manifest') + f = open(manifest, 'w') + try: + f.write(_MANIFEST_WITH_ONLY_MSVC_REFERENCE) + finally: + f.close() + + compiler = MSVCCompiler() + got = compiler._remove_visual_c_ref(manifest) + self.assertIs(got, None) + def test_suite(): return unittest.makeSuite(msvc9compilerTestCase) diff --git a/Misc/NEWS b/Misc/NEWS index b10ecf418b7..029f16ca54d 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,10 @@ What's New in Python 3.2.3? Core and Builtins ----------------- +- Issue #7833: Extension modules built using distutils on Windows will no + longer include a "manifest" to prevent them failing at import time in some + embedded situations. + - Issue #13063: the Windows error ERROR_NO_DATA (numbered 232 and described as "The pipe is being closed") is now mapped to POSIX errno EPIPE (previously EINVAL).