feat: comfyui create venv as needed#282
Conversation
should also fix Comfy-Org#263 tried to ensure python was only replaced where relevant, so no edits to uv.py or standalone.py, or the huggingface downloader
|
Fixed the merge conflicts; Haven't tested if the PR still works on any of Linux, Mac or Windows though cause its been a while since I last used comfyui. Change from draft to open to hopefully increase visibility. Am open to fixing any new bugs since June. |
|
bump |
bigcat88
left a comment
There was a problem hiding this comment.
- venv creation is too eager
find_or_create_python_env() is called in setup_workspace_manager(), which runs on every command via the main callback. This means:
- Running comfy env or even comfy --help could trigger venv creation
- Running comfy install for a fresh install would fail because the workspace directory doesn't exist yet
My suggestion: Make venv creation lazy - only create when actually needed (e.g., during install, launch, update), or/and add a guard:
if self.workspace_path and os.path.isdir(self.workspace_path):
self.python_exe = self.find_or_create_python_env()
else:
self.python_exe = sys.executable # fallback
- No workspace existence check
The function doesn't verify self.workspace_path exists before trying to create a venv in it:
os.path.join(self.workspace_path, default_name) # Could fail if workspace_path is None or doesn't exist
--upgrade-depsrequires network
This downloads the latest pip/setuptools during venv creation. It will:
- Fail in offline environments
- Slow down venv creation
Fixes #263, #272.
As comfy-cli might be installed globally via
pipxoruv, as per linked issue, it ends up installing the comfyui dependencies into the managed environment. Especially since comfy-cli can manage several comfyui installs, it should install into environments per each install. Hence the below logic:.venv/venvis present in comfyui folder, use that..venvusing the currently active Python in--copiesmode to prevent broken symlinks if user were to later delete that Python version.I have thus far tested only on Linux, but I assume it should work for MacOS and Windows too. That said, I only tested if
comfy installandcomfy launchwork as expected.I tried to selectively change
sys.executableto use the venv where appropriate. However, as I am new to this codebase, I might have missed out some places or changed some places I should not, so I need help to check that. The riskiest part,comfy standaloneseems to be broken for me before I made my changes, so I can't tell if I may have broke that.Also, while this approach works perfectly fine for venvs, it is limited for Conda environments. Since it doesn't properly activate Conda, tools installed in the Conda environment like
nvccaren't available as expected. I think this can be solved as a separate issue.