[issue36085] Enable better DLL resolution

Change by STINNER Victor : -- nosy: -vstinner ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue36085] Enable better DLL resolution

David Miguel Susano Pinto added the comment: I have just found out that commit 2438cdf0e93 which added the winmode argument and the documentation for it disagree. Documentation states that default is zero while the real default in code is None. I have opened PR 19167 on github to address it

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Heh, and I was so sure I'd copy pasted the right number after getting it wrong so often. Just tag it against this bug. -- ___ Python tracker

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: Steve, in what's new and the installer, you've referenced KB2533625 instead of KB2533623. Do we have to open a new issue for this minor search and replace fix? -- ___ Python tracker

[issue36085] Enable better DLL resolution

Change by Steve Dower : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___ ___

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Thanks, David. I looked at the log quickly and it's what I expected, so I'll merge the PR and start advertising the change. Thanks everyone! -- ___ Python tracker

[issue36085] Enable better DLL resolution

Steve Dower added the comment: New changeset 79da388a4016e24c4258dcc62cd0fa9dde0acb5b by Steve Dower in branch 'master': bpo-36085: Add installer check for KB2533625 (GH-12636) https://github.com/python/cpython/commit/79da388a4016e24c4258dcc62cd0fa9dde0acb5b --

[issue36085] Enable better DLL resolution

David Bolen added the comment: Ok, I've verified that on a Win7 system with SP1 but without KB2533625 I get the expected block screen at startup. On my worker (SP1 with KB2533625) it proceeds to the regular installation main dialog. I'm attaching a copy of the install log in the blocking

[issue36085] Enable better DLL resolution

Steve Dower added the comment: > any chance at a 32-bit version of the installer Done (on the PR). Thanks! -- ___ Python tracker ___

[issue36085] Enable better DLL resolution

David Bolen added the comment: I can help with a Win7 test of the installer, but my currently available systems are all 32-bit - any chance at a 32-bit version of the installer? -- ___ Python tracker

[issue36085] Enable better DLL resolution

Steve Dower added the comment: New changeset ac19d9652799412404aef6b357a01057df34e005 by Steve Dower in branch 'master': bpo-36085: Add additional load flag to ensure DLL loads successfully (GH-12633) https://github.com/python/cpython/commit/ac19d9652799412404aef6b357a01057df34e005

[issue36085] Enable better DLL resolution

Steve Dower added the comment: I attached a build of the updated installer to PR 12636 (it's too big to attach here) in case anyone can help me test. It should block right at the start if you don't have the update, or else it'll go to the usual screen. The message just says "install SP1 and

[issue36085] Enable better DLL resolution

Change by Steve Dower : -- pull_requests: +12568 ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Thanks for the info, David! I guess the update isn't part of SP1. I'll add a check to the installer and update the note in What's New. Eryk - my thought on CWD was that the new process was not starting in the correct directory, which can sometimes happen. I

[issue36085] Enable better DLL resolution

David Bolen added the comment: I just wanted to acknowledge that this was breaking on my Windows 7 builder (with a bad DLL load parameter in both pythoninfo and test steps). It looks like I was missing the required KB2533625 (the machine is mostly a virgin SP1 install), so I've installed

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: WinDLL('./_sqlite3.dll') succeeds, which just delays the call to GetFullPathNameW to the CDLL constructor, so I don't see how the working directory is a factor. The difference I see is the lack of the LOAD_LIBRARY_SEARCH_DEFAULT_DIRS flag. Try including the

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Guess the link in the devguide needs fixing... I'm out for the next few hours but will give it a go when I'm back. -- ___ Python tracker ___

[issue36085] Enable better DLL resolution

Zachary Ware added the comment: Try https://buildbot.python.org/all/#/builders?tags=%2Bcustom instead :) You can trigger a build by pushing to the `buildbot-custom` branch and if need be I can grant you SSH or RDP access to that worker, just let me know. --

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Pushed a potential fix, assuming there's something weird going on with relying on cwd rather than full paths (I know there's some weirdness here when paths get too long, for example). Zach - this is one of your buildbots. Can we trigger a run from my branch?

[issue36085] Enable better DLL resolution

Change by Steve Dower : -- pull_requests: +12564 stage: commit review -> patch review ___ Python tracker ___ ___ Python-bugs-list

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Acknowledging the buildbot failure at https://buildbot.python.org/all/#builders/12/builds/2181 I'll try and take a look today. Apparently Windows 8 has a slightly different understanding of the flags used in ctypes tests? -- nosy: +pablogsal, vstinner

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Leaving this in commit review for a couple of days, then I'll close. -- stage: patch review -> commit review ___ Python tracker ___

[issue36085] Enable better DLL resolution

Steve Dower added the comment: New changeset 2438cdf0e932a341c7613bf4323d06b91ae9f1f1 by Steve Dower in branch 'master': bpo-36085: Enable better DLL resolution on Windows (GH-12302) https://github.com/python/cpython/commit/2438cdf0e932a341c7613bf4323d06b91ae9f1f1 --

[issue36085] Enable better DLL resolution

Steve Dower added the comment: So symlinking didn't work (Python is too clever for that these days ;) ), but straight copying the exe and required DLLs is fine. It puts python.exe, python38.dll and vcruntime140.dll (properly discovered of course) into a temp directory, puts _sqlite3.pyd

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: > There are no specific "import" tests, because it's such a pain to set > those up (I need to delete files from the build directory during the > test, and if other tests have already used them that will fail, or I > need to copy the Python install elsewhere so it

[issue36085] Enable better DLL resolution

Steve Dower added the comment: I implemented the feature Eryk was asking for, as it's also by far the easiest way to test the add_dll_directory function. So now ctypes.CDLL (and subclasses) have a `winmode` argument that gets turned directly into LoadLibraryEx flags (changing `mode` would

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: > Any comments, questions or more feedback on the PR? I commented on the PR that I'm concerned that ctypes.CDLL will no longer open a path with slashes in it (e.g. CDLL("./spam.dll") or CDLL("eggs/spam.dll")) relative to the working directory, and may

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Since there's no chance of getting old SQL Server fixed, I think we should just merge it without the SetDefaultDllDirectories change. Any comments, questions or more feedback on the PR? -- ___ Python tracker

[issue36085] Enable better DLL resolution

Change by Brett Cannon : -- nosy: -brett.cannon ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue36085] Enable better DLL resolution

Steve Dower added the comment: I added some logging for the AppVeyor build at https://ci.appveyor.com/project/python/cpython/builds/23258953 Looks like the offending DLLs are: - perf-MSSQL$SQL2017-sqlctr14.0.1000.169.dll - perf-MSSQL$SQL2016-sqlctr13.1.4474.0.dll Since the events are pulled

[issue36085] Enable better DLL resolution

Zachary Ware added the comment: I've found AppVeyor's support forum (https://help.appveyor.com/) to be fairly responsive; it may be worth asking them about the issue there. -- ___ Python tracker

[issue36085] Enable better DLL resolution

Steve Dower added the comment: I think we'll be keeping Win7 with the KB. However, we've discovered in the PR that changing the default DLL lookup may cause Python to crash when accessing HKEY_PERFORMANCE_DATA (which fails to delay-load a DLL). This occurs because accessing that key

[issue36085] Enable better DLL resolution

Change by Ralf Gommers : -- nosy: +ralf.gommers ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue36085] Enable better DLL resolution

Łukasz Langa added the comment: Personally I am fine with Python 3.8 dropping Windows 7 support entirely if this makes it work better in Windows 8+. However, the 3 month overlap here would set a precedent that we don't have to adhere to self-imposed timing restrictions which is dangerous

[issue36085] Enable better DLL resolution

mattip added the comment: @eryksun - is there a sample resource: blog post, code snippet, msdn documentation, that demonstrates how that all works? I personally find the MSDN documentation of "what happens when I call LoadLibraryEx" not very user friendly. They seem to be written to

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: > Since I just dug enough to find it, the best way to diagnose problems > with dependent DLLs not being found is probably to run Process Monitor > [1] while doing the import and checking its logs. It should show the > paths that were attempted to be accessed.

[issue36085] Enable better DLL resolution

Steve Dower added the comment: PR has been posted, but it's incomplete (docs, news, etc.) And unfortunately longer than I'd hoped, since we have to use GetProcAddress for these function on Windows 7 still (even if it has the required update), but since it's coming from kernel32 (which is

[issue36085] Enable better DLL resolution

Change by Steve Dower : -- keywords: +patch pull_requests: +12277 stage: -> patch review ___ Python tracker ___ ___

[issue36085] Enable better DLL resolution

Steve Dower added the comment: It's different from ctypes because I can update ctypes as part of this change :) The reason it matters is that it's basically a wrapper around LoadLibrary, and that's what is going to change here. Hopefully we won't cause too much trouble for their users.

[issue36085] Enable better DLL resolution

mattip added the comment: I have left a note for arigato, but why is CFFI different than ctypes or c-extension modules? All will need adjustment. -- ___ Python tracker ___

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Actually, CFFI is probably going to be most affected. Who do we know from that project who should be nosied on this? -- ___ Python tracker ___

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: Okay. Sorry for adding noise. My mental hiccup was in thinking it would continue to use LOAD_WITH_ALTERED_SEARCH_PATH in conjunction with SetDefaultDllDirectories: LOAD_LIBRARY_SEARCH_DEFAULT_DIRS. I forgot that it's documented that they shouldn't be combined.

[issue36085] Enable better DLL resolution

Paul Moore added the comment: > Since everyone seems to have misunderstood at least part of the proposal, I'm > not going to explain any more until I have a patch. Excluding boilerplate and > docs, it'll be about ten lines of code. +1 on that. Code is much harder to misunderstand :-) Paul

[issue36085] Enable better DLL resolution

Paul Moore added the comment: OK, I don't really follow enough of the details here to comment properly. But clearly Steve and Eryk are not yet in agreement. My personal view is that this is something where we should be trying *very* hard to preserve backward compatibility. The proposal here

[issue36085] Enable better DLL resolution

Steve Dower added the comment: > The alternative ... Is what I proposed in the first place. Adding the SetDefaultDllDirectories call to Py_Main (that is, NOT for embedders) is to ensure consistency throughout the process. It'll only affect extension module dependencies that do their own

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: > will this change just affect the embedded Python, or will it affect > the whole process SetDefaultDllDirectories affects the whole process and cannot be reverted back to the legacy search path that includes "%SystemRoot%", "%SystemRoot%\System" (the old

[issue36085] Enable better DLL resolution

Paul Moore added the comment: > > This bothers me - how will backward compatibility work in that case? > > The new search order is compatible with the old search order, so you can > update all your layouts to have DLL dependencies in suitable locations and > you'll be fine. OK, cool. But

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Since I just dug enough to find it, the best way to diagnose problems with dependent DLLs not being found is probably to run Process Monitor [1] while doing the import and checking its logs. It should show the paths that were attempted to be accessed. [1]:

[issue36085] Enable better DLL resolution

Steve Dower added the comment: > That will require rewriting many scripts and packages that use ctypes or cffi > to load DLLs. It would also break DLLs that internally rely on modifying PATH > for a delayed load, though I hope that's uncommon. I think it's easier for > everyone else if we

[issue36085] Enable better DLL resolution

Paul Moore added the comment: > Also, if I'm understanding your intention, loading an extension may fail when > Python is embedded if the process is using the legacy DLL search path. So, > like with ctypes, we'll be forcing embedding applications to update how they > load DLLs in order to

[issue36085] Enable better DLL resolution

mattip added the comment: Correct me if I'm wrong, but once SetDefaultDllDirectories() is used, there is no going back: PATH no longer can change the search path no matter what flags are used with LoadLibrary* calls (see the recent Anaconda issue when they did this and broke NumPy).

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: > call SetDefaultDllDirectories() in Py_Main (i.e. not when embedded) > to ensure secure search paths are always used That will require rewriting many scripts and packages that use ctypes or cffi to load DLLs. It would also break DLLs that internally rely on

[issue36085] Enable better DLL resolution

Steve Dower added the comment: In the absence of any other comments, here's my proposal. * call SetDefaultDllDirectories() in Py_Main (i.e. not when embedded) to ensure secure search paths are always used * ensure LoadLibrary when used in ctypes or importing will use the correct flags * add

[issue36085] Enable better DLL resolution

Steve Dower added the comment: > Would this be a hard drop, i.e. would installing 3.8 be prevented in Windows > 7? Or would it install but require users to manually install KB2533623? That's the question I'm asking :) Python 3.9 is currently going to be a hard drop, according to our policy,

[issue36085] Enable better DLL resolution

Paul Moore added the comment: As someone whose work environment is still Windows 7, I'd prefer it if it were a soft desupport (i.e., we require users to manually ensure that the KB fix is installed, but we don't do anything specific to refuse to install Python on Win7). I'd rather not drop

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: > Alternatively, I'm totally happy to make a three month exception to > PEP 11 and just drop Win7 completely for 3.8. But I think that needs > to be made official as early as possible Windows 7 is still used on about 40% of Windows desktops, and will likely

[issue36085] Enable better DLL resolution

Steve Dower added the comment: Adding Łukasz for his RM opinion on Win7 support for 3.8. According to PEP 11, we've already dropped support for Win7 without Service Pack 1. Win7 SP1 would be dropped ~2-3 months after 3.8 releases, which means we still have to support it for all of 3.8. My

[issue36085] Enable better DLL resolution

Nick Coghlan added the comment: As a note in favour of the "Allow package nesting to be encoded in names, not just directories" approach, we actually have a similar problem affecting builtin modules: they're currently limited to top-level modules, with no way for the module to indicate that

[issue36085] Enable better DLL resolution

Steve Dower added the comment: > Correct me if I'm wrong, don't process launches use the `env` kwarg for > Popen, not the raw os.environ['PATH']? If you don't provide env, it'll use the current process's environment, and if you do provide it without copying at least some entries, chances

[issue36085] Enable better DLL resolution

mattip added the comment: > legitimately modify PATH for process launches Correct me if I'm wrong, don't process launches use the `env` kwarg for Popen, not the raw os.environ['PATH']? -- ___ Python tracker

[issue36085] Enable better DLL resolution

Steve Dower added the comment: > Even better would be official python/Microsoft support for a CLI version of > depends.exe like ldd on linux The dumpbin.exe tool with /IMPORTS is a good start, and I've definitely wrapped it in Python before to do this kind of analysis (not reproducibly,

[issue36085] Enable better DLL resolution

mattip added the comment: Clear documentation would go a long way toward onboarding package providers. Of course this does not solve the problem for packages with no active ongoing support for windows, and will annoy developers whose code base is full of `os.environ['PATH']` games. Perhaps

[issue36085] Enable better DLL resolution

Steve Dower added the comment: > Should we support a convenient syntax for including the current value of PATH > at extension-module load time? No. This is the bit that I refuse to add back, at least in CPython itself (if someone does it themselves then it's their bug). Private directories

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: > I'm very against doing magic to extract the names from the DLL, but > but maybe we could have a search path in the parent package? Then > add/remove it around importing the module. That works, too. I'm happy either way. We still can't load multiple DLLs with

[issue36085] Enable better DLL resolution

Steve Dower added the comment: I'm very against doing magic to extract the names from the DLL, but maybe we could have a search path in the parent package? Then add/remove it around importing the module. I think you're right that we just need to update the LoadLibrary flags, but will those

[issue36085] Enable better DLL resolution

Eryk Sun added the comment: > Proposal #1: CPython calls SetDefaultDllDirectories() [2] on startup SetDefaultDllDirectories() affects the entire process, so it would needlessly break the world -- especially for embedding applications and ctypes users that have relied on adding directories

[issue36085] Enable better DLL resolution

Change by Jeremy Kloth : -- nosy: +jkloth ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue36085] Enable better DLL resolution

Steve Dower added the comment: I nosied both Windows and import experts, and I'm about to go ping relevant numpy/scipy people on https://github.com/numpy/numpy/pull/13019 Personally, I would prefer option #1, despite the pain it would cause. It is the better long-term supported option, and

[issue36085] Enable better DLL resolution

New submission from Steve Dower : So the fundamental problem is that the default DLL search path on Windows changes in various contexts, and the only consistent approach is the most difficult to support with current packaging tools. The result is .pyd files that need to resolve .dll