[fix] inconsistent handling of symbolic links

[zEdS15B3GCwq]

Pull Request Check List

Resolves: #5425 describes some symptoms, but the root cause seems a bit more complicated.

• [+/-] Added tests for changed code. Well, not, but this fixes some failing tests.
• Updated documentation for changed code. No need.

In the code, paths (i.e. for file and directory package sources) are often normalised using Path.resolve(). This "Make the path absolute, resolving any symlinks" (quote). I've got the impression that the general expectation is that all of these locations should be normalised this way, however, the code isn't completely consistent in this regard. There are some pathways in the code where location data isn't treated in the same way, e.g. project root folders are used as supplied by cwd, which doesn't expand symbolic links. There are several tests as well, that do not take symbolic links into account.

Issue #5425 describes the problems I've noticed when coding or when executing Poetry tests in a folder that contained a symbolic link in the path. These are caused by:

1. Locker using relpath with resolved package source but unresolved project root paths, leading to unintended complicated relative paths (e.g. for a sub-module located under the project folder)
2. tests not taking symbolic links into account, supplying unresolved paths to Factory.add_dependency and Factory.create_dependency, which don't resolve them either
3. expected results in 1 test (test_solver_can_resolve_directory_dependencies_nested_editable) include unresolved paths, causing test to fail because Poetry does resolve those paths

In this PR I'm proposing simple fixes to points 1 and 3. The methods in point 2 are in poetry-core's BaseFactory, and although it doesn't require a complicated fix either, it's not as trivial a simply adding resolve() to a code line, at least not in here in Poetry, so I'm holding back until I get some feedback. Honestly, this entire thing is probably not a high priority task. I haven't had any real problems when using Poetry, except for the slightly annoying relative path in the locker file, and I haven't seen other related issues either. The only errors I've encountered are poetry's own tests failing. However, I do believe that the code is a bit sloppy in how it handles paths - path sanitization is left up to each module, which do that in different ways, sometimes resolving sometimes not, and test writers obviously didn't think of resolving paths either. My proposed code changes don't fix that. Therefore, yet unknown bugs may remain.

Some details about the failing tests I mentioned above:

Pytest results in a folder that contains a symlink in its path:

    1011 passed
      15 failed
         - ..\Lib\poetry\tests\installation/test_installer.py:1321 test_run_installs_with_local_setuptools_directory
         - ..\Lib\poetry\tests\installation/test_installer.py:1287 test_run_installs_with_local_poetry_file_transitive
         - ..\Lib\poetry\tests\installation/test_installer.py:1255 test_run_installs_with_local_poetry_directory_transitive
         - ..\Lib\poetry\tests\installation/test_installer.py:1225 test_run_installs_with_local_poetry_directory_and_extras
         - ..\Lib\poetry\tests\installation/test_installer.py:1170 test_run_installs_with_local_file
         - ..\Lib\poetry\tests\installation/test_installer.py:1197 test_run_installs_wheel_with_no_requires_dist
         - ..\Lib\poetry\tests\installation/test_installer_old.py:860 test_run_installs_wheel_with_no_requires_dist
         - ..\Lib\poetry\tests\installation/test_installer_old.py:839 test_run_installs_with_local_file
         - ..\Lib\poetry\tests\installation/test_installer_old.py:907 test_run_installs_with_local_poetry_directory_transitive
         - ..\Lib\poetry\tests\installation/test_installer_old.py:939 test_run_installs_with_local_poetry_file_transitive
         - tests\puzzle/test_solver.py:2421 test_solver_can_resolve_sdist_dependencies_with_extras
         - tests\puzzle/test_solver.py:2493 test_solver_can_resolve_wheel_dependencies_with_extras
         - tests\puzzle/test_solver.py:2289 test_solver_can_resolve_directory_dependencies_nested_editable
         - tests\puzzle/test_solver.py:2461 test_solver_can_resolve_wheel_dependencies
         - tests\puzzle/test_solver.py:2389 test_solver_can_resolve_sdist_dependencies
       2 xfailed
       5 skipped

PR applied:

    1020 passed
       6 failed
         - ..\Lib\poetry\tests\installation/test_installer_old.py:860 test_run_installs_wheel_with_no_requires_dist
         - ..\Lib\poetry\tests\installation/test_installer_old.py:839 test_run_installs_with_local_file
         - tests\puzzle/test_solver.py:2389 test_solver_can_resolve_sdist_dependencies
         - tests\puzzle/test_solver.py:2421 test_solver_can_resolve_sdist_dependencies_with_extras
         - tests\puzzle/test_solver.py:2461 test_solver_can_resolve_wheel_dependencies
         - tests\puzzle/test_solver.py:2493 test_solver_can_resolve_wheel_dependencies_with_extras
       2 xfailed
       5 skipped
      21 deselected

9 tests are fixed. All the remaining fails are due to Point 2 above. Plus, after PR, relative paths in the locker file are correct (as in path to a sub-folder will be just the sub-folder's name, instead of pointing back to the sub-folder under symlink's target).

[zEdS15B3GCwq]

That failing check on macOS / 3.11-dev is a bit weird. It worked on the last commit, and the only difference since then is that the code was formatted by black.

[onerandomusername]

That failing check on macOS / 3.11-dev is a bit weird. It worked on the last commit, and the only difference since then is that the code was formatted by black.

This occurred on another pr too, I think it was the same error as well.

[zEdS15B3GCwq]

That failing check on macOS / 3.11-dev is a bit weird. It worked on the last commit, and the only difference since then is that the code was formatted by black.

This occurred on another pr too, I think it was the same error as well.

Any idea how to fix it?

If it's simple, I'd be happy to implement it any time. Otherwise, I'll probably wait until when/if it becomes the main hurdle to this PR being accepted.

[spoorn]

I just noticed, I had already submitted a PR for this same fix, except I added some extra tests for it: #5850

[neersighted]

Duplicated #5850 which is merged

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.