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 all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/configuration/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -1758,7 +1758,7 @@ Explicitly set the config root for resolving all configs. When used with the --r

## Resolve All Configs

Tells isort to resolve the configs for all sub-directories and sort files in terms of its closest config files.
Tells isort to resolve the configs for all sub-directories and sort files in terms of its closest config files. When using this option, `src_paths` will resolve relative to the directory that contains the config that was used by default.

**Type:** Bool
**Default:** `False`
Expand Down
50 changes: 34 additions & 16 deletions isort/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
Dict,
FrozenSet,
Iterable,
Iterator,
List,
Optional,
Pattern,
Expand Down Expand Up @@ -806,26 +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:
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
14 changes: 14 additions & 0 deletions tests/unit/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ def test_find_all_configs(tmpdir):
pyproject_toml = """
[tool.isort]
profile = "hug"
src_paths = ["src"]
"""

isort_cfg = """
Expand All @@ -257,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 @@ -275,19 +278,30 @@ 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"))
assert config_info_1[0] == str(setup_cfg_file)
assert config_info_1[0] == str(setup_cfg_file) and config_info_1[1]["profile"] == "django"
assert set(Config(**config_info_1[1]).src_paths) == {Path(dir1), Path(dir1, "src")}

config_info_2 = config_trie.search(str(dir2 / "test2.py"))
assert config_info_2[0] == str(pyproject_toml_file)
assert config_info_2[0] == str(pyproject_toml_file) and config_info_2[1]["profile"] == "hug"
assert set(Config(**config_info_2[1]).src_paths) == {Path(dir2, "src")}

config_info_3 = config_trie.search(str(dir3 / "test3.py"))
assert config_info_3[0] == str(isort_cfg_file)
assert config_info_3[0] == str(isort_cfg_file) and config_info_3[1]["profile"] == "black"
assert set(Config(**config_info_3[1]).src_paths) == {Path(dir3), Path(dir3, "src")}

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")}