Issue #7833: Ext. modules built using distutils on Windows no longer get a manifest

3 files changed, 106 insertions, 23 deletions

diff --git a/Lib/distutils/msvc9compiler.py b/Lib/distutils/msvc9compiler.py
index 0cddb5c8e0..0807a36095 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 = "<dependentAssembly>\s*</dependentAssembly>"
             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"""<assemblyIdentity.*?name=(?:"|')(.+?)(?:"|')"""
+                r""".*?(?:/>|</assemblyIdentity>)""", 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 a0d62efcfd..5fa1ca10d4 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 <security> reference the assembly is removed - that is
+# currently a "feature", not a bug :)
+_MANIFEST_WITH_ONLY_MSVC_REFERENCE = """\
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<assembly xmlns="urn:schemas-microsoft-com:asm.v1"
+          manifestVersion="1.0">
+  <trustInfo xmlns="urn:schemas-microsoft-com:asm.v3">
+    <security>
+      <requestedPrivileges>
+        <requestedExecutionLevel level="asInvoker" uiAccess="false">
+        </requestedExecutionLevel>
+      </requestedPrivileges>
+    </security>
+  </trustInfo>
+  <dependency>
+    <dependentAssembly>
+      <assemblyIdentity type="win32" name="Microsoft.VC90.CRT"
+         version="9.0.21022.8" processorArchitecture="x86"
+         publicKeyToken="XXXX">
+      </assemblyIdentity>
+    </dependentAssembly>
+  </dependency>
+</assembly>
+"""
+
+# 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 = """\
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
 <assembly xmlns="urn:schemas-microsoft-com:asm.v1"
           manifestVersion="1.0">
@@ -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 b10ecf418b..029f16ca54 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).