mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-10-12 12:32:11 -03:00
fix(pending-delete): drop the staging container with its last batch (#1145)
Deleting a folder that holds models answered "A deletion is still pending inside this folder. Wait for the undo window to expire." even though nothing was staged and no window was running. `.lm-pending-delete` is only a container: the purge, the undo and the staging rollback all removed the batch dir inside it and left the container behind, and the folder manifest matched the directory *name* alone. Any folder where a model had ever been deleted with the undo window was therefore undeletable for good, and the same name match refused folder renames. * `staging_dir_has_content()` is the shared answer to "is there still a batch here?": an empty container is a leftover, a quarantined batch (`*.orphaned`) is preserved data and still counts, and an unreadable directory counts too — never delete over something that could not be listed. * The container is removed with its last batch (purge, undo, staging rollback, merged-loser cleanup). `rmdir` only ever succeeds on an empty directory, and the path is checked to really be a `.lm-pending-delete`, so a model folder is never at risk. * A manifest-less batch dir with nothing in it is a crash leftover between `makedirs` and the manifest write: it is dropped instead of quarantined, which would have left an empty container that blocks deletion just the same. Verified: purge, undo and the empty-manifest-less sweep all leave no container behind, the container survives while another batch is staged, and an empty one no longer blocks folder delete or rename while a real (or quarantined) batch still does. 3818 backend tests pass.
This commit is contained in:
@@ -12,7 +12,10 @@ from ..utils.recipes_paths import path_is_or_contains_recipes_dir
|
||||
from ..utils.sidecar_paths import is_centralized, resolve_centralized_dir_for_dir
|
||||
from ..services.settings_manager import get_settings_manager
|
||||
from ..services.model_lifecycle_service import _require_path_in_library_roots
|
||||
from ..services.pending_delete_service import PENDING_DELETE_DIR_NAME
|
||||
from ..services.pending_delete_service import (
|
||||
PENDING_DELETE_DIR_NAME,
|
||||
staging_dir_has_content,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
@@ -881,7 +884,12 @@ class ModelMoveService:
|
||||
excluded_paths = self._excluded_model_paths()
|
||||
|
||||
for dirpath, dirnames, filenames in os.walk(absolute_path):
|
||||
if PENDING_DELETE_DIR_NAME in dirnames:
|
||||
# The container alone means nothing: an empty `.lm-pending-delete`
|
||||
# is a leftover from a purge, an undo or a failed staging, and
|
||||
# treating it as pending made the folder undeletable for good.
|
||||
if PENDING_DELETE_DIR_NAME in dirnames and staging_dir_has_content(
|
||||
os.path.join(dirpath, PENDING_DELETE_DIR_NAME)
|
||||
):
|
||||
pending_delete_job = True
|
||||
|
||||
for name in dirnames:
|
||||
@@ -1072,9 +1080,15 @@ class ModelMoveService:
|
||||
|
||||
@staticmethod
|
||||
def _has_pending_delete_job(absolute_path: str) -> bool:
|
||||
"""Return True when a staged-delete batch lives inside the subtree."""
|
||||
for _dirpath, dirnames, _filenames in os.walk(absolute_path):
|
||||
if PENDING_DELETE_DIR_NAME in dirnames:
|
||||
"""Return True when a staged-delete batch lives inside the subtree.
|
||||
|
||||
An empty ``.lm-pending-delete`` container does not count: it is a
|
||||
leftover, not a batch, and a rename must not be refused because of one.
|
||||
"""
|
||||
for dirpath, dirnames, _filenames in os.walk(absolute_path):
|
||||
if PENDING_DELETE_DIR_NAME in dirnames and staging_dir_has_content(
|
||||
os.path.join(dirpath, PENDING_DELETE_DIR_NAME)
|
||||
):
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
@@ -55,6 +55,28 @@ MANIFEST_FILE_NAME = "manifest.json"
|
||||
# deleted by the sweep.
|
||||
ORPHANED_SUFFIX = ".orphaned"
|
||||
|
||||
|
||||
def staging_dir_has_content(directory: str) -> bool:
|
||||
"""Whether a staging directory still holds a batch (or a quarantined one).
|
||||
|
||||
``.lm-pending-delete`` is only a container: the batch dir inside it is what
|
||||
carries the staged files, and the purge, the undo and the staging rollback
|
||||
all remove it. An empty container left behind must not be mistaken for a
|
||||
staged delete -- it used to make every later folder delete answer "A
|
||||
deletion is still pending inside this folder" forever, with no undo window
|
||||
to wait for and nothing in the UI to clear it (issue #1145 follow-up).
|
||||
|
||||
Quarantined batches (``*.orphaned``) are kept on purpose, so anything left
|
||||
inside still counts. An unreadable directory counts as content too: never
|
||||
delete over something we could not list.
|
||||
"""
|
||||
|
||||
try:
|
||||
return bool(os.listdir(directory))
|
||||
except OSError:
|
||||
return True
|
||||
|
||||
|
||||
# Map scanner.model_type (singular) to the manifest page type values.
|
||||
_MODEL_TYPE_PAGE_MAP = {
|
||||
"lora": "loras",
|
||||
@@ -201,6 +223,7 @@ class PendingDeleteService:
|
||||
if batch_dir:
|
||||
self._rollback_model_staging(batch_dir, staged_pairs)
|
||||
self._remove_empty_dir(batch_dir)
|
||||
self._remove_empty_staging_parent(batch_dir)
|
||||
return None
|
||||
except Exception as exc: # defensive - never block the delete flow
|
||||
logger.warning(
|
||||
@@ -266,6 +289,7 @@ class PendingDeleteService:
|
||||
if batch_dir:
|
||||
self._rollback_recipe_staging(batch_dir, staged_pairs)
|
||||
self._remove_empty_dir(batch_dir)
|
||||
self._remove_empty_staging_parent(batch_dir)
|
||||
return None
|
||||
except Exception as exc: # defensive - never block the delete flow
|
||||
logger.warning(
|
||||
@@ -454,6 +478,7 @@ class PendingDeleteService:
|
||||
# Remove the manifest + batch dir only after all entries restored.
|
||||
self._remove_manifest(batch_dir)
|
||||
self._remove_empty_dir(batch_dir)
|
||||
self._remove_empty_staging_parent(batch_dir)
|
||||
await self._forget_batch(batch_id)
|
||||
# Clean up merged loser dirs (their staged files were restored
|
||||
# above) and drop them from the registry too.
|
||||
@@ -832,6 +857,7 @@ class PendingDeleteService:
|
||||
continue
|
||||
self._remove_manifest(src)
|
||||
self._remove_empty_dir(src)
|
||||
self._remove_empty_staging_parent(src)
|
||||
removed.append(os.path.basename(src))
|
||||
return removed
|
||||
|
||||
@@ -842,6 +868,13 @@ class PendingDeleteService:
|
||||
|
||||
manifest = self._read_manifest(batch_dir)
|
||||
if manifest is None:
|
||||
if not staging_dir_has_content(batch_dir):
|
||||
# Crash leftover: the batch dir was created but nothing was ever
|
||||
# staged into it. Quarantining an empty directory would leave a
|
||||
# container that blocks folder deletion for good.
|
||||
self._remove_empty_dir(batch_dir)
|
||||
self._remove_empty_staging_parent(batch_dir)
|
||||
return True
|
||||
# Corrupted or manifest-less batch: quarantine, NEVER delete the
|
||||
# staged files (they may be the only copy of the user's data).
|
||||
self._quarantine_batch_dir(batch_dir)
|
||||
@@ -885,6 +918,7 @@ class PendingDeleteService:
|
||||
|
||||
self._remove_manifest(batch_dir)
|
||||
self._remove_empty_dir(batch_dir)
|
||||
self._remove_empty_staging_parent(batch_dir)
|
||||
self._remove_merged_batch_dirs(manifest)
|
||||
return True
|
||||
|
||||
@@ -1132,6 +1166,18 @@ class PendingDeleteService:
|
||||
except OSError as exc:
|
||||
logger.debug("Directory %s not empty or missing: %s", directory, exc)
|
||||
|
||||
def _remove_empty_staging_parent(self, batch_dir: str) -> None:
|
||||
"""Drop the staging container once its last batch is gone.
|
||||
|
||||
Only an empty ``.lm-pending-delete`` directory is removed (``rmdir``
|
||||
refuses anything else), and only when the path really is one, so a model
|
||||
folder is never at risk.
|
||||
"""
|
||||
parent = os.path.dirname(batch_dir)
|
||||
if os.path.basename(parent) != PENDING_DELETE_DIR_NAME:
|
||||
return
|
||||
self._remove_empty_dir(parent)
|
||||
|
||||
def _new_batch_id(self) -> str:
|
||||
return uuid.uuid4().hex
|
||||
|
||||
|
||||
Reference in New Issue
Block a user