ntpath.realpath() does not fully resolve relative paths

[zooba]

BPO	38453
Nosy	@pfmoore, @vstinner, @tjguk, @zware, @eryksun, @zooba, @miss-islington
PRs	bpo-38453: Resolve test directories before chdir to them #16723 [3.8] bpo-38453: Resolve test directories before chdir to them (GH-16723) #16783 bpo-38453: Ensure ntpath.realpath correctly resolves relative paths #16967 [3.8] bpo-38453: Ensure ntpath.realpath correctly resolves relative paths (GH-16967) #17174 bpo-38453: Ensure correct short path is obtained for test #17184 [3.8] bpo-38453: Ensure correct short path is obtained for test (GH-17184) #17186

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/zooba'
closed_at = <Date 2019-11-18.17:32:26.461>
created_at = <Date 2019-10-11.22:27:20.160>
labels = ['type-bug', '3.8', '3.9', 'OS-windows']
title = 'ntpath.realpath() does not fully resolve relative paths'
updated_at = <Date 2019-11-18.17:32:26.446>
user = 'https://github.com/zooba'

bugs.python.org fields:

activity = <Date 2019-11-18.17:32:26.446>
actor = 'steve.dower'
assignee = 'steve.dower'
closed = True
closed_date = <Date 2019-11-18.17:32:26.461>
closer = 'steve.dower'
components = ['Windows']
creation = <Date 2019-10-11.22:27:20.160>
creator = 'steve.dower'
dependencies = []
files = []
hgrepos = []
issue_num = 38453
keywords = ['patch']
message_count = 18.0
messages = ['354509', '354511', '354512', '354638', '354640', '355574', '355575', '355680', '356548', '356701', '356702', '356722', '356723', '356726', '356727', '356733', '356739', '356888']
nosy_count = 7.0
nosy_names = ['paul.moore', 'vstinner', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'miss-islington']
pr_nums = ['16723', '16783', '16967', '17174', '17184', '17186']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue38453'
versions = ['Python 3.8', 'Python 3.9']

[zooba]

Because the abspath() call is deferred until the end of the resolution process, if the current working directory is not already a real path, it will not be fully resolved.

By passing it through abspath() at the start of the function, we ensure that the correct path is resolved.

This will help resolve some inconsistencies in the test suite when tests are launched with a short filename (FILENA~1) in the working directory (though some other fixes are probably needed to help here).

[zooba]

Updating change_cwd() in Lib/test/support/init.py to call chdir(realpath(path)) also fixes the test issues I was facing, but the realpath() implementation needs fixing.

[zooba]

PR 16723 is just for the test fix.

The realpath() fix should get tests that chdir (without change_cwd()) into a symlink and then realpath() on a relative path. Today, that should fail to resolve the sections in the CWD, and with the fix it should resolve the entire path.

[zooba]

New changeset d83fc27 by Steve Dower in branch 'master':
bpo-38453: Resolve test directories before chdir to them (GH-16723)
d83fc27

[miss-islington]

New changeset aa909b6 by Miss Islington (bot) in branch '3.8':
bpo-38453: Resolve test directories before chdir to them (GH-16723)
aa909b6

[zooba]

PR 16967 should fix the relative path resolution property, by joining with cwd if the original path is unprefixed and not absolute, and then joining with any symlink directories if their link is not absolute.

[zooba]

I also had to special-case "realpath('nul')" to maintain the behaviour that we deliberately maintained in another bug, and I think isabs() should always return true for "\\?\" prefixed paths (given that join() will promote a "\\?\" prefixed path to the root).

[zooba]

The discussion on the PR has raised one scenario where our algorithm conflicts with what we can reasonably emulate from the IO manager (which is itself not entirely self-consistent):

If you call ntpath.realpath(r"C:\spam\eggs") where "spam" is a symlink/junction to "D:\foo\baz" and "D:\foo\baz\eggs" is a relative symlink to "..\bar" and "D:\foo\bar" does not exist, we will incorrectly return "C:\bar".

Note that if "D:\foo\bar" *does* exist, we will return the correct result. And if "C:\spam" is *not* a symlink/junction, we will return the correct result.

Essentially, we fail in the case where ntpath.exists(X) fails and realpath(dirname(X)) != dirname(X) and readlink(X).startswith("..\\") is True.

Considering it's taken months of patient explanation from Eryk Sun for me to understand this, and nobody has ever complained about it, I'm prepared to call it a known limitation.

[zooba]

My latest push to PR 16967 is a significant change to what ntpath.realpath will return for broken symlinks, but I think it's the right change.

Basically, if the OS fully resolves the path, great! We'll return that (and handle \\?\ stripping)

If the OS doesn't, we'll assume it's a broken link and try and read the link target from the path. Most of the time, we'll get "not a link", but if it succeeds then we'll return here. For absolute links, we'll return it. For relative symlinks, we'll join with the parent directory. For relative anything-else, we'll return the path to the link itself.

If we can't read a link (most likely), then we'll work up the path doing the same thing. Once we successfully get further than the original path, we'll join the unprocessed tail on and return without attempting to resolve anything more (assuming that the OS would have done it earlier if it was possible).

So if you look at the changed test_ntpath.py you'll see the different results (the test_shutil.py changes are mainly to avoid long/short name comparison failures when realpath() fixes them, by putting the test files in the actual test directory instead of global TEMP).

[zooba]

New changeset abde52c by Steve Dower in branch 'master':
bpo-38453: Ensure ntpath.realpath correctly resolves relative paths (GH-16967)
abde52c

[zooba]

I think the PR as it stands is a net improvement over where we currently are, so I'll merge it. We can make more tweaks as necessary.

[vstinner]

AMD64 Windows8.1 Non-Debug 3.x is grumpy:
https://buildbot.python.org/all/#/builders/12/builds/3459

======================================================================
FAIL: test_realpath_cwd (test.test_ntpath.TestNtpath)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "D:\buildarea\3.x.ware-win81-release.nondebug\build\lib\test\test_ntpath.py", line 424, in test_realpath_cwd
    self.assertPathEqual(test_file_long, ntpath.realpath(test_file_short))
  File "D:\buildarea\3.x.ware-win81-release.nondebug\build\lib\test\test_ntpath.py", line 62, in assertPathEqual
    self.assertEqual(path1, path2)
AssertionError: 'D:\\[88 chars]worker_4476\\@test_4476_tmp\\MyVeryLongDirectoryName\\file.txt' != 'D:\\[88 chars]worker_4476\\@test_4476_tmp\\MYVERY~1\\file.txt'
- D:\buildarea\3.x.ware-win81-release.nondebug\build\build\test_python_4088\test_python_worker_4476\@test_4476_tmp\MyVeryLongDirectoryName\file.txt
?                                                                                                                   ^ ^^^^^^^^^^^^^^^^^^^^
+ D:\buildarea\3.x.ware-win81-release.nondebug\build\build\test_python_4088\test_python_worker_4476\@test_4476_tmp\MYVERY~1\file.txt
?                                                                                                                   ^ ^^^^^

[vstinner]

Grumpy AMD64 Windows10 3.x:
https://buildbot.python.org/all/#/builders/3/builds/3788

======================================================================
FAIL: test_realpath_cwd (test.test_ntpath.TestNtpath)
----------------------------------------------------------------------

Traceback (most recent call last):
  File "D:\buildarea\3.x.bolen-windows10\build\lib\test\test_ntpath.py", line 424, in test_realpath_cwd
    self.assertPathEqual(test_file_long, ntpath.realpath(test_file_short))
  File "D:\buildarea\3.x.bolen-windows10\build\lib\test\test_ntpath.py", line 62, in assertPathEqual
    self.assertEqual(path1, path2)
AssertionError: 'D:\\[76 chars]worker_6956\\@test_6956_tmp\\MyVeryLongDirectoryName\\file.txt' != 'D:\\[76 chars]worker_6956\\@test_6956_tmp\\MYVERY~1\\file.txt'
- D:\buildarea\3.x.bolen-windows10\build\build\test_python_6236\test_python_worker_6956\@test_6956_tmp\MyVeryLongDirectoryName\file.txt
?                                                                                                       ^ ^^^^^^^^^^^^^^^^^^^^
+ D:\buildarea\3.x.bolen-windows10\build\build\test_python_6236\test_python_worker_6956\@test_6956_tmp\MYVERY~1\file.txt
?                                                                                                       ^ ^^^^^

pythoninfo:

windows.RtlAreLongPathsEnabled: False

[zooba]

New changeset 66c0f01 by Steve Dower in branch '3.8':
bpo-38453: Ensure ntpath.realpath correctly resolves relative paths (GH-16967)
66c0f01

[zooba]

Thanks, Victor! That looks different from the one I was originally fixing (even though the long/short path mismatch is there), so I'll take a look.

[zooba]

New changeset 7c6130c by Steve Dower in branch 'master':
bpo-38453: Ensure correct short path is obtained for test (GH-17184)
7c6130c

[miss-islington]

New changeset 6f602fb by Miss Islington (bot) in branch '3.8':
bpo-38453: Ensure correct short path is obtained for test (GH-17184)
6f602fb

[zooba]

Closing this as I believe it's done, but happy to reopen if we find another edge case (or start a new issue).