From cd805b3202ef08bed9b7f206f355991784df3733 Mon Sep 17 00:00:00 2001 From: "Dr.Lt.Data" <128333288+ltdrdata@users.noreply.github.com> Date: Sun, 22 Mar 2026 20:21:03 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20Windows=20git=20clone=20failures=20?= =?UTF-8?q?=E2=80=94=20URL=20reinstall=20+=20pipe=20deadlock=20+=20file=20?= =?UTF-8?q?lock=20(#2726)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes for Windows E2E failures: 1. cm_cli reinstall_node(): URL-based node specs used the full URL as lookup key, but internal dicts are keyed by repo basename or cnr_id. Use get_cnr_by_repo() for CNR-aware lookup with correct is_unknown flag. 2. git_helper.py gitclone(): disable tqdm progress when stderr is piped (sys.stderr.isatty() gate) to prevent pipe buffer deadlock. Also move stale directories from previous failed clones into .disabled/.trash/ before cloning (GitPython handle leak on Windows). 3. try_rmtree(): 3-tier deletion strategy for Windows file locks: retry 3x with delay, rename into .disabled/.trash/, then lazy-delete via reserve_script as final fallback. --- cm_cli/__main__.py | 29 ++++++++++++++++-------- comfyui_manager/common/git_helper.py | 20 +++++++++++++++-- comfyui_manager/glob/manager_core.py | 31 ++++++++++++++++++++++---- comfyui_manager/legacy/manager_core.py | 31 ++++++++++++++++++++++---- pyproject.toml | 2 +- 5 files changed, 93 insertions(+), 20 deletions(-) diff --git a/cm_cli/__main__.py b/cm_cli/__main__.py index b302872c..554a7e03 100644 --- a/cm_cli/__main__.py +++ b/cm_cli/__main__.py @@ -238,18 +238,29 @@ def install_node(node_spec_str, is_all=False, cnt_msg='', **kwargs): def reinstall_node(node_spec_str, is_all=False, cnt_msg=''): - node_spec = unified_manager.resolve_node_spec(node_spec_str) + if core.is_valid_url(node_spec_str): + # URL-based: resolve_node_spec returns the full URL as node_name, + # but internal dicts are keyed by repo basename or cnr_id. + url = node_spec_str.rstrip('/') + cnr = unified_manager.get_cnr_by_repo(url) + if cnr: + node_id = cnr['id'] + unified_manager.unified_uninstall(node_id, False) + unified_manager.purge_node_state(node_id) + else: + repo_name = os.path.splitext(os.path.basename(url))[0] + unified_manager.unified_uninstall(repo_name, True) + unified_manager.purge_node_state(repo_name) - node_name, version_spec, _ = node_spec + install_node(node_spec_str, is_all=is_all, cnt_msg=cnt_msg, raise_on_fail=True) + else: + node_spec = unified_manager.resolve_node_spec(node_spec_str) + node_name, version_spec, _ = node_spec - # Best-effort uninstall via normal path - unified_manager.unified_uninstall(node_name, version_spec == 'unknown') + unified_manager.unified_uninstall(node_name, version_spec == 'unknown') + unified_manager.purge_node_state(node_name) - # Fallback: purge all state and directories regardless of categorization - # Handles categorization mismatch between cm_cli invocations (e.g. unknown→nightly) - unified_manager.purge_node_state(node_name) - - install_node(node_name, is_all=is_all, cnt_msg=cnt_msg, raise_on_fail=True) + install_node(node_name, is_all=is_all, cnt_msg=cnt_msg, raise_on_fail=True) def fix_node(node_spec_str, is_all=False, cnt_msg=''): diff --git a/comfyui_manager/common/git_helper.py b/comfyui_manager/common/git_helper.py index c8ae6cb2..11605b74 100644 --- a/comfyui_manager/common/git_helper.py +++ b/comfyui_manager/common/git_helper.py @@ -100,8 +100,24 @@ def gitclone(custom_nodes_path, url, target_hash=None, repo_path=None): if repo_path is None: repo_path = os.path.join(custom_nodes_path, repo_name) - # Clone the repository from the remote URL - repo = git.Repo.clone_from(url, repo_path, recursive=True, progress=GitProgress()) + # On Windows, previous failed clones may leave directories with locked + # .git/objects/pack/* files (GitPython memory-mapped handle leak). + # Rename stale directory out of the way so clone can proceed. + if os.path.exists(repo_path): + import shutil + import uuid as _uuid + trash_dir = os.path.join(custom_nodes_path, '.disabled', '.trash') + os.makedirs(trash_dir, exist_ok=True) + trash = os.path.join(trash_dir, repo_name + f'_{_uuid.uuid4().hex[:8]}') + try: + os.rename(repo_path, trash) + shutil.rmtree(trash, ignore_errors=True) + except OSError: + shutil.rmtree(repo_path, ignore_errors=True) + + # Disable tqdm progress when stderr is piped to avoid deadlock on Windows. + progress = GitProgress() if sys.stderr.isatty() else None + repo = git.Repo.clone_from(url, repo_path, recursive=True, progress=progress) if target_hash is not None: print(f"CHECKOUT: {repo_name} [{target_hash}]") diff --git a/comfyui_manager/glob/manager_core.py b/comfyui_manager/glob/manager_core.py index 8b166cb3..3c9eb23f 100644 --- a/comfyui_manager/glob/manager_core.py +++ b/comfyui_manager/glob/manager_core.py @@ -1854,11 +1854,34 @@ def reserve_script(repo_path, install_cmds): def try_rmtree(title, fullpath): + # Tier 1: retry with delay for transient Windows file locks + for attempt in range(3): + try: + shutil.rmtree(fullpath) + return + except OSError: + if attempt < 2: + time.sleep(1) + + # Tier 2: rename into .disabled/.trash/ so scanner ignores it + trash_dir = os.path.join(os.path.dirname(fullpath), '.disabled', '.trash') + os.makedirs(trash_dir, exist_ok=True) + trash = os.path.join(trash_dir, os.path.basename(fullpath) + f'_{uuid.uuid4().hex[:8]}') try: - shutil.rmtree(fullpath) - except Exception as e: - logging.warning(f"[ComfyUI-Manager] An error occurred while deleting '{fullpath}', so it has been scheduled for deletion upon restart.\nEXCEPTION: {e}") - reserve_script(title, ["#LAZY-DELETE-NODEPACK", fullpath]) + os.rename(fullpath, trash) + shutil.rmtree(trash, ignore_errors=True) + if not os.path.exists(trash): + return + # Rename succeeded but delete failed — schedule trash path for lazy delete + logging.warning(f"[ComfyUI-Manager] Renamed '{fullpath}' to '{trash}' but could not delete; scheduled for restart.") + reserve_script(title, ["#LAZY-DELETE-NODEPACK", trash]) + return + except OSError: + pass + + # Tier 3: lazy delete on restart (ComfyUI GUI fallback) + logging.warning(f"[ComfyUI-Manager] An error occurred while deleting '{fullpath}', so it has been scheduled for deletion upon restart.") + reserve_script(title, ["#LAZY-DELETE-NODEPACK", fullpath]) def try_install_script(url, repo_path, install_cmd, instant_execution=False): diff --git a/comfyui_manager/legacy/manager_core.py b/comfyui_manager/legacy/manager_core.py index de6b29f3..5d522149 100644 --- a/comfyui_manager/legacy/manager_core.py +++ b/comfyui_manager/legacy/manager_core.py @@ -1833,11 +1833,34 @@ def reserve_script(repo_path, install_cmds): def try_rmtree(title, fullpath): + # Tier 1: retry with delay for transient Windows file locks + for attempt in range(3): + try: + shutil.rmtree(fullpath) + return + except OSError: + if attempt < 2: + time.sleep(1) + + # Tier 2: rename into .disabled/.trash/ so scanner ignores it + trash_dir = os.path.join(os.path.dirname(fullpath), '.disabled', '.trash') + os.makedirs(trash_dir, exist_ok=True) + trash = os.path.join(trash_dir, os.path.basename(fullpath) + f'_{uuid.uuid4().hex[:8]}') try: - shutil.rmtree(fullpath) - except Exception as e: - logging.warning(f"[ComfyUI-Manager] An error occurred while deleting '{fullpath}', so it has been scheduled for deletion upon restart.\nEXCEPTION: {e}") - reserve_script(title, ["#LAZY-DELETE-NODEPACK", fullpath]) + os.rename(fullpath, trash) + shutil.rmtree(trash, ignore_errors=True) + if not os.path.exists(trash): + return + # Rename succeeded but delete failed — schedule trash path for lazy delete + logging.warning(f"[ComfyUI-Manager] Renamed '{fullpath}' to '{trash}' but could not delete; scheduled for restart.") + reserve_script(title, ["#LAZY-DELETE-NODEPACK", trash]) + return + except OSError: + pass + + # Tier 3: lazy delete on restart (ComfyUI GUI fallback) + logging.warning(f"[ComfyUI-Manager] An error occurred while deleting '{fullpath}', so it has been scheduled for deletion upon restart.") + reserve_script(title, ["#LAZY-DELETE-NODEPACK", fullpath]) def try_install_script(url, repo_path, install_cmd, instant_execution=False): diff --git a/pyproject.toml b/pyproject.toml index ce654979..600ebc69 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ build-backend = "setuptools.build_meta" [project] name = "comfyui-manager" license = { text = "GPL-3.0-only" } -version = "4.1b7" +version = "4.1b8" requires-python = ">= 3.9" description = "ComfyUI-Manager provides features to install and manage custom nodes for ComfyUI, as well as various functionalities to assist with ComfyUI." readme = "README.md"