fix: Windows git clone failures — URL reinstall + pipe deadlock + file lock (#2726)

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.
This commit is contained in:
Dr.Lt.Data
2026-03-22 20:21:03 +09:00
committed by GitHub
parent 099aed1ad4
commit cd805b3202
5 changed files with 93 additions and 20 deletions

View File

@ -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=''):

View File

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

View File

@ -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):

View File

@ -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):

View File

@ -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"