PyCQA / isort

A Python utility / library to sort imports.
https://pycqa.github.io/isort/
MIT License
6.49k stars 580 forks source link

Always ignore symlinks that point outside of the current VCS project #1788

Open ucodery opened 3 years ago

ucodery commented 3 years ago

1772 fixed errors from trying to ask git about files outside its knowledge by ignoring these kind of external links, but did not stop isort from continuing to follow, and possibly edit, these files.

I think that when users have these kind of links, they don't expect them to be followed. Symlinks that point within the same project should probably continue to be followed. This has already been an issue to users running either the Spack package manager or the Bazel build tool. This would also help in aligning running isort with black. Since black never follows these sorts of links, a project that had them and wanted isort to check the same files would have to pass each link as an extra ignore path in addition to those being kept in sync with black.

To properly make this change universally I think there are multiple places isort will have to look for links and check if they are external or internal. At least files.py but also main.py and api.py; it partially depends on if isort should support following an external link if it is explicitly passed in by the user. Additionally, the check for links in _check_folder_gitignore() is probably not needed once this is a default check on all paths.

Also, some food for thought, black will ignore any file that is on the other side of a symlink that points outside of the project's root. It might be good for isort to take the same line and always ignore these sorts of files. isort does have an older userbase, and works in more than just git repositories, so this might break some workflows though.

Originally posted by @ucodery in https://github.com/PyCQA/isort/issues/1772#issuecomment-873826801

ucodery commented 3 years ago

I have adapted and example from #1771 to show how even now skipping gitignore doesn't stop isort leaving the project root.

❯ mkdir test
❯ echo 'import foo' >> test/a.py
❯ echo 'import bar' >> test/a.py
❯ mkdir test/proj
❯ cp test/a.py test/proj/b.py
❯ ln -s test/a.py test/proj/c.py
❯ mkdir test/other
❯ cp test/a.py test/other/d.py
❯ cd test/proj
❯ ln -s ../other dir
❯ ls
total 8
-rw-r--r--  1 jeremyp  staff    22B Jul 20 17:06 b.py
lrwxr-xr-x  1 jeremyp  staff     9B Jul 20 17:08 c.py -> test/a.py
lrwxr-xr-x  1 jeremyp  staff    11B Jul 20 17:12 dir -> ../other
❯ isort . --show-files
./c.py
./b.py
./dir/d.py
❯ isort . --show-files --dont-follow-links
./c.py
./b.py
❯ git init
...
❯ echo 'c.py' >> .gitignore
❯ echo 'dir' >> .gitignore
❯ echo '[tool.isort]' >> pyproject.toml
❯ echo 'skip_gitignore = true' >> pyproject.toml
❯ isort . --show-files
./b.py
./dir/d.py
❯ isort . --show-files --dont-follow-links
./b.py
❯ git status -s
?? .gitignore
?? b.py

This shows that a combination of --show-files and --don't-follow-links get the desired behavior. However, this would unilaterally block symlinks, even if they remained within the project. Debatable if that is a useful workflow, but this also isn't default isort behavior.

RazCrimson commented 2 years ago

Is there any progress on this? Or waiting for contributions? 🤔

ucodery commented 2 years ago

I have not picked up this ticket at all yet. If you would like make a PR to address it, contributions are very welcome!