--skip-gitignore still traverses files/directories in .gitignore'd directories

[bram-tv]

Preamble

There are several issues about .gitignore that looks similar but none are quite the same..
The one that comes closest is #1912 (comment) and more exact the comment added in #1900 (comment) but it doesn't look like a specific issues was created for it. (#1912 is still open tho but the title does not really match with this issue).

Example

Let's start with an example repository:

$ tree -a  -I '.git'
.
├── bar.py
├── dir1
│   ├── .gitignore
│   ├── sub1
│   │   ├── dir1_sub1_file1.py
│   │   └── dir1_sub1_file2.py
│   ├── sub2
│   │   ├── dir1_sub2_file1.py
│   │   └── dir1_sub2_file2.py
│   └── sub3
│       ├── dir1_sub3_file1.py
│       └── dir1_sub3_file2.py
└── foo.py

4 directories, 9 files

The contents of dir1/.gitignore:

sub1/
sub2/
sub3/

Just for reference: the output of git ls-files

$ git ls-files
bar.py
dir1/.gitignore
foo.py
$ git ls-files --others --exclude-standard
$

Running isort --skip-gitignore --verbose .:

$ isort --check --skip-gitignore  --verbose .
...
.git was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub3_file2.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub3_file1.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub2_file2.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub2_file1.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub1_file2.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub1_file1.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
Skipped 7 files

It did skip the files inside dir1/sub1, dir1/sub2/ and dir1/sub3/ which is good but it really shouldn't be aware that these file exist. It means it did traverse into the sub-directories and then listed the files in it..

Work-around

Using --extend-skip-glob as a work-around and specifying the contents of the .gitignore file:

$ isort --skip-gitignore  --verbose --extend-skip-glob='dir1/sub1' --extend-skip-glob='dir1/sub2' --extend-skip-glob='dir1/sub3' .
...
.git was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub3 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub2 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub1 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
Skipped 4 files

Now it skipped the sub directories entirely and didn't traverse inside the directories to list the files (to then ignore these)

[bram-tv]

Looking at the is_skipped code in settings.py, it ends with:

isort/isort/settings.py

Line 646 in 7de1829

and not file_path.is_dir()

            ...

            # git_ls_files are good files you should parse. If you're not in the allow list, skip.

            if (
                git_folder
                and not file_path.is_dir()
                and str(file_path.resolve()) not in self.git_ls_files[git_folder]
            ):
                return True

        return False

When the file_path is a directory it will return False which will cause it to traverse the directory.

A possible fix: when file_path is a directory then check if there git_ls_files contains a file inside that directory.

Something like:

diff --git a/isort/settings.py b/isort/settings.py
index a3658fff..67acdbf5 100644
--- a/isort/settings.py
+++ b/isort/settings.py
@@ -641,11 +641,14 @@ class Config(_Config):

             # git_ls_files are good files you should parse. If you're not in the allow list, skip.

-            if (
-                git_folder
-                and not file_path.is_dir()
-                and str(file_path.resolve()) not in self.git_ls_files[git_folder]
-            ):
+            if git_folder:
+                if file_path.is_dir():
+                    dir = file_path.resolve()
+                    if not any(file.startswith(f"{dir}/") for file in self.git_ls_files[git_folder]):
+                        return True
+
+                elif str(file_path.resolve()) not in self.git_ls_files[git_folder]:
+                    return True
                 return True

         return False

With the example repository listed in the description the output is:

...
sub3 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub2 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub1 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled....
...

which is good.

However it's not yet perfect...
When the repository is changed into:

tree -a  -I '.git'
.
├── bar.py
├── dir1
│   ├── sub1
│   │   ├── dir1_sub1_file1.py
│   │   └── dir1_sub1_file2.py
│   ├── sub2
│   │   ├── dir1_sub2_file1.py
│   │   └── dir1_sub2_file2.py
│   └── sub3
│       ├── dir1_sub3_file1.py
│       └── dir1_sub3_file2.py
├── foo.py
└── .gitignore

4 directories, 9 files

where the .gitignore file now contains:

dir1/sub1/
dir1/sub2/
dir1/sub3/

then the output is:

...
dir1 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
...

which is a bit confusing: in the .gitignore file it's specified to ignore dir1/sub1/, dir1/sub2 and dir1/sub3 but the isort output makes it appear as if the entire directory was ignored..
In a way it was because there were no other files in dir1/ but still the message doesn't "feel" right..

[bram-tv]

Also: note that this solution is still suboptimal is some cases: it can still cause too many files/directories to be traversed.

Assume the layout:

$ tree -a  -I '.git'
.
├── bar.py
├── dir1
│   ├── sub1
│   │   ├── dir1_sub1_file1.py
│   │   ├── dir1_sub1_file2.py
│   │   ├── subsub1
│   │   │   └── dir1_sub1_subsub1_file_1.py
│   │   ├── subsub2
│   │   │   └── dir1_sub1_subsub2_file_1.py
│   │   ├── subsub3
│   │   │   └── dir1_sub1_subsub3_file_1.py
│   │   ├── subsub4
│   │   │   └── dir1_sub1_subsub4_file_1.py
│   │   ├── subsub5
│   │   │   └── dir1_sub1_subsub5_file_1.py
│   │   ├── subsub6
│   │   │   └── dir1_sub1_subsub6_file_1.py
│   │   ├── subsub7
│   │   │   └── dir1_sub1_subsub7_file_1.py
│   │   ├── subsub8
│   │   │   └── dir1_sub1_subsub8_file_1.py
│   │   └── subsub9
│   │       └── dir1_sub1_subsub9_file_1.py
│   ├── sub2
│   │   ├── dir1_sub2_file1.py
│   │   └── dir1_sub2_file2.py
│   └── sub3
│       ├── dir1_sub3_file1.py
│       └── dir1_sub3_file2.py
├── foo.py
└── .gitignore

13 directories, 18 files

And assume the file dir1/sub1/subsub5/dir1_sub1_subsub5_file_1.py was committed (despite being ignored, i.e. it was added using git add -f)

And .gitignore contains:

dir1/sub1/
dir1/sub2/
dir1/sub3/

The git ls-files output for reference:

$ git ls-files
.gitignore
bar.py
dir1/sub1/subsub5/dir1_sub1_subsub5_file_1.py
foo.py
$ git ls-files --others --exclude-standard
$

then isort output is:

...
sub3 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
sub2 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub1 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub7 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub2 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub4 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub9 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub3 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub6 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
subsub8 was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub1_file2.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
dir1_sub1_file1.py was skipped as it's listed in 'skip' setting, matches a glob in 'skip_glob' setting, or is in a .gitignore file with --skip-gitignore enabled.
...

so it still did some unneeded directory traversal.

I'm not sure if the structure of the code allows this to be (easily) fixed tho..
We know the files that may need to be checked (i.e. the files in self.git_ls_files) but that's not how the code loops..