From 4e845f24cee9c56e99a2ba420ca001cff667d25c Mon Sep 17 00:00:00 2001 From: Sean Link Date: Mon, 18 May 2026 11:40:13 -0700 Subject: [PATCH] fix(jlcpcb): use platform user data dir for parts database (#167) JLCPCBPartsManager defaulted db_path to a "data/" directory computed relative to __file__, which fails with read-only filesystems when the package is installed to a system-managed prefix (e.g. /nix/store, an immutable container image, or /usr/lib). The same pattern in download_jlcpcb.py would silently scatter the ~1.5 GB JLCPCB cache inside the install tree even when it is writable. The original integration plan (docs/archive/JLCPCB_INTEGRATION_PLAN.md) called for a per-user database under ~/.kicad-mcp/. This change moves the default to the platform-appropriate user data directory by adding a new PlatformHelper.get_data_dir() helper that mirrors the existing get_config_dir() / get_cache_dir() conventions: - Linux: XDG_DATA_HOME/kicad-mcp or ~/.local/share/kicad-mcp - macOS: ~/Library/Application Support/kicad-mcp - Windows: %USERPROFILE%\.kicad-mcp\data Both JLCPCBPartsManager and download_jlcpcb.py now resolve their database paths through this helper. ensure_directories() and detect_platform() include the new directory. Unit tests parallel to the existing config_dir/cache_dir cases cover platform-appropriate paths and the relative-XDG_DATA_HOME edge case. --- download_jlcpcb.py | 9 +++++--- python/commands/jlcpcb_parts.py | 12 +++++----- python/utils/platform_helper.py | 33 +++++++++++++++++++++++++++ tests/test_platform_helper.py | 40 +++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 8 deletions(-) diff --git a/download_jlcpcb.py b/download_jlcpcb.py index 886f434..bfb1f91 100644 --- a/download_jlcpcb.py +++ b/download_jlcpcb.py @@ -19,11 +19,14 @@ import sys import time from pathlib import Path -DATA_DIR = Path(__file__).parent / "data" -DATA_DIR.mkdir(exist_ok=True) +sys.path.insert(0, str(Path(__file__).parent / "python")) +from utils.platform_helper import PlatformHelper # noqa: E402 + +DATA_DIR = PlatformHelper.get_data_dir() +DATA_DIR.mkdir(parents=True, exist_ok=True) CACHE_DIR = DATA_DIR / "jlcparts_cache" -CACHE_DIR.mkdir(exist_ok=True) +CACHE_DIR.mkdir(parents=True, exist_ok=True) BASE_URL = "https://yaqwsx.github.io/jlcparts/data" MAX_PARTS = 30 # probe up to this many split volumes diff --git a/python/commands/jlcpcb_parts.py b/python/commands/jlcpcb_parts.py index 16de179..9698f66 100644 --- a/python/commands/jlcpcb_parts.py +++ b/python/commands/jlcpcb_parts.py @@ -13,6 +13,8 @@ from datetime import datetime from pathlib import Path from typing import Any, Callable, Dict, List, Optional, Tuple +from utils.platform_helper import PlatformHelper + logger = logging.getLogger("kicad_interface") @@ -28,13 +30,13 @@ class JLCPCBPartsManager: Initialize parts database manager Args: - db_path: Path to SQLite database file (default: data/jlcpcb_parts.db) + db_path: Path to SQLite database file (default: platform-specific + user data directory, e.g. ~/.local/share/kicad-mcp/jlcpcb_parts.db + on Linux). See PlatformHelper.get_data_dir() for platform paths. """ if db_path is None: - # Default to data directory in project root - project_root = Path(__file__).parent.parent.parent - data_dir = project_root / "data" - data_dir.mkdir(exist_ok=True) + data_dir = PlatformHelper.get_data_dir() + data_dir.mkdir(parents=True, exist_ok=True) db_path = str(data_dir / "jlcpcb_parts.db") self.db_path = db_path diff --git a/python/utils/platform_helper.py b/python/utils/platform_helper.py index d6c61c1..8cdd86f 100644 --- a/python/utils/platform_helper.py +++ b/python/utils/platform_helper.py @@ -261,6 +261,37 @@ class PlatformHelper: else: return PlatformHelper.get_config_dir() / "cache" + @staticmethod + def get_data_dir() -> Path: + r""" + Get appropriate data directory for current platform + + Used for application state that should persist across runs and is not + a transient cache (e.g. the JLCPCB parts database). + + Follows platform conventions: + - Windows: %USERPROFILE%\.kicad-mcp\data + - Linux: $XDG_DATA_HOME/kicad-mcp or ~/.local/share/kicad-mcp + - macOS: ~/Library/Application Support/kicad-mcp + + Returns: + Path to data directory + """ + if PlatformHelper.is_windows(): + return PlatformHelper.get_config_dir() / "data" + elif PlatformHelper.is_linux(): + xdg_data = os.environ.get("XDG_DATA_HOME") + if xdg_data: + xdg_data_path = Path(xdg_data).expanduser() + if xdg_data_path.is_absolute(): + return xdg_data_path / "kicad-mcp" + logger.warning("Ignoring relative XDG_DATA_HOME: %s", xdg_data) + return Path.home() / ".local" / "share" / "kicad-mcp" + elif PlatformHelper.is_macos(): + return Path.home() / "Library" / "Application Support" / "kicad-mcp" + else: + return PlatformHelper.get_config_dir() / "data" + @staticmethod def ensure_directories() -> None: """Create all necessary directories if they don't exist""" @@ -268,6 +299,7 @@ class PlatformHelper: PlatformHelper.get_config_dir(), PlatformHelper.get_log_dir(), PlatformHelper.get_cache_dir(), + PlatformHelper.get_data_dir(), ] for directory in dirs_to_create: @@ -317,6 +349,7 @@ def detect_platform() -> dict: "config_dir": str(PlatformHelper.get_config_dir()), "log_dir": str(PlatformHelper.get_log_dir()), "cache_dir": str(PlatformHelper.get_cache_dir()), + "data_dir": str(PlatformHelper.get_data_dir()), "kicad_python_paths": [str(p) for p in PlatformHelper.get_kicad_python_paths()], } diff --git a/tests/test_platform_helper.py b/tests/test_platform_helper.py index 4fcf0e3..1a58b50 100644 --- a/tests/test_platform_helper.py +++ b/tests/test_platform_helper.py @@ -69,6 +69,45 @@ class TestPathGeneration: assert cache_dir.exists(), f"Cache dir should exist: {cache_dir}" assert cache_dir.is_dir(), f"Cache dir should be a directory: {cache_dir}" + def test_data_dir_exists_after_ensure(self): + """Test that data directory is created""" + PlatformHelper.ensure_directories() + data_dir = PlatformHelper.get_data_dir() + assert data_dir.exists(), f"Data dir should exist: {data_dir}" + assert data_dir.is_dir(), f"Data dir should be a directory: {data_dir}" + + def test_data_dir_is_platform_appropriate(self): + """Test that data directory follows platform conventions""" + data_dir = PlatformHelper.get_data_dir() + + if PlatformHelper.is_linux(): + # Should be ~/.local/share/kicad-mcp or $XDG_DATA_HOME/kicad-mcp + xdg = os.environ.get("XDG_DATA_HOME") + if xdg and Path(xdg).is_absolute(): + expected = Path(xdg) / "kicad-mcp" + else: + expected = Path.home() / ".local" / "share" / "kicad-mcp" + assert data_dir == expected + + elif PlatformHelper.is_windows(): + # Should be %USERPROFILE%\.kicad-mcp\data + expected = Path.home() / ".kicad-mcp" / "data" + assert data_dir == expected + + elif PlatformHelper.is_macos(): + # Should be ~/Library/Application Support/kicad-mcp + expected = Path.home() / "Library" / "Application Support" / "kicad-mcp" + assert data_dir == expected + + def test_data_dir_ignores_relative_xdg_data_home(self, monkeypatch): + """Relative XDG_DATA_HOME should be ignored on Linux.""" + monkeypatch.setattr(PlatformHelper, "is_linux", staticmethod(lambda: True)) + monkeypatch.setattr(PlatformHelper, "is_windows", staticmethod(lambda: False)) + monkeypatch.setattr(PlatformHelper, "is_macos", staticmethod(lambda: False)) + monkeypatch.setenv("XDG_DATA_HOME", "relative/data") + + assert PlatformHelper.get_data_dir() == Path.home() / ".local" / "share" / "kicad-mcp" + def test_config_dir_is_platform_appropriate(self): """Test that config directory follows platform conventions""" config_dir = PlatformHelper.get_config_dir() @@ -146,6 +185,7 @@ class TestDetectPlatform: "config_dir", "log_dir", "cache_dir", + "data_dir", "kicad_python_paths", ] for key in required_keys: