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.

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

BUG: Handle import of distutils for python 3.12 #438

Merged
merged 1 commit into from
Jun 22, 2024

Conversation

ERosendo
Copy link
Contributor

This PR adds code to handle the import of the sysconfig module for Python >= 3.11

@ERosendo ERosendo marked this pull request as ready for review February 13, 2024 02:19
Comment on lines +490 to +492
if sys.version_info >= (3, 11):
from sysconfig import get_path
libdir = get_path("platlib")
Copy link
Member

Choose a reason for hiding this comment

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

Looks like sysconfig is a module all since Python 3.2 ... Maybe we can just skip the if?

@ferdnyc
Copy link

ferdnyc commented Mar 31, 2024

None of this code will work anyway — the code is looking for *.egg-link files, which haven't been supported in forever. They're as deprecated as distutils. Anything installed with pip install -e in a venv will have a .dist-info directory instead.

(Of course, so will most other packages installed in the venv, so it's necessary to look for a direct_url.json file inside the .dist-info dir to determine whether or not the package is editable, and the path to its source files.)

@ferdnyc
Copy link

ferdnyc commented Mar 31, 2024

IIUC correctly, based on the comment above the code that uses distutils, it's there to support one very particular use case: When the user is working in a virtualenv, but runs a pdoc script installed outside of that virtualenv (in a system or user path).

Honestly, the most sensible solution, to me, seems to be: Just don't support that. It's a lot of effort for very little reward. When working in a venv, install pdoc3 in the venv. Simple as that, and all of the path shenanigans go away.

Otherwise, checking for a VIRTUAL_ENV environment variable is the wrong way to detect when running in a venv (though it is a clever way to detect a non-venv-installed package being run from an activated venv).

But if pdoc is being executed under a non-venv Python interpreter, even with an activated venv, distutils.sysconfig.get_python_lib() / sysconfig.get_path() won't return the paths inside the venv. They'll return the system paths.

And if pdoc is actually being executed in a venv (that it's installed in), then there's no need to screw with sys.path, it'll already point into the venv's site-packages dir. If there are any editable installs, importlib.import_module('packagename') will work to import those packages, and the path to the source files is just packagename.__path__[0].

So I really don't get what that code is trying to do anyway.

cclauss added a commit to cclauss/filetype.py that referenced this pull request May 13, 2024
@kernc kernc merged commit f7f053e into pdoc3:master Jun 22, 2024
@kernc
Copy link
Member

kernc commented Jun 22, 2024

And if pdoc is actually being executed in a venv (that it's installed in), then there's no need to screw with sys.path, it'll already point into the venv's site-packages dir. If there are any editable installs, importlib.import_module('packagename') will work to import those packages, and the path to the source files is just packagename.__path__[0].

@ferdnyc Apparenly, this being a non-issue hasn't always been the case ... 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants