diff --git a/src/lehrer/core/codejail.py b/src/lehrer/core/codejail.py index 4f764e37..69292299 100644 --- a/src/lehrer/core/codejail.py +++ b/src/lehrer/core/codejail.py @@ -1,8 +1,12 @@ """Generic codejail service build for Open edX operators.""" +import shlex + import dagger from dagger import dag, function, object_type +from lehrer.core.pip_compile_bridge import python_deps_install_script + @object_type class OpenedxCodejail: @@ -95,6 +99,7 @@ async def _build( "-y", "--no-install-recommends", "build-essential", + "curl", "python3-virtualenv", "python3-pip", "git", @@ -144,10 +149,12 @@ async def _build( .with_exec(["chown", "-R", "sandbox:sandbox", "/sandbox/venv"]) ) - # Update PATH to use virtualenv + # Update PATH to use virtualenv. VIRTUAL_ENV is also needed by any + # `uv sync --active` call below (python_deps_install_script's + # uv.lock path) -- it targets that env var, not just PATH. container = container.with_env_variable( "PATH", "/sandbox/venv/bin:/usr/local/bin:/usr/bin:/bin" - ) + ).with_env_variable("VIRTUAL_ENV", "/sandbox/venv") # Clone codejail service container = container.with_workdir("/codejail").with_exec( @@ -167,27 +174,74 @@ async def _build( sudoers_file = codejail_config.file("01-sandbox") container = container.with_file("/etc/sudoers.d/01-sandbox", sudoers_file) - # Install dependencies + # Install dependencies. codejailservice has no uv.lock today (plain + # pip-compile requirements/), but sourcing it through the same + # uv.lock-vs-legacy bridge as every other Python build here means + # this keeps working with no lehrer change if that ever changes -- + # see python_deps_install_script. container = container.with_exec( - ["pip", "install", "--no-cache-dir", "-r", "requirements/base.txt"] + [ + "sh", + "-c", + python_deps_install_script( + workdir="/codejail", + legacy_requirements=["requirements/base.txt"], + ensure_uv=True, + ), + ] ).with_exec(["pip", "install", "--no-cache-dir", "gunicorn"]) - # Install edx-platform sandbox requirements in virtualenv - # The URL pattern differs based on whether it's a release or master - sandbox_req_url = ( - f"https://raw.githubusercontent.com/openedx/edx-platform/master/requirements/edx-sandbox/releases/{release_name}.txt" - if release_name != "master" - else "https://raw.githubusercontent.com/openedx/edx-platform/master/requirements/edx-sandbox/base.txt" - ) - - import shlex + # Install edx-platform sandbox requirements in virtualenv. + # + # Named releases haven't picked up the pip-compile -> uv migration + # and keep publishing a per-release export; read it directly as + # before. master's sandbox is now its own standalone uv project + # (requirements/edx-sandbox/{pyproject.toml,uv.lock}), with + # requirements/edx-sandbox/base.txt kept only as a temporary + # machine-generated compat export slated for removal once known + # consumers (including this one) stop reading it -- see + # openedx/public-engineering#552. Try the compat export first (fast, + # no uv needed) and fall back to a uv sync of the sub-project once + # it's gone, so this keeps working across that removal without a + # lehrer change. --inexact on that sync is required: codejailservice's + # own requirements/base.txt and gunicorn (installed above, into this + # same /sandbox/venv) aren't part of the sandbox project's uv.lock, and + # a plain sync would prune them back out, leaving the image without + # its gunicorn entrypoint. + if release_name != "master": + sandbox_req_url = ( + "https://raw.githubusercontent.com/openedx/edx-platform/master/" + f"requirements/edx-sandbox/releases/{release_name}.txt" + ) + install_sandbox_reqs = ( + f"pip install --no-cache-dir -r {shlex.quote(sandbox_req_url)}" + ) + else: + sandbox_base_url = ( + "https://raw.githubusercontent.com/openedx/edx-platform/master/" + "requirements/edx-sandbox" + ) + install_sandbox_reqs = ( + "set -eu && " + "if curl -fsSL -o /tmp/edx_sandbox.txt " + f"{shlex.quote(sandbox_base_url + '/base.txt')}; then " + "pip install --no-cache-dir -r /tmp/edx_sandbox.txt; " + "else " + "mkdir -p /tmp/edx-sandbox-uv && cd /tmp/edx-sandbox-uv && " + "curl -fsSL -o pyproject.toml " + f"{shlex.quote(sandbox_base_url + '/pyproject.toml')} && " + f"curl -fsSL -o uv.lock {shlex.quote(sandbox_base_url + '/uv.lock')} && " + "pip install --quiet uv && " + "uv sync --locked --active --no-install-project --inexact; " + "fi" + ) container = container.with_exec( [ "bash", "-c", f"source /sandbox/venv/bin/activate && " - f"pip install --no-cache-dir -r {shlex.quote(sandbox_req_url)} && " + f"{install_sandbox_reqs} && " f"deactivate", ] ) diff --git a/src/lehrer/core/notes.py b/src/lehrer/core/notes.py index 3c02ccc2..d2cfd623 100644 --- a/src/lehrer/core/notes.py +++ b/src/lehrer/core/notes.py @@ -3,6 +3,8 @@ import dagger from dagger import dag, function, object_type +from lehrer.core.pip_compile_bridge import python_deps_install_script + @object_type class OpenedxNotes: @@ -106,9 +108,18 @@ async def _build( ] ) - # Set working directory and PATH - container = container.with_workdir("/app/edx-notes-api").with_env_variable( - "PATH", "/app/.local/bin:/usr/local/bin:/usr/bin:/bin" + # Set working directory and PATH. /opt/notes-venv holds the app's own + # deps -- a real venv (not just system site-packages) is what + # `uv sync --active` needs a target for, should edx-notes-api's own + # requirements ever migrate to uv.lock. See python_deps_install_script. + container = ( + container.with_workdir("/app/edx-notes-api") + .with_exec(["python3", "-m", "venv", "/opt/notes-venv"]) + .with_env_variable( + "PATH", + "/opt/notes-venv/bin:/app/.local/bin:/usr/local/bin:/usr/bin:/bin", + ) + .with_env_variable("VIRTUAL_ENV", "/opt/notes-venv") ) # Get edx-notes-api source from local directory or Git @@ -130,12 +141,22 @@ async def _build( else: raise ValueError("Must provide either notes_code or notes_repo") - # Install Python dependencies as root (system site-packages), then fix ownership + # Install Python dependencies (uv sync against a uv.lock, or the + # legacy pip-compile requirements/base.txt, whichever the checkout + # has -- see python_deps_install_script), then fix ownership. container = ( container.with_exec( - ["pip", "install", "--no-cache-dir", "-r", "requirements/base.txt"] + [ + "sh", + "-c", + python_deps_install_script( + workdir="/app/edx-notes-api", + legacy_requirements=["requirements/base.txt"], + ensure_uv=True, + ), + ] ) - .with_exec(["chown", "-R", "app:app", "/app"]) + .with_exec(["chown", "-R", "app:app", "/app", "/opt/notes-venv"]) .with_user("1000") ) diff --git a/src/lehrer/core/pip_compile_bridge.py b/src/lehrer/core/pip_compile_bridge.py new file mode 100644 index 00000000..703fd716 --- /dev/null +++ b/src/lehrer/core/pip_compile_bridge.py @@ -0,0 +1,90 @@ +"""Shared pip-compile -> uv.lock bridge for Python service builds. + +Several Open edX services are (or may eventually be) migrating their Python +dependency management from pip-compile-generated ``requirements/*.txt`` +files to ``pyproject.toml`` + ``uv.lock`` as the source of truth -- +openedx-platform's own migration is tracked in +openedx/public-engineering#552. Every Python build in lehrer sources its +dependencies through :func:`python_deps_install_script`, which detects the +track from the checkout itself (``uv.lock`` present or not) rather than +hardcoding which release or service has migrated. That keeps a build +working across a service's migration landing without a lehrer change -- +and costs nothing for a checkout that never migrates: the branch just +always takes the legacy arm. +""" + +import shlex + + +def python_deps_install_script( + *, + workdir: str, + legacy_requirements: list[str], + sync_groups: list[str] | None = None, + legacy_installer: list[str] | None = None, + ensure_uv: bool = False, +) -> str: + """Shell script installing a checkout's Python deps. + + Runs ``uv sync --locked`` against a committed ``uv.lock`` when present, + otherwise falls back to installing the legacy pip-compile + ``requirements/*.txt`` files. + + ``--inexact`` is always passed on the sync path: callers commonly layer + other installs into the same environment around this call -- a + deployment's own pinned package list, another service's base + requirements, an editable framework checkout installed just before it. + A plain (exact) sync prunes anything not in the checkout's own lock + resolution, which would silently strip those back out. + + Args: + workdir: Directory containing the checkout (``pyproject.toml``/ + ``uv.lock`` or the legacy ``requirements/`` tree). + legacy_requirements: Requirement file paths (relative to + ``workdir``) to install when no ``uv.lock`` is present. + sync_groups: Dependency groups to select when ``uv.lock`` IS + present, via ``--no-default-groups --group `` for each. + Omit for just the project's own default dependencies. + legacy_installer: Command prefix for the legacy branch. Default + ``["pip", "install", "--no-cache-dir"]``. Pass + ``["uv", "pip", "install"]`` for a container where uv is + already the installer of record. + ensure_uv: Install uv via pip before the sync branch runs. Default + ``False`` for containers (e.g. the edx-platform build) that + already have uv on PATH; set ``True`` for containers (e.g. + codejail, notes) that only have plain pip. + + Must run with ``VIRTUAL_ENV`` set to the target environment -- the sync + branch uses ``--active``. + """ + installer = list(legacy_installer or ["pip", "install", "--no-cache-dir"]) + legacy_args: list[str] = [] + for req in legacy_requirements: + legacy_args += ["-r", req] + legacy_cmd = " ".join(shlex.quote(part) for part in [*installer, *legacy_args]) + + sync_parts = [ + "uv", + "sync", + "--locked", + "--active", + "--no-install-project", + "--inexact", + ] + if sync_groups: + sync_parts.append("--no-default-groups") + for group in sync_groups: + sync_parts += ["--group", group] + sync_cmd = " ".join(shlex.quote(part) for part in sync_parts) + if ensure_uv: + sync_cmd = f"pip install --quiet uv && {sync_cmd}" + + return ( + "set -eu\n" + f"cd {shlex.quote(workdir)}\n" + "if [ -f uv.lock ]; then\n" + f" {sync_cmd}\n" + "else\n" + f" {legacy_cmd}\n" + "fi\n" + ) diff --git a/src/lehrer/core/platform.py b/src/lehrer/core/platform.py index edc164c4..d84034b9 100644 --- a/src/lehrer/core/platform.py +++ b/src/lehrer/core/platform.py @@ -18,6 +18,7 @@ from dagger import dag, function, object_type from lehrer.core.build_manifest import BuildManifest, Cell +from lehrer.core.pip_compile_bridge import python_deps_install_script from lehrer.core.plugin_imports import plugin_distributions from lehrer.core.plugin_tests import ( combined_pytest_script, @@ -457,6 +458,55 @@ def _derive_test_settings(service: str) -> str: ) +def _edx_base_deps_script(*, include_dev: bool = False) -> str: + """Shell script that installs edx-platform's base + assets (+ dev) deps. + + Delegates to :func:`python_deps_install_script` -- see its docstring for + the ``uv.lock``-presence branch and ``--inexact`` rationale, and + openedx/public-engineering#552 for openedx-platform's own migration. + Named releases haven't picked up the migration and won't grow a + ``uv.lock`` until they do, so this keeps both tracks working, and picks + up master's migration automatically without a lehrer change once it + lands. + + ``include_dev`` pulls in openedx-platform's ``dev`` dependency group + (mypy, tox, django-debug-toolbar, ...) — or, pre-migration, + ``requirements/edx/development.txt`` — for local-dev builds; production + builds leave it out (the default). + + Must run with ``VIRTUAL_ENV`` set to the target environment. + """ + legacy_requirements = [ + "requirements/edx/base.txt", + "requirements/edx/assets.txt", + ] + sync_groups = ["assets"] + if include_dev: + legacy_requirements.append("requirements/edx/development.txt") + sync_groups.append("dev") + return python_deps_install_script( + workdir="/openedx/edx-platform", + legacy_requirements=legacy_requirements, + sync_groups=sync_groups, + legacy_installer=["uv", "pip", "install"], + ) + + +def _edx_testing_deps_script() -> str: + """Shell script that installs edx-platform's test-suite deps. + + Runs after ``install_deps`` has already layered the deployment's own + packages into the environment, so ``python_deps_install_script``'s + ``--inexact`` sync is what keeps those from being pruned back out. + """ + return python_deps_install_script( + workdir="/openedx/edx-platform", + legacy_requirements=["requirements/edx/testing.txt"], + sync_groups=["testing"], + legacy_installer=["uv", "pip", "install"], + ) + + @object_type class OpenedxPlatform: """Generic edx-platform build pipeline. @@ -684,6 +734,7 @@ def install_deps( packages_to_remove: list[str] | None = None, extra_npm_packages: list[str] | None = None, install_node: bool = True, + include_dev_dependencies: bool = False, # noqa: FBT001, FBT002 ) -> dagger.Container: """Install Python and Node.js dependencies using uv @@ -707,6 +758,11 @@ def install_deps( ``False`` for Python-only consumers (e.g. plugin import checks, settings regeneration) that never build webpack assets — the Python environment above is complete without it. + include_dev_dependencies: Also install edx-platform's ``dev`` + dependency group (mypy, tox, django-debug-toolbar, ...) — or, + pre-migration, ``requirements/edx/development.txt``. Default + ``False`` for production builds; set ``True`` for local-dev + images. Returns: Container with all dependencies installed @@ -723,33 +779,23 @@ def install_deps( .with_mounted_directory( "/root/pip_package_overrides", pip_package_overrides ) - # Copy base requirements from edx-platform + # Install base + assets (+ dev) deps from edx-platform (uv sync + # against uv.lock, or the legacy pip-compile .txt, whichever the + # checkout has), then layer the deployment's own pinned package + # list on top. .with_exec( [ "sh", "-c", - "cp /openedx/edx-platform/requirements/edx/base.txt /root/pip_package_lists/edx_base.txt", + _edx_base_deps_script(include_dev=include_dev_dependencies), ] ) - .with_exec( - [ - "sh", - "-c", - "cp /openedx/edx-platform/requirements/edx/assets.txt /root/pip_package_lists/edx_assets.txt", - ] - ) - # Install base Python dependencies using uv (much faster than pip) - # uv automatically uses the VIRTUAL_ENV set in apt_base .with_exec( [ "uv", "pip", "install", "-r", - "/root/pip_package_lists/edx_base.txt", - "-r", - "/root/pip_package_lists/edx_assets.txt", - "-r", f"/root/pip_package_lists/{release_name}/{deployment_name}.txt", ] ) @@ -1574,40 +1620,29 @@ def _python_only_env( edx_platform_git_repo=cell.platform_repo, edx_platform_git_branch=cell.platform_branch, ) - container = ( - container.with_mounted_directory( - "/root/pip_package_lists", cell.pip_package_lists - ) - .with_mounted_directory( - "/root/pip_package_overrides", cell.pip_package_overrides - ) - .with_exec( - [ - "sh", - "-c", - "cp /openedx/edx-platform/requirements/edx/base.txt" - " /root/pip_package_lists/edx_base.txt" - " && cp /openedx/edx-platform/requirements/edx/assets.txt" - " /root/pip_package_lists/edx_assets.txt", - ] - ) + container = container.with_mounted_directory( + "/root/pip_package_lists", cell.pip_package_lists + ).with_mounted_directory( + "/root/pip_package_overrides", cell.pip_package_overrides ) + # Installed *before* the base deps sync/install below so its version + # satisfies the pinned django-aqueduct== constraint and uv skips the + # PyPI fetch. The base step's --inexact flag is what keeps this from + # being pruned back out. if aqueduct_source is not None: container = container.with_mounted_directory( "/root/django-aqueduct", aqueduct_source ).with_exec(["uv", "pip", "install", "-e", "/root/django-aqueduct"]) container = container.with_exec( + ["sh", "-c", _edx_base_deps_script()] + ).with_exec( [ "uv", "pip", "install", "-r", - "/root/pip_package_lists/edx_base.txt", - "-r", - "/root/pip_package_lists/edx_assets.txt", - "-r", f"/root/pip_package_lists/{release_name}/{deployment_name}.txt", ] ) @@ -1668,6 +1703,7 @@ async def build_platform( extra_npm_packages: list[str] | None = None, verify_boot: bool = True, # noqa: FBT001, FBT002 strict_translations: bool = False, # noqa: FBT001, FBT002 + include_dev_dependencies: bool = False, # noqa: FBT001, FBT002 ) -> dagger.Container: """Build a complete openedx-platform image @@ -1717,6 +1753,11 @@ async def build_platform( pull/compile step fails, instead of warning (default: False — a missing plugin translation is normal, so this is opt-in until the per-step baseline is known). + include_dev_dependencies: Also install edx-platform's ``dev`` + dependency group (mypy, tox, django-debug-toolbar, ...) — or, + pre-migration, ``requirements/edx/development.txt``. Default + ``False`` for production builds; set ``True`` to build a + local-dev image. Returns: Container ready to be deployed @@ -1828,6 +1869,7 @@ async def build_platform( node_version=node_version, packages_to_remove=packages_to_remove, extra_npm_packages=extra_npm_packages, + include_dev_dependencies=include_dev_dependencies, ) # ── Clean base ──────────────────────────────────────────────────────── @@ -2294,7 +2336,7 @@ async def test( # noqa: PLR0913 container = ( container.with_workdir("/openedx/edx-platform") .with_exec(["uv", "pip", "install", "-e", "."]) - .with_exec(["uv", "pip", "install", "-r", "requirements/edx/testing.txt"]) + .with_exec(["sh", "-c", _edx_testing_deps_script()]) ) # When folding the plugins' own suites into this run, derive the plugin diff --git a/tests/core/test_pip_compile_bridge.py b/tests/core/test_pip_compile_bridge.py new file mode 100644 index 00000000..35b7f942 --- /dev/null +++ b/tests/core/test_pip_compile_bridge.py @@ -0,0 +1,67 @@ +from __future__ import annotations + +from lehrer.core.pip_compile_bridge import python_deps_install_script + + +def test_branches_on_uv_lock_presence() -> None: + script = python_deps_install_script( + workdir="/app/service", + legacy_requirements=["requirements/base.txt"], + ) + assert "cd /app/service" in script + assert "if [ -f uv.lock ]; then" in script + assert "uv sync --locked --active --no-install-project --inexact" in script + assert "pip install --no-cache-dir -r requirements/base.txt" in script + + +def test_multiple_legacy_requirements_each_get_their_own_flag() -> None: + script = python_deps_install_script( + workdir=".", + legacy_requirements=["requirements/base.txt", "requirements/assets.txt"], + ) + assert ( + "pip install --no-cache-dir -r requirements/base.txt" + " -r requirements/assets.txt" in script + ) + + +def test_sync_is_always_inexact() -> None: + # Load-bearing: callers commonly layer other installs (a deployment's own + # package list, an editable framework checkout, another service's base + # requirements) into the same environment around this call. A plain + # (exact) sync prunes anything not in the checkout's own lock resolution, + # which would silently strip those back out. + script = python_deps_install_script(workdir=".", legacy_requirements=[]) + assert "--inexact" in script + + +def test_no_sync_groups_by_default() -> None: + script = python_deps_install_script(workdir=".", legacy_requirements=[]) + assert "--group" not in script + assert "--no-default-groups" not in script + + +def test_sync_groups_use_no_default_groups() -> None: + script = python_deps_install_script( + workdir=".", legacy_requirements=[], sync_groups=["assets", "dev"] + ) + assert "--no-default-groups --group assets --group dev" in script + + +def test_legacy_installer_override() -> None: + script = python_deps_install_script( + workdir=".", + legacy_requirements=["requirements/base.txt"], + legacy_installer=["uv", "pip", "install"], + ) + assert "uv pip install -r requirements/base.txt" in script + assert "pip install --no-cache-dir" not in script + + +def test_ensure_uv_installs_uv_before_sync() -> None: + without = python_deps_install_script(workdir=".", legacy_requirements=[]) + with_ensure = python_deps_install_script( + workdir=".", legacy_requirements=[], ensure_uv=True + ) + assert "pip install --quiet uv" not in without + assert "pip install --quiet uv && uv sync" in with_ensure diff --git a/tests/core/test_platform_test_helpers.py b/tests/core/test_platform_test_helpers.py index 6993cfec..0811c43b 100644 --- a/tests/core/test_platform_test_helpers.py +++ b/tests/core/test_platform_test_helpers.py @@ -4,7 +4,12 @@ import pytest -from lehrer.core.platform import _derive_test_settings, _test_paths +from lehrer.core.platform import ( + _derive_test_settings, + _edx_base_deps_script, + _edx_testing_deps_script, + _test_paths, +) @pytest.mark.parametrize("service", ["lms", "cms"]) @@ -44,3 +49,50 @@ def test_derive_test_settings_is_valid_python(service: str) -> None: def test_derive_test_settings_rejects_unknown_service() -> None: with pytest.raises(ValueError, match="service must be one of"): _derive_test_settings("workers") + + +def test_edx_base_deps_script_branches_on_uv_lock() -> None: + script = _edx_base_deps_script() + assert "cd /openedx/edx-platform" in script + assert "if [ -f uv.lock ]; then" in script + # uv.lock branch: a direct sync, no intermediate requirements file. + assert "uv sync --locked --active --no-install-project" in script + # legacy branch: same base+assets requirements the pre-migration path used. + assert ( + "uv pip install -r requirements/edx/base.txt" + " -r requirements/edx/assets.txt" in script + ) + + +def test_edx_base_deps_script_sync_is_inexact() -> None: + # Load-bearing: callers layer an editable django-aqueduct install and the + # deployment's own pinned package list on top of this sync, before or + # after. A plain (exact) sync prunes anything not in edx-platform's own + # lock resolution, which would silently strip those back out. + script = _edx_base_deps_script() + assert "--inexact" in script + + +def test_edx_base_deps_script_excludes_dev_group_by_default() -> None: + script = _edx_base_deps_script() + assert "--group assets" in script + assert "--group dev" not in script + assert "development.txt" not in script + + +def test_edx_base_deps_script_include_dev() -> None: + script = _edx_base_deps_script(include_dev=True) + assert "--group assets" in script + assert "--group dev" in script + # legacy branch also needs the pre-migration development.txt equivalent. + assert "-r requirements/edx/development.txt" in script + + +def test_edx_testing_deps_script() -> None: + script = _edx_testing_deps_script() + assert "cd /openedx/edx-platform" in script + assert "if [ -f uv.lock ]; then" in script + assert "--inexact" in script + assert "--group testing" in script + # legacy branch: same testing.txt the pre-migration path installed. + assert "uv pip install -r requirements/edx/testing.txt" in script