Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to stubtest's new check for missing stdlib modules #15960

Merged
merged 5 commits into from
Aug 27, 2023
Merged

Fixes to stubtest's new check for missing stdlib modules #15960

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eab1df1
Try to fix stubtest
AlexWaygood Aug 25, 2023
ee2b44f
fix stdlib-finding logic
AlexWaygood Aug 25, 2023
1e59296
More
AlexWaygood Aug 25, 2023
258fca7
improve comment
AlexWaygood Aug 25, 2023
bf9fdad
Add comments
AlexWaygood Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 22 additions & 11 deletions mypy/stubtest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import typing
import typing_extensions
import warnings
from collections import defaultdict
from contextlib import redirect_stderr, redirect_stdout
from functools import singledispatch
from pathlib import Path
Expand Down Expand Up @@ -1679,16 +1680,16 @@ def get_importable_stdlib_modules() -> set[str]:
all_stdlib_modules = sys.stdlib_module_names
else:
all_stdlib_modules = set(sys.builtin_module_names)
Copy link
Member Author

@AlexWaygood AlexWaygood Aug 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you still need sys.builtin_module_names the way that snippet does though

@hauntsaninja we start off with the sys.builtin_module_names modules being in the set

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh oops, I missed that. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No worries — thanks for the review! :D

python_exe_dir = Path(sys.executable).parent
modules_by_finder: defaultdict[importlib.machinery.FileFinder, set[str]] = defaultdict(set)
for m in pkgutil.iter_modules():
finder = m.module_finder
if isinstance(finder, importlib.machinery.FileFinder):
finder_path = Path(finder.path)
if (
python_exe_dir in finder_path.parents
and "site-packages" not in finder_path.parts
):
all_stdlib_modules.add(m.name)
if isinstance(m.module_finder, importlib.machinery.FileFinder):
modules_by_finder[m.module_finder].add(m.name)
for finder, module_group in modules_by_finder.items():
if (
"site-packages" not in Path(finder.path).parents
and {"json", "_json", "_queue"} & module_group
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these modules special?

Copy link
Member Author

@AlexWaygood AlexWaygood Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment in bf9fdad. If there was a sure-fire way of finding out where the pure-Python and extension stdlib modules are located on any Python build, we could filter according to the .path attribute of the finder, which would be more principled. There might be a sure-fire way of doing so using sysconfig, not sure.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these?

>>> sysconfig.get_path("stdlib")
'/Users/jelle/.pyenv/versions/3.11.1/lib/python3.11'
>>> sysconfig.get_path("platstdlib")
'/Users/jelle/py/venvs/py311/lib/python3.11'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these?

They don't do the right thing on Windows, unfortunately:

>>> import sysconfig, pkgutil
>>> sysconfig.get_path("stdlib")
'C:\\Users\\alexw\\AppData\\Local\\Programs\\Python\\Python311\\Lib'
>>> sysconfig.get_path("platstdlib")
'C:\\Users\\alexw\\AppData\\Local\\Programs\\Python\\Python311\\Lib'
>>> m = next(m for m in pkgutil.iter_modules() if m.name == "_queue")
>>> m.module_finder.path
'C:\\Users\\alexw\\AppData\\Local\\Programs\\Python\\Python311\\DLLs'

Copy link
Member Author

@AlexWaygood AlexWaygood Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.path.join(sysconfig.get_config_var("BINDIR"), sysconfig.get_config_var("platlibdir")) might be the incantation I'm looking for...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

os.path.join(sysconfig.get_config_var("BINDIR"), sysconfig.get_config_var("platlibdir")) might be the incantation I'm looking for...

no, that doesn't work on linux.

There's a StackOverflow answer here using distutils.sysconfig.get_python_lib, but it's nontrivial, and distutils is all deprecated anyway: https://stackoverflow.com/a/8992937/13990016. Not sure if it's worth it.

):
all_stdlib_modules.update(module_group)

importable_stdlib_modules: set[str] = set()
for module_name in all_stdlib_modules:
Expand Down Expand Up @@ -1719,13 +1720,23 @@ def get_importable_stdlib_modules() -> set[str]:
# The idlelib.* submodules are similarly annoying in opening random tkinter windows,
# and we're unlikely to ever add stubs for idlelib in typeshed
# (see discussion in https://github.com/python/typeshed/pull/9193)
if submodule_name.endswith(".__main__") or submodule_name.startswith("idlelib."):
#
# test.* modules do weird things like raising exceptions in __del__ methods,
# leading to unraisable exceptions being logged to the terminal
# as a warning at the end of the stubtest run
if (
submodule_name.endswith(".__main__")
or submodule_name.startswith("idlelib.")
or submodule_name.startswith("test.")
):
continue

try:
silent_import_module(submodule_name)
except KeyboardInterrupt:
raise
# importing multiprocessing.popen_forkserver on Windows raises AttributeError...
AlexWaygood marked this conversation as resolved.
Show resolved Hide resolved
except Exception:
except BaseException:
continue
else:
importable_stdlib_modules.add(submodule_name)
Expand Down