Resolve symlinks in the import path

[keichi]

Currently, module import fails when the import path contains a symlink.

An example is Python installed on macOS using Homebrew where /opt/homebrew/opt/python is a symlink to /opt/homebrew/Cellar/[email protected]/3.9.8 (exact version varies). As a result, get_python_lib(standard_lib=True) returns /opt/homebrew/opt/[email protected]/Frameworks/Python.framework/Versions/3.9/lib/python3.9 while sys.path is [..., '/opt/homebrew/Cellar/[email protected]/3.9.8/Frameworks/Python.framework/Versions/3.9/lib/python3.9', ...]. The following tests fail due to this inconsistency:

FAILED tests/unittest_brain.py::MultiprocessingBrainTest::test_multiprocessing_manager - AssertionError: Uninferable != 'array.array'
FAILED tests/unittest_brain_ctypes.py::test_cdata_member_access - astroid.exceptions.InferenceError: StopIteration raised without ...
FAILED tests/unittest_builder.py::BuilderTest::test_socket_build - AssertionError: 'connect' not found in <ClassDef.socket l.214 a...
FAILED tests/unittest_modutils.py::StandardLibModuleTest::test_4 - AssertionError: False is not true
FAILED tests/unittest_modutils.py::StandardLibModuleTest::test_datetime - AssertionError: False is not true
FAILED tests/unittest_regrtest.py::NonRegressionTests::test_ssl_protocol - AssertionError: Uninferable is not an instance of <clas...

Description

Type of Changes

	Type
✓	🐛 Bug fix
	✨ New feature
	🔨 Refactoring
	📜 Docs

Related Issue

Fixes #823

[Pierre-Sassoulas]

I wonder if this also fix pylint-dev/pylint#4798. (We could ping DudeNr33 if no one can test this on Mac)

[keichi]

@Pierre-Sassoulas Just checked, it does!

pylint 2.11 w/ astroid 2.8.5

keichi@isa3-dhcp-190-228 tmp2 % pylint socket_test.py
************* Module socket_test
socket_test.py:14:13: E1101: Module 'socket' has no 'gethostbyname_ex' member (no-member)
socket_test.py:15:7: E1101: Module 'socket' has no 'gaierror' member (no-member)

------------------------------------
Your code has been rated at -2.50/10

pylint 2.11 w/ astroid (this branch)

keichi@isa3-dhcp-190-228 tmp2 % pylint socket_test.py

---------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: -2.50/10, +12.50)

Would you like me to update the change log?

[DanielNoord]

Do we think pylint-dev/pylint#4798 is indeed similar to pylint-dev/pylint#4759? We could then also close that issue!

[Pierre-Sassoulas]

I think #4759 would be valuable to do even if we fixed this particular issue thanks to @keichi

[Pierre-Sassoulas]

Would you like me to update the change log?

Yes please :) And thank you for fixing this issue in the first place, it's really hard to investigate without access to a Mac !

[keichi]

Yep, the problems reported in pylint-dev/pylint#4759 are fixed 🎉 Maybe we can run macOS CI jobs in this repository too?

[keichi]

Turns out this fixes pylint-dev/pylint#4302 and pylint-dev/pylint#5081 as well.

[keichi]

pylint-dev/pylint#3499 is also fixed. To summarize, here are the pylint issues resolved with this PR:

• Contents of native modules not found when running in a virtualenv pylint#3499
• False positive on gethostname in socket module pylint#4302
• False positives in socket library pylint#4798
• Regression: False positive I1101(c-extension-no-member) for math.iclose pylint#5081

[Pierre-Sassoulas]

Wow, so many fixes at once, this is 🔥

[Pierre-Sassoulas]

Sorry, I missed that this was not merged yet with the release of pylint 2.12. Thanks a lot for the numerous fixes !

[DanielNoord]

Can't leave this opportunity to add the typing already.

Should we add tests to this?

[Pierre-Sassoulas]

Should we add tests to this?

I think pylint-dev/pylint#4759 would be part of the solution ? But there might be a trick to test if on any platform ?

[DanielNoord]

I think we should add a test specific to this. I'm not sure: can we create a temporary symlink with pytest, sys or os or something? This seems to warrant an actual test.

[Pierre-Sassoulas]

Unit testing is not really hard, maybe a mix of os.symlink() on a tmp_path fixture ? But the integration testing means we need to create a new job to launch pylint from a symlinked environment ?

[DanielNoord]

Unit testing is not really hard, maybe a mix of os.symlink() on a tmp_path fixture ? But the integration testing means we need to create a new job to launch pylint from a symlinked environment ?

I don't really follow you here 😅

What do you mean with integration testing (I'm not that experienced with testing frameworks so forgive me if this is obvious)?

[Pierre-Sassoulas]

By integration testing I mean the generic computer science term, i.e. testing that modules using this new _normalize_path work properly. As opposed to unit tests that would just test the _normalize_path function output directly (not very useful). I was thinking more of "system testing" than integration testing now that I had to read the definition (testing that pylint work with the new _normalize_path).

[DanielNoord]

I have a Mac so I'll work on creating a test for this. I played around for a bit already and noticed that one test is actually breaking because of this change on Mac, so this can't be merged in its current state.

[DanielNoord]

I looked into this but I'm not sure we can create a test. The problem is not that what we import contains a symlink but that the python path is a symlink. This means that sysconfig.get_python_lib will return that symlink instead of the actual python directory.

I don't think we can add a symlink to the python executable/directory while we are already running a python instance.. Setting up a completely new action with a symlinked install of python just to test this does not seem worth it. I think we should go ahead and merge this without tests (sadly..)

[Pierre-Sassoulas]

I think we should go ahead and merge this without tests (sadly..)

I came to the same conclusion. I think the lack of tests is the reason why we did not merge this earlier. Some things are really hard to test. So really useful MR are rotting in astroid because of that. Same problem with #1207, it's not impossible to test, but it's hard.

[DanielNoord]

Can we auto-close the pylint issues? Or do you want to add something to the pylint changelog and do it then?

[Pierre-Sassoulas]

Generally I add a regression test in pylint before closing but right now testing is just as hard. I added them to the 2.13.0 milestone but we could close it when upgrading pylint to astroid 2.9.1 ?

[DanielNoord]

Fine with me!

[Pierre-Sassoulas]

pylint-dev/pylint#5529