Browse Source

fix(updates): run pip install in app_dir, not base_dir, on native installs

  Native-install upgrade via the in-app Apply Update button got the new
  code in via `git reset --hard origin/main` but then logged

    ERROR: Could not open requirements file:
    [Errno 2] No such file or directory: 'requirements.txt'

  and continued. The new deps never installed, leaving the user with
  new code but stale dependencies — surfaces as cryptic import errors
  on the next restart.

  Root cause: `pip install -r requirements.txt` ran with
  `cwd=settings.base_dir`. On a native install, systemd sets
  DATA_DIR=$INSTALL_PATH/data so base_dir resolves to the data dir
  (e.g. /opt/bambuddy/data), not the source tree. Pip doesn't walk up
  looking for the requirements file the way git walks up looking for
  .git, so it fails. Same bug affected the optional npm step
  (`frontend_dir = base_dir / "frontend"` doesn't exist).

  Fix: introduce `settings.app_dir` pointing at the source-tree root
  (distinct from `base_dir` only on native installs) and run pip +
  npm with `cwd=settings.app_dir`. Git ops keep using `base_dir`
  because they already work (git walks up).

  Docker users were unaffected — Docker doesn't use the in-app updater
  (image pull replaces it).

  Regression test in test_updates_api.py mocks every subprocess in
  _perform_update, captures their cwd, and asserts the pip step runs
  in app_dir and that requirements.txt actually exists there. Any
  future refactor that re-introduces cwd=base_dir for the pip step
  fails CI before another user trips over it.
maziggy 4 weeks ago
parent
commit
a85855f2dd

+ 9 - 3
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 = {

+ 6 - 0
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

+ 63 - 1
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()