Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix ignoring input files for symlink reasons
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
- Loading branch information