Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions comfy_cli/command/custom_nodes/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,13 +171,11 @@ def execute_install_script(repo_path):
if os.path.exists(requirements_path):
print("Install: pip packages")
python = resolve_workspace_python(workspace_manager.workspace_path)
with open(requirements_path, encoding="utf-8") as requirements_file:
for line in requirements_file:
package_name = line.strip()
if package_name and not package_name.startswith("#"):
install_cmd = [python, "-m", "pip", "install", package_name]
if package_name.strip() != "":
try_install_script(repo_path, install_cmd)
# Absolute path so pip doesn't re-resolve it against cwd=repo_path
# in try_install_script, which would double the path if repo_path
# is relative.
install_cmd = [python, "-m", "pip", "install", "-r", os.path.abspath(requirements_path)]
try_install_script(repo_path, install_cmd)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if os.path.exists(install_script_path):
print("Install: install script")
Expand Down
20 changes: 19 additions & 1 deletion comfy_cli/registry/config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
URLs,
)

# Mirrors pip's requirements-file comment rule: `#` only starts a comment when
# preceded by whitespace, so VCS URL fragments (`#subdirectory=`, `#egg=`) and
# direct-URL hashes (`#sha256=`) survive.
_inline_comment_re: re.Pattern[str] = re.compile(r"(^|\s+)#.*$")


def create_comfynode_config():
# Create the initial structure of the TOML document
Expand Down Expand Up @@ -249,7 +254,20 @@ def initialize_project_config():
# Handle dependencies
if os.path.exists("requirements.txt"):
with open("requirements.txt") as req_file:
dependencies = [line.strip() for line in req_file if line.strip()]
dependencies: list[str] = []
for raw in req_file:
# Strip inline/full-line comments, then skip pip-requirements-file
# options (-r, -e, -c, --index-url, ...) which are not valid
# PEP 508 deps and would break downstream build tooling.
line = _inline_comment_re.sub("", raw).strip()
if not line:
continue
if line.startswith("-"):
print(
f"Warning: skipping pip-only option from requirements.txt (not valid as PEP 508 dep): {line!r}"
)
Comment thread
bigcat88 marked this conversation as resolved.
continue
dependencies.append(line)
project["dependencies"] = dependencies
else:
print("Warning: 'requirements.txt' not found. No dependencies will be added.")
Expand Down
9 changes: 7 additions & 2 deletions comfy_cli/uv.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ def _check_call(cmd: list[str], cwd: PathLike | None = None):

_req_name_re: re.Pattern[str] = re.compile(r"require\s([\w-]+)")

# Mirrors pip's requirements-file comment rule (pip._internal.req.req_file.COMMENT_RE):
# `#` only starts a comment when preceded by whitespace (or starts the line), so
# VCS URL fragments like `#subdirectory=pkg` and `#egg=foo` survive.
_inline_comment_re: re.Pattern[str] = re.compile(r"(^|\s+)#.*$")
Comment thread
bigcat88 marked this conversation as resolved.


def _req_re_closure(name: str) -> re.Pattern[str]:
return re.compile(rf"({name}\S+)")
Expand Down Expand Up @@ -64,8 +69,8 @@ def parse_req_file(rf: PathLike, skips: list[str] | None = None):
opts: list[str] = []
with open(rf) as f:
for line in f:
line = line.strip()
if not line or line.startswith("#"):
line = _inline_comment_re.sub("", line).strip()
if not line:
continue
elif "==" in line and line.split("==")[0] in skips:
continue
Expand Down
97 changes: 97 additions & 0 deletions tests/comfy_cli/registry/test_config_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -399,3 +399,100 @@ def test_initialize_project_config_ssh_remote(tmp_path, monkeypatch):
assert urls["Bug Tracker"] == "https://github.com/user/ComfyUI-TestNode/issues"
assert data["project"]["name"] == "testnode"
assert data["tool"]["comfy"]["DisplayName"] == "ComfyUI-TestNode"


# Issue #431: requirements.txt → pyproject.toml migration must produce
# valid PEP 508 dependency specifiers. Inline comments, full-line comments,
# and pip-specific options (-r, -e, --index-url, ...) are not valid deps.


def _init_git_repo_with_reqs(tmp_path, requirements_content: str) -> None:
subprocess.run(["git", "init"], cwd=tmp_path, check=True, capture_output=True)
subprocess.run(
["git", "remote", "add", "origin", "https://github.com/user/ComfyUI-TestNode.git"],
cwd=tmp_path,
check=True,
capture_output=True,
)
(tmp_path / "requirements.txt").write_text(requirements_content)


def test_initialize_project_config_strips_inline_comments(tmp_path, monkeypatch):
monkeypatch.chdir(tmp_path)
_init_git_repo_with_reqs(
tmp_path,
"matplotlib>=3.3.0 # For visualization\nnumpy>=1.0 # trailing\n",
)
initialize_project_config()
with open(tmp_path / "pyproject.toml") as f:
data = tomlkit.parse(f.read())
deps = [str(d) for d in data["project"]["dependencies"]]
assert deps == ["matplotlib>=3.3.0", "numpy>=1.0"]


def test_initialize_project_config_skips_full_line_comments(tmp_path, monkeypatch):
monkeypatch.chdir(tmp_path)
_init_git_repo_with_reqs(
tmp_path,
"# heading comment\nfoo>=1.0\n # indented comment\nbar\n",
)
initialize_project_config()
with open(tmp_path / "pyproject.toml") as f:
data = tomlkit.parse(f.read())
deps = [str(d) for d in data["project"]["dependencies"]]
assert deps == ["foo>=1.0", "bar"]


def test_initialize_project_config_skips_pip_options(tmp_path, monkeypatch, capsys):
# `-r`, `-e`, `-c`, `--index-url`, `--extra-index-url`, `--find-links`
# are pip-requirements-file syntax, not PEP 508 dep specifiers. They must
# not land in [project.dependencies] where downstream build tools will
# error trying to parse them. Each skipped line must also produce a
# visible warning so silent data loss is avoided.
monkeypatch.chdir(tmp_path)
_init_git_repo_with_reqs(
tmp_path,
"-r other.txt\n"
"-e .\n"
"--index-url https://pypi.org/simple\n"
"--extra-index-url https://example.com/simple\n"
"--find-links ./local-wheels\n"
"foo>=1.0\n",
)
initialize_project_config()
with open(tmp_path / "pyproject.toml") as f:
data = tomlkit.parse(f.read())
deps = [str(d) for d in data["project"]["dependencies"]]
assert deps == ["foo>=1.0"]
out = capsys.readouterr().out
for dropped in ["-r other.txt", "-e .", "--index-url", "--extra-index-url", "--find-links"]:
assert dropped in out, f"missing skip warning for {dropped!r}"


def test_initialize_project_config_preserves_vcs_subdirectory_fragment(tmp_path, monkeypatch):
# Regression guard against a naive `split("#")[0]` fix — VCS fragments
# must survive because `#` is only a comment marker when preceded by
# whitespace (pip's rule).
monkeypatch.chdir(tmp_path)
_init_git_repo_with_reqs(
tmp_path,
"git+https://github.com/org/mono.git#subdirectory=pkg\n",
)
initialize_project_config()
with open(tmp_path / "pyproject.toml") as f:
data = tomlkit.parse(f.read())
deps = [str(d) for d in data["project"]["dependencies"]]
assert deps == ["git+https://github.com/org/mono.git#subdirectory=pkg"]


def test_initialize_project_config_vcs_with_inline_comment(tmp_path, monkeypatch):
monkeypatch.chdir(tmp_path)
_init_git_repo_with_reqs(
tmp_path,
"git+https://github.com/org/mono.git#subdirectory=pkg # monorepo dep\n",
)
initialize_project_config()
with open(tmp_path / "pyproject.toml") as f:
data = tomlkit.parse(f.read())
deps = [str(d) for d in data["project"]["dependencies"]]
assert deps == ["git+https://github.com/org/mono.git#subdirectory=pkg"]
67 changes: 67 additions & 0 deletions tests/comfy_cli/test_custom_nodes_python_resolution.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
from unittest.mock import patch

from comfy_cli.command.custom_nodes import command
Expand Down Expand Up @@ -63,6 +64,72 @@ def test_install_py_uses_resolved_python(self, tmp_path):
cmd = mock_check_call.call_args[0][0]
assert cmd == ["/resolved/python", "install.py"]

def test_inline_comment_not_passed_to_pip(self, tmp_path):
# Issue #431 regression: inline comments in requirements.txt must not
# survive into the argv handed to pip. Pre-fix, the raw line was passed
# verbatim (e.g. "matplotlib>=3.3.0 # note") and pip rejected it.
bad_spec = "matplotlib>=3.3.0 # For visualization"
(tmp_path / "requirements.txt").write_text(f"{bad_spec}\n")

with (
patch(
"comfy_cli.command.custom_nodes.command.resolve_workspace_python",
return_value="/resolved/python",
),
patch.object(command.workspace_manager, "workspace_path", str(tmp_path)),
patch("comfy_cli.command.custom_nodes.command.subprocess.check_call") as mock_check_call,
):
command.execute_install_script(str(tmp_path))

for call in mock_check_call.call_args_list:
argv = call[0][0]
assert bad_spec not in argv, f"raw comment-laden spec leaked into pip argv: {argv!r}"

def test_uses_pip_install_r(self, tmp_path):
# Option C: delegate requirements-file parsing to pip via `-r <path>`.
# This lets pip handle inline comments, line continuations, VCS URL
# fragments, env markers, -e, -r, --index-url, etc.
requirements_path = tmp_path / "requirements.txt"
requirements_path.write_text("numpy>=1.0\n")

with (
patch(
"comfy_cli.command.custom_nodes.command.resolve_workspace_python",
return_value="/resolved/python",
),
patch.object(command.workspace_manager, "workspace_path", str(tmp_path)),
patch("comfy_cli.command.custom_nodes.command.subprocess.check_call") as mock_check_call,
):
command.execute_install_script(str(tmp_path))

mock_check_call.assert_called_once()
argv = mock_check_call.call_args[0][0]
assert argv == ["/resolved/python", "-m", "pip", "install", "-r", str(requirements_path)]

def test_requirements_path_is_absolute_when_repo_path_is_relative(self, tmp_path, monkeypatch):
# try_install_script runs pip with cwd=repo_path. If requirements_path
# were relative, pip would resolve `-r <rel>/requirements.txt` against
# that cwd, producing a doubled path like <rel>/<rel>/requirements.txt.
# Guard: the `-r` target must be absolute regardless of the input.
(tmp_path / "requirements.txt").write_text("numpy>=1.0\n")
monkeypatch.chdir(tmp_path.parent)
relative_repo = tmp_path.name # e.g. "test_requirements_path..." relative to cwd

with (
patch(
"comfy_cli.command.custom_nodes.command.resolve_workspace_python",
return_value="/resolved/python",
),
patch.object(command.workspace_manager, "workspace_path", str(tmp_path)),
patch("comfy_cli.command.custom_nodes.command.subprocess.check_call") as mock_check_call,
):
command.execute_install_script(relative_repo)

argv = mock_check_call.call_args[0][0]
target = argv[-1]
assert os.path.isabs(target), f"-r target is not absolute: {target!r}"
assert target == str(tmp_path / "requirements.txt")


class TestUpdateNodeIdCache:
def test_uses_resolved_python(self, tmp_path):
Expand Down
68 changes: 67 additions & 1 deletion tests/uv/test_uv.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from comfy_cli import ui
from comfy_cli.constants import GPU_OPTION
from comfy_cli.uv import DependencyCompiler, _check_call
from comfy_cli.uv import DependencyCompiler, _check_call, parse_req_file

hereDir = Path(__file__).parent.resolve()
mockComfyDir = hereDir / "mock_comfy"
Expand Down Expand Up @@ -360,3 +360,69 @@ def test_check_call_no_hint_for_pip_install_uv(capsys):

captured = capsys.readouterr().out
assert "network filesystem" not in captured


# Issue #431: parse_req_file feeds its output into pip argv (pip download /
# pip wheel). Inline comments would be rejected by pip; VCS URL fragments must
# be preserved verbatim (e.g. `#subdirectory=pkg`, `#egg=foo`).


def test_parse_req_file_strips_inline_comments(tmp_path):
rf = tmp_path / "requirements.txt"
rf.write_text("foo>=1.0 # trailing comment\n")
assert parse_req_file(rf) == ["foo>=1.0"]


def test_parse_req_file_strips_inline_comment_with_single_space(tmp_path):
rf = tmp_path / "requirements.txt"
rf.write_text("bar==2.3 # single space before hash\n")
assert parse_req_file(rf) == ["bar==2.3"]


def test_parse_req_file_skips_full_line_comments(tmp_path):
rf = tmp_path / "requirements.txt"
rf.write_text("# heading\nfoo>=1.0\n # indented heading\nbaz\n")
assert parse_req_file(rf) == ["foo>=1.0", "baz"]


def test_parse_req_file_preserves_vcs_subdirectory_fragment(tmp_path):
# Regression guard: any naive `split("#")[0]` would break this. `#` is only
# a comment marker when preceded by whitespace (pip's rule).
rf = tmp_path / "requirements.txt"
rf.write_text("git+https://github.com/org/mono.git#subdirectory=pkg\n")
assert parse_req_file(rf) == ["git+https://github.com/org/mono.git#subdirectory=pkg"]


def test_parse_req_file_preserves_vcs_egg_fragment(tmp_path):
rf = tmp_path / "requirements.txt"
rf.write_text("git+https://github.com/org/repo.git@main#egg=foo\n")
assert parse_req_file(rf) == ["git+https://github.com/org/repo.git@main#egg=foo"]


def test_parse_req_file_preserves_direct_url_hash(tmp_path):
rf = tmp_path / "requirements.txt"
rf.write_text("foo @ https://host/f.whl#sha256=abc123\n")
assert parse_req_file(rf) == ["foo @ https://host/f.whl#sha256=abc123"]


def test_parse_req_file_vcs_with_inline_comment_strips_only_comment(tmp_path):
# The trickiest case: a VCS spec with a fragment AND a trailing comment.
# Comment is preceded by whitespace so it must be stripped; fragment is
# part of the URL and must survive.
rf = tmp_path / "requirements.txt"
rf.write_text("git+https://host/r.git#subdirectory=pkg # note\n")
assert parse_req_file(rf) == ["git+https://host/r.git#subdirectory=pkg"]


def test_parse_req_file_preserves_double_dash_options(tmp_path):
rf = tmp_path / "requirements.txt"
rf.write_text("--extra-index-url https://example.com/simple\nfoo\n")
assert parse_req_file(rf) == ["--extra-index-url", "https://example.com/simple", "foo"]
Comment thread
bigcat88 marked this conversation as resolved.


def test_parse_req_file_handles_crlf_line_endings(tmp_path):
# Windows-authored requirements.txt files use CRLF. Verify the comment
# stripper + .strip() cleanly handles the trailing \r.
rf = tmp_path / "requirements.txt"
rf.write_bytes(b"foo>=1.0 # note\r\nbar>=2.0\r\n")
assert parse_req_file(rf) == ["foo>=1.0", "bar>=2.0"]
Loading