Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make src_paths behave as expected when using --resolve-all-configs and improve performance #2142

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 34 additions & 18 deletions isort/settings.py
Expand Up @@ -19,6 +19,7 @@
Dict,
FrozenSet,
Iterable,
Iterator,
List,
Optional,
Pattern,
Expand Down Expand Up @@ -806,28 +807,43 @@ def find_all_configs(path: str) -> Trie:
"""
trie_root = Trie("default", {})

for dirpath, _, _ in os.walk(path):
for config_file_name in CONFIG_SOURCES:
potential_config_file = os.path.join(dirpath, config_file_name)
if os.path.isfile(potential_config_file):
config_data: Dict[str, Any]
try:
config_data = _get_config_data(
potential_config_file, CONFIG_SECTIONS[config_file_name]
)
except Exception:
warn(f"Failed to pull configuration information from {potential_config_file}")
config_data = {}

if config_data:
if "directory" not in config_data:
config_data["directory"] = dirpath
trie_root.insert(potential_config_file, config_data)
break
config_sources_set = set(CONFIG_SOURCES)
for potential_config_file in _scanwalk_files(
path, exclude_fn=lambda entry: entry.name in DEFAULT_SKIP
):
if potential_config_file.name not in config_sources_set:
continue
try:
config_data = _get_config_data(
potential_config_file.path, CONFIG_SECTIONS[potential_config_file.name]
)
if "directory" not in config_data:
config_data["directory"] = os.path.dirname(potential_config_file.path)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main logic to fix the src_paths resolution.

trie_root.insert(potential_config_file.path, config_data)
except Exception:
warn(f"Failed to pull configuration information from {potential_config_file.path}")

return trie_root


def _scanwalk_files(
root_path: str, exclude_fn: Callable[[os.DirEntry], bool] = None
) -> Iterator[os.DirEntry]:
# depth-first walk of file system starting with root_path
stack: List[str] = [root_path]
while stack:
dir_path = stack.pop()
with os.scandir(dir_path) as it:
for entry in it:
# Avoid processing excluded files/dirs and their descendants
if exclude_fn and exclude_fn(entry):
continue
if entry.is_dir():
stack.append(entry.path)
elif entry.is_file():
yield entry


def _get_config_data(file_path: str, sections: Tuple[str, ...]) -> Dict[str, Any]:
settings: Dict[str, Any] = {}

Expand Down
9 changes: 9 additions & 0 deletions tests/unit/test_settings.py
Expand Up @@ -258,11 +258,13 @@ def test_find_all_configs(tmpdir):
dir2 = tmpdir / "subdir2"
dir3 = tmpdir / "subdir3"
dir4 = tmpdir / "subdir4"
dir_skip = tmpdir / ".venv"

dir1.mkdir()
dir2.mkdir()
dir3.mkdir()
dir4.mkdir()
dir_skip.mkdir()

setup_cfg_file = dir1 / "setup.cfg"
setup_cfg_file.write_text(setup_cfg, "utf-8")
Expand All @@ -276,6 +278,9 @@ def test_find_all_configs(tmpdir):
pyproject_toml_file_broken = dir4 / "pyproject.toml"
pyproject_toml_file_broken.write_text(pyproject_toml_broken, "utf-8")

pyproject_toml_file_skip = dir_skip / "pyproject.toml"
pyproject_toml_file_skip.write_text(pyproject_toml, "utf-8")

config_trie = settings.find_all_configs(str(tmpdir))

config_info_1 = config_trie.search(str(dir1 / "test1.py"))
Expand All @@ -296,3 +301,7 @@ def test_find_all_configs(tmpdir):
config_info_4 = config_trie.search(str(tmpdir / "file4.py"))
assert config_info_4[0] == "default"
assert set(Config(**config_info_4[1]).src_paths) == {Path.cwd(), Path.cwd().joinpath("src")}

config_info_skip = config_trie.search(str(dir_skip / "skip.py"))
assert config_info_skip[0] == "default"
assert set(Config(**config_info_skip[1]).src_paths) == {Path.cwd(), Path.cwd().joinpath("src")}