bdist_wininst installers fail to load extensions built with Issue4120 patch

@loewis

BPO	7833
Nosy	@loewis, @mhammond, @vstinner, @ericvsmith, @tjguk, @tarekziade, @merwok, @zware, @zooba, @dstufft
Files	bdist_wininst.vcproj.patch test-bdist_wininst.zip msvc9compiler_stripruntimes_revised.patch msvc9compiler_stripruntimes_trunk.patch bug-7833-overridable-manifest-settings.patch: patch with same behaviour but more override friendly bug-7833-overridable-manifest-settings-with-test.patch bug-7833-tweaks-plus-news.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/mhammond'
closed_at = <Date 2021-01-11.12:51:13.069>
created_at = <Date 2010-02-01.22:12:51.098>
labels = ['library', 'OS-windows']
title = 'bdist_wininst installers fail to load extensions built with Issue4120 patch'
updated_at = <Date 2021-01-11.14:02:38.455>
user = 'https://bugs.python.org/cgohlke'

bugs.python.org fields:

activity = <Date 2021-01-11.14:02:38.455>
actor = 'mark.dickinson'
assignee = 'mhammond'
closed = True
closed_date = <Date 2021-01-11.12:51:13.069>
closer = 'vstinner'
components = ['Distutils', 'Windows']
creation = <Date 2010-02-01.22:12:51.098>
creator = 'cgohlke'
dependencies = []
files = ['16086', '16088', '16121', '17286', '23305', '23341', '23400']
hgrepos = []
issue_num = 7833
keywords = ['patch']
message_count = 38.0
messages = ['98694', '98696', '98699', '98805', '104316', '104318', '104341', '104382', '105473', '106855', '106867', '128775', '129610', '129611', '129612', '129617', '130205', '144806', '145004', '145048', '145116', '145117', '145151', '145206', '145210', '145211', '145496', '145534', '145654', '145655', '145691', '145692', '152447', '152448', '152585', '176062', '233204', '384818']
nosy_count = 20.0
nosy_names = ['loewis', 'mhammond', 'vstinner', 'sable', 'eric.smith', 'techtonik', 'tim.golden', 'koen', 'tarek', 'eric.araujo', 'cgohlke', 'Garen', 'alexis', 'John.Cary', 'python-dev', 'abenard', 'almar', 'zach.ware', 'steve.dower', 'dstufft']
pr_nums = []
priority = 'normal'
resolution = 'wont fix'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue7833'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

Wininst-9.0.exe and wininst-9.0-amd64.exe are missing MSVCRT90 dependencies in the embedded manifest.

While testing installers of pywin32 <http://sourceforge.net/projects/pywin32/\> version 214 built with Python 2.6.4 and the msvc9compiler_stripruntimes_regexp2.diff patch from bpo-4120 <http://bugs.python.org/issue4120\>, I noticed that the post_install script fails with an ImportError while trying to load extensions without embedded MSVCRT90 manifest:

Traceback (most recent call last):
  File "<string>", line 601, in <module>
  File "<string>", line 311, in install
  File "<string>", line 149, in LoadSystemModule
ImportError: DLL load failed: The specified module could not be found.

The code that fails is:

    mod = imp.load_module(modname, None, filename, ('.dll', 'rb', imp.C_EXTENSION))

where modname='pywintypes' and filename points to an existing 'pywintypes26.dll' file.

The post_install script runs fine when executed from the main python.exe interpreter.

A possible fix is to embed a MSVCRT90 dependency into wininst-9.0.exe and wininst-9.0-amd64.exe using the following linker switches:

/MANIFESTDEPENDENCY:"type='Win32' name='Microsoft.VC90.CRT' version='9.0.21022.8' processorArchitecture='X86' publicKeyToken='1fc8b3b9a1e18e3b'"

/MANIFESTDEPENDENCY:"type='Win32' name='Microsoft.VC90.CRT' version='9.0.21022.8' processorArchitecture='amd64' publicKeyToken='1fc8b3b9a1e18e3b'"

See also <http://msdn.microsoft.com/en-us/library/ew0y5khy.aspx\>

A patch against the Python 2.6 PCbuild\bdist_wininst.vcproj is attached. I tested it with Python 2.6.4, 32 and 64 bit, and pywin32 214 installers built with the following command:

python.exe setup.py bdist_wininst --user-access-control=auto --install-script=wxpython_win_post_install.py

This doesn't look right. IIUC, wininst.exe loads python26.dll, which in turn loads pywintypes.dll. So it should be python26.dll which loads the CRT.

IIUC, wininst.exe is linked statically, precisely to avoid a dependency on the CRT assembly.

The last line of my previous post should actually read
python.exe setup.py bdist_wininst

Anyway, here are three files (also attached) that can reproduce the problem:

setup.py

from distutils.core import setup, Extension
setup(name='testpyd', scripts = ["testpyd_postinstall.py"],
    ext_modules=[Extension('testpyd', ['testpyd.c'],)],
    options = {"bdist_wininst": {"install_script": 
        "testpyd_postinstall.py", "user_access_control": "auto"},})

testpyd.c

#include "Python.h"
PyMethodDef methods[] = {{NULL, NULL},};
void inittestpyd() {(void)Py_InitModule("testpyd", methods);}

testpyd_postinstall.py

#!python
import testpyd

Build the installer with python 2.6 and bpo-4120 patch applied:
python setup.py bdist_wininst

Run the installer:
dist\testpyd-0.0.0.win32-py2.6.exe

The postinstall script fails with:

Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: DLL load failed: The specified module could not be found.

According to Sysinternals process monitor python26.dll is loaded from the system32 directory, the testpyd.pyd extension is found at the right place, and then the Windows path is searched in vain for MSVCR90.DLL.

Tested on Windows 7 Pro 64 bit.

I thought one conclusion of the discussion on bpo-4120 was that any executable, which embeds Python and imports MSVCR9 dependent extensions, must now provide the manifest for the MSVCR9 runtimes, either embedded or as a separate file. See <http://bugs.python.org/issue4120#msg94644\> and responses. Why shouldn't this apply to wininst executables?

Another observation: importing the winsound extension in the postinstall script works even though it also depends on MSVCR90.DLL. Winsound.pyd does not have an embedded manifest. It is linked with /MANIFEST:NO. Apparently there is a difference between an "empty" manifest and no manifest.

How about a patch that does:

not embed any manifest into PYD files if the manifest contains only a single MSVCR90 dependentAssembly element.
remove the MSVCR90 dependentAssembly element from manifests embedded into PYD files if the manifest defines additional dependentAssembly elements (e.g. for MFC or Common Controls). This is essentially what the msvc9compiler_stripruntimes patch does now.
not embed manifests into any file (EXE, PYD, or DLL) if '/MANIFEST:NO' is defined in extra_link_args. Extension developers can then provide external manifest files.
not touch the default manifests embedded into EXE and DLL files. The msvc9compiler_stripruntimes currently produces DLLs that can not be used standalone, such as pythoncom26.dll from the pywin32 package (I can file a separate bug if requested). Pythoncom26.dll is meant to be placed in the system32 folder and to be used outside of a Python environment, i.e. from the Windows Scripting Host. Several pywin32 tests fail with the pythoncom26.dll built with the msvc9compiler_stripruntimes patch. Placing a MSVCR9 manifest file into the system32 folder next to the pythoncom26.dll is not an option.

A tentative patch against the Python 2.6 branch is attached. I will test it further.

(1) and (4) will solve the original pywin32 wininstaller problem without changing wininst.exe.

As it is now the installer and some functionality of the pywin32 package will likely break if pywin32 is built on Python 2.6.5.

There should be no manifest embedded into wininst, because then the cases which bpo-4120 fixed (a CRT installed into a local folder, instead of system-wide, due to limited access rights), will 'break' again: the installer can then no longer work unless there is a system-wide installation of the CRT.

Option #1, #2 and #3 all sound reasonable (and #2 is the current situation).

I have some doubts about option #4: it is a very specific use case, and then the whole benefit of bpo-4120 is lost, because stripping runtimes would have to be removed again. Why is putting a separate manifest file next to the DLL not an option? Combined with #3 (allow extension developer to disable embedding of manifests) a separate manifest can fix the problem.

Concerning the patch: what happens when the developer already added /MANIFEST:NO to the flags, and the code deduces that MSVCR9 is the only runtime, e.g. the case where /MANIFEST:NO is in the flags twice? Does the linker handle this OK, or does there need to be an additional check as to not have it twice?

I have some doubts about option #4: it is a very specific use case, and then the whole benefit of bpo-4120 is lost

Pythoncom are pywintypes are indeed special cases: Out of the 170 DLL files in my Python site-packages directory, these seem to be the only ones built with distutils. All other DLLs are apparently built without Python involvement using make, nmake, CMake, or Visual Studio and most of them contain embedded manifests, which is the default when using nmake, CMake, or Visual Studio. Practically, to make a standalone distribution of any Python 2.6/3.1 application with external DLL dependencies likely requires to provide external manifest files. The bpo-4120 patch does not change this situation and I don't see any sane way to patch Python/distutils that could. The main benefit of the bpo-4120 patch, as I see it, is that PYD files produced by distutils work in a standalone distribution without any further attention. Msvc9compiler_stripruntimes_revised.patch does not change this.

My reasoning for this patch (besides fixing the bdist_wininst installer issue) was to allow the popular pywin32 package to build without changes, and offer a way for other extension packages to exclude manifests from DLL files if required (apparently not that common). Alternatively one could provide a mechanism to embed specific manifests into DLLs. Is that currently possible? Then pywin32 setup.py could be fixed.

Why is putting a separate manifest file next to the DLL not an option?

Because the pythoncom dll is currently installed into the Windows system directory. Putting manifest files there will pollute the system directory even more and possibly interfere with other system components if not done right (not tested). But again, pywin32 setup.py could be fixed to not install the DLL/manifest files into the system directory.

Which Python packages other than pywin32 build DLL files via distutils? I don't know any.

Can anyone provide a minimal setup.py script and C file that produces a DLL file for testing?

the pywin32 DLLs have 2 heads. They are Python extension modules as well as regular DLLs. They are built by distutils and therefore have no manifests - I think many packages use distutils to build their extension modules - it is just that they usually don't have a .dll extension.

I fear that simply adding a manifest to those DLLs will put them in the same position we have before bpo-4120 was addressed, and these .dlls do need a way to be installed into System32 (or somewhere else on the global PATH) to function as COM servers etc. I need to experiment with this.

The bdist_wininst and DLL build issues also exist in Python 2.7b2. A patch against svn trunk is attached.

The pywin32 v214 package fails as reported earlier when built with Python 2.7b2. It installs and functions when built with this patch.

I see that this issue mentions --user-access-control option. Can somebody also check if bpo-8870 and bpo-8871 are related to this one?

bpo-8870 and bpo-8871 are not related to this one. There, the UAC elevation fails, here the issue is with the MS runtimes, elevation is working fine.

I'm failing to get a new pywin32 out of the door due to this issue. I've spent a few hours playing with this and I think the analysis is generally correct here. The key thing is that when using distutils, the extensions end up with a manifest (albeit one without a reference to the CRT) whereas the extensions shipped with Python have no manifest at all.

I agree with Martin that it seems strange the CRT fails to be used even though the CRT is obviously already loaded, but it seems to be a fact. I can't find much on this, but suspect it relates to the different "activation contexts" in use and how the activation contexts are designed to allow side-by-side loading of DLLs; Windows doesn't know if the version of the DLL already loaded is suitable. I also guess that the fact the DLL has *any* manifest means they use a more strict interpretation of things (ie, refuse to use already loaded ones) whereas a dll with no manifest gets given a little more slack.

I can confirm that with the attached patch, pywin32 builds and installs fine on a machine without the CRT installed globally - so I'm +1 on this patch with one caveat: The check for '.pyd' should either be expanded to include '.dll', or else the check should just use the 'target_desc == CCompiler.EXECUTABLE' condition already used.

I'm happy to make the change once I get some feedback and/or guidance about where I should check this in - I believe it is too late for python 2.6 which is a shame...

New version of the patch with the small tweaks requested plus a NEWS entry.

Looks good.

Style nit: I don’t put backslashes at end-of-lines in parens (in one re.compile call you have that). Also, I use -- where I can’t use —.

New changeset fb886858024c by Mark Hammond in branch '2.7':
Issue bpo-7833: Ext. modules built using distutils on Windows no longer get a manifest
http://hg.python.org/cpython/rev/fb886858024c

New changeset 9caeb7215344 by Mark Hammond in branch '3.2':
Issue bpo-7833: Ext. modules built using distutils on Windows no longer get a manifest
http://hg.python.org/cpython/rev/9caeb7215344

New changeset 3073ef853647 by Mark Hammond in branch 'default':
Issue bpo-7833: Ext. modules built using distutils on Windows no longer get a manifest
http://hg.python.org/cpython/rev/3073ef853647

I pushed the changes to 2.7, 3.2 and 3.3. I'm happy to help with distutils2/packaging but I'll need to do that later once I work out where to start :) Therefore I'm not yet closing this issue.

To port the patch to packaging, go into your CPython 3.3 checkout and edit Lib/packaging/compiler/msvc9compiler.py (and its test :).

To port the patch to distutils2, clone http://hg.python.org/distutils2/ and edit distutils/compiler/msvc9compiler.py (same :). Test with Python 2.4, 2.5, 2.6 and 2.7. Then hg update python3, hg merge default, test with Python 3.1, 3.2 and 3.3. Then you can push :)

If you don’t have the necessary Pythons or roundtuits, I’ll port the patch when I get a Windows VM. There’s plenty of time before Python 3.3 is out.

s/distutils/distutils2/ !

Mark: Possibly a stupid question, but in your commit (see snippet below), why is the processorArchitecture hard coded to "x86"? Is it appropriate to replace this with "amd64" for 64-bit builds, or should it always be "x86"?

+ <dependentAssembly>
+ <assemblyIdentity type="win32" name="Microsoft.VC90.CRT"
+ version="9.0.21022.8" processorArchitecture="x86"
+ publicKeyToken="XXXX">
+ </assemblyIdentity>
+ </dependentAssembly>

ack - that is a really good point. IIRC it can be "*". I'll try and look at this over the next day or 2.

Actually, I think this is OK - the reference to the "x86" is in the tests and those tests don't actually perform a build, just check the manifest is detected and stripped (ie, the test should still work fine on 64bit boxes). Ideally the test could also check a manifest with a 64bit architecture, but I don't think that's really necessary...

Just checking in to point out a possible problem with the code that strips the MSVCR dependency from the embedded manifest. The used regexpr is too greedy: the first bit can trigger on an earlier "assemblyIdentity" tag, so that after the removal the manifest is no longer valid XML.

The key problem is that the "<assemblyIdentity" and the name attribute are allowed to be on a separate lines. To fix this I removed the re.DOTALL flag and replaced the second occurrence of ".*?" to also allow newlines:

  pattern = re.compile(
-     r"""<assemblyIdentity.*?name=("|')Microsoft\."""\
-     r"""VC\d{2}\.CRT("|').*?(/>|</assemblyIdentity>)""",
-     re.DOTALL)
+     r"""<assemblyIdentity.*?name=("|')Microsoft\."""\
+     r"""VC\d{2}\.CRT("|')(.|\r|\r)*?(/>|</assemblyIdentity>)""")

It is well possible that the problem does not causes any problems for the intended usage of the code. I'm using the code to strip other DLL's and ran into this issue (with tk85.dll to be precise).

Almar

This is referenced from bpo-4431 which has been closed for over six years but keeps getting comments.

The distutils bdist_wininst command has been removed in Python 3.10: see bpo-42802.

Navigation Menu

Repository navigation

bdist_wininst installers fail to load extensions built with Issue4120 patch #52081

16 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

bdist_wininst installers fail to load extensions built with Issue4120 patch #52081

Description

Activity

cgohlke commented on Feb 1, 2010

loewis commented on Feb 1, 2010

cgohlke commented on Feb 2, 2010

cgohlke commented on Feb 4, 2010

koen commented on Apr 27, 2010

koen commented on Apr 27, 2010

cgohlke commented on Apr 27, 2010

mhammond commented on Apr 27, 2010

cgohlke commented on May 10, 2010

techtonik commented on Jun 1, 2010

koen commented on Jun 1, 2010

mhammond commented on Feb 18, 2011

16 remaining items

mhammond commented on Oct 14, 2011

merwok commented on Oct 14, 2011

python-dev commented on Oct 17, 2011

mhammond commented on Oct 17, 2011

merwok commented on Oct 17, 2011

merwok commented on Oct 17, 2011

mdickinson commented on Feb 2, 2012

mhammond commented on Feb 2, 2012

mhammond commented on Feb 4, 2012

almar commented on Nov 21, 2012

BreamoreBoy commented on Dec 30, 2014

vstinner commented on Jan 11, 2021

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions