mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-03-28 08:28:53 -03:00
fix(paths): deduplicate LoRA path overlap (#871)
This commit is contained in:
@@ -322,3 +322,182 @@ def test_extra_paths_deduplication(monkeypatch, tmp_path):
|
||||
|
||||
assert config_instance.loras_roots == [str(loras_dir)]
|
||||
assert config_instance.extra_loras_roots == [str(extra_loras_dir)]
|
||||
|
||||
|
||||
def test_apply_library_settings_ignores_extra_lora_path_overlapping_primary_symlink(
|
||||
monkeypatch, tmp_path, caplog
|
||||
):
|
||||
"""Extra LoRA paths should be ignored when they resolve to the same target as a primary root."""
|
||||
real_loras_dir = tmp_path / "loras_real"
|
||||
real_loras_dir.mkdir()
|
||||
loras_link = tmp_path / "loras_link"
|
||||
loras_link.symlink_to(real_loras_dir, target_is_directory=True)
|
||||
|
||||
config_instance = config_module.Config()
|
||||
|
||||
library_config = {
|
||||
"folder_paths": {
|
||||
"loras": [str(loras_link)],
|
||||
"checkpoints": [],
|
||||
"unet": [],
|
||||
"embeddings": [],
|
||||
},
|
||||
"extra_folder_paths": {
|
||||
"loras": [str(real_loras_dir)],
|
||||
"checkpoints": [],
|
||||
"unet": [],
|
||||
"embeddings": [],
|
||||
},
|
||||
}
|
||||
|
||||
with caplog.at_level("WARNING", logger=config_module.logger.name):
|
||||
config_instance.apply_library_settings(library_config)
|
||||
|
||||
assert config_instance.loras_roots == [str(loras_link)]
|
||||
assert config_instance.extra_loras_roots == []
|
||||
|
||||
warning_messages = [
|
||||
record.message
|
||||
for record in caplog.records
|
||||
if record.levelname == "WARNING"
|
||||
and "same lora folder" in record.message.lower()
|
||||
]
|
||||
assert len(warning_messages) == 1
|
||||
assert "comfyui model paths" in warning_messages[0].lower()
|
||||
assert "extra folder paths" in warning_messages[0].lower()
|
||||
assert "duplicate items" in warning_messages[0].lower()
|
||||
|
||||
|
||||
def test_apply_library_settings_detects_overlap_case_insensitively(
|
||||
monkeypatch, tmp_path, caplog
|
||||
):
|
||||
"""Overlap detection should use case-insensitive comparison on Windows-like paths."""
|
||||
real_loras_dir = tmp_path / "loras_real"
|
||||
real_loras_dir.mkdir()
|
||||
loras_link = tmp_path / "loras_link"
|
||||
loras_link.symlink_to(real_loras_dir, target_is_directory=True)
|
||||
|
||||
original_exists = config_module.os.path.exists
|
||||
original_realpath = config_module.os.path.realpath
|
||||
original_normcase = config_module.os.path.normcase
|
||||
|
||||
def fake_exists(path):
|
||||
if isinstance(path, str) and path.lower() in {
|
||||
str(loras_link).lower(),
|
||||
str(real_loras_dir).lower(),
|
||||
str(loras_link).upper().lower(),
|
||||
str(real_loras_dir).upper().lower(),
|
||||
}:
|
||||
return True
|
||||
return original_exists(path)
|
||||
|
||||
def fake_realpath(path, *args, **kwargs):
|
||||
if isinstance(path, str):
|
||||
lowered = path.lower()
|
||||
if lowered == str(loras_link).lower():
|
||||
return str(real_loras_dir)
|
||||
if lowered == str(real_loras_dir).lower():
|
||||
return str(real_loras_dir)
|
||||
return original_realpath(path, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(config_module.os.path, "exists", fake_exists)
|
||||
monkeypatch.setattr(config_module.os.path, "realpath", fake_realpath)
|
||||
monkeypatch.setattr(
|
||||
config_module.os.path,
|
||||
"normcase",
|
||||
lambda value: original_normcase(value).lower(),
|
||||
)
|
||||
|
||||
config_instance = config_module.Config()
|
||||
primary_path = str(loras_link).replace("loras_link", "LORAS_LINK")
|
||||
extra_path = str(real_loras_dir).replace("loras_real", "loras_real")
|
||||
|
||||
library_config = {
|
||||
"folder_paths": {
|
||||
"loras": [primary_path],
|
||||
"checkpoints": [],
|
||||
"unet": [],
|
||||
"embeddings": [],
|
||||
},
|
||||
"extra_folder_paths": {
|
||||
"loras": [extra_path.upper()],
|
||||
"checkpoints": [],
|
||||
"unet": [],
|
||||
"embeddings": [],
|
||||
},
|
||||
}
|
||||
|
||||
with caplog.at_level("WARNING", logger=config_module.logger.name):
|
||||
config_instance.apply_library_settings(library_config)
|
||||
|
||||
assert config_instance.loras_roots == [primary_path]
|
||||
assert config_instance.extra_loras_roots == []
|
||||
assert any("same lora folder" in record.message.lower() for record in caplog.records)
|
||||
|
||||
|
||||
def test_apply_library_settings_ignores_missing_extra_lora_paths(monkeypatch, tmp_path, caplog):
|
||||
"""Missing extra paths should be ignored without overlap warnings."""
|
||||
loras_dir = tmp_path / "loras"
|
||||
loras_dir.mkdir()
|
||||
missing_extra = tmp_path / "missing_loras"
|
||||
|
||||
config_instance = config_module.Config()
|
||||
library_config = {
|
||||
"folder_paths": {
|
||||
"loras": [str(loras_dir)],
|
||||
"checkpoints": [],
|
||||
"unet": [],
|
||||
"embeddings": [],
|
||||
},
|
||||
"extra_folder_paths": {
|
||||
"loras": [str(missing_extra)],
|
||||
"checkpoints": [],
|
||||
"unet": [],
|
||||
"embeddings": [],
|
||||
},
|
||||
}
|
||||
|
||||
with caplog.at_level("WARNING", logger=config_module.logger.name):
|
||||
config_instance.apply_library_settings(library_config)
|
||||
|
||||
assert config_instance.loras_roots == [str(loras_dir)]
|
||||
assert config_instance.extra_loras_roots == []
|
||||
assert not any("same lora folder" in record.message.lower() for record in caplog.records)
|
||||
|
||||
|
||||
def test_apply_library_settings_ignores_extra_lora_path_overlapping_primary_root_symlink(
|
||||
tmp_path, caplog
|
||||
):
|
||||
"""Extra LoRA paths should be ignored when already reachable via a first-level symlink under the primary root."""
|
||||
loras_dir = tmp_path / "loras"
|
||||
loras_dir.mkdir()
|
||||
external_dir = tmp_path / "external_loras"
|
||||
external_dir.mkdir()
|
||||
link_dir = loras_dir / "link"
|
||||
link_dir.symlink_to(external_dir, target_is_directory=True)
|
||||
|
||||
config_instance = config_module.Config()
|
||||
library_config = {
|
||||
"folder_paths": {
|
||||
"loras": [str(loras_dir)],
|
||||
"checkpoints": [],
|
||||
"unet": [],
|
||||
"embeddings": [],
|
||||
},
|
||||
"extra_folder_paths": {
|
||||
"loras": [str(external_dir)],
|
||||
"checkpoints": [],
|
||||
"unet": [],
|
||||
"embeddings": [],
|
||||
},
|
||||
}
|
||||
|
||||
with caplog.at_level("WARNING", logger=config_module.logger.name):
|
||||
config_instance.apply_library_settings(library_config)
|
||||
|
||||
assert config_instance.loras_roots == [str(loras_dir)]
|
||||
assert config_instance.extra_loras_roots == []
|
||||
assert any(
|
||||
"same lora folder" in record.message.lower()
|
||||
for record in caplog.records
|
||||
)
|
||||
|
||||
@@ -73,6 +73,15 @@ class DummyScanner(ModelScanner):
|
||||
}
|
||||
|
||||
|
||||
class MultiRootDummyScanner(DummyScanner):
|
||||
def __init__(self, roots: List[Path]):
|
||||
self._roots = [str(root) for root in roots]
|
||||
super().__init__(roots[0])
|
||||
|
||||
def get_model_roots(self) -> List[str]:
|
||||
return list(self._roots)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def reset_model_scanner_singletons():
|
||||
ModelScanner._instances.clear()
|
||||
@@ -559,3 +568,59 @@ async def test_count_model_files_handles_symlink_loops(tmp_path: Path):
|
||||
count = scanner._count_model_files()
|
||||
|
||||
assert count == 2
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_initialize_cache_dedupes_files_reachable_via_primary_symlink_and_extra_root(
|
||||
tmp_path: Path,
|
||||
):
|
||||
loras_root = tmp_path / "loras"
|
||||
loras_root.mkdir()
|
||||
extra_root = tmp_path / "extra"
|
||||
extra_root.mkdir()
|
||||
(extra_root / "one.txt").write_text("one", encoding="utf-8")
|
||||
(loras_root / "link").symlink_to(extra_root, target_is_directory=True)
|
||||
|
||||
scanner = MultiRootDummyScanner([loras_root, extra_root])
|
||||
|
||||
await scanner._initialize_cache()
|
||||
cache = await scanner.get_cached_data()
|
||||
|
||||
assert len(cache.raw_data) == 1
|
||||
assert cache.raw_data[0]["file_path"] == _normalize_path(loras_root / "link" / "one.txt")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reconcile_cache_removes_duplicate_alias_when_same_real_file_seen_once(
|
||||
tmp_path: Path,
|
||||
):
|
||||
loras_root = tmp_path / "loras"
|
||||
loras_root.mkdir()
|
||||
extra_root = tmp_path / "extra"
|
||||
extra_root.mkdir()
|
||||
real_file = extra_root / "one.txt"
|
||||
real_file.write_text("one", encoding="utf-8")
|
||||
(loras_root / "link").symlink_to(extra_root, target_is_directory=True)
|
||||
|
||||
scanner = MultiRootDummyScanner([loras_root, extra_root])
|
||||
await scanner._initialize_cache()
|
||||
|
||||
duplicate_entry = {
|
||||
"file_path": _normalize_path(extra_root / "one.txt"),
|
||||
"folder": "",
|
||||
"sha256": "hash-one",
|
||||
"tags": ["alpha"],
|
||||
"model_name": "one",
|
||||
"size": 1,
|
||||
"modified": 1.0,
|
||||
"license_flags": DEFAULT_LICENSE_FLAGS,
|
||||
}
|
||||
scanner._cache.raw_data.append(duplicate_entry)
|
||||
scanner._cache.add_to_version_index(duplicate_entry)
|
||||
scanner._hash_index.add_entry("hash-one", duplicate_entry["file_path"])
|
||||
|
||||
await scanner._reconcile_cache()
|
||||
|
||||
cache = await scanner.get_cached_data()
|
||||
cached_paths = {item["file_path"] for item in cache.raw_data}
|
||||
assert cached_paths == {_normalize_path(loras_root / "link" / "one.txt")}
|
||||
|
||||
@@ -590,6 +590,97 @@ def test_extra_paths_validation_no_overlap_with_other_libraries(manager, tmp_pat
|
||||
manager.update_extra_folder_paths({"loras": [str(lora_dir1)]})
|
||||
|
||||
|
||||
def test_extra_paths_validation_no_overlap_with_active_primary_lora_root(manager, tmp_path):
|
||||
"""Test that extra LoRA paths cannot overlap the active library primary LoRA roots."""
|
||||
real_lora_dir = tmp_path / "loras_real"
|
||||
real_lora_dir.mkdir()
|
||||
lora_link = tmp_path / "loras_link"
|
||||
lora_link.symlink_to(real_lora_dir, target_is_directory=True)
|
||||
|
||||
manager.create_library(
|
||||
"library1",
|
||||
folder_paths={"loras": [str(lora_link)]},
|
||||
activate=True,
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError, match="overlap with the active library's primary LoRA roots"):
|
||||
manager.update_extra_folder_paths({"loras": [str(real_lora_dir)]})
|
||||
|
||||
|
||||
def test_extra_paths_validation_no_overlap_with_active_primary_lora_root_case_insensitive(
|
||||
manager, monkeypatch, tmp_path
|
||||
):
|
||||
"""Overlap validation should treat differently-cased Windows-like paths as the same path."""
|
||||
real_lora_dir = tmp_path / "loras_real"
|
||||
real_lora_dir.mkdir()
|
||||
lora_link = tmp_path / "loras_link"
|
||||
lora_link.symlink_to(real_lora_dir, target_is_directory=True)
|
||||
|
||||
manager.create_library(
|
||||
"library1",
|
||||
folder_paths={"loras": [str(lora_link)]},
|
||||
activate=True,
|
||||
)
|
||||
|
||||
original_exists = settings_manager_module.os.path.exists
|
||||
original_realpath = settings_manager_module.os.path.realpath
|
||||
original_normcase = settings_manager_module.os.path.normcase
|
||||
|
||||
def fake_exists(path):
|
||||
if isinstance(path, str) and path.lower() in {str(lora_link).lower(), str(real_lora_dir).lower()}:
|
||||
return True
|
||||
return original_exists(path)
|
||||
|
||||
def fake_realpath(path):
|
||||
if isinstance(path, str) and path.lower() == str(lora_link).lower():
|
||||
return str(real_lora_dir)
|
||||
return original_realpath(path)
|
||||
|
||||
monkeypatch.setattr(settings_manager_module.os.path, "exists", fake_exists)
|
||||
monkeypatch.setattr(settings_manager_module.os.path, "realpath", fake_realpath)
|
||||
monkeypatch.setattr(settings_manager_module.os.path, "normcase", lambda value: original_normcase(value).lower())
|
||||
|
||||
with pytest.raises(ValueError, match="overlap with the active library's primary LoRA roots"):
|
||||
manager.update_extra_folder_paths({"loras": [str(real_lora_dir).upper()]})
|
||||
|
||||
|
||||
def test_extra_paths_validation_allows_missing_non_overlapping_lora_root(manager, tmp_path):
|
||||
"""Missing non-overlapping extra LoRA paths should not be rejected."""
|
||||
lora_dir = tmp_path / "loras"
|
||||
lora_dir.mkdir()
|
||||
missing_extra = tmp_path / "missing_loras"
|
||||
|
||||
manager.create_library(
|
||||
"library1",
|
||||
folder_paths={"loras": [str(lora_dir)]},
|
||||
activate=True,
|
||||
)
|
||||
|
||||
manager.update_extra_folder_paths({"loras": [str(missing_extra)]})
|
||||
|
||||
extra_paths = manager.get_extra_folder_paths()
|
||||
assert extra_paths["loras"] == [str(missing_extra)]
|
||||
|
||||
|
||||
def test_extra_paths_validation_rejects_primary_root_first_level_symlink_target(manager, tmp_path):
|
||||
"""Extra LoRA paths should be rejected when already reachable via a first-level symlink under the primary root."""
|
||||
lora_dir = tmp_path / "loras"
|
||||
lora_dir.mkdir()
|
||||
external_dir = tmp_path / "external_loras"
|
||||
external_dir.mkdir()
|
||||
link_dir = lora_dir / "link"
|
||||
link_dir.symlink_to(external_dir, target_is_directory=True)
|
||||
|
||||
manager.create_library(
|
||||
"library1",
|
||||
folder_paths={"loras": [str(lora_dir)]},
|
||||
activate=True,
|
||||
)
|
||||
|
||||
with pytest.raises(ValueError, match="overlap with the active library's primary LoRA roots"):
|
||||
manager.update_extra_folder_paths({"loras": [str(external_dir)]})
|
||||
|
||||
|
||||
def test_delete_library_switches_active(manager, tmp_path):
|
||||
other_dir = tmp_path / "other"
|
||||
other_dir.mkdir()
|
||||
|
||||
Reference in New Issue
Block a user