-
Notifications
You must be signed in to change notification settings - Fork 192
Prepare manager pip support #1450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a6d3637
f5ce780
27d557c
9014605
e5dd2f1
752dc92
9c5d402
2df110e
2e1a5b5
32c0b42
db1f2bb
3d63152
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,34 +1,49 @@ | ||
| /* eslint-disable @typescript-eslint/no-unsafe-member-access */ | ||
| /* eslint-disable @typescript-eslint/no-unsafe-argument */ | ||
| import fs from 'node:fs'; | ||
| import path from 'node:path'; | ||
|
|
||
| /** | ||
| * Verify the app build for the current platform. | ||
| * Check that all required paths are present. | ||
| */ | ||
| const PATHS = { | ||
| /** | ||
| * @typedef {{ base: string; required: string[] }} VerifyConfig | ||
| */ | ||
|
|
||
| const PATHS = /** @type {Record<'mac' | 'windows', VerifyConfig>} */ ({ | ||
| mac: { | ||
| base: 'dist/mac-arm64/ComfyUI.app/Contents/Resources', | ||
| required: ['ComfyUI', 'ComfyUI/custom_nodes/ComfyUI-Manager', 'UI', 'uv/macos/uv', 'uv/macos/uvx'], | ||
| required: ['ComfyUI', 'UI', 'uv/macos/uv', 'uv/macos/uvx'], | ||
| }, | ||
| windows: { | ||
| base: 'dist/win-unpacked/resources', | ||
| required: [ | ||
| // Add Windows-specific paths here | ||
| 'ComfyUI', | ||
| 'ComfyUI/custom_nodes/ComfyUI-Manager', | ||
| 'UI', | ||
| 'uv/win/uv.exe', | ||
| 'uv/win/uvx.exe', | ||
| ], | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| /** | ||
| * @param {VerifyConfig} config | ||
| */ | ||
| function verifyConfig(config) { | ||
| const required = [...config.required]; | ||
| const managerRequirementsPath = path.join(config.base, 'ComfyUI', 'manager_requirements.txt'); | ||
| const legacyManagerPath = path.join(config.base, 'ComfyUI', 'custom_nodes', 'ComfyUI-Manager'); | ||
| if (fs.existsSync(managerRequirementsPath)) { | ||
| required.push('ComfyUI/manager_requirements.txt'); | ||
| } else if (fs.existsSync(legacyManagerPath)) { | ||
| required.push('ComfyUI/custom_nodes/ComfyUI-Manager'); | ||
| } else { | ||
| required.push('ComfyUI/manager_requirements.txt'); | ||
| } | ||
|
Comment on lines
+40
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider adding a comment to clarify fallback intent. The else block adds } else {
+ // Fallback: require new manager format, which will fail verification if neither config exists
required.push(path.join('ComfyUI', 'manager_requirements.txt'));
}
🤖 Prompt for AI Agents |
||
|
|
||
| const missingPaths = []; | ||
|
|
||
| for (const requiredPath of config.required) { | ||
| for (const requiredPath of required) { | ||
| const fullPath = path.join(config.base, requiredPath); | ||
| if (!fs.existsSync(fullPath)) { | ||
| missingPaths.push(requiredPath); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { app } from 'electron'; | |
| import log from 'electron-log/main'; | ||
| import pty from 'node-pty'; | ||
| import { ChildProcess, spawn } from 'node:child_process'; | ||
| import { existsSync } from 'node:fs'; | ||
| import { readdir, rm } from 'node:fs/promises'; | ||
| import os, { EOL } from 'node:os'; | ||
| import path from 'node:path'; | ||
|
|
@@ -116,6 +117,7 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |
| readonly pythonInterpreterPath: string; | ||
| readonly comfyUIRequirementsPath: string; | ||
| readonly comfyUIManagerRequirementsPath: string; | ||
| readonly legacyComfyUIManagerRequirementsPath: string; | ||
| readonly selectedDevice: TorchDeviceType; | ||
| readonly telemetry: ITelemetry; | ||
| readonly pythonMirror?: string; | ||
|
|
@@ -186,13 +188,18 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |
| this.venvPath = path.join(basePath, '.venv'); | ||
| const resourcesPath = app.isPackaged ? path.join(process.resourcesPath) : path.join(app.getAppPath(), 'assets'); | ||
| this.comfyUIRequirementsPath = path.join(resourcesPath, 'ComfyUI', 'requirements.txt'); | ||
| this.comfyUIManagerRequirementsPath = path.join( | ||
| const managerRequirementsPath = path.join(resourcesPath, 'ComfyUI', 'manager_requirements.txt'); | ||
| this.legacyComfyUIManagerRequirementsPath = path.join( | ||
| resourcesPath, | ||
| 'ComfyUI', | ||
| 'custom_nodes', | ||
| 'ComfyUI-Manager', | ||
| 'requirements.txt' | ||
| ); | ||
| this.comfyUIManagerRequirementsPath = this.resolveManagerRequirementsPath( | ||
| managerRequirementsPath, | ||
| this.legacyComfyUIManagerRequirementsPath | ||
| ); | ||
|
|
||
| this.cacheDir = path.join(basePath, 'uv-cache'); | ||
|
|
||
|
|
@@ -231,6 +238,12 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |
| } | ||
| } | ||
|
|
||
| private resolveManagerRequirementsPath(primary: string, legacy: string) { | ||
| if (existsSync(primary)) return primary; | ||
| if (existsSync(legacy)) return legacy; | ||
| return primary; | ||
| } | ||
|
|
||
| public async create(callbacks?: ProcessCallbacks): Promise<void> { | ||
| try { | ||
| await this.createEnvironment(callbacks); | ||
|
|
@@ -377,6 +390,9 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |
| ); | ||
| return this.manualInstall(callbacks); | ||
| } | ||
|
|
||
| // Ensure Manager requirements are installed even if the compiled file did not include them. | ||
| await this.installComfyUIManagerRequirements(callbacks); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -641,6 +657,13 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |
| }) | ||
| ); | ||
|
|
||
| if (!(await pathAccessible(this.comfyUIManagerRequirementsPath))) { | ||
| throw new Error( | ||
| `Manager requirements file was not found at ${this.comfyUIManagerRequirementsPath}. ` + | ||
| `If you are using a legacy build, ensure the ComfyUI-Manager custom node is present at ${this.legacyComfyUIManagerRequirementsPath}.` | ||
| ); | ||
| } | ||
|
Comment on lines
+660
to
+665
|
||
|
|
||
benceruleanlu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| log.info(`Installing ComfyUIManager requirements from ${this.comfyUIManagerRequirementsPath}`); | ||
| const installCmd = getPipInstallArgs({ | ||
| requirementsFile: this.comfyUIManagerRequirementsPath, | ||
|
|
@@ -738,6 +761,12 @@ export class VirtualEnvironment implements HasTelemetry, PythonExecutor { | |
| }; | ||
|
|
||
| const coreOutput = await checkRequirements(this.comfyUIRequirementsPath); | ||
| if (!(await pathAccessible(this.comfyUIManagerRequirementsPath))) { | ||
| throw new Error( | ||
| `Manager requirements file was not found at ${this.comfyUIManagerRequirementsPath}. ` + | ||
| `If you are using a legacy build, ensure the ComfyUI-Manager custom node is present at ${this.legacyComfyUIManagerRequirementsPath}.` | ||
| ); | ||
| } | ||
| const managerOutput = await checkRequirements(this.comfyUIManagerRequirementsPath); | ||
|
|
||
| const coreOk = hasAllPackages(coreOutput); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| # Dummy manager requirements file for tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial
Use path.join() for cross-platform path construction.
Lines 37, 39, and 41 push strings with hardcoded forward slashes. While
path.join()normalizes these later, explicitly constructing these paths ensures consistency across platforms.Based on learnings, prefer
path.join()for all path construction to ensure cross-platform compatibility.Apply this diff:
const managerRequirementsPath = path.join(config.base, 'ComfyUI', 'manager_requirements.txt'); const legacyManagerPath = path.join(config.base, 'ComfyUI', 'custom_nodes', 'ComfyUI-Manager'); if (fs.existsSync(managerRequirementsPath)) { - required.push('ComfyUI/manager_requirements.txt'); + required.push(path.join('ComfyUI', 'manager_requirements.txt')); } else if (fs.existsSync(legacyManagerPath)) { - required.push('ComfyUI/custom_nodes/ComfyUI-Manager'); + required.push(path.join('ComfyUI', 'custom_nodes', 'ComfyUI-Manager')); } else { - required.push('ComfyUI/manager_requirements.txt'); + required.push(path.join('ComfyUI', 'manager_requirements.txt')); }🤖 Prompt for AI Agents