mirror of
https://github.com/Comfy-Org/ComfyUI-Manager.git
synced 2026-03-08 10:37:37 +08:00
* feat(deps): add unified dependency resolver using uv pip compile - Add UnifiedDepResolver module with 7 FRs: collect, compile, install pipeline - Integrate startup batch resolution in prestartup_script.py (module scope) - Skip per-node pip install in execute_install_script() when unified mode active - Add use_unified_resolver config flag following use_uv pattern - Input sanitization: reject -r, -e, --find-links, @ file://, path separators - Handle --index-url/--extra-index-url separation with credential redaction - Fallback to per-node pip on resolver failure or uv unavailability - Add 98 unit tests across 20 test classes - Add PRD and Design docs with cm_global integration marked as DEFERRED * fix(deps): reset use_unified_resolver flag on startup fallback When the unified resolver fails at startup (compile error, install error, uv unavailable, or generic exception), the runtime flag was not being reset to False. This caused subsequent runtime installs to incorrectly defer pip dependencies instead of falling back to per-node pip install. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(deps): add manual test cases for unified dependency resolver Add environment setup guide and 16 test cases covering: - Normal batch resolution (TC-1), disabled state (TC-2) - Fallback paths: uv unavailable (TC-3), compile fail (TC-4), install fail (TC-5), generic exception (TC-16) - install.py preservation (TC-6), runtime defer (TC-13) - Input sanitization: dangerous patterns (TC-7), path separators (TC-8), index-url separation (TC-9), credential redaction (TC-10) - Disabled pack exclusion (TC-11), no-deps path (TC-12) - Both unified resolver guard paths (TC-14), post-fallback (TC-15) Includes API reference, traceability matrix, and out-of-scope items. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deps): prevent read_config() from overriding resolver fallback state read_config() in manager_core.py unconditionally re-read use_unified_resolver from config.ini, undoing the False set by prestartup_script.py on resolver fallback. This caused runtime installs to still defer deps even after a startup batch failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deps): support multiple index URLs per line and optimize downgrade check - Rewrite _split_index_url() to handle multiple --index-url / --extra-index-url options on a single requirements.txt line using regex-based parsing instead of single split. - Cache installed_packages snapshot in collect_requirements() to avoid repeated subprocess calls during downgrade blacklist checks. - Add unit tests for multi-URL lines and bare --index-url edge case. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(deps): add E2E scripts and update test documentation Add automated E2E test scripts for unified dependency resolver: - setup_e2e_env.sh: idempotent environment setup (clone ComfyUI, create venv, install deps, symlink Manager, write config.ini) - start_comfyui.sh: foreground-blocking launcher using tail -f | grep -q readiness detection - stop_comfyui.sh: graceful SIGTERM → SIGKILL shutdown Update test documentation reflecting E2E testing findings: - TEST-environment-setup.md: add automated script usage, document caveats (PYTHONPATH, config.ini path, Manager v4 /v2/ prefix, Blocked by policy, bash ((var++)) trap, git+https:// rejection) - TEST-unified-dep-resolver.md: add TC-17 (restart dependency detection), TC-18 (real node pack integration), Validated Behaviors section, normalize API port to 8199 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deps): harden input sanitization, expand test coverage, bump version Security: - Add _INLINE_DANGEROUS_OPTIONS regex to catch pip options after package names (--find-links, --constraint, --requirement, --editable, --trusted-host, --global-option, --install-option and short forms) - Stage index URLs in pending_urls, commit only after full line validation to prevent URL injection from rejected lines Tests: - Add 50 new tests: inline sanitization, false-positive guards, parse helpers (_parse_conflicts, _parse_install_output), exception paths (91 → 141 total, all pass) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(cli): add uv-compile command and --uv-compile flag for batch dependency resolution Add two CLI entry points for the unified dependency resolver: - `cm_cli uv-compile`: standalone batch resolution of all installed node pack dependencies via uv pip compile - `cm_cli install --uv-compile`: skip per-node pip, batch-resolve all deps after install completes (mutually exclusive with --no-deps) Both use a shared `_run_unified_resolve()` helper that passes real cm_global values (pip_blacklist, pip_overrides, pip_downgrade_blacklist) and guarantees PIPFixer.fix_broken() runs via try/finally. Update DESIGN, PRD, and TEST docs for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
778 lines
33 KiB
Markdown
778 lines
33 KiB
Markdown
# Architecture Design: Unified Dependency Resolver
|
|
|
|
## 1. System Architecture
|
|
|
|
### 1.1 Module Location
|
|
|
|
```
|
|
comfyui_manager/
|
|
├── glob/
|
|
│ └── manager_core.py # Existing: execute_install_script() call sites (2 locations)
|
|
├── common/
|
|
│ ├── manager_util.py # Existing: get_pip_cmd(), PIPFixer, use_uv flag
|
|
│ ├── cm_global.py # Existing: pip_overrides, pip_blacklist (runtime dynamic assignment)
|
|
│ └── unified_dep_resolver.py # New: Unified dependency resolution module
|
|
├── prestartup_script.py # Existing: config reading, remap_pip_package, cm_global initialization
|
|
└── legacy/
|
|
└── manager_core.py # Legacy (not a modification target)
|
|
cm_cli/
|
|
└── __main__.py # CLI entry: uv-compile command (on-demand batch resolution)
|
|
```
|
|
|
|
The new module `unified_dep_resolver.py` is added to the `comfyui_manager/common/` directory.
|
|
It reuses `manager_util` utilities and `cm_global` global state from the same package.
|
|
|
|
> **Warning**: `cm_global.pip_overrides`, `pip_blacklist`, `pip_downgrade_blacklist` are
|
|
> NOT defined in `cm_global.py`. They are **dynamically assigned** during `prestartup_script.py` execution.
|
|
> In v1 unified mode, these are **not applied** — empty values are passed to the resolver constructor.
|
|
> The constructor interface accepts them for future extensibility (defaults to empty when `None`).
|
|
>
|
|
> **[DEFERRED]** Reading actual `cm_global` values at startup is deferred to a future version.
|
|
> The startup batch resolver in `prestartup_script.py` currently passes `blacklist=set()`,
|
|
> `overrides={}`, `downgrade_blacklist=[]`. The constructor and internal methods
|
|
> (`_remap_package`, `_is_downgrade_blacklisted`, blacklist check) are fully implemented
|
|
> and will work once real values are provided.
|
|
|
|
### 1.2 Overall Flow
|
|
|
|
```mermaid
|
|
flowchart TD
|
|
subgraph INSTALL_TIME["Install Time (immediate)"]
|
|
MC["manager_core.py<br/>execute_install_script() — 2 locations"]
|
|
MC -->|"use_unified_resolver=True"| SKIP["Skip per-node pip install<br/>(deps deferred to restart)"]
|
|
MC -->|"use_unified_resolver=False"| PIP["Existing pip install loop"]
|
|
SKIP --> INST["Run install.py"]
|
|
PIP --> INST
|
|
end
|
|
|
|
subgraph STARTUP["ComfyUI Restart (prestartup_script.py)"]
|
|
CHK{use_unified_resolver?}
|
|
CHK -->|Yes| UDR
|
|
CHK -->|No| LAZY["execute_lazy_install_script()<br/>per-node pip install (existing)"]
|
|
|
|
subgraph UDR["UnifiedDepResolver (batch)"]
|
|
S1["1. collect_requirements()<br/>(ALL installed node packs)"]
|
|
S2["2. compile_lockfile()"]
|
|
S3["3. install_from_lockfile()"]
|
|
S1 --> S2 --> S3
|
|
end
|
|
|
|
UDR -->|Success| FIX["PIPFixer.fix_broken()"]
|
|
UDR -->|Failure| LAZY
|
|
LAZY --> FIX2["PIPFixer.fix_broken()"]
|
|
end
|
|
```
|
|
|
|
> **Key design change**: The unified resolver runs at **startup time** (module scope), not at install time.
|
|
> At install time, `execute_install_script()` skips the pip loop when unified mode is active.
|
|
> At startup, `prestartup_script.py` runs the resolver at module scope — unconditionally when enabled,
|
|
> independent of `install-scripts.txt` existence. Blacklist/overrides/downgrade_blacklist are bypassed
|
|
> (empty values passed); `uv pip compile` handles all conflict resolution natively.
|
|
>
|
|
> **Note**: `execute_install_script()` exists in 2 locations in the codebase (excluding legacy module).
|
|
> - `UnifiedManager.execute_install_script()` (class method): Used for CNR installs, etc.
|
|
> - Standalone function `execute_install_script()`: Used for updates, git installs, etc.
|
|
> Both skip per-node pip install when unified mode is active.
|
|
|
|
### 1.3 uv Command Strategy
|
|
|
|
**`uv pip compile`** → Generates pinned requirements.txt (pip-compatible)
|
|
- Do not confuse with `uv lock`
|
|
- `uv lock` generates `uv.lock` (TOML) — cross-platform but incompatible with pip workflows
|
|
- This design uses a pip-compatible workflow (`uv pip compile` → `uv pip install -r`)
|
|
|
|
**`uv pip install -r`** ← Used instead of `uv pip sync`
|
|
- `uv pip sync`: **Deletes** packages not in lockfile → Risk of removing torch, ComfyUI deps
|
|
- `uv pip install -r`: Only performs additive installs, preserves existing packages → Safe
|
|
|
|
---
|
|
|
|
## 2. Class Design
|
|
|
|
### 2.1 UnifiedDepResolver
|
|
|
|
```python
|
|
class UnifiedDepResolver:
|
|
"""
|
|
Unified dependency resolver.
|
|
|
|
Resolves and installs all dependencies of (installed node packs + new node packs)
|
|
at once using uv.
|
|
|
|
Responsibility scope: Dependency resolution and installation only.
|
|
install.py execution and PIPFixer calls are handled by the caller (manager_core).
|
|
"""
|
|
|
|
def __init__(
|
|
self,
|
|
node_pack_paths: list[str],
|
|
base_requirements: list[str] | None = None,
|
|
blacklist: set[str] | None = None,
|
|
overrides: dict[str, str] | None = None,
|
|
downgrade_blacklist: list[str] | None = None,
|
|
):
|
|
"""
|
|
Args:
|
|
node_pack_paths: List of node pack directory paths
|
|
base_requirements: Base dependencies (ComfyUI requirements, etc.)
|
|
blacklist: Blacklisted package set (default: empty set; not applied in v1 unified mode)
|
|
overrides: Package name remapping dict (default: empty dict; not applied in v1 unified mode)
|
|
downgrade_blacklist: Downgrade-prohibited package list (default: empty list; not applied in v1 unified mode)
|
|
"""
|
|
|
|
def resolve_and_install(self) -> ResolveResult:
|
|
"""Execute full pipeline: stale cleanup → collect → compile → install.
|
|
Calls cleanup_stale_tmp() at start to clean up residual files from previous abnormal terminations."""
|
|
|
|
def collect_requirements(self) -> CollectedDeps:
|
|
"""Collect dependencies from all node packs"""
|
|
|
|
def compile_lockfile(self, deps: CollectedDeps) -> LockfileResult:
|
|
"""Generate pinned requirements via uv pip compile"""
|
|
|
|
def install_from_lockfile(self, lockfile_path: str) -> InstallResult:
|
|
"""Install from pinned requirements (uv pip install -r)"""
|
|
```
|
|
|
|
### 2.2 Data Classes
|
|
|
|
```python
|
|
@dataclass
|
|
class PackageRequirement:
|
|
"""Individual package dependency"""
|
|
name: str # Package name (normalized)
|
|
spec: str # Original spec (e.g., "torch>=2.0")
|
|
source: str # Source node pack path
|
|
|
|
@dataclass
|
|
class CollectedDeps:
|
|
"""All collected dependencies"""
|
|
requirements: list[PackageRequirement] # Collected deps (duplicates allowed, uv resolves)
|
|
skipped: list[tuple[str, str]] # (package_name, skip_reason)
|
|
sources: dict[str, list[str]] # {package_name: [source_node_packs]}
|
|
extra_index_urls: list[str] # Additional index URLs separated from --index-url entries
|
|
|
|
@dataclass
|
|
class LockfileResult:
|
|
"""Compilation result"""
|
|
success: bool
|
|
lockfile_path: str | None # pinned requirements.txt path
|
|
conflicts: list[str] # Conflict details
|
|
stderr: str # uv error output
|
|
|
|
@dataclass
|
|
class InstallResult:
|
|
"""Installation result (uv pip install -r is atomic: all-or-nothing)"""
|
|
success: bool
|
|
installed: list[str] # Installed packages (stdout parsing)
|
|
skipped: list[str] # Already installed (stdout parsing)
|
|
stderr: str # uv stderr output (for failure analysis)
|
|
|
|
@dataclass
|
|
class ResolveResult:
|
|
"""Full pipeline result"""
|
|
success: bool
|
|
collected: CollectedDeps | None
|
|
lockfile: LockfileResult | None
|
|
install: InstallResult | None
|
|
error: str | None
|
|
```
|
|
|
|
---
|
|
|
|
## 3. Core Logic Details
|
|
|
|
### 3.1 Dependency Collection (`collect_requirements`)
|
|
|
|
```python
|
|
# Input sanitization: dangerous patterns to reject
|
|
_DANGEROUS_PATTERNS = re.compile(
|
|
r'^(-r\b|--requirement\b|-e\b|--editable\b|-c\b|--constraint\b'
|
|
r'|--find-links\b|-f\b|.*@\s*file://)',
|
|
re.IGNORECASE
|
|
)
|
|
|
|
def collect_requirements(self) -> CollectedDeps:
|
|
requirements = []
|
|
skipped = []
|
|
sources = defaultdict(list)
|
|
extra_index_urls = []
|
|
|
|
for path in self.node_pack_paths:
|
|
# Exclude disabled node packs (directory-based mechanism)
|
|
# Disabled node packs are actually moved to custom_nodes/.disabled/,
|
|
# so they should already be excluded from input at this point.
|
|
# Defensive check: new style (.disabled/ directory) + old style ({name}.disabled suffix)
|
|
if ('/.disabled/' in path
|
|
or os.path.basename(os.path.dirname(path)) == '.disabled'
|
|
or path.rstrip('/').endswith('.disabled')):
|
|
continue
|
|
|
|
req_file = os.path.join(path, "requirements.txt")
|
|
if not os.path.exists(req_file):
|
|
continue
|
|
|
|
# chardet-based encoding detection (existing robust_readlines pattern)
|
|
for line in self._read_requirements(req_file):
|
|
line = line.split('#')[0].strip()
|
|
if not line:
|
|
continue
|
|
|
|
# 0. Input sanitization (security)
|
|
if self._DANGEROUS_PATTERNS.match(line):
|
|
skipped.append((line, f"rejected: dangerous pattern in {path}"))
|
|
logging.warning(f"[UnifiedDepResolver] rejected dangerous line: '{line}' from {path}")
|
|
continue
|
|
|
|
# 1. Separate --index-url / --extra-index-url handling
|
|
# (BEFORE path separator check, because URLs contain '/')
|
|
if '--index-url' in line or '--extra-index-url' in line:
|
|
pkg_spec, index_url = self._split_index_url(line)
|
|
if index_url:
|
|
extra_index_urls.append(index_url)
|
|
line = pkg_spec
|
|
if not line:
|
|
# Standalone option line (no package prefix)
|
|
continue
|
|
|
|
# 1b. Reject path separators in package name portion
|
|
pkg_name_part = re.split(r'[><=!~;]', line)[0]
|
|
if '/' in pkg_name_part or '\\' in pkg_name_part:
|
|
skipped.append((line, f"rejected: path separator in package name"))
|
|
continue
|
|
|
|
# 2. Apply remap_pip_package (using cm_global.pip_overrides)
|
|
pkg_spec = self._remap_package(line)
|
|
|
|
# 3. Blacklist check (cm_global.pip_blacklist)
|
|
pkg_name = self._extract_package_name(pkg_spec)
|
|
if pkg_name in self.blacklist:
|
|
skipped.append((pkg_spec, "blacklisted"))
|
|
continue
|
|
|
|
# 4. Downgrade blacklist check (includes version comparison)
|
|
if self._is_downgrade_blacklisted(pkg_name, pkg_spec):
|
|
skipped.append((pkg_spec, "downgrade blacklisted"))
|
|
continue
|
|
|
|
# 5. Collect (no dedup — uv handles resolution)
|
|
req = PackageRequirement(
|
|
name=pkg_name,
|
|
spec=pkg_spec,
|
|
source=path,
|
|
)
|
|
requirements.append(req)
|
|
sources[pkg_name].append(path)
|
|
|
|
return CollectedDeps(
|
|
requirements=requirements,
|
|
skipped=skipped,
|
|
sources=dict(sources),
|
|
extra_index_urls=list(set(extra_index_urls)), # Deduplicate
|
|
)
|
|
|
|
def _split_index_url(self, line: str) -> tuple[str, str | None]:
|
|
"""Split 'package_name --index-url URL' → (package_name, URL).
|
|
|
|
Also handles standalone ``--index-url URL`` and
|
|
``--extra-index-url URL`` lines (with no package prefix).
|
|
"""
|
|
# Handle --extra-index-url first (contains '-index-url' as substring
|
|
# but NOT '--index-url' due to the extra-index prefix)
|
|
for option in ('--extra-index-url', '--index-url'):
|
|
if option in line:
|
|
parts = line.split(option, 1)
|
|
pkg_spec = parts[0].strip()
|
|
url = parts[1].strip() if len(parts) == 2 else None
|
|
return pkg_spec, url
|
|
return line, None
|
|
|
|
def _is_downgrade_blacklisted(self, pkg_name: str, pkg_spec: str) -> bool:
|
|
"""Reproduce the downgrade version comparison from existing is_blacklisted() logic.
|
|
|
|
Same logic as manager_core.py's is_blacklisted():
|
|
- No version spec and already installed → block (prevent reinstall)
|
|
- Operator is one of ['<=', '==', '<', '~='] and
|
|
installed version >= requested version → block (prevent downgrade)
|
|
- Version comparison uses manager_util.StrictVersion (NOT packaging.version)
|
|
"""
|
|
if pkg_name not in self.downgrade_blacklist:
|
|
return False
|
|
|
|
installed_packages = manager_util.get_installed_packages()
|
|
|
|
# Version spec parsing (same pattern as existing is_blacklisted())
|
|
pattern = r'([^<>!~=]+)([<>!~=]=?)([^ ]*)'
|
|
match = re.search(pattern, pkg_spec)
|
|
|
|
if match is None:
|
|
# No version spec: prevent reinstall if already installed
|
|
if pkg_name in installed_packages:
|
|
return True
|
|
elif match.group(2) in ['<=', '==', '<', '~=']:
|
|
# Downgrade operator: block if installed version >= requested version
|
|
if pkg_name in installed_packages:
|
|
try:
|
|
installed_ver = manager_util.StrictVersion(installed_packages[pkg_name])
|
|
requested_ver = manager_util.StrictVersion(match.group(3))
|
|
if installed_ver >= requested_ver:
|
|
return True
|
|
except (ValueError, TypeError):
|
|
logging.warning(f"[UnifiedDepResolver] version parse failed: {pkg_spec}")
|
|
return False
|
|
|
|
return False
|
|
|
|
def _remap_package(self, pkg: str) -> str:
|
|
"""Package name remapping based on cm_global.pip_overrides.
|
|
Reuses existing remap_pip_package() logic."""
|
|
if pkg in self.overrides:
|
|
remapped = self.overrides[pkg]
|
|
logging.info(f"[UnifiedDepResolver] '{pkg}' remapped to '{remapped}'")
|
|
return remapped
|
|
return pkg
|
|
```
|
|
|
|
### 3.2 Lockfile Generation (`compile_lockfile`)
|
|
|
|
**Behavior:**
|
|
1. Create a unique temp directory (`tempfile.mkdtemp(prefix="comfyui_resolver_")`) for concurrency safety
|
|
2. Write collected requirements and base constraints to temp files
|
|
3. Execute `uv pip compile` with options:
|
|
- `--output-file` (pinned requirements path within temp dir)
|
|
- `--python` (current interpreter)
|
|
- `--constraint` (base dependencies)
|
|
- `--extra-index-url` (from `CollectedDeps.extra_index_urls`, logged via `_redact_url()`)
|
|
4. Timeout: 300s — returns `LockfileResult(success=False)` on `TimeoutExpired`
|
|
5. On `returncode != 0`: parse stderr for conflict details via `_parse_conflicts()`
|
|
6. Post-success verification: confirm lockfile was actually created (handles edge case of `returncode==0` without output)
|
|
7. Temp directory cleanup: `shutil.rmtree()` in `except` block; on success, caller (`resolve_and_install`'s `finally`) handles cleanup
|
|
|
|
### 3.3 Dependency Installation (`install_from_lockfile`)
|
|
|
|
**Behavior:**
|
|
1. Execute `uv pip install --requirement <lockfile_path> --python <sys.executable>`
|
|
- **NOT `uv pip sync`** — sync deletes packages not in lockfile (dangerous for torch, ComfyUI deps)
|
|
2. `uv pip install -r` is **atomic** (all-or-nothing): no partial failure
|
|
3. Timeout: 600s — returns `InstallResult(success=False)` on `TimeoutExpired`
|
|
4. On success: parse stdout via `_parse_install_output()` to populate `installed`/`skipped` lists
|
|
5. On failure: `stderr` captures the failure cause; `installed=[]` (atomic model)
|
|
|
|
### 3.4 uv Command Resolution
|
|
|
|
**`_get_uv_cmd()` resolution order** (mirrors existing `get_pip_cmd()` pattern):
|
|
1. **Module uv**: `[sys.executable, '-m', 'uv']` (with `-s` flag for embedded Python — note: `python_embeded` spelling is intentional, matching ComfyUI Windows distribution path)
|
|
2. **Standalone uv**: `['uv']` via `shutil.which('uv')`
|
|
3. **Not found**: raises `UvNotAvailableError` → caught by caller for pip fallback
|
|
|
|
### 3.5 Stale Temp File Cleanup
|
|
|
|
**`cleanup_stale_tmp(max_age_seconds=3600)`** — classmethod, called at start of `resolve_and_install()`:
|
|
- Scans `tempfile.gettempdir()` for directories with prefix `comfyui_resolver_`
|
|
- Deletes directories older than `max_age_seconds` (default: 1 hour)
|
|
- Silently ignores `OSError` (permission issues, etc.)
|
|
|
|
### 3.6 Credential Redaction
|
|
|
|
```python
|
|
_CREDENTIAL_PATTERN = re.compile(r'://([^@]+)@')
|
|
|
|
def _redact_url(self, url: str) -> str:
|
|
"""Mask authentication info in URLs. user:pass@host → ****@host"""
|
|
return self._CREDENTIAL_PATTERN.sub('://****@', url)
|
|
```
|
|
|
|
All `--extra-index-url` logging passes through `_redact_url()`:
|
|
```python
|
|
# Logging example within compile_lockfile()
|
|
for url in deps.extra_index_urls:
|
|
logging.info(f"[UnifiedDepResolver] extra-index-url: {self._redact_url(url)}")
|
|
cmd += ["--extra-index-url", url] # Original URL passed to actual command
|
|
```
|
|
|
|
---
|
|
|
|
## 4. Existing Code Integration
|
|
|
|
### 4.1 manager_core.py Modification Points
|
|
|
|
**2 `execute_install_script()` locations — both skip deps in unified mode:**
|
|
|
|
#### 4.1.1 UnifiedManager.execute_install_script() (Class Method)
|
|
|
|
#### 4.1.2 Standalone Function execute_install_script()
|
|
|
|
**Both locations use the same pattern when unified mode is active:**
|
|
|
|
1. `lazy_mode=True` → schedule and return early (unchanged)
|
|
2. If `not no_deps and manager_util.use_unified_resolver`:
|
|
- **Skip** the `requirements.txt` pip install loop entirely (deps deferred to startup)
|
|
- Log: `"[UnifiedDepResolver] deps deferred to startup batch resolution"`
|
|
3. If `not manager_util.use_unified_resolver`: existing pip install loop runs (unchanged)
|
|
4. `install.py` execution: **always runs immediately** regardless of resolver mode
|
|
|
|
> **Parameter ordering differs:**
|
|
> - Method: `(self, url, repo_path, instant_execution, lazy_mode, no_deps)`
|
|
> - Standalone: `(url, repo_path, lazy_mode, instant_execution, no_deps)`
|
|
|
|
### 4.1.3 Startup Batch Resolver (`prestartup_script.py`)
|
|
|
|
**New**: Runs unified resolver at **module scope** — unconditionally when enabled, independent of `install-scripts.txt` existence.
|
|
|
|
**Execution point**: After config reading and `cm_global` initialization, **before** the `execute_startup_script()` gate.
|
|
|
|
**Logic** (uses module-level helpers from `unified_dep_resolver.py`):
|
|
1. `collect_node_pack_paths(folder_paths.get_folder_paths('custom_nodes'))` — enumerate all installed node pack directories
|
|
2. `collect_base_requirements(comfy_path)` — read `requirements.txt` + `manager_requirements.txt` from ComfyUI root (base deps only)
|
|
3. Create `UnifiedDepResolver` with **empty** blacklist/overrides/downgrade_blacklist (uv handles resolution natively; interface preserved for extensibility)
|
|
4. Call `resolve_and_install()` → on success set `_unified_resolver_succeeded = True`
|
|
5. On failure (including `UvNotAvailableError`): log warning, fall back to per-node pip
|
|
|
|
> `manager_requirements.txt` is read **only** from `comfy_path` (ComfyUI base), never from node packs.
|
|
> Node packs' `requirements.txt` are collected by the resolver's `collect_requirements()` method.
|
|
|
|
### 4.1.5 `execute_lazy_install_script()` Modification
|
|
|
|
When unified resolver **succeeds**, `execute_lazy_install_script()` skips the per-node pip install loop
|
|
(deps already batch-resolved at module scope). `install.py` still runs per node pack.
|
|
|
|
```python
|
|
# In execute_lazy_install_script():
|
|
if os.path.exists(requirements_path) and not _unified_resolver_succeeded:
|
|
# Per-node pip install: only runs if unified resolver is disabled or failed
|
|
...
|
|
# install.py always runs regardless
|
|
```
|
|
|
|
> **Note**: Gated on `_unified_resolver_succeeded` (success flag), NOT `use_unified_resolver` (enable flag).
|
|
> If the resolver is enabled but fails, `_unified_resolver_succeeded` remains False → per-node pip runs as fallback.
|
|
|
|
### 4.1.6 CLI Integration
|
|
|
|
Two entry points expose the unified resolver in `cm_cli`:
|
|
|
|
#### 4.1.6.1 Standalone Command: `cm_cli uv-compile`
|
|
|
|
On-demand batch resolution — independent of ComfyUI startup.
|
|
|
|
```bash
|
|
cm_cli uv-compile [--user-directory DIR]
|
|
```
|
|
|
|
Resolves all installed node packs' dependencies at once. Useful for environment
|
|
recovery or initial setup without starting ComfyUI.
|
|
`PIPFixer.fix_broken()` runs after resolution (via `finally` — runs on both success and failure).
|
|
|
|
#### 4.1.6.2 Install Flag: `cm_cli install --uv-compile`
|
|
|
|
```bash
|
|
cm_cli install <node1> [node2 ...] --uv-compile [--mode remote]
|
|
```
|
|
|
|
When `--uv-compile` is set:
|
|
1. `no_deps` is forced to `True` → per-node pip install is skipped during each node installation
|
|
2. After **all** nodes are installed, runs unified batch resolution over **all installed node packs**
|
|
(not just the newly installed ones — `uv pip compile` needs the complete dependency graph)
|
|
3. `PIPFixer.fix_broken()` runs after resolution (via `finally` — runs on both success and failure)
|
|
|
|
This differs from per-node pip install: instead of resolving each node pack's
|
|
`requirements.txt` independently, all deps are compiled together to avoid conflicts.
|
|
|
|
#### Shared Design Decisions
|
|
|
|
- **Uses real `cm_global` values**: Unlike the startup path (4.1.3) which passes empty
|
|
blacklist/overrides, CLI commands pass `cm_global.pip_blacklist`,
|
|
`cm_global.pip_overrides`, and `cm_global.pip_downgrade_blacklist` — already
|
|
initialized at `cm_cli/__main__.py` module scope (lines 45-60).
|
|
- **No `_unified_resolver_succeeded` flag**: Not needed — these are one-shot commands,
|
|
not startup gates.
|
|
- **Shared helper**: Both entry points delegate to `_run_unified_resolve()` which
|
|
handles resolver instantiation, execution, and result reporting.
|
|
- **Error handling**: `UvNotAvailableError` / `ImportError` → exit 1 with message.
|
|
Both entry points use `try/finally` to guarantee `PIPFixer.fix_broken()` runs
|
|
regardless of resolution outcome.
|
|
|
|
**Node pack discovery**: Uses `cmd_ctx.get_custom_nodes_paths()` → `collect_node_pack_paths()`,
|
|
which is the CLI-native path resolution (respects `--user-directory` and `folder_paths`).
|
|
|
|
### 4.2 Configuration Addition (config.ini)
|
|
|
|
```ini
|
|
[default]
|
|
# Existing settings...
|
|
use_unified_resolver = false # Enable unified dependency resolution
|
|
```
|
|
|
|
### 4.3 Configuration Reading
|
|
|
|
Follows the existing `read_uv_mode()` / `use_uv` pattern:
|
|
- `prestartup_script.py`: `read_unified_resolver_mode()` reads from `default_conf` → sets `manager_util.use_unified_resolver`
|
|
- `manager_core.py`: `read_config()` / `write_config()` / `get_config()` include `use_unified_resolver` key
|
|
- `read_config()` exception fallback must include `use_unified_resolver` key to prevent `KeyError` in `write_config()`
|
|
|
|
### 4.4 manager_util.py Extension
|
|
|
|
```python
|
|
# manager_util.py
|
|
use_unified_resolver = False # New global flag (separate from use_uv)
|
|
```
|
|
|
|
---
|
|
|
|
## 5. Error Handling Strategy
|
|
|
|
```mermaid
|
|
flowchart TD
|
|
STARTUP["prestartup_script.py startup"]
|
|
STARTUP --> CHK{use_unified_resolver?}
|
|
CHK -->|No| SKIP_UDR["Skip → execute_lazy_install_script per-node pip"]
|
|
|
|
CHK -->|Yes| RAI["run_unified_resolver()"]
|
|
RAI --> STALE["cleanup_stale_tmp()<br/>Clean stale temp dirs (>1 hour old)"]
|
|
STALE --> UV_CHK{uv installed?}
|
|
UV_CHK -->|No| UV_ERR["UvNotAvailableError<br/>→ Fallback: execute_lazy_install_script per-node pip"]
|
|
|
|
UV_CHK -->|Yes| CR["collect_requirements()<br/>(ALL installed node packs)"]
|
|
CR --> CR_DIS[".disabled/ path → auto-skip"]
|
|
CR --> CR_PARSE["Parse failure → skip node pack, continue"]
|
|
CR --> CR_ENC["Encoding detection failure → assume UTF-8"]
|
|
CR --> CR_DANGER["Dangerous pattern detected → reject line + log"]
|
|
CR --> CR_DG["Downgrade blacklist → skip after version comparison"]
|
|
|
|
CR --> CL["compile_lockfile()"]
|
|
CL --> CL_CONFLICT["Conflict → report + per-node pip fallback"]
|
|
CL --> CL_TIMEOUT["TimeoutExpired 300s → per-node pip fallback"]
|
|
CL --> CL_NOFILE["Lockfile not created → failure + fallback"]
|
|
CL --> CL_TMP["Temp directory → finally block cleanup"]
|
|
|
|
CL -->|Success| IL["install_from_lockfile()"]
|
|
IL --> IL_OK["Total success → parse installed/skipped"]
|
|
IL --> IL_FAIL["Total failure → stderr + per-node pip fallback (atomic)"]
|
|
IL --> IL_TIMEOUT["TimeoutExpired 600s → fallback"]
|
|
|
|
IL_OK --> PF["PIPFixer.fix_broken()<br/>Restore torch/opencv/frontend"]
|
|
PF --> LAZY["execute_lazy_install_script()<br/>(install.py only, deps skipped)"]
|
|
```
|
|
|
|
> **Fallback model**: On resolver failure at startup, `execute_lazy_install_script()` runs normally
|
|
> (per-node pip install), providing the same behavior as if unified mode were disabled.
|
|
|
|
---
|
|
|
|
## 6. File Structure
|
|
|
|
### 6.1 New Files
|
|
|
|
```
|
|
comfyui_manager/common/unified_dep_resolver.py # Main module (~350 lines, includes sanitization/downgrade logic)
|
|
tests/test_unified_dep_resolver.py # Unit tests
|
|
```
|
|
|
|
### 6.2 Modified Files
|
|
|
|
```
|
|
comfyui_manager/glob/manager_core.py # Skip per-node pip in unified mode (2 execute_install_script locations)
|
|
comfyui_manager/common/manager_util.py # Add use_unified_resolver flag
|
|
comfyui_manager/prestartup_script.py # Config reading + startup batch resolver + execute_lazy_install_script modification
|
|
```
|
|
|
|
> **Not modified**: `comfyui_manager/legacy/manager_core.py` (legacy paths retain existing pip behavior)
|
|
|
|
---
|
|
|
|
## 7. Dependencies
|
|
|
|
| Dependency | Purpose | Notes |
|
|
|-----------|---------|-------|
|
|
| `uv` | Dependency resolution and installation | Already included in project dependencies |
|
|
| `cm_global` | pip_overrides, pip_blacklist, pip_downgrade_blacklist | Reuse existing global state (runtime dynamic assignment) |
|
|
| `manager_util` | StrictVersion, get_installed_packages, use_unified_resolver flag | Reuse existing utilities |
|
|
| `tempfile` | Temporary requirements files, mkdtemp | Standard library |
|
|
| `subprocess` | uv process execution | Standard library |
|
|
| `dataclasses` | Result data structures | Standard library |
|
|
| `re` | Input sanitization, version spec parsing, credential redaction | Standard library |
|
|
| `shutil` | uv lookup (`which`), temp directory cleanup | Standard library |
|
|
| `time` | Stale temp file age calculation | Standard library |
|
|
| `logging` | Per-step logging | Standard library |
|
|
|
|
No additional external dependencies.
|
|
|
|
---
|
|
|
|
## 8. Sequence Diagram
|
|
|
|
### Install Time + Startup Batch Resolution
|
|
|
|
```mermaid
|
|
sequenceDiagram
|
|
actor User
|
|
participant MC as manager_core
|
|
participant PS as prestartup_script
|
|
participant UDR as UnifiedDepResolver
|
|
participant UV as uv (CLI)
|
|
|
|
Note over User,MC: Install Time (immediate)
|
|
User->>MC: Install node pack X
|
|
MC->>MC: Git clone / download X
|
|
MC->>MC: Skip per-node pip (unified mode)
|
|
MC->>MC: Run X's install.py
|
|
MC-->>User: Node pack installed (deps pending)
|
|
|
|
Note over User,UV: ComfyUI Restart
|
|
User->>PS: Start ComfyUI
|
|
PS->>PS: Check use_unified_resolver
|
|
PS->>UDR: Create resolver (module scope)
|
|
UDR->>UDR: collect_requirements()<br/>(ALL installed node packs)
|
|
UDR->>UV: uv pip compile --output-file
|
|
UV-->>UDR: pinned reqs.txt
|
|
UDR->>UV: uv pip install -r
|
|
UV-->>UDR: Install result
|
|
UDR-->>PS: ResolveResult(success=True)
|
|
PS->>PS: PIPFixer.fix_broken()
|
|
PS->>PS: execute_lazy_install_script()<br/>(install.py only, deps skipped)
|
|
PS-->>User: ComfyUI ready
|
|
```
|
|
|
|
---
|
|
|
|
## 9. Test Strategy
|
|
|
|
### 9.1 Unit Tests
|
|
|
|
| Test Target | Cases |
|
|
|------------|-------|
|
|
| `collect_requirements` | Normal parsing, empty file, blacklist filtering, comment handling, remap application |
|
|
| `.disabled` filtering | Exclude node packs within `.disabled/` directory path (directory-based mechanism) |
|
|
| Input sanitization | Reject lines with `-r`, `-e`, `--find-links`, `@ file://`, path separators |
|
|
| `--index-url` / `--extra-index-url` separation | `package --index-url URL`, standalone `--index-url URL`, standalone `--extra-index-url URL`, `package --extra-index-url URL` → package spec + extra_index_urls separation |
|
|
| Downgrade blacklist | Installed + lower version request → skip, not installed → pass, same/higher version → pass |
|
|
| `compile_lockfile` | Normal compilation, conflict detection, TimeoutExpired, constraint application, --output-file verification |
|
|
| Lockfile existence verification | Failure handling when file not created despite returncode==0 |
|
|
| `extra_index_urls` passthrough | Verify `--extra-index-url` argument included in compile command |
|
|
| `install_from_lockfile` | Normal install, total failure, TimeoutExpired |
|
|
| Atomic model | On failure: installed=[], stderr populated |
|
|
| `_get_uv_cmd` | Module uv, standalone uv, embedded python (`python_embeded`), not installed |
|
|
| `_remap_package` | pip_overrides remapping, unregistered packages |
|
|
| Blacklist | torch family, torchsde, custom blacklist |
|
|
| Duplicate handling | Same package with multiple specs → all passed to uv |
|
|
| Multiple paths | Collection from multiple custom_nodes paths |
|
|
| `cm_global` defense | Default values used when `pip_blacklist` etc. not assigned |
|
|
| Concurrency | Two resolver instances each use unique temp directories |
|
|
| Credential redaction | `user:pass@host` URL masked in log output |
|
|
| `_redact_url` | `://user:pass@host` → `://****@host` conversion, no-credential URL passthrough |
|
|
| `cleanup_stale_tmp` | Delete stale dirs >1 hour, preserve recent dirs, ignore permission errors |
|
|
| Downgrade operators | `<=`, `==`, `<`, `~=` blocked; `>=`, `>`, `!=` pass; no spec + installed → blocked |
|
|
| `StrictVersion` comparison | Verify `manager_util.StrictVersion` is used (not `packaging.version`) |
|
|
|
|
### 9.2 Integration Tests
|
|
|
|
- End-to-end test in real uv environment
|
|
- Existing pip fallback path test
|
|
- config.ini setting toggle test
|
|
- Environment integrity verification after PIPFixer call
|
|
- lazy_mode scheduling behavior verification (Windows simulation)
|
|
- `use_uv=False` + `use_unified_resolver=True` combination test
|
|
- Large-scale dependency (50+ node packs) performance test
|
|
|
|
---
|
|
|
|
## 10. Implementation Order
|
|
|
|
1. **Phase 1**: Data classes and `collect_requirements` implementation + tests
|
|
- PackageRequirement, CollectedDeps (including extra_index_urls) and other data classes
|
|
- Blacklist/override filtering
|
|
- **Downgrade blacklist** (version comparison logic included)
|
|
- **Input sanitization** (-r, -e, @ file:// etc. rejection)
|
|
- **`--index-url` / `--extra-index-url` separation handling** (package spec + extra_index_urls)
|
|
- **`.disabled` node pack filtering**
|
|
- Defensive cm_global access (getattr pattern)
|
|
2. **Phase 2**: `compile_lockfile` implementation + tests
|
|
- uv pip compile invocation
|
|
- --output-file, --constraint, --python options
|
|
- Conflict parsing logic
|
|
3. **Phase 3**: `install_from_lockfile` implementation + tests
|
|
- uv pip install -r invocation (NOT sync)
|
|
- Install result parsing
|
|
4. **Phase 4**: Integration — startup batch + install-time skip
|
|
- `prestartup_script.py`: Module-scope startup batch resolver + `execute_lazy_install_script()` deps skip
|
|
- `manager_core.py`: Skip per-node pip in 2 `execute_install_script()` locations
|
|
- `manager_util.py`: `use_unified_resolver` flag
|
|
- Config reading (`read_unified_resolver_mode()`, `read_config()`/`write_config()`)
|
|
5. **Phase 5**: Integration tests + fallback verification + startup batch tests
|
|
|
|
---
|
|
|
|
## Appendix A: Existing Code Reference
|
|
|
|
> **Note**: Line numbers may shift as code changes, so references use symbol names (function/class names).
|
|
> Use `grep -n` or IDE symbol search for exact locations.
|
|
|
|
### remap_pip_package Location (Code Duplication Exists)
|
|
|
|
```
|
|
comfyui_manager/glob/manager_core.py — def remap_pip_package(pkg)
|
|
comfyui_manager/prestartup_script.py — def remap_pip_package(pkg)
|
|
```
|
|
|
|
Both reference `cm_global.pip_overrides` with identical logic.
|
|
The unified resolver uses `cm_global.pip_overrides` directly to avoid adding more duplication.
|
|
|
|
### cm_global Global State
|
|
|
|
```python
|
|
# Dynamically assigned in prestartup_script.py (NOT defined in cm_global.py!)
|
|
cm_global.pip_blacklist = {'torch', 'torchaudio', 'torchsde', 'torchvision'} # set
|
|
cm_global.pip_overrides = {} # dict, loaded from JSON
|
|
cm_global.pip_downgrade_blacklist = [ # list
|
|
'torch', 'torchaudio', 'torchsde', 'torchvision',
|
|
'transformers', 'safetensors', 'kornia'
|
|
]
|
|
```
|
|
|
|
> **cm_cli path**: `cm_cli/__main__.py` also independently initializes these attributes.
|
|
> If the resolver may be called from the CLI path, this initialization should also be verified.
|
|
|
|
### PIPFixer Call Pattern
|
|
|
|
```python
|
|
# Within UnifiedManager.execute_install_script() method in manager_core.py:
|
|
pip_fixer = manager_util.PIPFixer(
|
|
manager_util.get_installed_packages(),
|
|
context.comfy_path,
|
|
context.manager_files_path
|
|
)
|
|
# ... (after installation)
|
|
pip_fixer.fix_broken()
|
|
```
|
|
|
|
The unified resolver does not call PIPFixer directly.
|
|
The caller (execute_install_script) calls PIPFixer as part of the existing flow.
|
|
|
|
### is_blacklisted() Logic (Must Be Reproduced in Unified Resolver)
|
|
|
|
```python
|
|
# manager_core.py — def is_blacklisted(name)
|
|
# 1. Simple pip_blacklist membership check
|
|
# 2. pip_downgrade_blacklist version comparison:
|
|
# - Parse spec with regex r'([^<>!~=]+)([<>!~=]=?)([^ ]*)'
|
|
# - match is None (no version spec) + installed → block
|
|
# - Operator in ['<=', '==', '<', '~='] + installed version >= requested version → block
|
|
# - Version comparison uses manager_util.StrictVersion (NOT packaging.version)
|
|
```
|
|
|
|
The unified resolver's `_is_downgrade_blacklisted()` method faithfully reproduces this logic.
|
|
It uses `manager_util.StrictVersion` instead of `packaging.version.parse()` to ensure consistency with existing behavior.
|
|
|
|
### Existing Code --index-url Handling (Asymmetric)
|
|
|
|
```python
|
|
# Only exists in standalone function execute_install_script():
|
|
if '--index-url' in package_name:
|
|
s = package_name.split('--index-url')
|
|
install_cmd = manager_util.make_pip_cmd(["install", s[0].strip(), '--index-url', s[1].strip()])
|
|
|
|
# UnifiedManager.execute_install_script() method does NOT have this handling
|
|
```
|
|
|
|
The unified resolver unifies both paths for consistent handling via `_split_index_url()`.
|