From 31202eaa5c735044d9007f448b7f41b2c00f5f69 Mon Sep 17 00:00:00 2001 From: Steve Dower Date: Wed, 5 Aug 2015 11:39:19 -0700 Subject: [PATCH] Issue #24798: _msvccompiler.py doesn't properly support manifests --- Lib/distutils/_msvccompiler.py | 167 +++++++---------------- Lib/distutils/tests/test_msvccompiler.py | 121 ---------------- Misc/NEWS | 2 + 3 files changed, 52 insertions(+), 238 deletions(-) diff --git a/Lib/distutils/_msvccompiler.py b/Lib/distutils/_msvccompiler.py index 4e2eed72a99..b344616e600 100644 --- a/Lib/distutils/_msvccompiler.py +++ b/Lib/distutils/_msvccompiler.py @@ -15,7 +15,6 @@ for older versions in distutils.msvc9compiler and distutils.msvccompiler. import os import subprocess -import re from distutils.errors import DistutilsExecError, DistutilsPlatformError, \ CompileError, LibError, LinkError @@ -182,7 +181,8 @@ class MSVCCompiler(CCompiler) : raise DistutilsPlatformError("Unable to find a compatible " "Visual Studio installation.") - paths = vc_env.get('path', '').split(os.pathsep) + self._paths = vc_env.get('path', '') + paths = self._paths.split(os.pathsep) self.cc = _find_exe("cl.exe", paths) self.linker = _find_exe("link.exe", paths) self.lib = _find_exe("lib.exe", paths) @@ -199,23 +199,41 @@ class MSVCCompiler(CCompiler) : self.add_library_dir(dir) self.preprocess_options = None + # Use /MT[d] to build statically, then switch from libucrt[d].lib to ucrt[d].lib + # later to dynamically link to ucrtbase but not vcruntime. self.compile_options = [ - '/nologo', '/Ox', '/MD', '/W3', '/GL', '/DNDEBUG' + '/nologo', '/Ox', '/MT', '/W3', '/GL', '/DNDEBUG' ] self.compile_options_debug = [ - '/nologo', '/Od', '/MDd', '/Zi', '/W3', '/D_DEBUG' + '/nologo', '/Od', '/MTd', '/Zi', '/W3', '/D_DEBUG' ] - self.ldflags_shared = [ - '/nologo', '/DLL', '/INCREMENTAL:NO', '/LTCG', '/nodefaultlib:libucrt.lib', 'ucrt.lib' + ldflags = [ + '/nologo', '/INCREMENTAL:NO', '/LTCG', '/nodefaultlib:libucrt.lib', 'ucrt.lib', ] - self.ldflags_shared_debug = [ - '/nologo', '/DLL', '/INCREMENTAL:no', '/LTCG', '/DEBUG:FULL', '/nodefaultlib:libucrtd.lib', 'ucrtd.lib' - ] - self.ldflags_static = [ - '/nologo' + ldflags_debug = [ + '/nologo', '/INCREMENTAL:NO', '/LTCG', '/DEBUG:FULL', '/nodefaultlib:libucrtd.lib', 'ucrtd.lib', ] + self.ldflags_exe = [*ldflags, '/MANIFEST:EMBED,ID=1'] + self.ldflags_exe_debug = [*ldflags_debug, '/MANIFEST:EMBED,ID=1'] + self.ldflags_shared = [*ldflags, '/DLL', '/MANIFEST:EMBED,ID=2', '/MANIFESTUAC:NO'] + self.ldflags_shared_debug = [*ldflags_debug, '/DLL', '/MANIFEST:EMBED,ID=2', '/MANIFESTUAC:NO'] + self.ldflags_static = [*ldflags] + self.ldflags_static_debug = [*ldflags_debug] + + self._ldflags = { + (CCompiler.EXECUTABLE, None): self.ldflags_exe, + (CCompiler.EXECUTABLE, False): self.ldflags_exe, + (CCompiler.EXECUTABLE, True): self.ldflags_exe_debug, + (CCompiler.SHARED_OBJECT, None): self.ldflags_shared, + (CCompiler.SHARED_OBJECT, False): self.ldflags_shared, + (CCompiler.SHARED_OBJECT, True): self.ldflags_shared_debug, + (CCompiler.SHARED_LIBRARY, None): self.ldflags_static, + (CCompiler.SHARED_LIBRARY, False): self.ldflags_static, + (CCompiler.SHARED_LIBRARY, True): self.ldflags_static_debug, + } + self.initialized = True # -- Worker methods ------------------------------------------------ @@ -224,9 +242,12 @@ class MSVCCompiler(CCompiler) : source_filenames, strip_dir=0, output_dir=''): - ext_map = {ext: self.obj_extension for ext in self.src_extensions} - ext_map.update((ext, self.res_extension) - for ext in self._rc_extensions + self._mc_extensions) + ext_map = { + **{ext: self.obj_extension for ext in self.src_extensions}, + **{ext: self.res_extension for ext in self._rc_extensions + self._mc_extensions}, + } + + output_dir = output_dir or '' def make_out_path(p): base, ext = os.path.splitext(p) @@ -237,18 +258,17 @@ class MSVCCompiler(CCompiler) : if base.startswith((os.path.sep, os.path.altsep)): base = base[1:] try: - return base + ext_map[ext] + # XXX: This may produce absurdly long paths. We should check + # the length of the result and trim base until we fit within + # 260 characters. + return os.path.join(output_dir, base + ext_map[ext]) except LookupError: # Better to raise an exception instead of silently continuing # and later complain about sources and targets having # different lengths raise CompileError("Don't know how to compile {}".format(p)) - output_dir = output_dir or '' - return [ - os.path.join(output_dir, make_out_path(src_name)) - for src_name in source_filenames - ] + return list(map(make_out_path, source_filenames)) def compile(self, sources, @@ -359,6 +379,7 @@ class MSVCCompiler(CCompiler) : if debug: pass # XXX what goes here? try: + log.debug('Executing "%s" %s', self.lib, ' '.join(lib_args)) self.spawn([self.lib] + lib_args) except DistutilsExecError as msg: raise LibError(msg) @@ -399,14 +420,9 @@ class MSVCCompiler(CCompiler) : output_filename = os.path.join(output_dir, output_filename) if self._need_link(objects, output_filename): - ldflags = (self.ldflags_shared_debug if debug - else self.ldflags_shared) - if target_desc == CCompiler.EXECUTABLE: - ldflags = ldflags[1:] + ldflags = self._ldflags[target_desc, debug] - export_opts = [] - for sym in (export_symbols or []): - export_opts.append("/EXPORT:" + sym) + export_opts = ["/EXPORT:" + sym for sym in (export_symbols or [])] ld_args = (ldflags + lib_opts + export_opts + objects + ['/OUT:' + output_filename]) @@ -425,8 +441,6 @@ class MSVCCompiler(CCompiler) : self.library_filename(dll_name)) ld_args.append ('/IMPLIB:' + implib_file) - self.manifest_setup_ldargs(output_filename, build_temp, ld_args) - if extra_preargs: ld_args[:0] = extra_preargs if extra_postargs: @@ -434,101 +448,20 @@ class MSVCCompiler(CCompiler) : self.mkpath(os.path.dirname(output_filename)) try: + log.debug('Executing "%s" %s', self.linker, ' '.join(ld_args)) self.spawn([self.linker] + ld_args) except DistutilsExecError as msg: raise LinkError(msg) - - # embed the manifest - # XXX - this is somewhat fragile - if mt.exe fails, distutils - # 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. - mfinfo = self.manifest_get_embed_info(target_desc, ld_args) - if mfinfo is not None: - mffilename, mfid = mfinfo - out_arg = '-outputresource:{};{}'.format(output_filename, mfid) - try: - self.spawn([self.mt, '-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): + def spawn(self, cmd): + old_path = os.getenv('path') try: - # Remove references to the Visual C runtime, so they will - # fall through to the Visual C dependency of Python.exe. - # This way, when installed for a restricted user (e.g. - # 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() - finally: - manifest_f.close() - pattern = re.compile( - r"""|)""", - re.DOTALL) - 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 OSError: - pass + os.environ['path'] = self._paths + return super().spawn(cmd) + finally: + os.environ['path'] = old_path # -- Miscellaneous methods ----------------------------------------- # These are all used by the 'gen_lib_options() function, in diff --git a/Lib/distutils/tests/test_msvccompiler.py b/Lib/distutils/tests/test_msvccompiler.py index b4407543b12..1f8890792e2 100644 --- a/Lib/distutils/tests/test_msvccompiler.py +++ b/Lib/distutils/tests/test_msvccompiler.py @@ -7,88 +7,6 @@ from distutils.errors import DistutilsPlatformError from distutils.tests import support from test.support import run_unittest -# 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 = """\ - - - - - - - - - - - - - - - - - - - - - - -""" - -_CLEANED_MANIFEST = """\ - - - - - - - - - - - - - - - - - - -""" SKIP_MESSAGE = (None if sys.platform == "win32" else "These tests are only for win32") @@ -114,45 +32,6 @@ class msvccompilerTestCase(support.TempdirManager, finally: _msvccompiler._find_vcvarsall = old_find_vcvarsall - def test_remove_visual_c_ref(self): - from distutils._msvccompiler import MSVCCompiler - tempdir = self.mkdtemp() - manifest = os.path.join(tempdir, 'manifest') - f = open(manifest, 'w') - try: - f.write(_MANIFEST_WITH_MULTIPLE_REFERENCES) - finally: - f.close() - - compiler = MSVCCompiler() - compiler._remove_visual_c_ref(manifest) - - # see what we got - f = open(manifest) - try: - # removing trailing spaces - content = '\n'.join([line.rstrip() for line in f.readlines()]) - finally: - f.close() - - # makes sure the manifest was properly cleaned - self.assertEqual(content, _CLEANED_MANIFEST) - - def test_remove_entire_manifest(self): - from distutils._msvccompiler 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.assertIsNone(got) - - def test_suite(): return unittest.makeSuite(msvccompilerTestCase) diff --git a/Misc/NEWS b/Misc/NEWS index 5fefcdbaae7..9dd07f15034 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -13,6 +13,8 @@ Core and Builtins Library ------- +- Issue #24798: _msvccompiler.py doesn't properly support manifests + - Issue #4395: Better testing and documentation of binary operators. Patch by Martin Panter.