Comfy-Org / comfy-cli

Command Line Interface for Managing ComfyUI
https://docs.comfy.org/comfy-cli/getting-started
GNU General Public License v3.0
291 stars 47 forks source link

Failure importing registry installed node due to custom node code dependent on case sensitive name #199

Open yondonfu opened 1 month ago

yondonfu commented 1 month ago

Describe the bug A clear and concise description of what the bug is.

Note: I think it is debatable whether this is a problem with registry installs using comfy-cli or whether it is problem with how the specific custom node is implemented, but I thought I'd open this issue to get some discussion going and am happy to move the issue to the most relevant repo if there is alignment. See Additional Context for how I fixed this locally and a summary understanding of why this problem occurs in the first place.

I installed the jovimetrix node from the registry by running:

comfy node registry-install jovimetrix

When I run comfy launch, the jovimetrix node(s) are not imported properly due to failures in its __init__.py:

2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:955 - list index out of range
2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:954 - module failed /Users/yondonfu/Development/ComfyUI/custom_nodes/jovimetrix/core/core/calc.py
2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:955 - list index out of range
2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:954 - module failed /Users/yondonfu/Development/ComfyUI/custom_nodes/jovimetrix/core/core/utility/batch.py
2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:955 - list index out of range
2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:954 - module failed /Users/yondonfu/Development/ComfyUI/custom_nodes/jovimetrix/core/core/utility/io.py
2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:955 - list index out of range
2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:954 - module failed /Users/yondonfu/Development/ComfyUI/custom_nodes/jovimetrix/core/core/utility/info.py
2024-10-18 09:04:11.877 | WARNING  | jovimetrix:__init__:955 - list index out of range
2024-10-18 09:04:11.878 | WARNING  | jovimetrix:__init__:954 - module failed /Users/yondonfu/Development/ComfyUI/custom_nodes/jovimetrix/core/utility/batch.py
2024-10-18 09:04:11.878 | WARNING  | jovimetrix:__init__:955 - list index out of range
2024-10-18 09:04:11.878 | WARNING  | jovimetrix:__init__:954 - module failed /Users/yondonfu/Development/ComfyUI/custom_nodes/jovimetrix/core/utility/io.py
2024-10-18 09:04:11.878 | WARNING  | jovimetrix:__init__:955 - list index out of range
2024-10-18 09:04:11.878 | WARNING  | jovimetrix:__init__:954 - module failed /Users/yondonfu/Development/ComfyUI/custom_nodes/jovimetrix/core/utility/info.py
2024-10-18 09:04:11.878 | WARNING  | jovimetrix:__init__:955 - list index out of range
2024-10-18 09:04:11.878 | INFO     | jovimetrix:__init__:988 - 0 nodes loaded

Import times for custom nodes:
   0.0 seconds: /Users/yondonfu/Development/ComfyUI/custom_nodes/websocket_image_save.py
   0.0 seconds: /Users/yondonfu/Development/ComfyUI/custom_nodes/jovimetrix

To Reproduce Steps to reproduce the behavior

  1. Run comfy node registry-install jovimetrix
  2. Run comfy launch.
  3. Observe warnings and failure to import jovimetrix nodes.

Expected behavior A clear and concise description of what you expected to happen.

I expect the jovimetrix nodes to be imported without any issue when running comfy launch after running comfy node registry-install jovimetrix.

Nice to have

Additional context Add any other context about the problem here.

Environment

The Problem

The problem seems to be that there are a few places in the code for jovimetrix that assumes that the directory name for the base project is Jovimetrix:

The registry install by comfy-cli uses a case insensitive name (i.e. jovimetrix vs. Jovimetrix) and sets the directory name under custom_nodes to jovimetrix which causes these areas of the code to break.

The Temporary Fix

I was able to fix the warnings and import failure by renaming custom_nodes/jovimetrix within my ComfyUI directory to custom_nodes/Jovimetrix.

Future

AFAICT most other package managers use case insensitive package names so should the onus be on the custom node developer to ensure that their code is structured to work with case insensitive names? The concern would be if this causes backwards compat issues.

Or should a registry install by comfy-cli respect case sensitive names for repos and ensure that the directory name in custom_nodes is exactly the same as the repo name?