isort icon indicating copy to clipboard operation
isort copied to clipboard

Always ignore symlinks that point outside of the current VCS project

Open ucodery opened this issue 3 years ago • 3 comments

#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 avatar Jul 20 '21 19:07 ucodery

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.

ucodery avatar Jul 21 '21 00:07 ucodery

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

RazCrimson avatar Mar 23 '22 16:03 RazCrimson

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

ucodery avatar Mar 30 '22 21:03 ucodery