Skip to content

Commit

Permalink
Apply .gitignore correctly in every source entry (#3336)
Browse files Browse the repository at this point in the history
When passing multiple src directories, the root gitignore was only
applied to the first processed source. The reason is that, in the
first source, exclude is `None`, but then the value gets overridden by
`re_compile_maybe_verbose(DEFAULT_EXCLUDES)`, so in the next iteration
where the source is a directory, the condition is not met and sets the
value of `gitignore` to `None`.

To fix this problem, we store a boolean indicating if `exclude` is
`None` and set the value of `exclude` to its default value if that's
the case. This makes sure that the flow enters the correct condition on
following iterations and also keeps the original value if the condition
is not met.

Also, the value of `gitignore` is initialized as `None` and overriden
if necessary. The value of `root_gitignore` is always calculated to
avoid using additional variables (at the small cost of additional
computations).

Signed-off-by: Antonio Ossa Guerra <aaossa@uc.cl>
  • Loading branch information
aaossa committed Nov 5, 2022
1 parent 2704dc7 commit 0e9d29a
Show file tree
Hide file tree
Showing 8 changed files with 26 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGES.md
Expand Up @@ -22,6 +22,9 @@

<!-- Changes to how Black can be configured -->

- Fix incorrectly ignoring .gitignore presence when more than one source directory is
specified (#3336)

### Packaging

<!-- Changes to how Black is packaged, such as dependency requirements -->
Expand Down
18 changes: 11 additions & 7 deletions src/black/__init__.py
Expand Up @@ -30,6 +30,7 @@
import click
from click.core import ParameterSource
from mypy_extensions import mypyc_attr
from pathspec import PathSpec
from pathspec.patterns.gitwildmatch import GitWildMatchPatternError

from _black_version import version as __version__
Expand Down Expand Up @@ -627,6 +628,11 @@ def get_sources(
sources: Set[Path] = set()
root = ctx.obj["root"]

exclude_is_None = exclude is None
exclude = re_compile_maybe_verbose(DEFAULT_EXCLUDES) if exclude is None else exclude
gitignore = None # type: Optional[PathSpec]
root_gitignore = get_gitignore(root)

for s in src:
if s == "-" and stdin_filename:
p = Path(stdin_filename)
Expand Down Expand Up @@ -660,16 +666,14 @@ def get_sources(

sources.add(p)
elif p.is_dir():
if exclude is None:
exclude = re_compile_maybe_verbose(DEFAULT_EXCLUDES)
gitignore = get_gitignore(root)
if exclude_is_None:
p_gitignore = get_gitignore(p)
# No need to use p's gitignore if it is identical to root's gitignore
# (i.e. root and p point to the same directory).
if gitignore != p_gitignore:
gitignore += p_gitignore
else:
gitignore = None
if root_gitignore == p_gitignore:
gitignore = root_gitignore
else:
gitignore = root_gitignore + p_gitignore
sources.update(
gen_python_files(
p.iterdir(),
Expand Down
1 change: 1 addition & 0 deletions tests/data/gitignore_used_on_multiple_sources/.gitignore
@@ -0,0 +1 @@
a.py
Empty file.
Empty file.
Empty file.
Empty file.
11 changes: 11 additions & 0 deletions tests/test_black.py
Expand Up @@ -1998,6 +1998,17 @@ def test_gitignore_used_as_default(self) -> None:
ctx.obj["root"] = base
assert_collected_sources(src, expected, ctx=ctx, extend_exclude=r"/exclude/")

def test_gitignore_used_on_multiple_sources(self) -> None:
root = Path(DATA_DIR / "gitignore_used_on_multiple_sources")
expected = [
root / "dir1" / "b.py",
root / "dir2" / "b.py",
]
ctx = FakeContext()
ctx.obj["root"] = root
src = [root / "dir1", root / "dir2"]
assert_collected_sources(src, expected, ctx=ctx)

@patch("black.find_project_root", lambda *args: (THIS_DIR.resolve(), None))
def test_exclude_for_issue_1572(self) -> None:
# Exclude shouldn't touch files that were explicitly given to Black through the
Expand Down

0 comments on commit 0e9d29a

Please sign in to comment.