python / cpython

The Python programming language
https://www.python.org
Other
63.17k stars 30.25k forks source link

sys.executable is broken in VirtualEnv in Windows for Python 3.11 using Nushell #102496

Open presidento opened 1 year ago

presidento commented 1 year ago

Bug report

Minimal reproducible example in Windows:

> py -3.11 -m venv .venv
> nu # <---- start Nushell
> .venv/Scripts/python.exe  -c "import sys; print(sys.version, sys.executable)"
3.11.2 (tags/v3.11.2:878ead1, Feb  7 2023, 16:38:35) [MSC v.1934 64 bit (AMD64)] \\?\C:\Users\MFarkas\git\python-bug\.venv\Scripts\python.exe

Your environment

Nushell 0.76.0, Windows 10 64bit. It works perfectly with system Pythons (no virtualenv), or in virtualenv except for Python 3.11 (this is why I think it is something related to Python, and not only to Nushell):

System Pythons
3.6.8 (tags/v3.6.8:3c6b436a57, Dec 24 2018, 00:16:47) [MSC v.1916 64 bit (AMD64)] C:\Users\MFarkas\AppData\Local\Programs\Python\Python36\python.exe
3.7.9 (tags/v3.7.9:13c94747c7, Aug 17 2020, 18:58:18) [MSC v.1900 64 bit (AMD64)] C:\Users\MFarkas\AppData\Local\Programs\Python\Python37\python.exe
3.8.10 (tags/v3.8.10:3d8993a, May  3 2021, 11:48:03) [MSC v.1928 64 bit (AMD64)] C:\Users\MFarkas\AppData\Local\Programs\Python\Python38\python.exe
3.9.13 (tags/v3.9.13:6de2ca5, May 17 2022, 16:36:42) [MSC v.1929 64 bit (AMD64)] C:\Users\MFarkas\AppData\Local\Programs\Python\Python39\python.exe
3.10.9 (tags/v3.10.9:1dd9be6, Dec  6 2022, 20:01:21) [MSC v.1934 64 bit (AMD64)] C:\Users\MFarkas\AppData\Local\Programs\Python\Python310\python.exe
3.11.2 (tags/v3.11.2:878ead1, Feb  7 2023, 16:38:35) [MSC v.1934 64 bit (AMD64)] C:\Users\MFarkas\AppData\Local\Programs\Python\Python311\python.exe

VirtualEnv Pythons
3.6.8 (tags/v3.6.8:3c6b436a57, Dec 24 2018, 00:16:47) [MSC v.1916 64 bit (AMD64)] C:\Users\MFarkas\git\python-bug\3.6\Scripts\python.exe
3.7.9 (tags/v3.7.9:13c94747c7, Aug 17 2020, 18:58:18) [MSC v.1900 64 bit (AMD64)] C:\Users\MFarkas\git\python-bug\3.7\Scripts\python.exe
3.8.10 (tags/v3.8.10:3d8993a, May  3 2021, 11:48:03) [MSC v.1928 64 bit (AMD64)] C:\Users\MFarkas\git\python-bug\3.8\Scripts\python.exe
3.9.13 (tags/v3.9.13:6de2ca5, May 17 2022, 16:36:42) [MSC v.1929 64 bit (AMD64)] C:\Users\MFarkas\git\python-bug\3.9\Scripts\python.exe
3.10.9 (tags/v3.10.9:1dd9be6, Dec  6 2022, 20:01:21) [MSC v.1934 64 bit (AMD64)] C:\Users\MFarkas\git\python-bug\3.10\Scripts\python.exe
3.11.2 (tags/v3.11.2:878ead1, Feb  7 2023, 16:38:35) [MSC v.1934 64 bit (AMD64)] \\?\C:\Users\MFarkas\git\python-bug\3.11\Scripts\python.exe

Because of this, some packages cannot be installed with pip. And everything is broken which needs to have a valid sys.executable.

eryksun commented 1 year ago

When Nushell resolves a command name from a relative path that has one or more slashes or backslashes, such as ".venv/Scripts/python.exe ", it seems to always create an extended path, i.e. a path that's prefixed by "\\?\". This gets set as the lpApplicationName parameter when the shell calls CreateProcessW(). This in turn is the result of calling GetModuleFileNameW(NULL, ...) in the spawned process.

Example in Nu:

C:\Program Files〉Python310\python.exe
Python 3.10.10 (tags/v3.10.10:aad5f6a, Feb  7 2023, 17:20:36) [MSC v.1929 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys, _winapi
>>> sys.orig_argv
['Python310\\python.exe']
>>> _winapi.GetModuleFileName(0)
'\\\\?\\C:\\Program Files\\Python310\\python.exe'
>>> sys.executable
'C:\\Program Files\\Python310\\python.exe'

Prior to Python 3.11, GetModuleFileNameW() is called at startup in order to get the path to set as sys.executable. So the question is, why isn't sys.executable an extended path in the above example? What's happening is that in 3.6 to 3.10, the initialization code in "PC/getpathp.c" converts an extended path into a regular path via WinAPI PathCchCanonicalizeEx().

Python 3.11 is the first version on Windows that sets sys.executable based on the first command-line argument (i.e. argv[0]) instead of basing it on GetModuleFileNameW(NULL, ...). It still falls back on on the latter, but only if the resolved argv[0] path doesn't exist. Usually argv[0] from the command line is either a regular absolute path or a relative path, in which case, if the path exists, sys.executable will also be a regular path.

On the other hand, when running from a virtual environment that uses a launcher, the value of sys.executable gets passed to the base Python process in the "__PYVENV_LAUNCHER__" environment variable. The venv launcher gets this value from calling GetModuleFileNameW() instead of from the command line. The problem is that the new implementation of "Modules/getpath.c" in 3.11 doesn't canonicalize the path. Thus sys.executable ends up being an extended path in this case.

Note that if you create the environment with symlinks instead of using a launcher, then sys.executable is based on argv[0]. For example:

C:\Temp〉py -3.11 -m venv --symlinks env311
C:\Temp〉env311\Scripts\python.exe
Python 3.11.2 (tags/v3.11.2:878ead1, Feb  7 2023, 16:38:35) [MSC v.1934 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys, _winapi
>>> sys.orig_argv
['env311\\Scripts\\python.exe']
>>> _winapi.GetModuleFileName(0)
'\\\\?\\C:\\Temp\\env311\\Scripts\\python.exe'
>>> sys.executable
'C:\\Temp\\env311\\Scripts\\python.exe'

I suppose the simplest fix would be to update the venv launcher to canonicalize the executable path in its get_process_name() function.

A more reliable fix would update "Modules/getpath.c" to always canonicalize the executable path, regardless of how it's determined.


Because of this, some packages cannot be installed with pip. And everything is broken which needs to have a valid sys.executable.

Can you provide some example packages that fail to install or misbehave in this case?

An extended path is a valid file path, valid as the process executable path, and valid in most other cases. However, an extended path should not be set as the current working directory. The Windows API doesn't support that.

presidento commented 1 year ago

For example I cannot install twine:

C:\Users\MFarkas\git\scripthelper〉^".venv/Scripts/python.exe" -m pip install --upgrade twine                                                             Collecting twine
  Using cached twine-4.0.2-py3-none-any.whl (36 kB)
Collecting pkginfo>=1.8.1
  Using cached pkginfo-1.9.6-py3-none-any.whl (30 kB)
Collecting readme-renderer>=35.0
  Using cached readme_renderer-37.3-py3-none-any.whl (14 kB)
Collecting requests>=2.20
  Using cached requests-2.28.2-py3-none-any.whl (62 kB)
Collecting requests-toolbelt!=0.9.0,>=0.8.0
  Using cached requests_toolbelt-0.10.1-py2.py3-none-any.whl (54 kB)
Collecting urllib3>=1.26.0
  Using cached urllib3-1.26.14-py2.py3-none-any.whl (140 kB)
Collecting importlib-metadata>=3.6
  Using cached importlib_metadata-6.0.0-py3-none-any.whl (21 kB)
Collecting keyring>=15.1
  Using cached keyring-23.13.1-py3-none-any.whl (37 kB)
Collecting rfc3986>=1.4.0
  Using cached rfc3986-2.0.0-py2.py3-none-any.whl (31 kB)
Collecting rich>=12.0.0
  Using cached rich-13.3.2-py3-none-any.whl (238 kB)
Collecting zipp>=0.5
  Using cached zipp-3.15.0-py3-none-any.whl (6.8 kB)
Collecting jaraco.classes
  Using cached jaraco.classes-3.2.3-py3-none-any.whl (6.0 kB)
Collecting pywin32-ctypes>=0.2.0
  Using cached pywin32_ctypes-0.2.0-py2.py3-none-any.whl (28 kB)
Collecting bleach>=2.1.0
  Using cached bleach-6.0.0-py3-none-any.whl (162 kB)
Collecting docutils>=0.13.1
  Using cached docutils-0.19-py3-none-any.whl (570 kB)
Collecting Pygments>=2.5.1
  Using cached Pygments-2.14.0-py3-none-any.whl (1.1 MB)
Collecting charset-normalizer<4,>=2
  Using cached charset_normalizer-3.1.0-cp311-cp311-win_amd64.whl (96 kB)
Collecting idna<4,>=2.5
  Using cached idna-3.4-py3-none-any.whl (61 kB)
Collecting certifi>=2017.4.17
  Using cached certifi-2022.12.7-py3-none-any.whl (155 kB)
Collecting markdown-it-py<3.0.0,>=2.2.0
  Using cached markdown_it_py-2.2.0-py3-none-any.whl (84 kB)
Collecting six>=1.9.0
  Using cached six-1.16.0-py2.py3-none-any.whl (11 kB)
Collecting webencodings
  Using cached webencodings-0.5.1-py2.py3-none-any.whl (11 kB)
Collecting mdurl~=0.1
  Using cached mdurl-0.1.2-py3-none-any.whl (10.0 kB)
Collecting more-itertools
  Using cached more_itertools-9.1.0-py3-none-any.whl (54 kB)
Installing collected packages: webencodings, pywin32-ctypes, zipp, urllib3, six, rfc3986, Pygments, pkginfo, more-itertools, mdurl, idna, docutils, charset-normalizer, certifi, requests, markdown-it-py, jaraco.classes, importlib-metadata, bleach, rich, requests-toolbelt, readme-renderer, keyring, twine
ERROR: Could not install packages due to an OSError: [Errno 22] Invalid argument: '\\\\?\\C:\\Users\\MFarkas\\.venv\\Lib\\site-packages\\../../Scripts/rst2html.py'

Unfortunately I got the same error even if I create the virtualenv using symlinks:

^".3.11.venv/Scripts/python.exe" -m pip install twine --quiet --upgrade
ERROR: Could not install packages due to an OSError: [Errno 22] Invalid argument: '\\\\?\\C:\\Users\\MFarkas\\.3.11.venv\\Lib\\site-packages\\../../Scripts/rst2html.py'

error: Recipe `bootstrap-with` failed on line 13 with exit code 1
C:\Users\MFarkas\git\scripthelper〉.3.11.venv\Scripts\python.exe                                                                                               03/07/2023 10:08:19
Python 3.11.1 (tags/v3.11.1:a7a450f, Dec  6 2022, 19:58:39) [MSC v.1934 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import sys
>>> sys.executable
'C:\\Users\\MFarkas\\git\\scripthelper\\.3.11.venv\\Scripts\\python.exe'
>>>

Also I need to run Python scripts from shell using the current Python interpreter, so I used:

result = subprocess.run(
    f"\"{sys.executable}\" {command}",
    stderr=subprocess.PIPE,
    stdout=subprocess.PIPE,
    check=subprocess_check,
    shell=True,
)

I assumed that sys.executable is executable... (It works with symlinks, but it needs elevated privileges, so not easy to use.)

eryksun commented 1 year ago

Unfortunately I got the same error even if I create the virtualenv using symlinks

I just installed twine successfully in a 3.11.2 virtual environment that's based on symlinks. It worked, as I expected it would, because sys.executable, the prefix path, and the site packages path are all regular paths in this case.


I assumed that sys.executable is executable... (It works with symlinks, but it needs elevated privileges, so not easy to use.)

By default, the right to create symbolic links (i.e. SeCreateSymbolicLinkPrivilege) is limited to the Administrators group. If you have administrator access, you can grant this privilege to your user account, a builtin group, or well-known group (e.g. "Authenticated Users"). Then you'll be able to create symlinks in a standard logon session. Run "secpol.msc", and add the desired user account or group to "Security Settings" -> "Local Policies" -> "User Rights Assignment" -> "Create symbolic links".


I also tried in a 3.11.2 virtual environment that's based on a launcher instead of symlinks. This case fails as follows:

  File "\\?\C:\Temp\env311_launcher\Lib\site-packages\pip\_internal\utils\misc.py", line 594, in hash_file
    with open(path, "rb") as f:
         ^^^^^^^^^^^^^^^^
OSError: [Errno 22] Invalid argument: '\\\\?\\C:\\Temp\\env311_launcher\\Lib\\site-packages\\../../Scripts/rst2html.py'

It fails because the prefix path and site packages path are extended paths:

>>> print(sys.prefix)
\\?\C:\Temp\env311_launcher
>>> print(*site.getsitepackages(), sep='\n')
\\?\C:\Temp\env311_launcher
\\?\C:\Temp\env311_launcher\Lib\site-packages

pip appends "../../Scripts/rst2html.py" to the site packages path and tries to open this path without first normalizing it. However, when opening an extended path, forward slashes and "." and ".." components are not supported. One has to manually normalize an extended path before opening it, such as via os.path.normpath(). This is undoubtedly a common problem when the process executable is opened as an extended path.

eryksun commented 1 year ago

@zooba

I suppose the simplest fix would be to update the venv launcher to canonicalize the executable path in its get_process_name() function.

A more reliable fix would update "Modules/getpath.c" to always canonicalize the executable path, regardless of how it's determined.

zooba commented 1 year ago

I'll have to give it a bit more thought, but initial reactions:

If it must go on the getpath side, I think I'd rather add a normpath implementation to getpath.c and then call it from getpath.py. The values set from the .py should be the final ones (one day, in my dreams, we'll set them directly on the sys module from there). But if containing it to the launcher is sufficient for enough cases, that's certainly preferable.

presidento commented 1 year ago

I also get this notification for an old version of pip:

[notice] A new release of pip available: 22.3.1 -> 23.0.1
[notice] To update, run: \\?\C:\Users\MFarkas\.venv\Scripts\python.exe -m pip install --upgrade pip
zooba commented 1 year ago

Yeah, makes sense - Python has been told that that's how to launch it, so it's going to keep showing you that whenever it shows how to launch itself.

I'm leaning towards saying this is not a bug, since there may legitimately be cases where sys.executable ought to be the path as provided, and they would have no workaround.

Our issue might be ntpath.join refusing to join path segments with the \\?\ prefix there, and potentially an issue where ntpath.normpath will ignore the prefix and interpret parts of the path under the normal rules (which could also break paths, but I'm pretty sure we discussed this thoroughly at the time, so it's probably OK).

eryksun commented 1 year ago

I'm leaning towards saying this is not a bug, since there may legitimately be cases where sys.executable ought to be the path as provided, and they would have no workaround.

There are too many problems with leaving sys.executable as an extended path if it doesn't have to be. This also affects sys.prefix and library paths added to sys.path.

Appending to an extended path requires the extra step of normalizing the result to replace forward slashes with backslashes, collapse repeated slashes, resolve any "." and ".." components, and strip trailing spaces and dots from the final path component. Pretty much no cross-platform code will do this. It's taken for granted that something like open(join(dir, '../spam.txt')) always works.

Also, setting an extended path as the working directory is not supported and buggy in the Windows API. I don't know why anyone would be setting sys.prefix or the site packages directory as the current working directory, but they shouldn't have to worry about whether it's supported.

Python 3.6-3.10 avoids these problems by getting a canonical path via PathCchCanonicalizeEx(). This function converts an extended path to a normal path if either it isn't a long path or the process supports long paths.

potentially an issue where ntpath.normpath will ignore the prefix and interpret parts of the path under the normal rules

Extended paths only bypass the normalization step when opened, not when explicitly normalized. ntpath.normpath() should support extended paths like any other device path, just as I think it does (at least nt._path_normpath() does), and just as WinAPI GetFullPathNameW() does. It's important to normalize an extended path that's being used to bypass the MAX_PATH length limit, to ensure that the path can be accessed normally by a process that supports long paths. Unfortunately our normpath() implementation is still missing an important step -- stripping trailing dots and spaces from the final component. Thus abspath() should be used instead.

zooba commented 1 year ago

There are too many problems with leaving sys.executable as an extended path if it doesn't have to be.

Right, but the "if it doesn't have to be" bit is why I'm hesitating. Who's supposed to judge that? What if we take a 263 character sys.executable, strip the \\?\ prefix, and then change the name to python312.dll and exceed the limit again?

It would be different if we were adding the prefix, but it's come from the person who launched us. So I'm not sure we ought to be overriding it.

Would this case be fixed if we make the venv launcher pass along GetModuleFileName instead of argv[0]. That one we can do.

eryksun commented 1 year ago

Would this case be fixed if we make the venv launcher pass along GetModuleFileName instead of argv[0]. That one we can do.

The venv launcher sets the "__PYVENVLAUNCHER_\" launcher environment variable to the value of argv0 = get_process_name(). The latter is actually based on GetModuleFileNameW(NULL, ...) instead of argv[0] from the command line. Generally, argv[0] from the command line could also be an extended path. However, in this case, argv[0] is a relative path.

The extended path comes from the lpApplicationName argument of the CreateProcessW() call that spawns the launcher. For a relative executable path that includes a path separator, Nushell apparently resolves the full executable path as an extended path, even if it isn't a long path. In the lpCommandLine argument, however, Nushell simply uses the relative path of the executable.

Resolving the full path of the executable and using lpApplicationName is normal for a shell, but needlessly resolving it as an extended path is weird.

Right, but the "if it doesn't have to be" bit is why I'm hesitating. Who's supposed to judge that? What if we take a 263 character sys.executable, strip the \?\ prefix, and then change the name to python312.dll and exceed the limit again?

So you're arguing for the view that what we used to do in Python 3.6 to 3.10 was actually wrong, and that we should retain the extended path even if it's not strictly necessary.

CPython isn't responsible for path handling bugs in pip and third-party modules. So it's your call whether you want to try to make life easier for them by avoiding extended paths when they're not necessary.

For internal use, we should test for normal long-path support at startup by calling PathCchCanonicalizeEx() on a test path such as "C:\\spam\\" + "a" * 255, with the option PATHCCH_ALLOW_LONG_PATHS. If the result is an extended path, then normal long paths aren't supported, in which case internal use of long paths should use extended paths.

I'm doubtful about converting paths that are visible to Python code into extended paths. For example, if normal long paths aren't supported by the current process, should _Py_normpath() convert a long path to extended form, assuming sufficient buffer space? We'd be ensuring that the path itself can be opened, but this could also break modules that don't support extended paths -- such as pip apparently.

zooba commented 1 year ago

CPython isn't responsible for path handling bugs in pip and third-party modules. So it's your call whether you want to try to make life easier for them by avoiding extended paths when they're not necessary.

I suspect the blame here is in ntpath.join, which basically does os.path.sep.join(*args) rather than any intelligent work (regardless of the prefix). Chances are, if join would split each argument on sep or altsep, process dots, and join things back up with sep, the path would be just fine.

I don't know whether that would backport to 3.11. I suspect it probably shouldn't, so would only be an improvement for 3.12.

So you're arguing for the view that what we used to do in Python 3.6 to 3.10 was actually wrong, and that we should retain the extended path even if it's not strictly necessary.

I guess so, but then, running it through an OS API at least means we aren't responsible for it changing the meaning of a path. So maybe what we could do is change anywhere we use GetModuleFileName to also PathCchCanonicalize the path?

Main thing I want to maintain here is that it's a single logical step. We don't have a "canonicalize" step on POSIX as part of path calculation, so I don't want one on Windows. We do have a "get the current process name" step, which is already implemented differently, so perhaps this is just a part of that step?

eryksun commented 1 year ago

Main thing I want to maintain here is that it's a single logical step. We don't have a "canonicalize" step on POSIX as part of path calculation, so I don't want one on Windows. We do have a "get the current process name" step, which is already implemented differently, so perhaps this is just a part of that step?

I'd prefer that this step was applied to the argv[0] path from the command line as well, but I guess we can assume that the command line is a user intention that should be respected. At least that's a sufficient assumption for this issue, since Nushell uses the relative path of the executable in the command line, and only the GetModuleFileNameW() result is an extended path.

Try it out and verify that none of sys.executable, sys.prefix, sys.exec_prefix, or any path in sys.path ends up being an extended path when the venv launcher is spawned with lpApplicationName as an extended path.

I suspect the blame here is in ntpath.join, which basically does os.path.sep.join(*args) rather than any intelligent work (regardless of the prefix). Chances are, if join would split each argument on sep or altsep, process dots, and join things back up with sep, the path would be just fine.

ntpath.join() shouldn't normalize the result in all cases. I'm fine with always normalizing a path that has a drive, per ntpath.splitroot(). Since extended paths always have a 'drive', ntpath.join() can normalize them without causing problems.

On the other hand, purely relative paths and relative rooted paths might be set as the relative target path of a symlink. In this case, resolving ".." components should be handled by the kernel. Also, an initial "." component may be significant in a path search context.

zooba commented 1 year ago

ntpath.join() shouldn't normalize the result in all cases.

I wasn't thinking of normalising the result, but the arguments. If any segment starts with \\?\ then it clearly gets left alone, but if it's being joined with ../../Scripts/rst2html.py then we should at least convert that into ..\..\Scripts\rst2html.py. It still won't resolve properly, because the \\?\ disables the handling of .. segments, but I'm not sure there's anything we can do about that at all? Unless it's okay for ntpath.join to apply .. to the path it knows about, and so won't drop them from the start of the result.

eryksun commented 1 year ago

I wasn't thinking of normalising the result, but the arguments. If any segment starts with \\?\ then it clearly gets left alone, but if it's being joined with ../../Scripts/rst2html.py then we should at least convert that into ..\..\Scripts\rst2html.py. It still won't resolve properly, because the \\?\ disables the handling of .. segments, but I'm not sure there's anything we can do about that at all? Unless it's okay for ntpath.join to apply .. to the path it knows about, and so won't drop them from the start of the result.

On Windows, if a path has a drive, it's okay to normalize it to use backslashes, collapse repeated slashes, and resolve ".." components as much as possible. For example, it would be okay if npath.join("//?/C:/spam/eggs', '../../foo') returned r'\\?\C:\foo'. For example:

>>> nt._getfullpathname('//?/C:/spam/eggs/../../foo')
'\\\\?\\C:\\foo'
>>> nt._getfullpathname(r'\\?\C:\spam\eggs\..\..\foo')
'\\\\?\\C:\\foo'

I wouldn't backport the new behavior to 3.11, but I think it's reasonable behavior for a high-level path operation. Someone who needs something weird like literal ".." component names or literal "/" characters in an extended path can simply do the extra work of constructing their weird path manually.

zooba commented 1 year ago

It would also make sense to have join('../foo/bar/baz', '../..') return r'..\foo', right? So the drive part isn't essential, as long as we preserve the leading dot segments and don't invade them (so join('../foo', '../../bar') returns r'..\..\bar'). This is the behaviour of _path_normpath, though it does strip a leading .\ segment.

The failing path above would have normalised fine:

>>> nt._path_normpath('\\\\?\\C:\\Temp\\env311_launcher\\Lib\\site-packages\\../../Scripts/rst2html.py')
'\\\\?\\C:\\Temp\\env311_launcher\\Scripts\\rst2html.py'

So maybe we should just add the normpath in? It's easiest to implement, and is only one more pass over the string so shouldn't be wildly expensive (potentially cheaper than adding the additional logic in Python code).


Other thought I had was to canonicalize __PYVENV_LAUNCHER__ when we read that. Since it's technically untrusted input, I think we can justifiably sanitise it, and discard anything so drastically invalid that the API gives up.

eryksun commented 1 year ago

Other thought I had was to canonicalize __PYVENV_LAUNCHER__ when we read that. Since it's technically untrusted input, I think we can justifiably sanitise it, and discard anything so drastically invalid that the API gives up.

Yes, that will be required to resolve this issue. The venv launcher gets the __PYVENV_LAUNCHER__ path from GetModuleFileNameW(). It should be made canonical via PathCchCanonicalizeEx() with the flag PATHCCH_ALLOW_LONG_PATHS. If it's not a long path, then an extended path will be converted to a regular path. If it's a long path, the result will be an extended path only if the process doesn't support regular long paths (i.e. for our "python[w].exe", that means it's either an older Windows system or that support for long paths is disabled at the system level).

It would also make sense to have join('../foo/bar/baz', '../..') return r'..\foo', right? So the drive part isn't essential, as long as we preserve the leading dot segments and don't invade them (so join('../foo', '../../bar') returns r'..\..\bar'). This is the behaviour of _path_normpath, though it does strip a leading .\ segment.

I wouldn't normalize the relative path "../foo/bar/baz/../.." in ntpath.join() because it could be used as the target path of a relative symlink. ntpath.join() should be able to construct such a path. For example, if "bar" or "baz" are symlinks, the kernel will resolve them first. "baz" could resolve to "Z:\some\path\qux\baz". Then applying "..\.." resolves to "Z:\some\path".

If a path has a 'drive' (including UNC paths), then as the target of a symlink CreateSymbolicLinkW() stores it as a normalized path, the same as if the path were being opened. Thus it will have no "." or ".." components and no forward slashes. This includes relative drive paths such as "C:spam", which get resolved against the working directory on the drive. Extended paths are excluded from this, just as they are for an opened path, but I'm okay with not excluding them in ntpath.join() because it's important to normalize them in almost all cases. Anyone who needs a special case can do so manually.

One normalization step that I'd like ntpath.join() to implement for all paths is to replace forward slashes with backslashes. For example, when the I/O manager resolves a symlink, it handles forward slash as a name character, not as a path separator. Then the filesystem fails the open with ERROR_INVALID_NAME (123). Unfortunately, CreateSymbolicLinkW() doesn't normalize slashes in the target of a relative symlink (i.e. SYMLINK_FLAG_RELATIVE), which is any target that has no drive.

In short, I would prefer ntpath.join() to replace forward slashes with backslashes, and if the result has a drive, I would prefer for the path to be normalized via normpath().

zooba commented 1 year ago

In short, I would prefer ntpath.join() to replace forward slashes with backslashes, and if the result has a drive, I would prefer for the path to be normalized via normpath().

That's reasonable. I'll file a new issue for it, since it is fairly independent from this one.

Meanwhile, this fix here is to modify env_to_dict in Modules/getpath.c to optionally canonicalize the value it gets (if any), and enable that for the values read that should only contain a single path. On Windows, this needs to be PathCchCanonicalizeEx to handle this issue, but on POSIX it can just use our _Py_normpath function.

stefaneidelloth commented 1 year ago

I use a relative path to the python interpreter of my portable python distribution on Windows:

..\\..\\App\\WinPython\\python-3.11.1.amd64\\python.exe

With python 3.10 that relative path worked just fine. However, with python 3.11 sys.executable yields a wrong absolute path when using the debug mode of a flask application.

D:\\python_env\\workspace\\my_project\\App\\WinPython\\python-3.11.1.amd64\\python.exe

instead of

D:\\python_env\\App\\WinPython\\python-3.11.1.amd64\\python.exe

=> When joining paths, the ..\\..\\ seems to be ignored/trimmed in python 3.11.

This has nothing to do with VirtualEnv. However, the underlying issue might be the same.

As a temporal workaround I used

sys.executable = sys.executable.replace('\\App', '\\..\\..\\App')

at the beginning of my main application.