[issue36085] Enable better DLL resolution

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[David Miguel Susano Pinto]

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

--
message_count: 60.0 -> 61.0
nosy: +carandraug
nosy_count: 14.0 -> 15.0
pull_requests: +18563
pull_request: https://github.com/python/cpython/pull/19167

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

Change by Steve Dower :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[David Bolen]

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 case to the PR as 
requested there.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

Steve Dower  added the comment:

> any chance at a 32-bit version of the installer

Done (on the PR). Thanks!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[David Bolen]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 all updates" as it always has, though 
the log file specifically refers to the KB. I doubt enough people are going to 
hit this for it to be a huge problem.

Also, I think Eryk was right that Windows 8 apparently does require the 
additional flag. It shouldn't affect what the test is testing, so I put it in 
for all versions. Just waiting on the custom buildbot to start to verify that 
the flag is all that's necessary and not the other changes I tried.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

Change by Steve Dower :


--
pull_requests: +12568

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 just started the custom 
buildbots with my fix, so we'll see if it's that or something else.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[David Bolen]

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 that now.  I've restarted the most 
recent build and it's already past both previous error points successfully.

Windows 7 is clearly on the wane, but as there may still be other systems in a 
similar state as my worker, the new KB requirement for 3.8 should probably be 
documented with the release.

--
nosy: +db3l

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 individual flags (i.e. 
LOAD_LIBRARY_SEARCH_SYSTEM32, LOAD_LIBRARY_SEARCH_APPLICATION_DIR, 
LOAD_LIBRARY_SEARCH_USER_DIRS) one by one until it works.

We could enable loader snaps in the registry for the Python executable; run it 
as a debugger; and log the debug output to see exactly what the loader is 
failing to find and where it's searching.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Zachary Ware]

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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? (As 
an aside, 
https://buildbot.python.org/all/#/builders?tags=custom.unstable&tags=custom.stable
 is currently showing no builders, but I'm not sure we have a custom one that 
matches the configuration here anyway.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

Change by Steve Dower :


--
pull_requests: +12564
stage: commit review -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 into a subdirectory and only 
allows imports from that directory and the pure stdlib (for encodings). Then we 
test with add_dll_directory(), then copy sqlite3.dll in and test again without.

Assuming tests all pass, I consider this complete now.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 doesn't pick those
> up). 

Instead of copying the whole install, you should be able to symlink the core 
binaries (e.g. python.exe, python38.dll, python3.dll, vcruntime140.dll) to a 
temporary directory and set PYTHONHOME. Most (or at least some) of the build 
bots should be set up to grant the symlink privilege to the current user or all 
standard users, and %TEMP% should be on an NTFS/ReFS volume that supports 
reparse points.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 have required a deprecation period, as it's explicitly 
documented as being ignored on Windows).

I believe the docs are updated as much as necessary, but if I missed something 
please call out.

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 doesn't pick those up). But with ctypes I can exclude 
the application directory from the search paths. Open to brilliant ideas here.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 accidentally match another directory 
in the search path. In POSIX, a path with a slash in it is always relative to 
the working directory and never searched for. In Windows, particularly for the 
loader, all relative paths are always searched for. This works with the current 
LoadLibraryW call, with minimal risk of name collisions, because only the 
application directory and system directories are checked before the working 
directory, which is checked before searching PATH. I suggest that with the 
change to use LOAD_LIBRARY_SEARCH_DEFAULT_DIRS, ctypes should first resolve 
such a path by calling GetFullPathNameW, as is suggested in the Windows docs.

I also had suggested documenting and exposing a subset of the loader flags for 
use with the CDLL `mode` parameter, which is currently unused in Windows. This 
would make it convenient for a user to include the flag 
LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR when the DLL path is fully qualified. However, 
that was deemed too peripheral to be worth the extra effort of supporting and 
documenting the parameter and adding the required tests, which is 
understandable. This can maybe be addressed in another issue, if the need 
arises. That said, it would be nice to provide parity with C extension modules 
here, which are always loaded with the latter flag. If given a fully-qualified 
path, or a relative path with slashes in it that's resolved via 
GetFullPathNameW, _ctypes.LoadLibrary could automatically include the 
LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR flag.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Brett Cannon]

Change by Brett Cannon :


--
nosy:  -brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 out in reverse order, I assume the first one is the 
problem. I'll see if I can ping the MSSQL team and find out if they know what's 
going on.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Zachary Ware]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 enumerates a set of 
installed services (presumably both 1st and 3rd party) and one of those fails 
on AppVeyor. (The Azure Pipelines tests are fine, as are all the local test 
machines I've used.) There's no indication what AppVeyor has installed that is 
causing the problem.

So it seems we'll have to not use the safe DLL lookup for all parts of CPython, 
and restrict it only to ctypes and extension module loading. (Or else drop 
AppVeyor as a required check.)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Ralf Gommers]

Change by Ralf Gommers :


--
nosy: +ralf.gommers

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Łukasz Langa]

Ł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 territory.

I think it's reasonable to leave Windows 7 support but *require* KB2533625 to 
be in. We've done similar things before on other platforms.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[mattip]

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 document the 
system calls and not to explain the user experience. A diagram with some 
examples of setting and debugging this would go a long way to helping users 
enter the right mindset to debug failures to load DLLs because the support dlls 
they depend on are not found

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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.

Don't forget loader snaps, which we can log using a standard debugger such as 
WinDbg or by attaching a Python script as a debugger (e.g. debug a child 
process via the DEBUG_PROCESS creation flag). For the latter, we need a 
debug-event loop (i.e. WaitForDebugEventEx via ctypes) that logs debug-string 
events. This will show the paths that the loader checks and the load attempts 
that fail with STATUS_DLL_NOT_FOUND (0xC135). We have to first enable 
loader snaps for the executable by setting a flag value of 2 in the 
"GlobalFlag" DWORD in the key "HKLM\Software\Microsoft\Windows 
NT\CurrentVersion\Image File Execution Options\". Or use 
gflags.exe to set this value.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 always loaded) and these are going to 
be rare calls I'm not too concerned. Still, as soon as we drop Win7, it'll be 
nice to clean these up.

I ended up making the functions public as os.add_dll_directory and 
os.remove_dll_directory. The return value is using a capsule (which is needed 
because it's an opaque pointer that you use to remove the directory later), and 
honestly I don't think it matters enough to give it its own type. Given the 
choice between making the functions private (and requiring "import nt; 
nt._add_dll_directory()") vs. implementing a whole type for one opaque value, 
I'll make the functions private :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

Change by Steve Dower :


--
keywords: +patch
pull_requests: +12277
stage:  -> patch review

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[mattip]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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. Instead we have to explicitly use 
LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | LOAD_LIBRARY_SEARCH_DEFAULT_DIRS in each 
LoadLibraryExW call in order to support loading DLLs beside the extension 
module. In this case, embedding applications that don't call 
SetDefaultDllDirectories won't have a problem loading extensions that rely on 
AddDllDirectory. It's only ctypes and cffi packages that will be forced to 
update if they currently rely on PATH or the working directory.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Paul Moore]

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

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Paul Moore]

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 is intended to solve 
the problem of making it easier for .pyd files to reliably load helper DLLs 
from shared locations. That's fine, and while it's an important use case (AIUI, 
it matters for a lot of the scientific stack) IMO it's *not* important enough 
to warrant breaking working scripts or embedding applications (particularly as 
this is a fairly obscure detail of how Windows works, so it's unlikely that 
people carefully follow "best practices" here).

I'm very concerned that comments I've seen here, specifically

>> 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 implement this just for extension-module loading with the
>> LoadLibraryExW flags.
>
> Only if they're loading them via PATH. If they're using full paths they'll be 
> fine, and if they're using system DLLs they'll be fine. In both cases, the 
> fix will work (better) with existing versions.
>
>> 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.
>
> That's true. "import" will always use the secure flags, and so if you were 
> relying on PATH to locate dependencies of the extension module (note that 
> extension modules themselves are loaded by full path, so it doesn't apply to 
> them), you need to stop doing that.

imply that it's OK to break working code "because they are doing things 
wrongly". That's not how backward compatibility works - we should avoid 
breaking *any* working code, no matter how ill-advised it seems to be.

If it's necessary to break code that (say) uses ctypes to load a DLL via PATH, 
or an embedding application that relies on getting DLLs using PATH, then we 
need to follow PEP 387 and go through a deprecation cycle for the existing 
behaviour.

For the ctypes case I assume we can detect where we found the DLL being loaded, 
so warning that behaviour will change is certainly possible.

For the embedding case, we could (for example) add an API 
Py_UseSecureSearchPath(bool) that embedders should call to opt into the new 
search semantics. With an explicit opt-in, we can then migrate that to be the 
default over time - have the Python API warn for a release if called without 
the opt-in, and then switch the default to be the secure search path, with 
applications that want to use the old search path being able to opt out using 
Py_UseSecureSearchPath(FALSE) for a release or two.

That proposal is very much off the top of my head. But the point is that it's 
not impossible to make the transition follow the normal backward compatibility 
rules, and so we should do so.

Of course, far simpler would be to choose a solution which *doesn't* break 
existing code :-)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 
delay loading and currently rely on unsupported resolve paths.

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 16-bit directory), the current working directory, and the PATH 
directories. Also, there's no LoadLibraryExW flag to use the legacy search 
path, either, so scripts and packages that use ctypes or cffi will have to be 
updated if they depend on PATH or changing the working directory. 

The alternative is to modify Python's importer to use the secure LoadLibraryExW 
flags (i.e. LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR | 
LOAD_LIBRARY_SEARCH_DEFAULT_DIRS), without affecting the rest of the process. 

LOAD_LIBRARY_SEARCH_DEFAULT_DIRS includes the application directory, the user 
directories added with AddDlldirectory or SetDllDirectoryW, and the System32 
directory. LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR prepends the directory of the 
target DLL, which must be passed as a fully-qualified path.

> The docs for SetDllDirectory seem to imply that there *is* a global
> impact

SetDllDirectoryW still works after calling SetDefaultDllDirectories, as long as 
we include either LOAD_LIBRARY_SEARCH_USER_DIRS or 
LOAD_LIBRARY_SEARCH_DEFAULT_DIRS. It only allows adding a single directory, so 
it's of limited use anyway.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Paul Moore]

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 one thing I'm not clear on, will this change just affect
the embedded Python, or will it affect the whole process - which would
mean that supporting an embedded Python 3.8 interpreter would mean
potentially reorganising the application layout. That may be quite a
cost, in some applications.

Note that this is all on the basis of "I don't understand the
implications, they should be documented" rather than being a specific
problem that I know will happen. My particular scenario, though, is an
application like Vim, that provides optional support for an "embedded
scripting" which may be any one of a number of Python versions, or
even other languages. In an application like that, costs for
supporting Python 3.8 may simply result in no (or delayed) support for
Python 3.8, rather than the application getting fixed.

> And if you're still writing code for Windows 7 with no security updates 
> installed, Python 3.8 isn't going to save you anyway.

Nobody's suggesting that it will. But maintaining *existing* code that
supports older Windows versions, while still allowing Python 3.8 to be
used as an embedded scripting language on systems that support it, is
an entirely reasonable proposal.

> > I really have no feel as to what practical impact there would be on an
> > embedded application.
>
> Since we're not going to change the default search directories for the entire 
> process when embedding

OK, if that's the case, then that alleviates most of my concerns. But
it really wasn't obvious to me, and it's something that I think should
be made clear in the docs, if only to reassure embedding applications
that Python isn't making global changes. The docs for SetDllDirectory
seem to imply that there *is* a global impact - "The SetDllDirectory
function affects all subsequent calls to the LoadLibrary and
LoadLibraryEx functions" (note - *all* subsequent calls, which implies
that behaviour will change for the embedding application once Python
has been loaded).

> the only practical impact is that your extension modules need to have their 
> dependent DLLs either:
> * in the system directory
> * adjacent to the .pyd file
> * in a directory added using AddDllDirectory

That seems fine, so let's just state that and keep things simple for
embedders to understand.

> And if the embedding application is already calling SetDefaultDllDirectories, 
> as has been recommended for years, then they're already experiencing this 
> change and won't have to update a thing.

Sadly, in my experience, an awful lot of projects (specifically, open
source projects that write mostly cross-platform code, with the
minimum of OS-specific differences) don't follow recommendations like
this. They use LoadLibrary without digging too deeply into the
implications or complexities, as long as it does what they want. And I
don't think MS helped themselves much here, either - the whole
business with SxS installs and assemblies was (IMO) *way* too much
complexity for most cross platform projects to bother with, and went
ignored. Even once things got simpler again, there remained a sense of
"don't go there, just get something that works". (And to be clear, I'm
not bashing on MS here - I find the Linux machinery around all of this
to be just as complex and confusing).

Anyhow, if as you say the only impact is that when a pyd file depends
on a DLL, that DLL needs to be located in one of three places, all of
which are equally valid on Python <=3.7, and there's no impact on the
non-Python part of the embedded application, then it's not a big deal.
Let's make the change, write up those points in What's New (at least),
and leave it at that.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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]: http://technet.microsoft.com/en-us/sysinternals/bb896645

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 implement this just for extension-module loading with the
> LoadLibraryExW flags.

Only if they're loading them via PATH. If they're using full paths they'll be 
fine, and if they're using system DLLs they'll be fine. In both cases, the fix 
will work (better) with existing versions.

> 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.

That's true. "import" will always use the secure flags, and so if you were 
relying on PATH to locate dependencies of the extension module (note that 
extension modules themselves are loaded by full path, so it doesn't apply to 
them), you need to stop doing that.

> Also, at runtime we can raise a SystemError if AddDllDirectory isn't found via
> GetProcAddress. This supports portable Python installations.

This adds a lot of complexity for very old Windows 7 installs. I'm not inclined 
to care that much for them - installing security updates isn't a big ask for a 
nearly EOL operating system.

> 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

Correct. But this is already the case if your sysadmin has enabled certain 
policies or if you're running via Store Python. So you can't rely on PATH 
anyway.

> Why is this CPython-specific and "private"? It seems like
> * it should be a public interface, used by all implementations consistently,
> as a policy decision for the win32 platform and documented as such

Not every implementation has to support Windows. We can certainly recommend 
that those that do copy it, but I'm not going to force MicroPython to declare 
an exception from a standard Python API.

> * located in os, much like os.environ (not critical)

Fair point. It can go into os. :)

> There should be some kind of debugging tool for when LoadLibraryEx fails, to
> indicate what might have gone wrong, perhaps os.getdlldirectory() would be
> helpful

I'd love to have this. Now someone just has to invent one that can be used from 
Python :) It's unrelated to this discussion - in fact, this change will make it 
so that you get the failure on _all_ machines, not just on some random user's 
machine.

We can't retrieve the true search path, only the ones that were added through 
an API that we control and making assumptions based on the documentation. I 
think this would be more of a distraction. The best way to diagnose actual 
LoadLibrary failures is to use a debugger, at which point simply getting the 
search paths doesn't add anything.

> 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.

And if you're still writing code for Windows 7 with no security updates 
installed, Python 3.8 isn't going to save you anyway.

> I really have no feel as to what practical impact there would be on an
> embedded application.

Since we're not going to change the default search directories for the entire 
process when embedding, the only practical impact is that your extension 
modules need to have their dependent DLLs either:
* in the system directory
* adjacent to the .pyd file
* in a directory added using AddDllDirectory

And if the embedding application is already calling SetDefaultDllDirectories, 
as has been recommended for years, then they're already experiencing this 
change and won't have to update a thing.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Paul Moore]

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 comply with us, else they'll have to accept that some 
> packages won't work without the SetDefaultDllDirectories call.

This bothers me - how will backward compatibility work in that case?
There are applications (for example, Vim) that can embed Python, and
it's possible to pick the Python version at compile time. Would Vim
need additional code (possibly guarded by some sort of "If this is
Python 3.8 or later" flag, which from my knowledge of the Vim code
would not be particularly easy to add in a backward compatible way) to
handle this change?

Actually, as a more general point, I have been following this
discussion, but I really have no feel as to what practical impact
there would be on an embedded application. I get that this is OS
functionality, and therefore it's not Python's place to explain the
details to users, but IMO it *is* Python's responsibility to explain
how embedding applications will need to change if we change how we
configure things. Even if users are currently using an approach that
is no longer encouraged (which is *I think* what you're saying about
putting DLLs on PATH) they are still using something that works right
now, and we're breaking it - so we need to describe what changed,
*why* we felt we should break their code, and what they need to do,
both to switch to the new model, and (if they have a requirement to do
so) to support the old and new models simultaneously in their code
(very few people, not even embedders, can suddenly say "we're dropping
support for Python 3.7 and older, we only allow 3.8+ from now on").

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[mattip]

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). Assuming that is true, then

> add sys._adddlldirectory() and sys._removedlldirectory() as CPython-specific 
> functions for extending the search path (for use by packages currently 
> modifying PATH at runtime)

Why is this CPython-specific and "private"? It seems like
* it should be a public interface, used by all implementations consistently, as 
a policy decision for the win32 platform and documented as such
* located in os, much like os.environ (not critical)

There should be some kind of debugging tool for when LoadLibraryEx fails, to 
indicate what might have gone wrong, perhaps os.getdlldirectory() would be 
helpful

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 modifying PATH 
for a delayed load, though I hope that's uncommon. I think it's easier for 
everyone else if we implement this just for extension-module loading with the 
LoadLibraryExW flags. 

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 comply with us, else they'll have to accept that some packages 
won't work without the SetDefaultDllDirectories call.

> ensure LoadLibrary when used in ctypes or importing will use the 
> correct flags

ctypes calls LoadLibraryW, which uses the default that's set by 
SetDefaultDllDirectories, if that's what we eventually decide is the best 
course of action.

If we decide to not call SetDefaultDllDirectories, then we should provide a way 
for ctypes packages to update to using the secure search path instead of 
relying on the legacy search path. We could rewrite the ctypes LoadLibrary 
wrapper to call LoadLibraryExW instead of LoadLibraryW and support the flags in 
the CDLL `mode` parameter, which is currently unused in Windows.

> add sys._adddlldirectory() and sys._removedlldirectory() as CPython-
> specific functions for extending the search path (for use by packages 
> currently modifying PATH at runtime)

I'd prefer some way for scripts and packages to configure their shared-library 
search paths as static data. The implementation would be kept private in the 
interpreter. 

I know there's debate about removing ".pth" files. But maybe we could  
implement something similar for the DLL search path with package and script 
".pthext" files. These would contain a list of directories (relative to the 
script or package) that extend the shared-library search path.

> add check for KB2533623 to the installer and block if it is not
> present

Also, at runtime we can raise a SystemError if AddDllDirectory isn't found via 
GetProcAddress. This supports portable Python installations.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 sys._adddlldirectory() and sys._removedlldirectory() as CPython-specific 
functions for extending the search path (for use by packages currently 
modifying PATH at runtime)
* add check for KB2533623 to the installer and block if it is not present

Any thoughts? The only one I'm not 100% committed to is the 
SetDefaultDllDirectories call, but I'd rather ship it in alpha/beta releases 
and pull it out later if necessary. Passing the flags to LoadLibrary should 
have the same effect anyway, so I don't think changing the defaults in 
python.exe will make the current scenarios worse.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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, and 
if Python 3.8 slips by 3 months then it will also be a hard drop unless we make 
an exception to the policy.

Paul's comment suggests we should avoid slipping/make the exception, and that 
it's okay to require the update, which is basically where I'm at too (provided 
the buildbot team are willing to keep an EOL'd OS running for as long as 3.8 is 
supported).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Paul Moore]

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 Win7 support in 3.8 completely - I feel like that's a bit 
too aggressive, as Eryk says, there's still a *lot* of Windows 7 usage.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 
remain popular for a few years after its scheduled end of life in January 2020. 
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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 concern is the KB2533623 I mentioned in the original post, which adds the 
ability to control the search path properly. I *think* it might be already 
included in Win7 SP1, in which case we're fine (I'm confirming this with some 
colleagues), but if it's optional on top of SP1 then I want to make it required 
for Python.

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, and should get python-dev exposure.

Łukasz - thoughts?

(Yes, I incorrectly typed the KB number at the top. Apparently I regularly fail 
to type numbers into bpo for some reason... doesn't happen elsewhere?)

--
nosy: +lukasz.langa

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Nick Coghlan]

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 it's actually part of the parent package. 
Details at https://bugs.python.org/issue1644818 (yes, that's a SourceForge era 
issue number).

The solutions may not overlap in practice, but they're conceptually related 
(i.e. outside frozen modules, the package topology is pretty tightly coupled to 
the underlying filesystem layout)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 are your 
process won't start (and in general, you copy the current value and add to it). 
I've never seen anyone try to reset PATH here, nor would I recommend it.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[mattip]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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, yet...).

Doing this kind of analysis live is oddly tough, but there may be an ETW 
provider that a debug hook could enable to get more of a trace. Again, we're 
deep in third-party tool territory, not a change to core CPython.

Certainly if we can drop support for base Windows 7 we will document how to use 
more recent OS support via whatever we add. Though to a certain extent those 
hitting problems are going deep enough that reading the MSDN docs will have to 
be mandatory (at least for those who want to know "why"). I really don't want 
to have to reproduce those docs and make them guaranteed Python semantics.

> will annoy developers whose code base is full of `os.environ['PATH']` games. 
> Perhaps the solution should come with a deprecation warning when setting 
> `os.environ['PATH']`.

Yeah, this is the downside of changing anything at all, though of course since 
resolving DLLs via PATH is already broken, those developers are already annoyed 
;) And we can't add warning that wouldn't annoy those who legitimately modify 
PATH for process launches. So I think it's mostly about providing a supported 
path for those developers to be able to port their code when they discover it's 
broken.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[mattip]

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 the solution should come with a deprecation 
warning when setting `os.environ['PATH']`.

It would be very helpful if failure to import a pyd (or for that matter to open 
a DLL with ctypes) due to missing support dlls could be easily debugged. Right 
now we get a message from windows that seems to suggest the file was not found.
- Python could check if the file exists on disk and print a more helpful message
- A debug hook to print the dll search path at the time of the failed 
LoadLibraryEx call, or perhaps adding it as an attribute of the Exception (this 
might be nice to have on Linux as well, even though there the Exception already 
includes the name of the missing *.so).

Even better would be official python/Microsoft support for a CLI version of 
depends.exe like ldd on linux, but that seems much harder and is out of scope 
for this issue.

--
nosy: +mattip

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 only.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 the same name with this technique. That 
requires private assembly packages, which is doable (in Windows 7+), but a bit 
complex and requires modifying the embedded #2 manifest of the extension 
module. The alternative is to rewrite the PE import tables of all DLLs to 
reference unique DLL names. Neither is necessary if everything is built against 
unique, versioned DLL names.

> I think you're right that we just need to update the LoadLibrary
> flags, but will those also apply to dependencies of the loaded 
> module? 

The DLL search path is computed once per LoadLibraryExW call based on either 
the call flags or the process default flags. We shouldn't mess with the process 
default, since there's no way to restore the legacy DLL search path, in 
particular this includes the Windows directory (%SystemRoot%), 16-bit System 
directory (%SystemRoot%\System), current directory, and PATH. 

Should we support a convenient syntax for including the current value of PATH 
at extension-module load time? This could be an entry that's exactly equal to 
"". (Less-than and greater-than are reserved as wildcard characters by 
all Windows file systems that I can think of.) It would iterate over PATH, 
adding each directory via AddDllDirectory. Of course, all added directories 
would subsequently be removed via RemoveDllDirectory after the LoadLibraryExW 
call.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 also apply to dependencies of the loaded module? I thought not...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Eryk Sun]

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 to PATH. When loading an extension module, we 
can simply replace LOAD_WITH_ALTERED_SEARCH_PATH in the LoadLibraryExW flags 
with LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR and LOAD_LIBRARY_SEARCH_DEFAULT_DIRS 
(application directory, System32 directory, and directories added via 
SetDllDirectoryW and AddDllDirectory). Writers of extension modules are 
constrained by our API. We'll simply mandate that PATH is no longer searched.

It's cumbersome to require packages to have to manually call AddDllDirectory 
before being able to import an extension module with dependencies. We could 
create a protocol to store relative paths as an embedded resource in the 
extension module, which would be similar to the RPATH/RUNPATH $ORIGIN field in 
POSIX. We'd first map the extension module as a data file via 
LOAD_LIBRARY_AS_DATAFILE | LOAD_LIBRARY_AS_IMAGE_RESOURCE. Load and resolve the 
relative paths, add them via AddDllDirectory. Call LoadLibraryExW with the 
above-mentioned flags. Then remove the directories via RemoveDllDirectory.

--
nosy: +eryksun

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Jeremy Kloth]

Change by Jeremy Kloth :


--
nosy: +jkloth

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 Anaconda has already adopted a patch 
that does this. However, I think it's most appropriate to be a change in 
CPython at a major release boundary so that we can provide proper porting 
information for users.

Option #2 is kind of neat, and honestly I thought this already worked when the 
fully-qualified .pyd was in a folder on sys.path. However, it's going to mess 
big time with all of our existing build tools. So I'm not thrilled about 
passing on that kind of pain - then again, most people don't need this, and 
those who do can do their own hacks to make it work (on the theory that they're 
already applying their own hacks anyway).

I'm totally open to other suggestions on how to make these situations workable, 
though I will (continue to) push back hard against ideas that simply bring back 
the security concerns that led us to this point :)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue36085] Enable better DLL resolution

[Steve Dower]

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 dependencies from directories *other* than where the 
.pyd file is located.

Here's a generic scenario:
* my_package.subpackage1.my_module is implemented as 
my_package/subpackage1/my_module.pyd
* my_package.subpackage2.my_module is implemented as 
my_package/subpackage2/my_module.pyd
* my_module.pyd in both cases depends on HelperLib.dll
* both modules must end up with the same instance of HelperLib.dll

While there are various ways for my_modules.pyd to locate HelperLib.dll, the 
only totally reliable way is to put HelperLib.dll alongside my_module.pyd. 
However, because it is needed twice, this means two copies of the DLL, which is 
unacceptable.

With Python 3.8, we are *nearly* dropping support for Windows 7, and I believe 
we can justify dropping support for Windows 7 without KB2533625 [1], which will 
have been released over eight years by the time 3.8 releases. This means the 
DLL search path enhancements are available.


Proposal #1: CPython calls SetDefaultDllDirectories() [2] on startup and 
exposes AddDllDirectory() [3] via the sys or os module.

This would ensure consistency in DLL search order regardless of security 
settings, and modules that have their own ".libs" directory have a supported 
API for adding it to the search path.

Past experience of forcing a consistent search path like this is that it has 
broken many users who expect features like %PATH% to locate DLL dependencies to 
work. For security reasons, this feature is already deprecated and often 
disabled (see [4]), so it can't be relied upon, but it makes it impossible for 
a single package to modify this setting or use the supported method for adding 
more DLL search directories.


Proposal #2: Resolve extension modules by full name

Without this proposal, the directory structure looks like:

my_package\
-subpackage1\
--__init__.py
--my_module.pyd
--HelperLib.dll
-subpackage2\
--__init__.py
--my_module.pyd
--HelperLib.dll

After this proposal, it could look like:

my_package\
-subpackage1
--__init__.py
-subpackage2\
--__init__.py
-my_package.subpackage1.my_module.pyd
-my_package.subpackage2.my_module.pyd
-HelperLib.dll

Essentially, when searching for modules, allow going up the package hierarchy 
and locating a fully-qualified name at any level of the import tree.

Note that since "import my_package.subpackage1.my_module" implies both "import 
my_package" and "import my_package.subpackage1", those have to succeed, but 
then the final part of the import would use subpackage1.__path__ to look for 
"my_module.pyd" and my_package.__path__ to look for 
"my_package.subpackage1.my_module.pyd".

This allows all extension modules to be co-located in the one (importable) 
directory, along with a single copy of any shared dependencies.

[1]: https://go.microsoft.com/fwlink/p/?linkid=217865
[2]: 
https://docs.microsoft.com/windows/desktop/api/libloaderapi/nf-libloaderapi-setdefaultdlldirectories
[3]: 
https://docs.microsoft.com/windows/desktop/api/libloaderapi/nf-libloaderapi-adddlldirectory
[4]: 
https://docs.microsoft.com/windows/desktop/Dlls/dynamic-link-library-search-order

--
assignee: steve.dower
components: Windows
messages: 336349
nosy: brett.cannon, eric.snow, ncoghlan, paul.moore, steve.dower, tim.golden, 
zach.ware
priority: normal
severity: normal
status: open
title: Enable better DLL resolution
type: enhancement
versions: Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com