-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add a --fast-deps option to comfy install/reinstall
that enables new faster/unified/uv-based dependency install
#141
Conversation
1ab6123
to
d742327
Compare
…s already installed
- output is currently unsatisfactory in the case of a dependency conflict - doesn't print the names of conflicting top-level packages. The uv maintainers have expressed interest in fixing this: astral-sh/uv#1854
Okay, the code in this PR is coming along. Fixed a bunch of bugs/problems, and thanks to #147 this PR is now much easier to test out for yourself. Just install this PR branch, then do: comfy install --fast-deps --manager-url=https://github.com/ltdrdata/ComfyUI-Manager.git@feat/cnr specifying any other args as normal. The fast-deps option will now also work with installing custom nodes, eg: comfy node install --fast-deps comfyui-kjnodes comfyui-animatediff-evolved |
Here's the features/TODO list for the PR:
So the big remaining issues are adding some kind of extension-vs-extension conflict resolution and finishing up any tests for the new installer. Unfortunately the obvious easy path to implement ext-vs-ext behavior is a no-go; due to an upstream issue there's no way currently for us to report to the user which extensions are actually in conflict, so we can't just print out the relevant info and then tell the user to manually resolve the conflict |
- will manually prompt user for resolution in case of extension-vs-extension dependency conflict, using `ui.prompt_select`
…r input now functional - needs better output
- still needs an actual assert at the end
comfy install/reintstall
that enables new faster/unified/uv-based dependency installcomfy install/reinstall
that enables new faster/unified/uv-based dependency install
Okay, unittest is now functional. Said test needs a small fix tho (I realized that currently it will break whenver sympy or numpy have a new release, which is not ideal). I'll add the fix and address all of the feedback tomorrow, and then I'll merge |
Sounds great! |
rocmPytorchUrl = "https://download.pytorch.org/whl/rocm6.0" | ||
nvidiaPytorchUrl = "https://download.pytorch.org/whl/cu121" | ||
|
||
overrideGpu = dedent(""" |
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.
What's the need for override.txt file?
self.override.unlink(missing_ok=True) | ||
|
||
with open(self.override, "w") as f: | ||
if self.gpu is not None: |
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.
if self.gpu is not None and self.gpuUrl is not None
This PR adds a new option
--fast-deps
, tocomfy install
andcomfy reinstall
. This option enables the new dependency handling implementation. The features of said new dependency handling currently cover:This PR depends on ltdrdata/ComfyUI-Manager#886, which adds support for a
--no-deps
option to the lower level cm-cli interface. Although ComfyUI-Manager#886 has been merged, the relevant code has not yet been pushed to the main branch of ComfyUI-Manager. So if you want to test this PR out yourself, you'll have to manually switch to thefeat/cnr
branch on the instance of ComfyUI-Manager that comfy-cli installs.