diff --git a/backend/app/api/routes/updates.py b/backend/app/api/routes/updates.py index 382252855..2e0a8f031 100644 --- a/backend/app/api/routes/updates.py +++ b/backend/app/api/routes/updates.py @@ -425,7 +425,13 @@ async def _perform_update(): "error": None, } - # Install Python dependencies + # Install Python dependencies — must run from the source-code directory + # (where requirements.txt lives), not the data dir. On native installs + # systemd sets DATA_DIR=INSTALL_PATH/data, so `base_dir` is the data dir, + # not the working tree. `git reset` above worked from base_dir because + # git walks up looking for .git, but `pip install -r requirements.txt` + # needs the file in cwd literally. + app_dir = settings.app_dir process = await asyncio.create_subprocess_exec( sys.executable, "-m", @@ -434,7 +440,7 @@ async def _perform_update(): "-r", "requirements.txt", "-q", - cwd=str(base_dir), + cwd=str(app_dir), stdout=asyncio.subprocess.PIPE, stderr=asyncio.subprocess.PIPE, ) @@ -445,7 +451,7 @@ async def _perform_update(): # Try to build frontend if npm is available (optional - static files are pre-built) npm_path = _find_executable("npm") - frontend_dir = base_dir / "frontend" + frontend_dir = app_dir / "frontend" if npm_path and frontend_dir.exists(): _update_status = { diff --git a/backend/app/core/config.py b/backend/app/core/config.py index a88836458..ae9692b5d 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -60,6 +60,12 @@ class Settings(BaseSettings): # Paths base_dir: Path = _data_dir # For backwards compatibility + # `app_dir` is where the source code is checked out — distinct from `base_dir` + # on native installs where DATA_DIR is set to a sibling like INSTALL_PATH/data. + # Use this when you need the working tree (requirements.txt, frontend/, etc.) + # rather than the data dir. On Docker / local dev where DATA_DIR is unset, + # app_dir == base_dir. + app_dir: Path = _app_dir archive_dir: Path = _data_dir / "archive" plate_calibration_dir: Path = _plate_cal_dir # Plate detection references static_dir: Path = _app_dir / "static" # Static files are part of app, not data diff --git a/backend/tests/integration/test_updates_api.py b/backend/tests/integration/test_updates_api.py index e52f28e63..f09df69a0 100644 --- a/backend/tests/integration/test_updates_api.py +++ b/backend/tests/integration/test_updates_api.py @@ -1,6 +1,7 @@ """Integration tests for Updates API endpoints.""" -from unittest.mock import AsyncMock, patch +from pathlib import Path +from unittest.mock import AsyncMock, MagicMock, patch import pytest from httpx import AsyncClient @@ -45,3 +46,64 @@ class TestUpdatesAPI: from backend.app.api.routes.updates import is_newer_version assert is_newer_version("0.1.5", "0.1.5b7") is True + + @pytest.mark.asyncio + async def test_perform_update_runs_pip_in_app_dir_not_data_dir(self, tmp_path): + """Native install: `requirements.txt` lives at INSTALL_PATH (the source- + code dir), NOT at DATA_DIR (where systemd sets DATA_DIR=INSTALL_PATH/data). + Pre-fix, the updater ran `pip install -r requirements.txt` with + `cwd=settings.base_dir`, which on a native install resolves to the data + dir — `requirements.txt` isn't there and pip fails with `Could not open + requirements file`. The fix: pip's cwd is `settings.app_dir` (the source + tree) so it can actually find the file. + + This test mocks every subprocess so it can capture the cwd of each call + and assert that the pip step runs in app_dir while git steps continue + to run in base_dir (their existing behaviour — git walks up to find + `.git` so that path keeps working).""" + from backend.app.api.routes import updates as updates_module + + # Set up fake install layout: app_dir has requirements.txt, data_dir is + # a sibling (mirroring `INSTALL_PATH=/opt/bambuddy`, `DATA_DIR=/opt/bambuddy/data`). + app_dir = tmp_path / "app" + data_dir = tmp_path / "app" / "data" + app_dir.mkdir() + data_dir.mkdir() + (app_dir / "requirements.txt").write_text("fastapi\n") + + # Capture every subprocess call's cwd + the executable token. + calls: list[dict] = [] + + async def fake_create_subprocess_exec(*args, **kwargs): + calls.append({"args": args, "cwd": kwargs.get("cwd")}) + proc = MagicMock() + proc.communicate = AsyncMock(return_value=(b"", b"")) + proc.returncode = 0 + return proc + + with ( + patch.object(updates_module.settings, "base_dir", data_dir), + patch.object(updates_module.settings, "app_dir", app_dir), + patch.object(updates_module, "_find_executable", return_value="/usr/bin/git"), + patch.object( + updates_module.asyncio, + "create_subprocess_exec", + side_effect=fake_create_subprocess_exec, + ), + ): + await updates_module._perform_update() + + # Find the pip invocation (sys.executable + "-m" + "pip" + "install"). + pip_calls = [c for c in calls if "pip" in c["args"] and "install" in c["args"]] + assert pip_calls, "pip install was never invoked. Captured: " + repr([c["args"] for c in calls]) + pip_cwd = pip_calls[0]["cwd"] + assert pip_cwd == str(app_dir), ( + f"pip install must run in app_dir ({app_dir}) so it finds " + f"requirements.txt; got cwd={pip_cwd}. Regression to base_dir " + f"breaks every native-install upgrade." + ) + + # Sanity check: the requirements.txt that pip would read actually exists + # at the captured cwd. If this fails the cwd is wrong even if it isn't + # base_dir — useful diagnostic if someone refactors path handling. + assert (Path(pip_cwd) / "requirements.txt").exists()