stdin-filename doesn't support UNC path in python 3.8/3.9 on Windows #4209
Labels
T: bug
Something isn't working
Comments
This was referenced Feb 5, 2024
justinchuby
added a commit
to justinchuby/lintrunner-adapters
that referenced
this issue
Feb 6, 2024
… black file path handling (#92) Fixes #93. This change uses `Path().resolve()` on the file path provided to black to avoid a bug described in psf/black#4209.
hauntsaninja
added a commit
to hauntsaninja/black
that referenced
this issue
Feb 11, 2024
This relates to psf#4015, psf#4161 and the behaviour of os.getcwd() Black is a big user of pathlib and as such loves doing `.resolve()`, since for a long time it was the only good way of getting an absolute path in pathlib. However, this has two problems: The first minor problem is performance, e.g. in psf#3751 I (safely) got rid of a bunch of `.resolve()` which made Black 40% faster on cached runs. The second more important problem is that always resolving symlinks results in unintuitive exclusion behaviour. For instance, a gitignored symlink should never alter formatting of your actual code. This was reported by users a few times. In psf#3846, I improved the exclusion rule logic for symlinks in `gen_python_files` and everything was good. But `gen_python_files` isn't enough, there's also `get_sources`, which handles user specified paths directly (instead of files Black discovers). So in psf#4015, I made a very similar change to psf#3846 for `get_sources`, and this is where some problems began. The core issue was the line: ``` root_relative_path = path.absolute().relative_to(root).as_posix() ``` The first issue is that despite root being computed from user inputs, we call `.resolve()` while computing it (likely unecessarily). Which means that `path` may not actually be relative to `root`. So I started off this PR trying to fix that, when I ran into the second issue. Which is that `os.getcwd()` (as called by `os.path.abspath` or `Path.absolute` or `Path.cwd`) also often resolves symlinks! ``` >>> import os >>> os.environ.get("PWD") '/Users/shantanu/dev/black/symlink/bug' >>> os.getcwd() '/Users/shantanu/dev/black/actual/bug' ``` This also meant that the breakage often would not show up when input relative paths. This doesn't affect `gen_python_files` / psf#3846 because things are always absolute and known to be relative to `root`. Anyway, it looks like psf#4161 fixed the crash by just swallowing the error and ignoring the file. Instead, we should just try to compute the actual relative path. I think this PR should be quite safe, but we could also consider reverting some of the previous changes; the associated issues weren't too popular. At the same time, I think there's still behaviour that can be improved and I kind of want to make larger changes, but maybe I'll save that for if we do something like psf#3952 Hopefully fixes psf#4205, fixes psf#4209, actual fix for psf#4077
hauntsaninja
added a commit
to hauntsaninja/black
that referenced
this issue
Feb 11, 2024
This relates to psf#4015, psf#4161 and the behaviour of os.getcwd() Black is a big user of pathlib and as such loves doing `.resolve()`, since for a long time it was the only good way of getting an absolute path in pathlib. However, this has two problems: The first minor problem is performance, e.g. in psf#3751 I (safely) got rid of a bunch of `.resolve()` which made Black 40% faster on cached runs. The second more important problem is that always resolving symlinks results in unintuitive exclusion behaviour. For instance, a gitignored symlink should never alter formatting of your actual code. This kind of thing was reported by users a few times. In psf#3846, I improved the exclusion rule logic for symlinks in `gen_python_files` and everything was good. But `gen_python_files` isn't enough, there's also `get_sources`, which handles user specified paths directly (instead of files Black discovers). So in psf#4015, I made a very similar change to psf#3846 for `get_sources`, and this is where some problems began. The core issue was the line: ``` root_relative_path = path.absolute().relative_to(root).as_posix() ``` The first issue is that despite root being computed from user inputs, we call `.resolve()` while computing it (likely unecessarily). Which means that `path` may not actually be relative to `root`. So I started off this PR trying to fix that, when I ran into the second issue. Which is that `os.getcwd()` (as called by `os.path.abspath` or `Path.absolute` or `Path.cwd`) also often resolves symlinks! ``` >>> import os >>> os.environ.get("PWD") '/Users/shantanu/dev/black/symlink/bug' >>> os.getcwd() '/Users/shantanu/dev/black/actual/bug' ``` This also meant that the breakage often would not show up when input relative paths. This doesn't affect `gen_python_files` / psf#3846 because things are always absolute and known to be relative to `root`. Anyway, it looks like psf#4161 fixed the crash by just swallowing the error and ignoring the file. Instead, we should just try to compute the actual relative path. I think this PR should be quite safe, but we could also consider reverting some of the previous changes; the associated issues weren't too popular. At the same time, I think there's still behaviour that can be improved and I kind of want to make larger changes, but maybe I'll save that for if we do something like psf#3952 Hopefully fixes psf#4205, fixes psf#4209, actual fix for psf#4077
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Describe the bug
In Windows, the UNC path is not supported when using as
--stdin-filename. For example, let's say one path is:\\?\D:\git\OLive\setup.pyWhen running
The following error message will be shown.
To Reproduce
Windows Python pathlib doesn't support UNC path.
Expected behavior
UNC path
\\?\D:\git\OLive\setup.pyshould be supported in Windows allow as stdin-filenameEnvironment
Additional context
The text was updated successfully, but these errors were encountered: