From 979b2054a1d58ba04f1309ff57a358d5ca34f830 Mon Sep 17 00:00:00 2001 From: William Schwartz Date: Fri, 15 Jan 2021 13:19:54 -0600 Subject: [PATCH 1/6] Simplify pywin32_bootstrap, avoid importing site Since pywin32 has dropped support for Python 2.7 since version 300 and support for Python 3.0 through 3.4 since version 223, we can now rely on [PEP-420] semantics when importing names of directories available on `sys.path` and not containing an `__init__.py`. This applies to to `pywin32_system32`. The previous version of `win32.lib.pywin32_boostrap` manually searched site-packages directories, stopping at the first one found. Now we pass this responsibility off to Python's import machinery. When we `import pywin32_system32`, if successful, `pywin32_system32` will likely be a `_frozen_importlib_external._NamespacePath` object. We obtain its first entry, being careful that it didn't support sequence indexing until Python 3.7 or so (and in any case, that's all undocumented; the [documentation promises] only that `__path__: Iterable[str]`). This first entry is the path to the directory containing the DLLs, just as before, so the remainder of the code is unchanged. The only semantic differences between this implementation and the previous one is that 1. we no longer import `site` (always better to import less!), and 2. `pywin32_system32` is no longer _required_ to be in a site-packages directory Just as with the previous implementation, this one does not raise an exception if `pywin32_system32` cannot be found. I did one annoying thing in this patch to minimize the diff, which is that I redefine the variable `pywin32_system32`: first it refers to a namespace package module object, and then later it refers to the `str` directory name. It might be reasonable after initial review of the patch to rename the latter instance of the variable to something like `path`. Finally, I added no tests and no change log entry, but can if necessary. [PEP-420]: https://www.python.org/dev/peps/pep-0420/#specification [documentation promises]: https://docs.python.org/3/reference/import.html#__path__ --- win32/Lib/pywin32_bootstrap.py | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/win32/Lib/pywin32_bootstrap.py b/win32/Lib/pywin32_bootstrap.py index 434ed598b5..392b795d59 100644 --- a/win32/Lib/pywin32_bootstrap.py +++ b/win32/Lib/pywin32_bootstrap.py @@ -7,22 +7,27 @@ # If Python has `os.add_dll_directory()`, we need to call it with this path. # Otherwise, we add this path to PATH. import os -import site -# The directory should be installed under site-packages. -dirname = os.path.dirname -# This is to get the "...\Lib\site-packages" directory -# out of this file name: "...\Lib\site-packages\win32\Lib\pywin32_bootstrap.py". -# It needs to be searched when installed in virtual environments. -level3_up_dir = dirname(dirname(dirname(__file__))) +try: + Exc = ModuleNotFoundError # Introduced in Python 3.6 +except NameError: + Exc = ImportError -site_packages_dirs = getattr(site, "getsitepackages", lambda: [])() -if level3_up_dir not in site_packages_dirs: - site_packages_dirs.insert(0, level3_up_dir) +try: + import pywin32_system32 +except Exc: + path_iterator = iter([]) +else: + # We're guaranteed only that __path__: Iterable[str] + # https://docs.python.org/3/reference/import.html#__path__ + path_iterator = iter(pywin32_system32.__path__) -for site_packages_dir in site_packages_dirs: - pywin32_system32 = os.path.join(site_packages_dir, "pywin32_system32") +try: + pywin32_system32 = next(path_iterator) +except StopIteration: + pass +else: if os.path.isdir(pywin32_system32): if hasattr(os, "add_dll_directory"): os.add_dll_directory(pywin32_system32) @@ -31,4 +36,3 @@ elif not os.environ["PATH"].startswith(pywin32_system32): os.environ["PATH"] = os.environ["PATH"].replace(os.pathsep + pywin32_system32, "") os.environ["PATH"] = pywin32_system32 + os.pathsep + os.environ["PATH"] - break From d9fb26fd5374b040fce987994fac2ab349ce18a0 Mon Sep 17 00:00:00 2001 From: William Schwartz Date: Tue, 19 Jan 2021 21:38:02 -0600 Subject: [PATCH 2/6] Add changelog entry for #1651 --- CHANGES.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.txt b/CHANGES.txt index 4991408d9a..c8116259e3 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -7,6 +7,10 @@ However contributors are encouraged to add their own entries for their work. Note that, baring some major issue building and cutting the release, build 228 will be the last version supporting Python 2. +Since build 300: +---------------- +* Shifted work in win32.lib.pywin32_bootstrap to Python's import system from + manual path manipulations (@wkschwartz in #1651) Since build 228: ---------------- From cb51ca489ce3f3cf83eb9caf34f1931bb284253a Mon Sep 17 00:00:00 2001 From: William Schwartz Date: Tue, 19 Jan 2021 21:38:27 -0600 Subject: [PATCH 3/6] Rename variable per review comment https://github.com/mhammond/pywin32/pull/1651#pullrequestreview-571815833 --- win32/Lib/pywin32_bootstrap.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/win32/Lib/pywin32_bootstrap.py b/win32/Lib/pywin32_bootstrap.py index 392b795d59..34b9c8fb98 100644 --- a/win32/Lib/pywin32_bootstrap.py +++ b/win32/Lib/pywin32_bootstrap.py @@ -24,15 +24,15 @@ path_iterator = iter(pywin32_system32.__path__) try: - pywin32_system32 = next(path_iterator) + path = next(path_iterator) except StopIteration: pass else: - if os.path.isdir(pywin32_system32): + if os.path.isdir(path): if hasattr(os, "add_dll_directory"): - os.add_dll_directory(pywin32_system32) + os.add_dll_directory(path) # This is to ensure the pywin32 path is in the beginning to find the # pywin32 DLLs first and prevent other PATH entries to shadow them - elif not os.environ["PATH"].startswith(pywin32_system32): - os.environ["PATH"] = os.environ["PATH"].replace(os.pathsep + pywin32_system32, "") - os.environ["PATH"] = pywin32_system32 + os.pathsep + os.environ["PATH"] + elif not os.environ["PATH"].startswith(path): + os.environ["PATH"] = os.environ["PATH"].replace(os.pathsep + path, "") + os.environ["PATH"] = path + os.pathsep + os.environ["PATH"] From df3e334cca650391aa4420832ea3dd950fa9fe9c Mon Sep 17 00:00:00 2001 From: William Schwartz Date: Tue, 19 Jan 2021 21:50:08 -0600 Subject: [PATCH 4/6] Simplify per review comment https://github.com/mhammond/pywin32/pull/1651#discussion_r560627958 This very slightly changes semantics: if `pywin32_system32` is found to have multiple entries in `__path__` and the first one is not an existing directory, now the search will continue whereas before it would give up. --- win32/Lib/pywin32_bootstrap.py | 27 +++++++++++---------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/win32/Lib/pywin32_bootstrap.py b/win32/Lib/pywin32_bootstrap.py index 34b9c8fb98..d2e5c21aaa 100644 --- a/win32/Lib/pywin32_bootstrap.py +++ b/win32/Lib/pywin32_bootstrap.py @@ -17,22 +17,17 @@ try: import pywin32_system32 except Exc: - path_iterator = iter([]) + pass else: # We're guaranteed only that __path__: Iterable[str] # https://docs.python.org/3/reference/import.html#__path__ - path_iterator = iter(pywin32_system32.__path__) - -try: - path = next(path_iterator) -except StopIteration: - pass -else: - if os.path.isdir(path): - if hasattr(os, "add_dll_directory"): - os.add_dll_directory(path) - # This is to ensure the pywin32 path is in the beginning to find the - # pywin32 DLLs first and prevent other PATH entries to shadow them - elif not os.environ["PATH"].startswith(path): - os.environ["PATH"] = os.environ["PATH"].replace(os.pathsep + path, "") - os.environ["PATH"] = path + os.pathsep + os.environ["PATH"] + for path in pywin32_system32.__path__: + if os.path.isdir(path): + if hasattr(os, "add_dll_directory"): + os.add_dll_directory(path) + # This is to ensure the pywin32 path is in the beginning to find the + # pywin32 DLLs first and prevent other PATH entries to shadow them + elif not os.environ["PATH"].startswith(path): + os.environ["PATH"] = os.environ["PATH"].replace(os.pathsep + path, "") + os.environ["PATH"] = path + os.pathsep + os.environ["PATH"] + break From 7a6de27fdfa3c9213e698981d40c4b29c3d75cbb Mon Sep 17 00:00:00 2001 From: William Schwartz Date: Tue, 19 Jan 2021 21:53:26 -0600 Subject: [PATCH 5/6] Use ImportError instead of ModuleNotFoundError Per review comment: https://github.com/mhammond/pywin32/pull/1651#discussion_r560627068 --- win32/Lib/pywin32_bootstrap.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/win32/Lib/pywin32_bootstrap.py b/win32/Lib/pywin32_bootstrap.py index d2e5c21aaa..b13add363c 100644 --- a/win32/Lib/pywin32_bootstrap.py +++ b/win32/Lib/pywin32_bootstrap.py @@ -9,14 +9,9 @@ import os -try: - Exc = ModuleNotFoundError # Introduced in Python 3.6 -except NameError: - Exc = ImportError - try: import pywin32_system32 -except Exc: +except ImportError: # Python ≥3.6: replace ImportError with ModuleNotFoundError pass else: # We're guaranteed only that __path__: Iterable[str] From 74c96dc1c6d8b6daf3bb08f3e9c77ee956943da1 Mon Sep 17 00:00:00 2001 From: William Schwartz Date: Tue, 19 Jan 2021 21:57:29 -0600 Subject: [PATCH 6/6] Avoid importing `os` unless necessary Normally I wouldn't push the line on PEP 8 for importing `os`, but `win32.lib.pywin32_bootstrap` is imported by `site` at Python startup via pywin32.pth. Programs that care a lot about startup time might appreciate our caution here. --- win32/Lib/pywin32_bootstrap.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/win32/Lib/pywin32_bootstrap.py b/win32/Lib/pywin32_bootstrap.py index b13add363c..686a59a8f7 100644 --- a/win32/Lib/pywin32_bootstrap.py +++ b/win32/Lib/pywin32_bootstrap.py @@ -6,7 +6,6 @@ # modules are imported. # If Python has `os.add_dll_directory()`, we need to call it with this path. # Otherwise, we add this path to PATH. -import os try: @@ -14,6 +13,7 @@ except ImportError: # Python ≥3.6: replace ImportError with ModuleNotFoundError pass else: + import os # We're guaranteed only that __path__: Iterable[str] # https://docs.python.org/3/reference/import.html#__path__ for path in pywin32_system32.__path__: