black
black copied to clipboard
black does not exclude files in git submodules
Describe the bug
black does not exclude files in git submodules when full filename is passed
To Reproduce
I created an example project with a git submodule to test. The super-project has a pyproject.toml which excludes the subproject:
[tool.black]
force-exclude = 'subproject'
Commands to clone and reproduce bug:
git clone --recurse-submodules [email protected]:janjachnik-dyson/black-submodule-test-super.git
cd black-submodule-test-super
black --check subproject/file.py
The output:
Identified `/tmp/black-submodule-test-super/subproject` as project root containing a .git directory.
Sources to be formatted: "file.py"
subproject/file.py already well formatted, good job.
All done! ✨ 🍰 ✨
1 file would be left unchanged.
Expected behavior
subproject/file.py
would be ignored because its path matches the force-exclude regex in pyproject.toml
Environment
- Black's version: 22.3.0
- OS and Python version: Ubuntu 20.04, Python 3.8.10
Additional context
We use black from within Visual Studio Code, where it is always called with the full filename. If black is manually run on the root folder of the super-project, the excludes are handled correctly. Within VSCode we pass the --config
argument to specify the correct configuration file of the super-project.
Possible solution
The problem occurs because black decides the project root by recursively searching parent directories for .git
, .hg
and pyproject.toml
files. In this case, the project root becomes the subproject folder (due to .git) and the exclude regex is compared against the path relative to the project root, which no longer contains the subproject
string.
As a possible fix, I added the option to manually specify the project root: https://github.com/janjachnik-dyson/black/commit/6fc33683f17ecbb689add9c722ba96be0428d78c
Then, the following command works to properly exclude the file:
black --project-root . subproject/file.py
If this sounds like a valid fix/workaround, I'll raise a PR. Or is there a better way to handle excludes with git submodules?
We've also been seeing issues where black doesn't correctly handle .gitignore
with submodules. In our case it's a top-level .gitignore
being incorrectly applied to files in submodules, eg.
$ black -v .
Identified `...` as project root containing a .git directory.
Sources to be formatted: "."
example.py ignored: matches the .gitignore file content
submodule/example.py ignored: matches the .gitignore file content
It would be nice for black to match git behaviour 1:1 by default, without needing CLI workarounds.
I've been trying to write a test for this and I'm concerned that the existing exclude tests do not reflect real-world usage.
The existing tests make use of the assert_collected_sources()
function: https://github.com/psf/black/blob/6c1bd08f16b636de38b92aeb2e0a1e8ebef0a0b1/tests/test_black.py#L1836 which in-term makes a call to get_sources()
: https://github.com/psf/black/blob/6c1bd08f16b636de38b92aeb2e0a1e8ebef0a0b1/src/black/init.py#L595
However, the result of get_sources()
is dependent on ctx.obj["root"]
. That root folder is set dynamically when you run black using the find_project_root()
function: https://github.com/psf/black/blob/6c1bd08f16b636de38b92aeb2e0a1e8ebef0a0b1/src/black/init.py#L458-L459
However, in all of the include/exclude tests the root folder is manually set. None of the tests will reproduce the behaviour that occurs if find_project_root()
finds a different folder, which is the behaviour when you actually use black.
In my branch, I have added a test which reproduces the behaviour of this bug: https://github.com/janjachnik-dyson/black/tree/feature/manually_specify_project_root
The problem only seems to occur when the full file path is given, I added tests for both scenarios (base path and full path).
To make sure the tests are run correctly, I now set ctx.obj['root']
inside assert_collected_sources()
by making a call to black.find_project_root()
- this mimics the behaviour of the black executable. Without that change, the test would pass.
So now we have the bug reproduced in a test, I'd like some feedback/guidance on how to fix it.
I've suggested one solution in a draft PR here: https://github.com/psf/black/pull/3116, which essentially gives the ability to manually override the project root.
Is my solution a viable option, or should the bug be fixed in a way that doesn't require manual intervention?
I was expecting that black would never try to format files in any submodule, but maybe our team has a different setup than other python project. We have googletest as a submodule (i think is the normal way to use gtest), which seems to have python2 scripts which makes black fail. I could file a different feature-request for "black option to exclude files in git submodules"? (Or would that be a dupe of this issue?)
Any news on this front? This is a major nuisance for us, would be great if a workaround was available.
@derpda No update currently. I'm open to specifying the project root. I'll try to take a look at the linked PR soon-ish. A admittedly hacky workaround is to create an empty file at the root of your project and pass that file along with the file from the submodule.
black empty.py submodule/yourcode.py
This way Black will be forced to use the common parent of the two files as the project root.
Thank you for getting back to me on this!
I tried this in VSCode, and it will sadly still format files that are in the force-exclude list.
Weird thing is though, if I copy the command from VSCode's Ouput
window and execute it in the shell it works as expected, ignoring the second file if it is in the exclude list. When formatting on save however, it will apply changes.
That is probably beyond the scope of this issue, but if you (or someone else) knows something, that would be much appreciated!
Still relevant.