Use sysconfig.get_path instead of distutils to find stdlib
#1323
Conversation
DanielNoord
added
the
Maintenance
astroid/modutils.py
Outdated
| # non-valid path, see https://bugs.pypy.org/issue1164 | ||
| except DistutilsPlatformError: | ||
| STD_LIB_DIRS = set() | ||
| STD_LIB_DIRS = {sysconfig.get_path("stdlib")} |
There was a problem hiding this comment.
Should we include sysconfig.get_path("platstdlib") too?
That would match get_python_lib(standard_lib=True, prefix=sys.prefix).
{
'platstdlib': '/home/runner/work/astroid/astroid/venv/lib/python3.10',
'stdlib': '/opt/hostedtoolcache/Python/3.10.2/x64/lib/python3.10'
}Usually stdlib should be enough. I just don't know if virtualenv or others do something unexpected with the lib files.
There was a problem hiding this comment.
I'm not sure? It was added in:
b8c4825
But there was not test or related issue that prompted the change?
There was a problem hiding this comment.
It would had to be a virtualenv issue from the looks of it. Tbh I don't know if it has been fixed in the meantime. Maybe we should include in for now, just to be safe. The additional path doesn't really hurt.
If we really want to, we could revisit this in the future and just remove it then to see if it causes any new regressions. I just wouldn't suggest to do it now. It would make it more difficult to see if the current changes were correct.
|
By the way, for some reason the previous crash no longer happens after merging |
Which crash? |
|
See failing test in the first commit and the issue I opened against |
It looks like the setuptools / distutils.version issue we dealt we back in December. So all good I think. |
Well.. That actually regressed 😓 See pylint-dev/pylint#5645. Hmm looking back the failing test is not actually related. Let me have another look. Edit: I'm mixing up my PRs. See #1326, that's where I opened an incorrect issue report against Python. |
😞 We did however remove the failing test case with it and it wasn't added back. That at least explains why this one isn't failing anymore. https://github.com/PyCQA/astroid/pull/1321/files#diff-fce15b6315286261c040399638ba825e411e7c0284c1006a50aa271f405beef6L75 It should be safe to move forward with this PR. The last open items are probably
|
There was a problem hiding this comment.
Just some last formatting suggesting. Looks good otherwise!
Thank you @DanielNoord for driving the effort to replace distutils!
Co-authored-by: Marc Mueller <[email protected]>
for more information, see https://pre-commit.ci
|
Think it's ready now 🥳 |
Steps
Description
2/3 of PRs mentioned in #1322
For the change in this PR:
get_python_libwill "If standard_lib is true, the directory for the standard library is returned rather than the directory for the installation of third-party extensions."This is thus the parent directory of
site-packages.This functionality is "replaced" by
sysconfig.get_pathwithstdlib.The commit that added this particular fix was 70b7afc
I have looked around but the closest I got to finding any information on the presumed bug was https://bugs.python.org/issue11320. I also found a number of older Python 2 and 3.6 < issues about
virtualenvanddistutils. I couldn't find the actual issue here..However, I think
sysconfigin combination with newer versions ofvirtualenvhave fixed these underlying issues. The tests seems to pass anyway, so I would argue to go for the YOLO-option here as well: we need to get rid ofdistutilssomeday anyway 😅Type of Changes
Related Issue