Skip to content

Commit a399d41

Browse files
committed
fix: handle inline comments and pip options in requirements.txt
Replace the per-line pip install loop in execute_install_script with `pip install -r <path>`, delegating requirements parsing to pip. This fixes inline comments being passed verbatim to pip (Invalid requirement errors) and incidentally handles line continuations, -e, --index-url, env markers, and hash specs. Also strip inline comments (matching pip's COMMENT_RE regex) in: - parse_req_file in uv.py, which feeds pip download/wheel argv. - initialize_project_config in registry/config_parser.py, which produces pyproject.toml [project.dependencies]; additionally skip pip-only options (-r, -e, --index-url, --find-links) that are not valid PEP 508 specifiers and would corrupt the generated TOML. The regex (^|\s+)#.*$ matches pip's exact rule: `#` only starts a comment when preceded by whitespace, so VCS URL fragments like Signed-off-by: Alexander Piskun <bigcat88@icloud.com> #subdirectory=pkg and direct-URL #sha256= hashes survive.
1 parent 5f8937f commit a399d41

6 files changed

Lines changed: 216 additions & 11 deletions

File tree

comfy_cli/command/custom_nodes/command.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -171,13 +171,8 @@ def execute_install_script(repo_path):
171171
if os.path.exists(requirements_path):
172172
print("Install: pip packages")
173173
python = resolve_workspace_python(workspace_manager.workspace_path)
174-
with open(requirements_path, encoding="utf-8") as requirements_file:
175-
for line in requirements_file:
176-
package_name = line.strip()
177-
if package_name and not package_name.startswith("#"):
178-
install_cmd = [python, "-m", "pip", "install", package_name]
179-
if package_name.strip() != "":
180-
try_install_script(repo_path, install_cmd)
174+
install_cmd = [python, "-m", "pip", "install", "-r", requirements_path]
175+
try_install_script(repo_path, install_cmd)
181176

182177
if os.path.exists(install_script_path):
183178
print("Install: install script")

comfy_cli/registry/config_parser.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@
1717
URLs,
1818
)
1919

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

2126
def create_comfynode_config():
2227
# Create the initial structure of the TOML document
@@ -249,7 +254,15 @@ def initialize_project_config():
249254
# Handle dependencies
250255
if os.path.exists("requirements.txt"):
251256
with open("requirements.txt") as req_file:
252-
dependencies = [line.strip() for line in req_file if line.strip()]
257+
dependencies: list[str] = []
258+
for raw in req_file:
259+
# Strip inline/full-line comments, then skip pip-requirements-file
260+
# options (-r, -e, -c, --index-url, ...) which are not valid
261+
# PEP 508 deps and would break downstream build tooling.
262+
line = _inline_comment_re.sub("", raw).strip()
263+
if not line or line.startswith("-"):
264+
continue
265+
dependencies.append(line)
253266
project["dependencies"] = dependencies
254267
else:
255268
print("Warning: 'requirements.txt' not found. No dependencies will be added.")

comfy_cli/uv.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ def _check_call(cmd: list[str], cwd: PathLike | None = None):
3737

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

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

4146
def _req_re_closure(name: str) -> re.Pattern[str]:
4247
return re.compile(rf"({name}\S+)")
@@ -64,8 +69,8 @@ def parse_req_file(rf: PathLike, skips: list[str] | None = None):
6469
opts: list[str] = []
6570
with open(rf) as f:
6671
for line in f:
67-
line = line.strip()
68-
if not line or line.startswith("#"):
72+
line = _inline_comment_re.sub("", line).strip()
73+
if not line:
6974
continue
7075
elif "==" in line and line.split("==")[0] in skips:
7176
continue

tests/comfy_cli/registry/test_config_parser.py

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,3 +399,96 @@ def test_initialize_project_config_ssh_remote(tmp_path, monkeypatch):
399399
assert urls["Bug Tracker"] == "https://github.com/user/ComfyUI-TestNode/issues"
400400
assert data["project"]["name"] == "testnode"
401401
assert data["tool"]["comfy"]["DisplayName"] == "ComfyUI-TestNode"
402+
403+
404+
# Issue #431: requirements.txt → pyproject.toml migration must produce
405+
# valid PEP 508 dependency specifiers. Inline comments, full-line comments,
406+
# and pip-specific options (-r, -e, --index-url, ...) are not valid deps.
407+
408+
409+
def _init_git_repo_with_reqs(tmp_path, requirements_content: str) -> None:
410+
subprocess.run(["git", "init"], cwd=tmp_path, check=True, capture_output=True)
411+
subprocess.run(
412+
["git", "remote", "add", "origin", "https://github.com/user/ComfyUI-TestNode.git"],
413+
cwd=tmp_path,
414+
check=True,
415+
capture_output=True,
416+
)
417+
(tmp_path / "requirements.txt").write_text(requirements_content)
418+
419+
420+
def test_initialize_project_config_strips_inline_comments(tmp_path, monkeypatch):
421+
monkeypatch.chdir(tmp_path)
422+
_init_git_repo_with_reqs(
423+
tmp_path,
424+
"matplotlib>=3.3.0 # For visualization\nnumpy>=1.0 # trailing\n",
425+
)
426+
initialize_project_config()
427+
with open(tmp_path / "pyproject.toml") as f:
428+
data = tomlkit.parse(f.read())
429+
deps = [str(d) for d in data["project"]["dependencies"]]
430+
assert deps == ["matplotlib>=3.3.0", "numpy>=1.0"]
431+
432+
433+
def test_initialize_project_config_skips_full_line_comments(tmp_path, monkeypatch):
434+
monkeypatch.chdir(tmp_path)
435+
_init_git_repo_with_reqs(
436+
tmp_path,
437+
"# heading comment\nfoo>=1.0\n # indented comment\nbar\n",
438+
)
439+
initialize_project_config()
440+
with open(tmp_path / "pyproject.toml") as f:
441+
data = tomlkit.parse(f.read())
442+
deps = [str(d) for d in data["project"]["dependencies"]]
443+
assert deps == ["foo>=1.0", "bar"]
444+
445+
446+
def test_initialize_project_config_skips_pip_options(tmp_path, monkeypatch):
447+
# `-r`, `-e`, `-c`, `--index-url`, `--extra-index-url`, `--find-links`
448+
# are pip-requirements-file syntax, not PEP 508 dep specifiers. They must
449+
# not land in [project.dependencies] where downstream build tools will
450+
# error trying to parse them.
451+
monkeypatch.chdir(tmp_path)
452+
_init_git_repo_with_reqs(
453+
tmp_path,
454+
"-r other.txt\n"
455+
"-e .\n"
456+
"--index-url https://pypi.org/simple\n"
457+
"--extra-index-url https://example.com/simple\n"
458+
"--find-links ./local-wheels\n"
459+
"foo>=1.0\n",
460+
)
461+
initialize_project_config()
462+
with open(tmp_path / "pyproject.toml") as f:
463+
data = tomlkit.parse(f.read())
464+
deps = [str(d) for d in data["project"]["dependencies"]]
465+
assert deps == ["foo>=1.0"]
466+
467+
468+
def test_initialize_project_config_preserves_vcs_subdirectory_fragment(tmp_path, monkeypatch):
469+
# Regression guard against a naive `split("#")[0]` fix — VCS fragments
470+
# must survive because `#` is only a comment marker when preceded by
471+
# whitespace (pip's rule).
472+
monkeypatch.chdir(tmp_path)
473+
_init_git_repo_with_reqs(
474+
tmp_path,
475+
"git+https://github.com/org/mono.git#subdirectory=pkg\n",
476+
)
477+
initialize_project_config()
478+
with open(tmp_path / "pyproject.toml") as f:
479+
data = tomlkit.parse(f.read())
480+
deps = [str(d) for d in data["project"]["dependencies"]]
481+
assert deps == ["git+https://github.com/org/mono.git#subdirectory=pkg"]
482+
483+
484+
def test_initialize_project_config_vcs_with_inline_comment(tmp_path, monkeypatch):
485+
monkeypatch.chdir(tmp_path)
486+
_init_git_repo_with_reqs(
487+
tmp_path,
488+
"git+https://github.com/org/mono.git#subdirectory=pkg # monorepo dep\n",
489+
)
490+
initialize_project_config()
491+
with open(tmp_path / "pyproject.toml") as f:
492+
data = tomlkit.parse(f.read())
493+
deps = [str(d) for d in data["project"]["dependencies"]]
494+
assert deps == ["git+https://github.com/org/mono.git#subdirectory=pkg"]

tests/comfy_cli/test_custom_nodes_python_resolution.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,47 @@ def test_install_py_uses_resolved_python(self, tmp_path):
6363
cmd = mock_check_call.call_args[0][0]
6464
assert cmd == ["/resolved/python", "install.py"]
6565

66+
def test_inline_comment_not_passed_to_pip(self, tmp_path):
67+
# Issue #431 regression: inline comments in requirements.txt must not
68+
# survive into the argv handed to pip. Pre-fix, the line was passed
69+
# verbatim (e.g. "matplotlib>=3.3.0 # note") and pip rejected it.
70+
(tmp_path / "requirements.txt").write_text("matplotlib>=3.3.0 # For visualization\n")
71+
72+
with (
73+
patch(
74+
"comfy_cli.command.custom_nodes.command.resolve_workspace_python",
75+
return_value="/resolved/python",
76+
),
77+
patch.object(command.workspace_manager, "workspace_path", str(tmp_path)),
78+
patch("comfy_cli.command.custom_nodes.command.subprocess.check_call") as mock_check_call,
79+
):
80+
command.execute_install_script(str(tmp_path))
81+
82+
for call in mock_check_call.call_args_list:
83+
argv = call[0][0]
84+
assert not any("#" in element for element in argv), f"inline comment leaked into pip argv: {argv!r}"
85+
86+
def test_uses_pip_install_r(self, tmp_path):
87+
# Option C: delegate requirements-file parsing to pip via `-r <path>`.
88+
# This lets pip handle inline comments, line continuations, VCS URL
89+
# fragments, env markers, -e, -r, --index-url, etc.
90+
requirements_path = tmp_path / "requirements.txt"
91+
requirements_path.write_text("numpy>=1.0\n")
92+
93+
with (
94+
patch(
95+
"comfy_cli.command.custom_nodes.command.resolve_workspace_python",
96+
return_value="/resolved/python",
97+
),
98+
patch.object(command.workspace_manager, "workspace_path", str(tmp_path)),
99+
patch("comfy_cli.command.custom_nodes.command.subprocess.check_call") as mock_check_call,
100+
):
101+
command.execute_install_script(str(tmp_path))
102+
103+
mock_check_call.assert_called_once()
104+
argv = mock_check_call.call_args[0][0]
105+
assert argv == ["/resolved/python", "-m", "pip", "install", "-r", str(requirements_path)]
106+
66107

67108
class TestUpdateNodeIdCache:
68109
def test_uses_resolved_python(self, tmp_path):

tests/uv/test_uv.py

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88
from comfy_cli import ui
99
from comfy_cli.constants import GPU_OPTION
10-
from comfy_cli.uv import DependencyCompiler, _check_call
10+
from comfy_cli.uv import DependencyCompiler, _check_call, parse_req_file
1111

1212
hereDir = Path(__file__).parent.resolve()
1313
mockComfyDir = hereDir / "mock_comfy"
@@ -360,3 +360,61 @@ def test_check_call_no_hint_for_pip_install_uv(capsys):
360360

361361
captured = capsys.readouterr().out
362362
assert "network filesystem" not in captured
363+
364+
365+
# Issue #431: parse_req_file feeds its output into pip argv (pip download /
366+
# pip wheel). Inline comments would be rejected by pip; VCS URL fragments must
367+
# be preserved verbatim (e.g. `#subdirectory=pkg`, `#egg=foo`).
368+
369+
370+
def test_parse_req_file_strips_inline_comments(tmp_path):
371+
rf = tmp_path / "requirements.txt"
372+
rf.write_text("foo>=1.0 # trailing comment\n")
373+
assert parse_req_file(rf) == ["foo>=1.0"]
374+
375+
376+
def test_parse_req_file_strips_inline_comment_with_single_space(tmp_path):
377+
rf = tmp_path / "requirements.txt"
378+
rf.write_text("bar==2.3 # single space before hash\n")
379+
assert parse_req_file(rf) == ["bar==2.3"]
380+
381+
382+
def test_parse_req_file_skips_full_line_comments(tmp_path):
383+
rf = tmp_path / "requirements.txt"
384+
rf.write_text("# heading\nfoo>=1.0\n # indented heading\nbaz\n")
385+
assert parse_req_file(rf) == ["foo>=1.0", "baz"]
386+
387+
388+
def test_parse_req_file_preserves_vcs_subdirectory_fragment(tmp_path):
389+
# Regression guard: any naive `split("#")[0]` would break this. `#` is only
390+
# a comment marker when preceded by whitespace (pip's rule).
391+
rf = tmp_path / "requirements.txt"
392+
rf.write_text("git+https://github.com/org/mono.git#subdirectory=pkg\n")
393+
assert parse_req_file(rf) == ["git+https://github.com/org/mono.git#subdirectory=pkg"]
394+
395+
396+
def test_parse_req_file_preserves_vcs_egg_fragment(tmp_path):
397+
rf = tmp_path / "requirements.txt"
398+
rf.write_text("git+https://github.com/org/repo.git@main#egg=foo\n")
399+
assert parse_req_file(rf) == ["git+https://github.com/org/repo.git@main#egg=foo"]
400+
401+
402+
def test_parse_req_file_preserves_direct_url_hash(tmp_path):
403+
rf = tmp_path / "requirements.txt"
404+
rf.write_text("foo @ https://host/f.whl#sha256=abc123\n")
405+
assert parse_req_file(rf) == ["foo @ https://host/f.whl#sha256=abc123"]
406+
407+
408+
def test_parse_req_file_vcs_with_inline_comment_strips_only_comment(tmp_path):
409+
# The trickiest case: a VCS spec with a fragment AND a trailing comment.
410+
# Comment is preceded by whitespace so it must be stripped; fragment is
411+
# part of the URL and must survive.
412+
rf = tmp_path / "requirements.txt"
413+
rf.write_text("git+https://host/r.git#subdirectory=pkg # note\n")
414+
assert parse_req_file(rf) == ["git+https://host/r.git#subdirectory=pkg"]
415+
416+
417+
def test_parse_req_file_preserves_double_dash_options(tmp_path):
418+
rf = tmp_path / "requirements.txt"
419+
rf.write_text("--extra-index-url https://example.com/simple\nfoo\n")
420+
assert parse_req_file(rf) == ["--extra-index-url", "https://example.com/simple", "foo"]

0 commit comments

Comments
 (0)