ltdrdata / ComfyUI-Manager

ComfyUI-Manager is an extension designed to enhance the usability of ComfyUI. It offers management functions to install, remove, disable, and enable various custom nodes of ComfyUI. Furthermore, this extension provides a hub feature and convenience functions to access a wide range of information within ComfyUI.
GNU General Public License v3.0
6.9k stars 893 forks source link

Custom_nodes path different from original ComfyUI folder_paths definition. #420

Open davertor opened 8 months ago

davertor commented 8 months ago

There is a mismatch of ComfyUI and ComfyUI-Manager folder paths, where you want to run ComfyUI with non-default paths by passing an extra_model_paths.yaml file. In this case, ComfyUI-Manager does not save custom modules where it should be. For example, this is a problem if you run ComfyUI in a docker container.

In comfyanonymous/ComfyUI, all the folder paths are defined under folder_paths.py module. For example, custom_nodes folder path is defined under folder_names_and_paths["custom_nodes"] variable.

However, ComfyUI-Manager instead of importing folder_names_and_paths["custom_nodes"], defines its own paths. For example, custom_nodes path is defined as

comfy_path = os.path.dirname(folder_paths.__file__)
custom_nodes_path = os.path.join(comfy_path, 'custom_nodes')

The solution would be to load all the paths from folder_paths.py module.

ltdrdata commented 8 months ago

This issue is not a simple one. In the case of some custom nodes, they identify the location of ComfyUI using a relative path at the install stage. If a custom node is installed in a different path, it can cause an error.

Even if Manager support this, a policy like changing the installation path depending on the presence or absence of install.py is needed. If install.py is added in the future and tries to identify the location of ComfyUI using a relative path, such a risk arises. Although the probability is very low, some measures are needed.

xingren23 commented 8 months ago

i have a suggestion for custom_nodes model paths:

  1. Support configuration of model paths in an extra_model_path.yaml file.
  2. Store models downloaded by users in the root directory /comfyui/models/xxxxx.
  3. Store models downloaded during runtime in either the root directory /comfyui/models/xxx or within the node itself at /comfyui/custom_nodes/xxxx/models/xxxx.
davertor commented 8 months ago

This issue is not a simple one. In the case of some custom nodes, they identify the location of ComfyUI using a relative path at the install stage. If a custom node is installed in a different path, it can cause an error.

Even if Manager support this, a policy like changing the installation path depending on the presence or absence of install.py is needed. If install.py is added in the future and tries to identify the location of ComfyUI using a relative path, such a risk arises. Although the probability is very low, some measures are needed.

I understand the issue you mentioned, but I would try to argue why I think that reading paths from folder_paths.py module is the way to go.

Although, it might broke some custom_nodes installation in ComfyManager, I think is the way to go. ComfyUI gives the option to pass an extra_model_paths.yaml to define new folder paths for your Comfy server. In this case, it could happen that your custom_nodes folder would be a different one from the default one. So, all the custom_nodes should deal with that option in order not to broke a comfyUI feature.

What do you think?

ltdrdata commented 8 months ago

This issue is not a simple one. In the case of some custom nodes, they identify the location of ComfyUI using a relative path at the install stage. If a custom node is installed in a different path, it can cause an error. Even if Manager support this, a policy like changing the installation path depending on the presence or absence of install.py is needed. If install.py is added in the future and tries to identify the location of ComfyUI using a relative path, such a risk arises. Although the probability is very low, some measures are needed.

I understand the issue you mentioned, but I would try to argue why I think that reading paths from folder_paths.py module is the way to go.

Although, it might broke some custom_nodes installation in ComfyManager, I think is the way to go. ComfyUI gives the option to pass an extra_model_paths.yaml to define new folder paths for your Comfy server. In this case, it could happen that your custom_nodes folder would be a different one from the default one. So, all the custom_nodes should deal with that option in order not to broke a comfyUI feature.

What do you think?

There's a bit of a tricky part about that. When a custom node is imported, it runs within the comfy process, so it can get the path based on folder_paths.

However, install.py runs separately from the comfy process. Therefore, if it is located in a path other than ComfyUI/custom_nodes, it cannot obtain path information.

For example, when registering additional models and such, it becomes impossible to determine the model path that needs to be shared.

shripadk commented 6 months ago

@ltdrdata First, want to thank you for making ComfyUI-Manager. Really pleasant to work with.

Coming to this issue: Is there a possibility to enable this feature through a flag? Something like --enable-extra-model-paths (while setting up ComfyUI-Manager or even later through the UI). That way we know exactly what we are getting into by enabling it. So if a custom node fails to load, we can file an issue or submit a PR with a fix for the same. Or just not use the custom node and use something else instead. But without a switch being enabled, we will never know which custom nodes need this fixed and which don't unless we go through each custom node and check the source (which is not feasible).

ltdrdata commented 6 months ago

Plans are currently underway to improve this issue.

shripadk commented 6 months ago

@ltdrdata Thank you 🎉

APZmedia commented 2 months ago

I'd like to follow up on this. Is there a way to add the custom node extra paths in the toml file? For example, new custom nodes use different controlnets and I'd like to store them in the main controlnets folder, which is in an extra path in order to share it with different intances of comfy instead of needing to download them again and again for each instance

ltdrdata commented 2 months ago

I'd like to follow up on this. Is there a way to add the custom node extra paths in the toml file? For example, new custom nodes use different controlnets and I'd like to store them in the main controlnets folder, which is in an extra path in order to share it with different intances of comfy instead of needing to download them again and again for each instance

When using a node that belongs to a different custom node pack, you can obtain the class by using NODE_CLASS_MAPPINGS['<node_name>'].