diff --git a/Dockerfile b/Dockerfile index 9b5d88e06..13db00df0 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,6 +3,22 @@ # For maximum integrity, set this to an immutable digest in CI/CD. ARG SWIPL_IMAGE=docker.io/library/swipl:10.0.2 +# Only plugin requirements reach the install step, so editing plugin code keeps its cache. +FROM ${SWIPL_IMAGE} AS plugin-requirements-collector + +COPY plugins /plugins +RUN mkdir -p /plugin-requirements \ + && for declared in /plugins/*/requirements.txt; do \ + [ -f "$declared" ] || continue; \ + plugin="$(basename "$(dirname "$declared")")"; \ + mkdir -p "/plugin-requirements/$plugin"; \ + cp -p "$declared" "/plugin-requirements/$plugin/requirements.txt"; \ + done + +FROM scratch AS plugin-requirements + +COPY --from=plugin-requirements-collector /plugin-requirements / + FROM ${SWIPL_IMAGE} AS builder SHELL ["/bin/bash", "-o", "pipefail", "-c"] @@ -52,12 +68,18 @@ RUN sh build.sh RUN mkdir -p /PeTTa/repos \ && git clone --depth 1 --branch "${CHROMADB_REF}" "${CHROMADB_REPO}" /PeTTa/repos/petta_lib_chromadb -COPY ./requirements.txt /tmp/requirements.txt -RUN python3 -m pip install --no-cache-dir --break-system-packages \ +# Torch comes from the CPU wheel index; its version is read from requirements.txt, not repeated here. +RUN --mount=type=bind,source=requirements.txt,target=/tmp/omega/requirements.txt \ + python3 -m pip install --no-cache-dir --break-system-packages \ --index-url https://download.pytorch.org/whl/cpu \ --extra-index-url https://pypi.org/simple/ \ - torch==2.12.1 \ - && python3 -m pip install --no-cache-dir --break-system-packages -r /tmp/requirements.txt + "$(grep -E '^torch(\[|[=<>!~]|$)' /tmp/omega/requirements.txt)" + +# Core's requirements and every plugin's, resolved together so a conflict fails the build. +RUN --mount=type=bind,source=requirements.txt,target=/tmp/omega/requirements.txt \ + --mount=type=bind,from=plugin-requirements,target=/tmp/omega/plugins \ + --mount=type=bind,source=scripts/install_dependencies.sh,target=/tmp/omega/scripts/install_dependencies.sh \ + /tmp/omega/scripts/install_dependencies.sh --no-cache-dir --break-system-packages # Pre-download the sentence-transformers model so runtime does not need network access. RUN mkdir -p "${HF_HOME}" "${SENTENCE_TRANSFORMERS_HOME}" \ diff --git a/README.md b/README.md index e0ced94a2..44f7ae323 100644 --- a/README.md +++ b/README.md @@ -57,12 +57,13 @@ source ./.venv/bin/activate If you have CPU only machine or don't want calculate embeddings on GPU: ``` -python3 -m pip install --index-url https://download.pytorch.org/whl/cpu torch +python3 -m pip install --index-url https://download.pytorch.org/whl/cpu \ + "$(grep -E '^torch(\[|[=<>!~]|$)' ./repos/Omega/requirements.txt)" ``` -Install Python dependencies: +Install Python dependencies — core's, and those of any plugin that declares its own: ``` -python3 -m pip install -r ./repos/Omega/requirements.txt +./repos/Omega/scripts/install_dependencies.sh ``` --- diff --git a/docs/reference-plugin-api.md b/docs/reference-plugin-api.md index b5427d6ac..2e69b21de 100644 --- a/docs/reference-plugin-api.md +++ b/docs/reference-plugin-api.md @@ -33,6 +33,34 @@ module. The plugin record has the following fields: `name` module is located. Can include `{REPO}` placeholder to designate the root folder of the Omega source repository. +## Plugin dependencies + +A plugin that imports a third-party package declares it in a `requirements.txt` +in the plugin's own directory. `scripts/install_dependencies.sh` installs those +alongside Omega's own, and both the image build and the source install in +[README.md](/README.md#installation) call it. A plugin that imports nothing +outside the standard library and Omega ships no such file and needs no entry +anywhere. + +Everything is installed by a single pip invocation, so Omega's own pins win. They +are exact, and a plugin asking for a different version of a package Omega pins +fails the install with a conflict rather than replacing it. Declare ranges wide +enough to include Omega's pin — `openai>=1.0.0` rather than `openai==2.1.0` — +and list only what the plugin actually imports. + +The file holds requirements and nothing else. pip reads an option written inside +a requirements file — `--index-url` and `--extra-index-url` above all — as an +instruction for the whole invocation rather than for the file carrying it, which +would let one plugin choose where Omega's own packages are fetched from. Sharing +one invocation is what makes Omega's pins win, so there is nowhere to scope such +a line to, and a plugin that carries one stops the install with the line quoted. + +Two limits are worth knowing. A plugin that pins a package Omega depends on +*indirectly* can change it without any error, because nothing is violated: keep +the list short for this reason. And a plugin mounted into an already built image +gets no installation at all, since the packages are baked in at build time — that +route needs an image built with the plugin present. + As an example of a MeTTa plugin one can look at the code of the [workflow plugin](/plugins/workflow/workflow.metta). As an example of a Python plugin one can look at the code of the [IRC communication channel](/channels/irc.py). diff --git a/plugins/openclaw/requirements.txt b/plugins/openclaw/requirements.txt new file mode 100644 index 000000000..f2293605c --- /dev/null +++ b/plugins/openclaw/requirements.txt @@ -0,0 +1 @@ +requests diff --git a/scripts/install_dependencies.sh b/scripts/install_dependencies.sh new file mode 100755 index 000000000..ce7fa639b --- /dev/null +++ b/scripts/install_dependencies.sh @@ -0,0 +1,50 @@ +#!/bin/sh +# Install core's Python dependencies, and those of every plugin that declares any. +# +# A plugin is a directory under plugins/. One that ships a requirements.txt gets +# it installed without core naming the plugin; one that ships none costs nothing. +# That file lists requirements and nothing else — see refuse_pip_options. +# +# Everything reaches a single pip invocation. Separate invocations resolve +# separately, so a plugin pinning a version core also pins would replace core's +# copy in silence — core is not a pip distribution, so nothing records what it +# needed and no warning fires. Resolved together, a real conflict fails here +# rather than at import time, and pip check catches what resolution let through. +# +# Arguments are passed through to pip, so an image build can add its own: +# +# ./scripts/install_dependencies.sh +# ./scripts/install_dependencies.sh --no-cache-dir --break-system-packages +set -eu + +repo="$(CDPATH= cd -- "$(dirname -- "$0")/.." && pwd)" + +# Stop on a plugin file that carries pip options instead of requirements. +# +# pip applies an option it reads inside a requirements file to the whole +# invocation rather than to the file carrying it, and it overrides the same +# option given on the command line. One plugin shipping an --index-url line +# therefore decides where every package is fetched from, core's own pinned ones +# included, and the install still reports success. Sharing one invocation is +# what makes core's pins win, so there is nowhere to scope such an option to, +# and the file is refused instead. +refuse_pip_options() { + options="$(grep -n '^[[:space:]]*-' "$1" || :)" + if [ -n "$options" ]; then + echo "$1: a plugin lists requirements here, and nothing else." >&2 + echo "pip would apply these to the whole install, core's own packages included:" >&2 + echo "$options" >&2 + exit 1 + fi +} + +set -- "$@" -r "$repo/requirements.txt" +for plugin_requirements in "$repo"/plugins/*/requirements.txt; do + if [ -f "$plugin_requirements" ]; then + refuse_pip_options "$plugin_requirements" + set -- "$@" -r "$plugin_requirements" + fi +done + +python3 -m pip install "$@" +python3 -m pip check diff --git a/tests/test_plugin_requirements.py b/tests/test_plugin_requirements.py new file mode 100644 index 000000000..225e6f7d3 --- /dev/null +++ b/tests/test_plugin_requirements.py @@ -0,0 +1,321 @@ +"""Installing the dependencies each plugin declares. + +A plugin is a directory under plugins/ that core loads at startup, and until now +the one thing it could not say was what it needs to import. Only core's own +requirements.txt reached pip, so a plugin's imports either happened to be +satisfied by core's tree or the loader failed and took start-up with it. +plugins/openclaw/openclaw.py imports requests, which no requirements file +declares; it works because chromadb pulls it in. + +scripts/install_dependencies.sh walks whatever plugins are present and adds each +requirements.txt it finds to the same pip invocation as core's own. Two details +carry most of the weight, so most of these tests are about them: the invocation +is single, and pip check runs after it. + +The script is the one definition of what installing Omega's dependencies means. +The image build and the README's source install both call it, and the last two +tests are what stops either of them growing a second, quietly different copy. +""" +import os +import re +import shutil +import subprocess +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +SCRIPT = REPO_ROOT / "scripts" / "install_dependencies.sh" +DOCKERFILE = REPO_ROOT / "Dockerfile" +README = REPO_ROOT / "README.md" +PLUGIN_API_DOC = REPO_ROOT / "docs" / "reference-plugin-api.md" +CORE_REQUIREMENTS = REPO_ROOT / "requirements.txt" + + +def _fake_repo(root, plugins): + """A tree shaped like core, holding the script and the requirements it walks. + + `plugins` maps a plugin directory name to the text of its requirements.txt, + or to None for a plugin that ships none. Omit plugins entirely by passing + None instead of a mapping, which leaves out the plugins directory. + """ + (root / "scripts").mkdir(parents=True) + shutil.copy(SCRIPT, root / "scripts" / SCRIPT.name) + (root / "requirements.txt").write_text("chromadb==1.3.0\n", encoding="utf-8") + + if plugins is not None: + (root / "plugins").mkdir() + for name, requirements in plugins.items(): + (root / "plugins" / name).mkdir() + if requirements is not None: + (root / "plugins" / name / "requirements.txt").write_text( + requirements, encoding="utf-8" + ) + return root + + +def _stub_pip(bin_dir, log, failing_subcommand): + """A python3 on PATH that records how pip was called instead of calling it.""" + bin_dir.mkdir() + stub = bin_dir / "python3" + fail = "" + if failing_subcommand: + fail = f'case " $* " in *" {failing_subcommand} "*) exit 1 ;; esac\n' + stub.write_text( + "#!/bin/sh\n" + f'printf "%s\\n" "$*" >> "{log}"\n' + f"{fail}" + "exit 0\n", + encoding="utf-8", + ) + stub.chmod(0o755) + return bin_dir + + +def _install(tmp_path, plugins, *pip_options, failing_subcommand=None): + """Run the script against a made-up tree; hand back its exit and pip's calls.""" + repo = _fake_repo(tmp_path / "omega", plugins) + log = tmp_path / "pip.log" + bin_dir = _stub_pip(tmp_path / "bin", log, failing_subcommand) + + result = subprocess.run( + [str(repo / "scripts" / SCRIPT.name), *pip_options], + env={"PATH": f"{bin_dir}:/usr/bin:/bin"}, + capture_output=True, + text=True, + check=False, + ) + calls = log.read_text(encoding="utf-8").splitlines() if log.exists() else [] + return repo, result, calls + + +def _installs(calls): + """Just the install calls, leaving out pip check and anything else.""" + return [call for call in calls if "pip install" in call] + + +def _unwrapped(text): + """The text's lines, with a command wrapped over several read as the one it is. + + A shell command wraps with a trailing backslash, in the README and in the + Dockerfile alike, so what a command carries often sits on a different line + from the command's name. + """ + commands = [] + for line in text.splitlines(): + if commands and commands[-1].endswith("\\"): + commands[-1] = commands[-1][:-1].rstrip() + " " + line.strip() + else: + commands.append(line) + return commands + + +def test_core_and_every_plugin_resolve_in_one_invocation(tmp_path): + """Separate pip runs resolve separately, and the loser is silent. + + A plugin that pins a version core also pins would, in its own invocation, + replace core's copy without complaint: core is not a pip distribution, so no + metadata records what it needed and nothing warns. Resolving everything at + once turns that into a failure instead. + """ + repo, result, calls = _install( + tmp_path, {"telegram": "aiogram>=3.0.0\n", "openclaw": "requests>=2.31.0\n"} + ) + + assert result.returncode == 0, result.stderr + assert len(_installs(calls)) == 1, calls + installed = _installs(calls)[0] + assert f"-r {repo}/requirements.txt" in installed + assert f"-r {repo}/plugins/telegram/requirements.txt" in installed + assert f"-r {repo}/plugins/openclaw/requirements.txt" in installed + + +def test_a_plugin_that_declares_nothing_costs_nothing(tmp_path): + """Neither plugin core ships today has a requirements.txt, so this is the + ordinary case, not an edge one.""" + repo, result, calls = _install(tmp_path, {"openclaw": None, "workflow": None}) + + assert result.returncode == 0, result.stderr + assert _installs(calls) == [ + f"-m pip install -r {repo}/requirements.txt" + ] + + +def test_a_tree_with_no_plugins_at_all_still_installs(tmp_path): + """A glob that matches nothing is a literal string in sh, which must not be + passed to pip or tested as a file. Neither an empty plugins directory nor a + missing one is an error: core's dependencies still install.""" + for plugins in ({}, None): + repo, result, calls = _install(tmp_path / str(plugins), plugins) + + assert result.returncode == 0, result.stderr + assert _installs(calls) == [f"-m pip install -r {repo}/requirements.txt"] + + +def test_pip_options_reach_pip(tmp_path): + """The image build needs its own flags, and passing them through is what + lets it share this script with a source install that wants none of them.""" + _, result, calls = _install( + tmp_path, {}, "--no-cache-dir", "--break-system-packages" + ) + + assert result.returncode == 0, result.stderr + assert _installs(calls)[0].startswith( + "-m pip install --no-cache-dir --break-system-packages -r " + ) + + +def test_the_result_is_checked(tmp_path): + """pip resolves what it was asked for and reports conflicts it could not + avoid; pip check is what turns the second into a failure.""" + _, result, calls = _install(tmp_path, {"telegram": "aiogram>=3.0.0\n"}) + + assert result.returncode == 0, result.stderr + assert calls[-1].split() == ["-m", "pip", "check"] + + +def test_an_unsatisfied_dependency_fails(tmp_path): + """The point of the check is the exit code, so the script has to stop on it.""" + _, result, _ = _install( + tmp_path, {"telegram": "aiogram>=3.0.0\n"}, failing_subcommand="check" + ) + + assert result.returncode != 0 + + +def test_an_install_failure_stops_before_the_check(tmp_path): + """A conflict pip refuses outright must not be followed by a check that + passes on the packages that did land.""" + _, result, calls = _install( + tmp_path, {"telegram": "aiogram>=3.0.0\n"}, failing_subcommand="install" + ) + + assert result.returncode != 0 + assert not [call for call in calls if "pip check" in call] + + +def test_a_plugin_cannot_choose_where_packages_come_from(tmp_path): + """pip reads an option written inside a requirements file as an instruction + for the whole invocation, and it beats the same option on the command line. + A plugin shipping one picks the index core's own pinned packages come from, + and the install still succeeds, so the file is refused before pip runs.""" + hijack = "--index-url https://elsewhere.example/simple\nrequests\n" + _, result, calls = _install(tmp_path, {"greedy": hijack}) + + assert result.returncode != 0, "the install went ahead" + assert not _installs(calls), "pip installed something anyway" + + +def test_the_refused_line_is_quoted(tmp_path): + """Whoever wrote the plugin has to be able to find it, so core names the + file, the line number and the line itself.""" + carried = "requests\n--extra-index-url https://elsewhere.example\n" + _, result, _ = _install(tmp_path, {"greedy": carried}) + + assert "plugins/greedy/requirements.txt" in result.stderr + assert "2:--extra-index-url https://elsewhere.example" in result.stderr + + +def test_a_comment_is_not_an_option(tmp_path): + """Refusing options must not refuse an ordinary file that explains itself.""" + ordinary = "# what the plugin imports\n\nrequests>=2.0\n" + _, result, calls = _install(tmp_path, {"polite": ordinary}) + + assert result.returncode == 0, result.stderr + assert _installs(calls), "an ordinary plugin file was not installed" + + +def test_the_script_names_no_plugin(): + """Core walks whatever is there. Naming a plugin would make the next one a + core change again, which is the whole problem.""" + present = sorted( + path.name for path in (REPO_ROOT / "plugins").iterdir() if path.is_dir() + ) + assert present, "no plugins in the tree; this test has nothing to check" + text = SCRIPT.read_text(encoding="utf-8") + DOCKERFILE.read_text(encoding="utf-8") + named = [ + name for name in present + if re.search(rf"(?!~]", dockerfile), \ + "the Dockerfile pins a torch version of its own" + assert re.search(r"^torch[=<>!~]", CORE_REQUIREMENTS.read_text(encoding="utf-8"), re.M), \ + "the CPU-index step reads torch's version from requirements.txt, which does not pin it" + + +def test_the_readme_installs_the_torch_version_core_pins(): + """The CPU wheel index serves a newer torch than core pins, so installing it + unpinned and then installing requirements.txt swaps the CPU build for PyPI's + CUDA one — a multi-gigabyte download, no error, and the wrong wheel.""" + for command in _unwrapped(README.read_text(encoding="utf-8")): + if "download.pytorch.org" in command: + assert "requirements.txt" in command, \ + f"README installs torch without core's pin: {command}" + + +def test_the_plugin_api_documents_declaring_dependencies(): + """The contract a plugin author reads. Without it the file is a convention + that happens to work, which is how the gap went unnoticed.""" + doc = PLUGIN_API_DOC.read_text(encoding="utf-8") + assert "requirements.txt" in doc + assert SCRIPT.name in doc