fix: handle inline comments in requirements.txt parsing (#433)

Signed-off-by: Alexander Piskun <bigcat88@icloud.com>

Author: bigcat88

comfy_cli/command/custom_nodes/command.py

@@ -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)
 
     if os.path.exists(install_script_path):
         print("Install: install script")

comfy_cli/registry/config_parser.py

@@ -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
@@ -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}"
+                    )
+                    continue
+                dependencies.append(line)
         project["dependencies"] = dependencies
     else:
         print("Warning: 'requirements.txt' not found. No dependencies will be added.")

comfy_cli/uv.py

@@ -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+)#.*$")
+
 
 def _req_re_closure(name: str) -> re.Pattern[str]:
     return re.compile(rf"({name}\S+)")
@@ -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

tests/comfy_cli/registry/test_config_parser.py

@@ -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"]

tests/comfy_cli/test_custom_nodes_python_resolution.py

@@ -1,3 +1,4 @@
+import os
 from unittest.mock import patch
 
 from comfy_cli.command.custom_nodes import command
@@ -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):

tests/uv/test_uv.py

@@ -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"
@@ -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"]
+
+
+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"]