From de6357edc59ad8c52740f108686b12d3d376a0f9 Mon Sep 17 00:00:00 2001 From: Josef Cada Date: Tue, 7 Jul 2026 08:40:39 +0200 Subject: [PATCH 1/8] feat: add kilm import command for SamacSys/Mouser ZIPs --- .../commands/import_zip/__init__.py | 13 + .../commands/import_zip/command.py | 413 ++++++++++++++++++ kicad_lib_manager/main.py | 6 + tests/test_import_zip_command.py | 229 ++++++++++ 4 files changed, 661 insertions(+) create mode 100644 kicad_lib_manager/commands/import_zip/__init__.py create mode 100644 kicad_lib_manager/commands/import_zip/command.py create mode 100644 tests/test_import_zip_command.py diff --git a/kicad_lib_manager/commands/import_zip/__init__.py b/kicad_lib_manager/commands/import_zip/__init__.py new file mode 100644 index 0000000..534f76a --- /dev/null +++ b/kicad_lib_manager/commands/import_zip/__init__.py @@ -0,0 +1,13 @@ +import typer + +from .command import import_zip + +import_zip_app = typer.Typer( + name="import", + help="Import SamacSys/Mouser KiCad ZIP(s) into the configured library", + rich_markup_mode="rich", + callback=import_zip, + invoke_without_command=True, +) + +__all__ = ["import_zip", "import_zip_app"] diff --git a/kicad_lib_manager/commands/import_zip/command.py b/kicad_lib_manager/commands/import_zip/command.py new file mode 100644 index 0000000..2259619 --- /dev/null +++ b/kicad_lib_manager/commands/import_zip/command.py @@ -0,0 +1,413 @@ +""" +Import command: unpack a SamacSys/Mouser KiCad ZIP into the configured library. +""" + +import re +import shutil +import subprocess +import tempfile +import zipfile +from pathlib import Path +from typing import Annotated, Optional + +import typer +from rich.console import Console + +from ...services.config_service import Config + +console = Console() + +# ── Symbol helpers ──────────────────────────────────────────────────────────── + +_SYM_BLOCK_RE = re.compile(r"^\t\(symbol \"") + + +def _paren_depth(line: str) -> int: + """Count net paren depth change in line, ignoring parens inside "..." strings.""" + depth = 0 + in_str = False + i = 0 + while i < len(line): + ch = line[i] + if in_str: + if ch == "\\" and i + 1 < len(line): + i += 2 # skip escaped character + continue + if ch == '"': + in_str = False + else: + if ch == '"': + in_str = True + elif ch == "(": + depth += 1 + elif ch == ")": + depth -= 1 + i += 1 + return depth + + +def _extract_symbol_blocks(text: str) -> list[str]: + blocks: list[str] = [] + current: list[str] = [] + depth = 0 + in_sym = False + for line in text.split("\n"): + if not in_sym and _SYM_BLOCK_RE.match(line): + in_sym = True + depth = _paren_depth(line) + current = [line] + elif in_sym: + current.append(line) + depth += _paren_depth(line) + if depth == 0: + blocks.append("\n".join(current)) + in_sym = False + current = [] + return blocks + + +def _symbol_name(block: str) -> str: + m = re.match(r'\t\(symbol "([^"]+)"', block) + return m.group(1) if m else "" + + +def _fix_footprint_ref(block: str, lib_name: str) -> str: + def _replace(m: re.Match[str]) -> str: + val = m.group(1) + if ":" not in val: + val = f"{lib_name}:{val}" + return f'"Footprint" "{val}"' + + return re.sub(r'"Footprint" "([^"]+)"', _replace, block) + + +def _merge_symbols( + src_file: Path, sym_lib: Path, lib_name: str, dry_run: bool +) -> tuple[list[str], list[str]]: + src_text = src_file.read_text(encoding="utf-8") + lpp_text = sym_lib.read_text(encoding="utf-8") + existing = {_symbol_name(b) for b in _extract_symbol_blocks(lpp_text)} + + added: list[str] = [] + skipped: list[str] = [] + new_blocks: list[str] = [] + + for block in _extract_symbol_blocks(src_text): + name = _symbol_name(block) + if name in existing: + skipped.append(name) + continue + block = _fix_footprint_ref(block, lib_name) + new_blocks.append(block) + added.append(name) + + if new_blocks and not dry_run: + insert = "\n".join(new_blocks) + "\n" + last = lpp_text.rfind(")") + sym_lib.write_text(lpp_text[:last] + insert + lpp_text[last:], encoding="utf-8") + + return added, skipped + + +# ── Footprint helpers ───────────────────────────────────────────────────────── + + +def _fix_3d_path(text: str, models_dir_name: str) -> str: + def _replace(m: re.Match[str]) -> str: + filename = Path(m.group(1).strip('"')).name + return f'(model "${{KICAD_3RD_PARTY}}/{models_dir_name}/{filename}"' + + text = re.sub(r'\(model\s+"([^"]+)"', _replace, text) + text = re.sub(r"\(model\s+(\S+\.(?:stp|step|stp\.gz))", _replace, text) + return text + + +def _upgrade_fp(path: Path, kicad_cli: Optional[Path]) -> None: + """Upgrade legacy (module ...) footprint format in-place using kicad-cli.""" + if kicad_cli is None or not kicad_cli.exists(): + return + first_line = path.read_text(encoding="utf-8", errors="replace").lstrip()[:20] + if not first_line.startswith("(module"): + return + + with tempfile.TemporaryDirectory(prefix="kilm_fp_upgrade_") as tmp: + in_pretty = Path(tmp) / "in.pretty" + in_pretty.mkdir() + shutil.copy2(path, in_pretty / path.name) + out_dir = Path(tmp) / "out" + + if kicad_cli.suffix.lower() == ".appimage": + cmd = [ + str(kicad_cli), + "kicad-cli", + "fp", + "upgrade", + "--force", + "--output", + str(out_dir), + str(in_pretty), + ] + else: + cmd = [ + str(kicad_cli), + "fp", + "upgrade", + "--force", + "--output", + str(out_dir), + str(in_pretty), + ] + + subprocess.run(cmd, capture_output=True, check=False) + upgraded = out_dir / path.name + if upgraded.exists(): + shutil.copy2(upgraded, path) + + +def _upgrade_sym(sym_file: Path, kicad_cli: Optional[Path]) -> None: + """Upgrade symbol file format in-place using kicad-cli.""" + if kicad_cli is None or not kicad_cli.exists(): + return + if kicad_cli.suffix.lower() == ".appimage": + cmd = [str(kicad_cli), "kicad-cli", "sym", "upgrade", "--force", str(sym_file)] + else: + cmd = [str(kicad_cli), "sym", "upgrade", "--force", str(sym_file)] + subprocess.run(cmd, capture_output=True, check=False) + + +# ── ZIP extraction ─────────────────────────────────────────────────────────── + + +def _safe_extractall(zf: zipfile.ZipFile, dest: Path) -> None: + """Extract ZIP, rejecting any member whose resolved path escapes dest. + + Python < 3.12 does not guard against zip-slip in extractall(). + """ + dest_resolved = dest.resolve() + for member in zf.namelist(): + member_path = (dest / member).resolve() + if ( + not str(member_path).startswith(str(dest_resolved) + "/") + and member_path != dest_resolved + ): + raise ValueError(f"Unsafe ZIP entry rejected: {member!r}") + zf.extractall(dest) + + +# ── Per-ZIP import ──────────────────────────────────────────────────────────── + + +def _import_zip( + zip_path: Path, + sym_lib: Path, + fp_dir: Path, + models_dir: Path, + lib_name: str, + kicad_cli: Optional[Path], + dry_run: bool, +) -> dict[str, list[str]]: + result: dict[str, list[str]] = { + "sym": [], + "sym_skipped": [], + "fp": [], + "models": [], + } + models_dir_name = models_dir.name + + with tempfile.TemporaryDirectory(prefix="kilm_import_") as tmp: + tmp_path = Path(tmp) + with zipfile.ZipFile(zip_path) as zf: + _safe_extractall(zf, tmp_path) + + kicad_dirs = list(tmp_path.rglob("KiCad")) + model_dirs = list(tmp_path.rglob("3D")) + + # 3D models + for d in model_dirs: + for f in d.iterdir(): + if f.suffix.lower() in (".stp", ".step") or f.name.endswith(".stp.gz"): + dest = models_dir / f.name + if dest.exists(): + console.print(f" 3D skip (exists): {f.name}") + else: + console.print(f" 3D add: {f.name}") + if not dry_run: + models_dir.mkdir(exist_ok=True) + shutil.copy2(f, dest) + result["models"].append(f.name) + + # Footprints + for d in kicad_dirs: + for f in d.glob("*.kicad_mod"): + dest = fp_dir / f.name + if dest.exists(): + console.print(f" FP skip (exists): {f.name}") + continue + console.print(f" FP add: {f.name}") + if not dry_run: + text = _fix_3d_path(f.read_text(encoding="utf-8"), models_dir_name) + f.write_text(text, encoding="utf-8") + _upgrade_fp(f, kicad_cli) + fp_dir.mkdir(exist_ok=True) + shutil.copy2(f, dest) + result["fp"].append(f.name) + + # Symbols + for d in kicad_dirs: + for f in d.glob("*.kicad_sym"): + if not dry_run: + _upgrade_sym(f, kicad_cli) + added, skipped = _merge_symbols(f, sym_lib, lib_name, dry_run) + for name in added: + console.print(f" SYM add: {name}") + for name in skipped: + console.print(f" SYM skip (exists): {name}") + result["sym"].extend(added) + result["sym_skipped"].extend(skipped) + + return result + + +# ── Command ─────────────────────────────────────────────────────────────────── + + +def _detect_kicad_cli() -> Optional[Path]: + """Return kicad-cli path if found on PATH or common locations.""" + if shutil.which("kicad-cli"): + return Path(shutil.which("kicad-cli")) # type: ignore[arg-type] + for candidate in [ + Path.home() / "AppImages" / "kicad.appimage", + Path("/usr/bin/kicad-cli"), + Path("/usr/local/bin/kicad-cli"), + Path("/Applications/KiCad/KiCad.app/Contents/MacOS/kicad-cli"), + ]: + if candidate.exists(): + return candidate + return None + + +def import_zip( + zip_files: Annotated[ + list[Path], typer.Argument(help="SamacSys/Mouser ZIP file(s) to import") + ], + library: Annotated[ + Optional[str], + typer.Option( + "--library", + "-l", + help="Target library name (default: first github library)", + ), + ] = None, + kicad_cli_path: Annotated[ + Optional[Path], + typer.Option( + "--kicad-cli", help="Path to kicad-cli or kicad.appimage for format upgrade" + ), + ] = None, + dry_run: Annotated[ + bool, + typer.Option( + "--dry-run", help="Show what would be imported without making changes" + ), + ] = False, +) -> None: + """Import SamacSys/Mouser KiCad ZIP(s) into the configured library. + + Each ZIP should be a standard SamacSys multi-EDA archive as downloaded + from Mouser or component search. The command extracts the KiCad files + and merges them into the library. Run 'kilm setup' afterwards to register + any newly added libraries in KiCad. + """ + config = Config() + github_libs = config.get_libraries(library_type="github") + if not github_libs: + console.print("[red]No github library configured. Run 'kilm init' first.[/red]") + raise typer.Exit(1) + + # Resolve target library + target_lib = None + for lib in github_libs: + if library is None or lib.get("name") == library: + target_lib = lib + break + + if target_lib is None: + console.print(f"[red]Library '{library}' not found in config.[/red]") + raise typer.Exit(1) + + lib_path = Path(target_lib["path"]) + if not lib_path.exists(): + console.print(f"[red]Library path does not exist: {lib_path}[/red]") + raise typer.Exit(1) + + # Find symbol lib and footprint dir + sym_candidates = ( + sorted((lib_path / "symbols").glob("*.kicad_sym")) + if (lib_path / "symbols").exists() + else [] + ) + fp_candidates = ( + sorted((lib_path / "footprints").glob("*.pretty")) + if (lib_path / "footprints").exists() + else [] + ) + + if not sym_candidates: + console.print(f"[red]No .kicad_sym file found under {lib_path}/symbols/[/red]") + raise typer.Exit(1) + if not fp_candidates: + console.print( + f"[red]No .pretty directory found under {lib_path}/footprints/[/red]" + ) + raise typer.Exit(1) + + sym_lib = sym_candidates[0] + fp_dir = fp_candidates[0] + lib_name = sym_lib.stem + models_dir = lib_path / f"{lib_name}.3dshapes" + + # Resolve kicad-cli + kicad_cli = kicad_cli_path if kicad_cli_path else _detect_kicad_cli() + if kicad_cli: + console.print(f"[dim]kicad-cli: {kicad_cli}[/dim]") + else: + console.print("[dim]kicad-cli not found - format upgrade skipped[/dim]") + + if dry_run: + console.print("[yellow]Dry run - no changes will be made[/yellow]") + + totals: dict[str, list[str]] = {"sym": [], "fp": [], "models": []} + + for zip_path in zip_files: + zip_path = zip_path.expanduser().resolve() + if not zip_path.exists(): + console.print(f"[yellow]Skipping {zip_path.name}: file not found[/yellow]") + continue + if not zipfile.is_zipfile(zip_path): + console.print(f"[yellow]Skipping {zip_path.name}: not a valid ZIP[/yellow]") + continue + + console.print(f"\n[cyan]Importing {zip_path.name}[/cyan]") + r = _import_zip( + zip_path, sym_lib, fp_dir, models_dir, lib_name, kicad_cli, dry_run + ) + totals["sym"].extend(r["sym"]) + totals["fp"].extend(r["fp"]) + totals["models"].extend(r["models"]) + + console.print("\n[bold]Summary:[/bold]") + console.print(f" Symbols added: {len(totals['sym'])}") + console.print(f" Footprints added: {len(totals['fp'])}") + console.print(f" 3D models added: {len(totals['models'])}") + + if any(totals.values()) and not dry_run: + console.print("\n[green]Import complete.[/green]") + console.print( + "[dim]Note: If this library is not yet configured in KiCad, run 'kilm setup' to register it.[/dim]" + ) + elif dry_run: + console.print( + "\n[dim]Dry run complete - run without --dry-run to apply changes.[/dim]" + ) + else: + console.print("\n[dim]Nothing new added.[/dim]") diff --git a/kicad_lib_manager/main.py b/kicad_lib_manager/main.py index 45fde94..d3ce059 100644 --- a/kicad_lib_manager/main.py +++ b/kicad_lib_manager/main.py @@ -16,6 +16,7 @@ from .commands.add_3d import add_3d_app from .commands.add_hook import add_hook_app from .commands.config import config_app +from .commands.import_zip import import_zip_app from .commands.init import init_app from .commands.list_libraries import list_app from .commands.pin import pin_app @@ -159,6 +160,11 @@ def main( app.add_typer(sync_app, name="sync", help="Update/sync library content") app.add_typer(update_app, name="update", help="Update KiLM itself") app.add_typer(add_hook_app, name="add-hook", help="Add project hooks") +app.add_typer( + import_zip_app, + name="import", + help="Import SamacSys/Mouser KiCad ZIP(s) into the library", +) if __name__ == "__main__": diff --git a/tests/test_import_zip_command.py b/tests/test_import_zip_command.py new file mode 100644 index 0000000..f3e819d --- /dev/null +++ b/tests/test_import_zip_command.py @@ -0,0 +1,229 @@ +""" +Tests for the kilm import command. +""" + +import zipfile +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest +from typer.testing import CliRunner + +from kicad_lib_manager.commands.import_zip.command import ( + _extract_symbol_blocks, + _fix_3d_path, + _fix_footprint_ref, + _merge_symbols, + _safe_extractall, + _symbol_name, +) +from kicad_lib_manager.main import app + +runner = CliRunner() + +# ── Unit tests for helpers ──────────────────────────────────────────────────── + +SAMPLE_SYM_LIB = """\ +(kicad_symbol_lib +\t(symbol "ExistingPart" +\t\t(property "Footprint" "LPP:ExistingPart") +\t) +) +""" + +INCOMING_SYM = """\ +(kicad_symbol_lib +\t(symbol "NewPart" +\t\t(property "Footprint" "NewPart") +\t) +\t(symbol "ExistingPart" +\t\t(property "Footprint" "LPP:ExistingPart") +\t) +) +""" + + +def test_extract_symbol_blocks(): + blocks = _extract_symbol_blocks(INCOMING_SYM) + assert len(blocks) == 2 + assert _symbol_name(blocks[0]) == "NewPart" + assert _symbol_name(blocks[1]) == "ExistingPart" + + +def test_fix_footprint_ref_adds_prefix(): + block = '\t(property "Footprint" "BareFootprint")' + result = _fix_footprint_ref(block, "LPP") + assert '"Footprint" "LPP:BareFootprint"' in result + + +def test_fix_footprint_ref_keeps_existing_prefix(): + block = '\t(property "Footprint" "OTHER:Footprint")' + result = _fix_footprint_ref(block, "LPP") + assert '"Footprint" "OTHER:Footprint"' in result + + +def test_extract_symbol_blocks_ignores_parens_in_strings(): + # A property value with unbalanced parens must not break depth tracking + text = ( + "(kicad_symbol_lib\n" + '\t(symbol "PartA"\n' + '\t\t(property "Description" "Filter (LC")\n' # unbalanced ( inside string + "\t)\n" + ")\n" + ) + blocks = _extract_symbol_blocks(text) + assert len(blocks) == 1 + assert _symbol_name(blocks[0]) == "PartA" + + +def test_safe_extractall_rejects_zip_slip(tmp_path: Path): + zip_path = tmp_path / "evil.zip" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr("../../outside.txt", "malicious content") + dest = tmp_path / "extract" + dest.mkdir() + with zipfile.ZipFile(zip_path) as zf, pytest.raises(ValueError, match="Unsafe ZIP entry"): + _safe_extractall(zf, dest) + assert not (tmp_path / "outside.txt").exists() + + +def test_fix_3d_path_normalises(): + text = '(model "C:/SamacSys/somepart.stp"' + result = _fix_3d_path(text, "LPP.3dshapes") + assert "${KICAD_3RD_PARTY}/LPP.3dshapes/somepart.stp" in result + + +def test_merge_symbols_adds_new_skips_existing(tmp_path: Path): + sym_lib = tmp_path / "LPP.kicad_sym" + sym_lib.write_text(SAMPLE_SYM_LIB, encoding="utf-8") + + src = tmp_path / "incoming.kicad_sym" + src.write_text(INCOMING_SYM, encoding="utf-8") + + added, skipped = _merge_symbols(src, sym_lib, "LPP", dry_run=False) + assert added == ["NewPart"] + assert skipped == ["ExistingPart"] + + merged = sym_lib.read_text(encoding="utf-8") + assert "NewPart" in merged + assert merged.count("ExistingPart") == 2 # original + new + + +def test_merge_symbols_dry_run_does_not_write(tmp_path: Path): + sym_lib = tmp_path / "LPP.kicad_sym" + sym_lib.write_text(SAMPLE_SYM_LIB, encoding="utf-8") + src = tmp_path / "incoming.kicad_sym" + src.write_text(INCOMING_SYM, encoding="utf-8") + + original_content = sym_lib.read_text(encoding="utf-8") + added, _ = _merge_symbols(src, sym_lib, "LPP", dry_run=True) + + assert added == ["NewPart"] + assert sym_lib.read_text(encoding="utf-8") == original_content + + +# ── CLI integration test ────────────────────────────────────────────────────── + + +def _make_samacsys_zip(tmp_path: Path, part_name: str) -> Path: + """Build a minimal SamacSys-style ZIP for testing.""" + zip_path = tmp_path / f"LIB_{part_name}.zip" + + sym_content = f"""\ +(kicad_symbol_lib +\t(symbol "{part_name}" +\t\t(property "Footprint" "{part_name}") +\t) +) +""" + fp_content = f"""\ +(footprint "{part_name}" +) +""" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr(f"{part_name}/KiCad/{part_name}.kicad_sym", sym_content) + zf.writestr(f"{part_name}/KiCad/{part_name}.kicad_mod", fp_content) + zf.writestr(f"{part_name}/3D/{part_name}.stp", "STEP data") + + return zip_path + + +@pytest.fixture +def library_tree(tmp_path: Path) -> Path: + """Set up a minimal library tree with kilm.yaml.""" + lib = tmp_path / "mylib" + (lib / "symbols").mkdir(parents=True) + (lib / "footprints" / "LPP.pretty").mkdir(parents=True) + (lib / "symbols" / "LPP.kicad_sym").write_text(SAMPLE_SYM_LIB, encoding="utf-8") + (lib / "kilm.yaml").write_text("name: mylib\n", encoding="utf-8") + return lib + + +@pytest.fixture +def mock_config(library_tree: Path, monkeypatch: pytest.MonkeyPatch) -> MagicMock: + config_mock = MagicMock() + config_mock.get_libraries.return_value = [ + {"name": "mylib", "path": str(library_tree), "type": "github"} + ] + monkeypatch.setattr( + "kicad_lib_manager.commands.import_zip.command.Config", lambda: config_mock + ) + return config_mock + + +def test_import_zip_cli_adds_part( + tmp_path: Path, library_tree: Path, mock_config: MagicMock +): + zip_path = _make_samacsys_zip(tmp_path, "TestPart") + + with patch( + "kicad_lib_manager.commands.import_zip.command._detect_kicad_cli", + return_value=None, + ): + result = runner.invoke(app, ["import", str(zip_path)]) + + assert result.exit_code == 0, result.output + assert "add: TestPart" in result.output + + sym_lib = library_tree / "symbols" / "LPP.kicad_sym" + assert "TestPart" in sym_lib.read_text(encoding="utf-8") + + fp_file = library_tree / "footprints" / "LPP.pretty" / "TestPart.kicad_mod" + assert fp_file.exists() + + model_file = library_tree / "LPP.3dshapes" / "TestPart.stp" + assert model_file.exists() + + +def test_import_zip_cli_dry_run_no_changes( + tmp_path: Path, library_tree: Path, mock_config: MagicMock +): + zip_path = _make_samacsys_zip(tmp_path, "DryPart") + + with patch( + "kicad_lib_manager.commands.import_zip.command._detect_kicad_cli", + return_value=None, + ): + result = runner.invoke(app, ["import", "--dry-run", str(zip_path)]) + + assert result.exit_code == 0, result.output + assert "add: DryPart" in result.output + + sym_lib = library_tree / "symbols" / "LPP.kicad_sym" + assert "DryPart" not in sym_lib.read_text(encoding="utf-8") + assert not (library_tree / "LPP.3dshapes" / "DryPart.stp").exists() + + +def test_import_zip_cli_skips_existing( + tmp_path: Path, library_tree: Path, mock_config: MagicMock +): + zip_path = _make_samacsys_zip(tmp_path, "ExistingPart") + + with patch( + "kicad_lib_manager.commands.import_zip.command._detect_kicad_cli", + return_value=None, + ): + result = runner.invoke(app, ["import", str(zip_path)]) + + assert result.exit_code == 0, result.output + assert "skip (exists): ExistingPart" in result.output From 918311ddc6de690fbb0d32e403d73e89234432ba Mon Sep 17 00:00:00 2001 From: Josef Cada Date: Tue, 7 Jul 2026 09:36:18 +0200 Subject: [PATCH 2/8] fix: harden import command against subprocess hangs, zip-slip on Windows, and per-zip errors --- .../commands/import_zip/command.py | 50 ++++++++++++------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/kicad_lib_manager/commands/import_zip/command.py b/kicad_lib_manager/commands/import_zip/command.py index 2259619..5d7f1d3 100644 --- a/kicad_lib_manager/commands/import_zip/command.py +++ b/kicad_lib_manager/commands/import_zip/command.py @@ -158,7 +158,14 @@ def _upgrade_fp(path: Path, kicad_cli: Optional[Path]) -> None: str(in_pretty), ] - subprocess.run(cmd, capture_output=True, check=False) + try: + subprocess.run(cmd, capture_output=True, check=True, timeout=30) + except subprocess.TimeoutExpired: + console.print(f"[yellow] warn: kicad-cli fp upgrade timed out for {path.name}, skipping[/yellow]") + return + except subprocess.CalledProcessError as exc: + console.print(f"[yellow] warn: kicad-cli fp upgrade failed for {path.name}: {exc.stderr.strip()}[/yellow]") + return upgraded = out_dir / path.name if upgraded.exists(): shutil.copy2(upgraded, path) @@ -172,7 +179,12 @@ def _upgrade_sym(sym_file: Path, kicad_cli: Optional[Path]) -> None: cmd = [str(kicad_cli), "kicad-cli", "sym", "upgrade", "--force", str(sym_file)] else: cmd = [str(kicad_cli), "sym", "upgrade", "--force", str(sym_file)] - subprocess.run(cmd, capture_output=True, check=False) + try: + subprocess.run(cmd, capture_output=True, check=True, timeout=30) + except subprocess.TimeoutExpired: + console.print(f"[yellow] warn: kicad-cli sym upgrade timed out for {sym_file.name}, skipping[/yellow]") + except subprocess.CalledProcessError as exc: + console.print(f"[yellow] warn: kicad-cli sym upgrade failed for {sym_file.name}: {exc.stderr.strip()}[/yellow]") # ── ZIP extraction ─────────────────────────────────────────────────────────── @@ -181,15 +193,13 @@ def _upgrade_sym(sym_file: Path, kicad_cli: Optional[Path]) -> None: def _safe_extractall(zf: zipfile.ZipFile, dest: Path) -> None: """Extract ZIP, rejecting any member whose resolved path escapes dest. - Python < 3.12 does not guard against zip-slip in extractall(). + Defense-in-depth against zip-slip: checks every member before extracting + regardless of Python version or platform. """ dest_resolved = dest.resolve() for member in zf.namelist(): member_path = (dest / member).resolve() - if ( - not str(member_path).startswith(str(dest_resolved) + "/") - and member_path != dest_resolved - ): + if member_path != dest_resolved and not member_path.is_relative_to(dest_resolved): raise ValueError(f"Unsafe ZIP entry rejected: {member!r}") zf.extractall(dest) @@ -270,17 +280,19 @@ def _import_zip( # ── Command ─────────────────────────────────────────────────────────────────── +_KICAD_CLI_CANDIDATES: tuple[Path, ...] = ( + Path.home() / "AppImages" / "kicad.appimage", + Path("/usr/bin/kicad-cli"), + Path("/usr/local/bin/kicad-cli"), + Path("/Applications/KiCad/KiCad.app/Contents/MacOS/kicad-cli"), +) + def _detect_kicad_cli() -> Optional[Path]: - """Return kicad-cli path if found on PATH or common locations.""" + """Return kicad-cli path if found on PATH or a location in _KICAD_CLI_CANDIDATES.""" if shutil.which("kicad-cli"): return Path(shutil.which("kicad-cli")) # type: ignore[arg-type] - for candidate in [ - Path.home() / "AppImages" / "kicad.appimage", - Path("/usr/bin/kicad-cli"), - Path("/usr/local/bin/kicad-cli"), - Path("/Applications/KiCad/KiCad.app/Contents/MacOS/kicad-cli"), - ]: + for candidate in _KICAD_CLI_CANDIDATES: if candidate.exists(): return candidate return None @@ -388,9 +400,13 @@ def import_zip( continue console.print(f"\n[cyan]Importing {zip_path.name}[/cyan]") - r = _import_zip( - zip_path, sym_lib, fp_dir, models_dir, lib_name, kicad_cli, dry_run - ) + try: + r = _import_zip( + zip_path, sym_lib, fp_dir, models_dir, lib_name, kicad_cli, dry_run + ) + except Exception as exc: + console.print(f"[red] error: {zip_path.name}: {exc}[/red]") + continue totals["sym"].extend(r["sym"]) totals["fp"].extend(r["fp"]) totals["models"].extend(r["models"]) From 2a75791479aaebc56559e144693f1d4dfe048742 Mon Sep 17 00:00:00 2001 From: Josef Cada Date: Wed, 8 Jul 2026 09:02:04 +0200 Subject: [PATCH 3/8] refactor: extract _build_kicad_cli_cmd helper, fix double shutil.which call --- .../commands/import_zip/command.py | 39 ++++++------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/kicad_lib_manager/commands/import_zip/command.py b/kicad_lib_manager/commands/import_zip/command.py index 5d7f1d3..d4a6ed1 100644 --- a/kicad_lib_manager/commands/import_zip/command.py +++ b/kicad_lib_manager/commands/import_zip/command.py @@ -136,27 +136,7 @@ def _upgrade_fp(path: Path, kicad_cli: Optional[Path]) -> None: shutil.copy2(path, in_pretty / path.name) out_dir = Path(tmp) / "out" - if kicad_cli.suffix.lower() == ".appimage": - cmd = [ - str(kicad_cli), - "kicad-cli", - "fp", - "upgrade", - "--force", - "--output", - str(out_dir), - str(in_pretty), - ] - else: - cmd = [ - str(kicad_cli), - "fp", - "upgrade", - "--force", - "--output", - str(out_dir), - str(in_pretty), - ] + cmd = _build_kicad_cli_cmd(kicad_cli, "fp", "upgrade", "--force", "--output", str(out_dir), str(in_pretty)) try: subprocess.run(cmd, capture_output=True, check=True, timeout=30) @@ -175,10 +155,7 @@ def _upgrade_sym(sym_file: Path, kicad_cli: Optional[Path]) -> None: """Upgrade symbol file format in-place using kicad-cli.""" if kicad_cli is None or not kicad_cli.exists(): return - if kicad_cli.suffix.lower() == ".appimage": - cmd = [str(kicad_cli), "kicad-cli", "sym", "upgrade", "--force", str(sym_file)] - else: - cmd = [str(kicad_cli), "sym", "upgrade", "--force", str(sym_file)] + cmd = _build_kicad_cli_cmd(kicad_cli, "sym", "upgrade", "--force", str(sym_file)) try: subprocess.run(cmd, capture_output=True, check=True, timeout=30) except subprocess.TimeoutExpired: @@ -187,6 +164,13 @@ def _upgrade_sym(sym_file: Path, kicad_cli: Optional[Path]) -> None: console.print(f"[yellow] warn: kicad-cli sym upgrade failed for {sym_file.name}: {exc.stderr.strip()}[/yellow]") +def _build_kicad_cli_cmd(kicad_cli: Path, *args: str) -> list[str]: + """Return the command list for kicad-cli, inserting the subcommand for AppImages.""" + if kicad_cli.suffix.lower() == ".appimage": + return [str(kicad_cli), "kicad-cli", *args] + return [str(kicad_cli), *args] + + # ── ZIP extraction ─────────────────────────────────────────────────────────── @@ -290,8 +274,9 @@ def _import_zip( def _detect_kicad_cli() -> Optional[Path]: """Return kicad-cli path if found on PATH or a location in _KICAD_CLI_CANDIDATES.""" - if shutil.which("kicad-cli"): - return Path(shutil.which("kicad-cli")) # type: ignore[arg-type] + on_path = shutil.which("kicad-cli") + if on_path is not None: + return Path(on_path) for candidate in _KICAD_CLI_CANDIDATES: if candidate.exists(): return candidate From 09eb38e55048bd3dbe1cff3610e8bed905cc9104 Mon Sep 17 00:00:00 2001 From: Josef Cada Date: Wed, 8 Jul 2026 09:15:02 +0200 Subject: [PATCH 4/8] fix: decode subprocess stderr as text in kicad-cli upgrade helpers --- kicad_lib_manager/commands/import_zip/command.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kicad_lib_manager/commands/import_zip/command.py b/kicad_lib_manager/commands/import_zip/command.py index d4a6ed1..4edc166 100644 --- a/kicad_lib_manager/commands/import_zip/command.py +++ b/kicad_lib_manager/commands/import_zip/command.py @@ -139,7 +139,7 @@ def _upgrade_fp(path: Path, kicad_cli: Optional[Path]) -> None: cmd = _build_kicad_cli_cmd(kicad_cli, "fp", "upgrade", "--force", "--output", str(out_dir), str(in_pretty)) try: - subprocess.run(cmd, capture_output=True, check=True, timeout=30) + subprocess.run(cmd, capture_output=True, text=True, check=True, timeout=30) except subprocess.TimeoutExpired: console.print(f"[yellow] warn: kicad-cli fp upgrade timed out for {path.name}, skipping[/yellow]") return @@ -157,7 +157,7 @@ def _upgrade_sym(sym_file: Path, kicad_cli: Optional[Path]) -> None: return cmd = _build_kicad_cli_cmd(kicad_cli, "sym", "upgrade", "--force", str(sym_file)) try: - subprocess.run(cmd, capture_output=True, check=True, timeout=30) + subprocess.run(cmd, capture_output=True, text=True, check=True, timeout=30) except subprocess.TimeoutExpired: console.print(f"[yellow] warn: kicad-cli sym upgrade timed out for {sym_file.name}, skipping[/yellow]") except subprocess.CalledProcessError as exc: From c051220350fde5a73b28036900b13c231357b4c2 Mon Sep 17 00:00:00 2001 From: Josef Cada Date: Wed, 8 Jul 2026 16:48:22 +0200 Subject: [PATCH 5/8] feat: support UltraLibrarian ZIPs in kilm import UltraLibrarian exports differ from SamacSys/Mouser: the KiCad dir is named "KiCADv6" (case differs, prior lookup was case-sensitive), footprints nest under a "*.pretty" subdir, and symbol files use 2-space/CRLF formatting instead of tabs. Match KiCad/3D dirs case-insensitively, glob footprints/symbols recursively, and rewrite symbol-block extraction to track paren depth instead of relying on a tab-indentation regex, so it works regardless of generator whitespace style. Verified against a real UltraLibrarian export. Co-Authored-By: Claude Sonnet 5 --- .../commands/import_zip/command.py | 86 +++++++++--------- tests/test_import_zip_command.py | 89 +++++++++++++++---- 2 files changed, 112 insertions(+), 63 deletions(-) diff --git a/kicad_lib_manager/commands/import_zip/command.py b/kicad_lib_manager/commands/import_zip/command.py index 4edc166..9e1b81c 100644 --- a/kicad_lib_manager/commands/import_zip/command.py +++ b/kicad_lib_manager/commands/import_zip/command.py @@ -1,5 +1,5 @@ """ -Import command: unpack a SamacSys/Mouser KiCad ZIP into the configured library. +Import command: unpack a SamacSys/Mouser/UltraLibrarian KiCad ZIP into the configured library. """ import re @@ -19,55 +19,47 @@ # ── Symbol helpers ──────────────────────────────────────────────────────────── -_SYM_BLOCK_RE = re.compile(r"^\t\(symbol \"") +def _extract_symbol_blocks(text: str) -> list[str]: + """Extract top-level `(symbol "...")` blocks by tracking paren depth. -def _paren_depth(line: str) -> int: - """Count net paren depth change in line, ignoring parens inside "..." strings.""" + Depth-based (not indentation-based) so it works regardless of whether + the generator indents with tabs (SamacSys/Mouser) or spaces + (UltraLibrarian), and regardless of line endings. + """ + blocks: list[str] = [] depth = 0 in_str = False + block_start: Optional[int] = None i = 0 - while i < len(line): - ch = line[i] + n = len(text) + while i < n: + ch = text[i] if in_str: - if ch == "\\" and i + 1 < len(line): - i += 2 # skip escaped character + if ch == "\\" and i + 1 < n: + i += 2 continue if ch == '"': in_str = False - else: - if ch == '"': - in_str = True - elif ch == "(": - depth += 1 - elif ch == ")": - depth -= 1 + i += 1 + continue + if ch == '"': + in_str = True + elif ch == "(": + if depth == 1 and text.startswith('(symbol "', i): + block_start = i + depth += 1 + elif ch == ")": + depth -= 1 + if depth == 1 and block_start is not None: + blocks.append(text[block_start : i + 1]) + block_start = None i += 1 - return depth - - -def _extract_symbol_blocks(text: str) -> list[str]: - blocks: list[str] = [] - current: list[str] = [] - depth = 0 - in_sym = False - for line in text.split("\n"): - if not in_sym and _SYM_BLOCK_RE.match(line): - in_sym = True - depth = _paren_depth(line) - current = [line] - elif in_sym: - current.append(line) - depth += _paren_depth(line) - if depth == 0: - blocks.append("\n".join(current)) - in_sym = False - current = [] return blocks def _symbol_name(block: str) -> str: - m = re.match(r'\t\(symbol "([^"]+)"', block) + m = re.match(r'\(symbol "([^"]+)"', block) return m.group(1) if m else "" @@ -213,8 +205,10 @@ def _import_zip( with zipfile.ZipFile(zip_path) as zf: _safe_extractall(zf, tmp_path) - kicad_dirs = list(tmp_path.rglob("KiCad")) - model_dirs = list(tmp_path.rglob("3D")) + all_dirs = [d for d in tmp_path.rglob("*") if d.is_dir()] + # "KiCad" (SamacSys/Mouser) or "KiCADv6" (UltraLibrarian), any case + kicad_dirs = [d for d in all_dirs if d.name.lower().startswith("kicad")] + model_dirs = [d for d in all_dirs if d.name.lower() == "3d"] # 3D models for d in model_dirs: @@ -230,9 +224,9 @@ def _import_zip( shutil.copy2(f, dest) result["models"].append(f.name) - # Footprints + # Footprints (UltraLibrarian nests these under a "*.pretty" subdir) for d in kicad_dirs: - for f in d.glob("*.kicad_mod"): + for f in d.rglob("*.kicad_mod"): dest = fp_dir / f.name if dest.exists(): console.print(f" FP skip (exists): {f.name}") @@ -248,7 +242,7 @@ def _import_zip( # Symbols for d in kicad_dirs: - for f in d.glob("*.kicad_sym"): + for f in d.rglob("*.kicad_sym"): if not dry_run: _upgrade_sym(f, kicad_cli) added, skipped = _merge_symbols(f, sym_lib, lib_name, dry_run) @@ -285,7 +279,8 @@ def _detect_kicad_cli() -> Optional[Path]: def import_zip( zip_files: Annotated[ - list[Path], typer.Argument(help="SamacSys/Mouser ZIP file(s) to import") + list[Path], + typer.Argument(help="SamacSys/Mouser/UltraLibrarian ZIP file(s) to import"), ], library: Annotated[ Optional[str], @@ -308,10 +303,11 @@ def import_zip( ), ] = False, ) -> None: - """Import SamacSys/Mouser KiCad ZIP(s) into the configured library. + """Import SamacSys/Mouser/UltraLibrarian KiCad ZIP(s) into the configured library. - Each ZIP should be a standard SamacSys multi-EDA archive as downloaded - from Mouser or component search. The command extracts the KiCad files + Each ZIP should be a standard SamacSys multi-EDA archive (as downloaded + from Mouser or component search) or an UltraLibrarian KiCad export. + The command extracts the KiCad files and merges them into the library. Run 'kilm setup' afterwards to register any newly added libraries in KiCad. """ diff --git a/tests/test_import_zip_command.py b/tests/test_import_zip_command.py index f3e819d..35c9ff2 100644 --- a/tests/test_import_zip_command.py +++ b/tests/test_import_zip_command.py @@ -26,7 +26,7 @@ SAMPLE_SYM_LIB = """\ (kicad_symbol_lib \t(symbol "ExistingPart" -\t\t(property "Footprint" "LPP:ExistingPart") +\t\t(property "Footprint" "SAMPLELIB:ExistingPart") \t) ) """ @@ -37,7 +37,7 @@ \t\t(property "Footprint" "NewPart") \t) \t(symbol "ExistingPart" -\t\t(property "Footprint" "LPP:ExistingPart") +\t\t(property "Footprint" "SAMPLELIB:ExistingPart") \t) ) """ @@ -52,13 +52,13 @@ def test_extract_symbol_blocks(): def test_fix_footprint_ref_adds_prefix(): block = '\t(property "Footprint" "BareFootprint")' - result = _fix_footprint_ref(block, "LPP") - assert '"Footprint" "LPP:BareFootprint"' in result + result = _fix_footprint_ref(block, "SAMPLELIB") + assert '"Footprint" "SAMPLELIB:BareFootprint"' in result def test_fix_footprint_ref_keeps_existing_prefix(): block = '\t(property "Footprint" "OTHER:Footprint")' - result = _fix_footprint_ref(block, "LPP") + result = _fix_footprint_ref(block, "SAMPLELIB") assert '"Footprint" "OTHER:Footprint"' in result @@ -89,18 +89,18 @@ def test_safe_extractall_rejects_zip_slip(tmp_path: Path): def test_fix_3d_path_normalises(): text = '(model "C:/SamacSys/somepart.stp"' - result = _fix_3d_path(text, "LPP.3dshapes") - assert "${KICAD_3RD_PARTY}/LPP.3dshapes/somepart.stp" in result + result = _fix_3d_path(text, "SAMPLELIB.3dshapes") + assert "${KICAD_3RD_PARTY}/SAMPLELIB.3dshapes/somepart.stp" in result def test_merge_symbols_adds_new_skips_existing(tmp_path: Path): - sym_lib = tmp_path / "LPP.kicad_sym" + sym_lib = tmp_path / "SAMPLELIB.kicad_sym" sym_lib.write_text(SAMPLE_SYM_LIB, encoding="utf-8") src = tmp_path / "incoming.kicad_sym" src.write_text(INCOMING_SYM, encoding="utf-8") - added, skipped = _merge_symbols(src, sym_lib, "LPP", dry_run=False) + added, skipped = _merge_symbols(src, sym_lib, "SAMPLELIB", dry_run=False) assert added == ["NewPart"] assert skipped == ["ExistingPart"] @@ -110,13 +110,13 @@ def test_merge_symbols_adds_new_skips_existing(tmp_path: Path): def test_merge_symbols_dry_run_does_not_write(tmp_path: Path): - sym_lib = tmp_path / "LPP.kicad_sym" + sym_lib = tmp_path / "SAMPLELIB.kicad_sym" sym_lib.write_text(SAMPLE_SYM_LIB, encoding="utf-8") src = tmp_path / "incoming.kicad_sym" src.write_text(INCOMING_SYM, encoding="utf-8") original_content = sym_lib.read_text(encoding="utf-8") - added, _ = _merge_symbols(src, sym_lib, "LPP", dry_run=True) + added, _ = _merge_symbols(src, sym_lib, "SAMPLELIB", dry_run=True) assert added == ["NewPart"] assert sym_lib.read_text(encoding="utf-8") == original_content @@ -148,13 +148,44 @@ def _make_samacsys_zip(tmp_path: Path, part_name: str) -> Path: return zip_path +def _make_ultralibrarian_zip(tmp_path: Path, part_name: str) -> Path: + """Build a minimal UltraLibrarian-style ZIP for testing. + + UltraLibrarian differs from SamacSys/Mouser: the KiCad dir is named + "KiCADv6" (not "KiCad"), footprints live in a nested "*.pretty" dir, + and symbol files use 2-space indents with CRLF line endings. + """ + zip_path = tmp_path / f"ul_{part_name}.zip" + + sym_content = ( + "(kicad_symbol_lib (version 20211014) (generator kicad_symbol_editor)\r\n" + f' (symbol "{part_name}" (in_bom yes) (on_board yes)\r\n' + f' (property "Footprint" "{part_name}")\r\n' + f' (symbol "{part_name}_0_1"\r\n' + " )\r\n" + " )\r\n" + ")\r\n" + ) + fp_content = f'(footprint "{part_name}"\n)\n' + + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr(f"{part_name}/KiCADv6/2026-01-01_00-00-00.kicad_sym", sym_content) + zf.writestr( + f"{part_name}/KiCADv6/footprints.pretty/{part_name}.kicad_mod", fp_content + ) + + return zip_path + + @pytest.fixture def library_tree(tmp_path: Path) -> Path: """Set up a minimal library tree with kilm.yaml.""" lib = tmp_path / "mylib" (lib / "symbols").mkdir(parents=True) - (lib / "footprints" / "LPP.pretty").mkdir(parents=True) - (lib / "symbols" / "LPP.kicad_sym").write_text(SAMPLE_SYM_LIB, encoding="utf-8") + (lib / "footprints" / "SAMPLELIB.pretty").mkdir(parents=True) + (lib / "symbols" / "SAMPLELIB.kicad_sym").write_text( + SAMPLE_SYM_LIB, encoding="utf-8" + ) (lib / "kilm.yaml").write_text("name: mylib\n", encoding="utf-8") return lib @@ -185,13 +216,13 @@ def test_import_zip_cli_adds_part( assert result.exit_code == 0, result.output assert "add: TestPart" in result.output - sym_lib = library_tree / "symbols" / "LPP.kicad_sym" + sym_lib = library_tree / "symbols" / "SAMPLELIB.kicad_sym" assert "TestPart" in sym_lib.read_text(encoding="utf-8") - fp_file = library_tree / "footprints" / "LPP.pretty" / "TestPart.kicad_mod" + fp_file = library_tree / "footprints" / "SAMPLELIB.pretty" / "TestPart.kicad_mod" assert fp_file.exists() - model_file = library_tree / "LPP.3dshapes" / "TestPart.stp" + model_file = library_tree / "SAMPLELIB.3dshapes" / "TestPart.stp" assert model_file.exists() @@ -209,9 +240,31 @@ def test_import_zip_cli_dry_run_no_changes( assert result.exit_code == 0, result.output assert "add: DryPart" in result.output - sym_lib = library_tree / "symbols" / "LPP.kicad_sym" + sym_lib = library_tree / "symbols" / "SAMPLELIB.kicad_sym" assert "DryPart" not in sym_lib.read_text(encoding="utf-8") - assert not (library_tree / "LPP.3dshapes" / "DryPart.stp").exists() + assert not (library_tree / "SAMPLELIB.3dshapes" / "DryPart.stp").exists() + + +def test_import_zip_cli_adds_ultralibrarian_part( + tmp_path: Path, library_tree: Path, mock_config: MagicMock +): + zip_path = _make_ultralibrarian_zip(tmp_path, "UlPart") + + with patch( + "kicad_lib_manager.commands.import_zip.command._detect_kicad_cli", + return_value=None, + ): + result = runner.invoke(app, ["import", str(zip_path)]) + + assert result.exit_code == 0, result.output + assert "SYM add: UlPart" in result.output + assert "FP add: UlPart.kicad_mod" in result.output + + sym_lib = library_tree / "symbols" / "SAMPLELIB.kicad_sym" + assert "UlPart" in sym_lib.read_text(encoding="utf-8") + + fp_file = library_tree / "footprints" / "SAMPLELIB.pretty" / "UlPart.kicad_mod" + assert fp_file.exists() def test_import_zip_cli_skips_existing( From 109c19392a9cf5991dd5e82c867764cc106b9637 Mon Sep 17 00:00:00 2001 From: Josef Cada Date: Fri, 10 Jul 2026 15:12:44 +0200 Subject: [PATCH 6/8] feat: support SnapMagic ZIPs in kilm import SnapMagic exports have no wrapping "KiCad"/"3D" directory at all - the .kicad_sym, .kicad_mod, and .step files sit flat at the ZIP root. Drop the vendor-specific directory-name matching (case-insensitive "kicad"/ "3d" dirs) in favor of searching the whole extracted tree by file extension, which also still covers SamacSys/Mouser and UltraLibrarian layouts. Verified against a real SnapMagic export. Co-Authored-By: Claude Sonnet 5 --- .../commands/import_zip/command.py | 89 +++++++++---------- tests/test_import_zip_command.py | 55 ++++++++++++ 2 files changed, 99 insertions(+), 45 deletions(-) diff --git a/kicad_lib_manager/commands/import_zip/command.py b/kicad_lib_manager/commands/import_zip/command.py index 9e1b81c..629fd23 100644 --- a/kicad_lib_manager/commands/import_zip/command.py +++ b/kicad_lib_manager/commands/import_zip/command.py @@ -1,5 +1,5 @@ """ -Import command: unpack a SamacSys/Mouser/UltraLibrarian KiCad ZIP into the configured library. +Import command: unpack a SamacSys/Mouser/UltraLibrarian/SnapMagic KiCad ZIP into the configured library. """ import re @@ -205,53 +205,50 @@ def _import_zip( with zipfile.ZipFile(zip_path) as zf: _safe_extractall(zf, tmp_path) - all_dirs = [d for d in tmp_path.rglob("*") if d.is_dir()] - # "KiCad" (SamacSys/Mouser) or "KiCADv6" (UltraLibrarian), any case - kicad_dirs = [d for d in all_dirs if d.name.lower().startswith("kicad")] - model_dirs = [d for d in all_dirs if d.name.lower() == "3d"] + # Vendor ZIPs disagree on directory layout (SamacSys/Mouser use + # "KiCad"/"3D" dirs, UltraLibrarian uses "KiCADv6" with a nested + # "*.pretty" dir, SnapMagic has no wrapping dir at all) so search + # the whole extracted tree by extension instead of by dir name. # 3D models - for d in model_dirs: - for f in d.iterdir(): - if f.suffix.lower() in (".stp", ".step") or f.name.endswith(".stp.gz"): - dest = models_dir / f.name - if dest.exists(): - console.print(f" 3D skip (exists): {f.name}") - else: - console.print(f" 3D add: {f.name}") - if not dry_run: - models_dir.mkdir(exist_ok=True) - shutil.copy2(f, dest) - result["models"].append(f.name) - - # Footprints (UltraLibrarian nests these under a "*.pretty" subdir) - for d in kicad_dirs: - for f in d.rglob("*.kicad_mod"): - dest = fp_dir / f.name + for f in tmp_path.rglob("*"): + if f.suffix.lower() in (".stp", ".step") or f.name.endswith(".stp.gz"): + dest = models_dir / f.name if dest.exists(): - console.print(f" FP skip (exists): {f.name}") - continue - console.print(f" FP add: {f.name}") - if not dry_run: - text = _fix_3d_path(f.read_text(encoding="utf-8"), models_dir_name) - f.write_text(text, encoding="utf-8") - _upgrade_fp(f, kicad_cli) - fp_dir.mkdir(exist_ok=True) - shutil.copy2(f, dest) - result["fp"].append(f.name) + console.print(f" 3D skip (exists): {f.name}") + else: + console.print(f" 3D add: {f.name}") + if not dry_run: + models_dir.mkdir(exist_ok=True) + shutil.copy2(f, dest) + result["models"].append(f.name) + + # Footprints + for f in tmp_path.rglob("*.kicad_mod"): + dest = fp_dir / f.name + if dest.exists(): + console.print(f" FP skip (exists): {f.name}") + continue + console.print(f" FP add: {f.name}") + if not dry_run: + text = _fix_3d_path(f.read_text(encoding="utf-8"), models_dir_name) + f.write_text(text, encoding="utf-8") + _upgrade_fp(f, kicad_cli) + fp_dir.mkdir(exist_ok=True) + shutil.copy2(f, dest) + result["fp"].append(f.name) # Symbols - for d in kicad_dirs: - for f in d.rglob("*.kicad_sym"): - if not dry_run: - _upgrade_sym(f, kicad_cli) - added, skipped = _merge_symbols(f, sym_lib, lib_name, dry_run) - for name in added: - console.print(f" SYM add: {name}") - for name in skipped: - console.print(f" SYM skip (exists): {name}") - result["sym"].extend(added) - result["sym_skipped"].extend(skipped) + for f in tmp_path.rglob("*.kicad_sym"): + if not dry_run: + _upgrade_sym(f, kicad_cli) + added, skipped = _merge_symbols(f, sym_lib, lib_name, dry_run) + for name in added: + console.print(f" SYM add: {name}") + for name in skipped: + console.print(f" SYM skip (exists): {name}") + result["sym"].extend(added) + result["sym_skipped"].extend(skipped) return result @@ -280,7 +277,9 @@ def _detect_kicad_cli() -> Optional[Path]: def import_zip( zip_files: Annotated[ list[Path], - typer.Argument(help="SamacSys/Mouser/UltraLibrarian ZIP file(s) to import"), + typer.Argument( + help="SamacSys/Mouser/UltraLibrarian/SnapMagic ZIP file(s) to import" + ), ], library: Annotated[ Optional[str], @@ -303,7 +302,7 @@ def import_zip( ), ] = False, ) -> None: - """Import SamacSys/Mouser/UltraLibrarian KiCad ZIP(s) into the configured library. + """Import SamacSys/Mouser/UltraLibrarian/SnapMagic KiCad ZIP(s) into the configured library. Each ZIP should be a standard SamacSys multi-EDA archive (as downloaded from Mouser or component search) or an UltraLibrarian KiCad export. diff --git a/tests/test_import_zip_command.py b/tests/test_import_zip_command.py index 35c9ff2..eab5d04 100644 --- a/tests/test_import_zip_command.py +++ b/tests/test_import_zip_command.py @@ -177,6 +177,35 @@ def _make_ultralibrarian_zip(tmp_path: Path, part_name: str) -> Path: return zip_path +def _make_snapmagic_zip(tmp_path: Path, part_name: str) -> Path: + """Build a minimal SnapMagic-style ZIP for testing. + + SnapMagic differs from every other vendor: there is no wrapping + "KiCad"/"3D" dir at all - the .kicad_sym, .kicad_mod, and .step files + sit directly at the ZIP root. + """ + zip_path = tmp_path / f"{part_name}.zip" + + sym_content = f"""\ +(kicad_symbol_lib +\t(symbol "{part_name}" +\t\t(property "Footprint" "FP_{part_name}") +\t) +) +""" + fp_content = f"""\ +(footprint "FP_{part_name}" +) +""" + with zipfile.ZipFile(zip_path, "w") as zf: + zf.writestr(f"{part_name}.kicad_sym", sym_content) + zf.writestr(f"FP_{part_name}.kicad_mod", fp_content) + zf.writestr(f"{part_name}.step", "STEP data") + zf.writestr("how-to-import.htm", "") + + return zip_path + + @pytest.fixture def library_tree(tmp_path: Path) -> Path: """Set up a minimal library tree with kilm.yaml.""" @@ -267,6 +296,32 @@ def test_import_zip_cli_adds_ultralibrarian_part( assert fp_file.exists() +def test_import_zip_cli_adds_snapmagic_part( + tmp_path: Path, library_tree: Path, mock_config: MagicMock +): + zip_path = _make_snapmagic_zip(tmp_path, "SnapPart") + + with patch( + "kicad_lib_manager.commands.import_zip.command._detect_kicad_cli", + return_value=None, + ): + result = runner.invoke(app, ["import", str(zip_path)]) + + assert result.exit_code == 0, result.output + assert "SYM add: SnapPart" in result.output + assert "FP add: FP_SnapPart.kicad_mod" in result.output + assert "3D add: SnapPart.step" in result.output + + sym_lib = library_tree / "symbols" / "SAMPLELIB.kicad_sym" + assert "SnapPart" in sym_lib.read_text(encoding="utf-8") + + fp_file = library_tree / "footprints" / "SAMPLELIB.pretty" / "FP_SnapPart.kicad_mod" + assert fp_file.exists() + + model_file = library_tree / "SAMPLELIB.3dshapes" / "SnapPart.step" + assert model_file.exists() + + def test_import_zip_cli_skips_existing( tmp_path: Path, library_tree: Path, mock_config: MagicMock ): From eb29a984dd14c4fe761689fefe87188f71b52286 Mon Sep 17 00:00:00 2001 From: Josef Cada Date: Fri, 10 Jul 2026 16:11:48 +0200 Subject: [PATCH 7/8] fix: stop _fix_3d_path from truncating .stp.gz/.step.gz model paths Two sequential re.sub passes let the second (unquoted-path) regex re-match inside the first pass's already-substituted output, and the extension alternation tried "step"/"stp" before their compound forms "step.gz"/"stp.gz" (which contain them as literal prefixes) - both issues caused any gzipped 3D model reference to be truncated mid extension, leaving a dangling ".gz" and a malformed quoted string in the rewritten footprint file. Replaced with a single combined regex with compound extensions ordered first. --- .../commands/import_zip/command.py | 19 ++++++++++++----- tests/test_import_zip_command.py | 21 +++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/kicad_lib_manager/commands/import_zip/command.py b/kicad_lib_manager/commands/import_zip/command.py index 629fd23..f2f5c94 100644 --- a/kicad_lib_manager/commands/import_zip/command.py +++ b/kicad_lib_manager/commands/import_zip/command.py @@ -104,14 +104,21 @@ def _merge_symbols( # ── Footprint helpers ───────────────────────────────────────────────────────── +_MODEL_PATH_RE = re.compile( + # Longer/compound extensions must come before their literal prefixes + # (e.g. "step.gz" before "step"), or the alternation matches the + # shorter one first and leaves the rest of the extension dangling. + r'\(model\s+(?:"([^"]+)"|(\S+\.(?:step\.gz|stp\.gz|step|stp)))' +) + + def _fix_3d_path(text: str, models_dir_name: str) -> str: def _replace(m: re.Match[str]) -> str: - filename = Path(m.group(1).strip('"')).name + raw = m.group(1) if m.group(1) is not None else m.group(2) + filename = Path(raw).name return f'(model "${{KICAD_3RD_PARTY}}/{models_dir_name}/{filename}"' - text = re.sub(r'\(model\s+"([^"]+)"', _replace, text) - text = re.sub(r"\(model\s+(\S+\.(?:stp|step|stp\.gz))", _replace, text) - return text + return _MODEL_PATH_RE.sub(_replace, text) def _upgrade_fp(path: Path, kicad_cli: Optional[Path]) -> None: @@ -212,7 +219,9 @@ def _import_zip( # 3D models for f in tmp_path.rglob("*"): - if f.suffix.lower() in (".stp", ".step") or f.name.endswith(".stp.gz"): + if f.suffix.lower() in (".stp", ".step") or f.name.lower().endswith( + (".stp.gz", ".step.gz") + ): dest = models_dir / f.name if dest.exists(): console.print(f" 3D skip (exists): {f.name}") diff --git a/tests/test_import_zip_command.py b/tests/test_import_zip_command.py index eab5d04..247a577 100644 --- a/tests/test_import_zip_command.py +++ b/tests/test_import_zip_command.py @@ -93,6 +93,27 @@ def test_fix_3d_path_normalises(): assert "${KICAD_3RD_PARTY}/SAMPLELIB.3dshapes/somepart.stp" in result +@pytest.mark.parametrize( + "raw", + [ + '(model "C:/Vendor/somepart.step"', + '(model "C:/Vendor/somepart.stp.gz"', + '(model "C:/Vendor/somepart.step.gz"', + "(model somepart.stp", + "(model somepart.step", + "(model somepart.stp.gz", + "(model somepart.step.gz", + ], +) +def test_fix_3d_path_handles_all_extensions_without_truncation(raw: str): + # Regression: chained re.sub passes (or misordered alternation) previously + # matched a compound extension's own prefix (e.g. "step" inside + # "step.gz"), leaving the rest of the extension as dangling text. + result = _fix_3d_path(raw, "SAMPLELIB.3dshapes") + assert result.count('"') == 2 + assert result.endswith('"') + + def test_merge_symbols_adds_new_skips_existing(tmp_path: Path): sym_lib = tmp_path / "SAMPLELIB.kicad_sym" sym_lib.write_text(SAMPLE_SYM_LIB, encoding="utf-8") From e360253749a218040b431d0de0cf7b53d42c42e4 Mon Sep 17 00:00:00 2001 From: Josef Cada Date: Fri, 10 Jul 2026 16:13:05 +0200 Subject: [PATCH 8/8] fix: address import command review feedback - Reformat with black (command.py, test file were failing black --check despite passing ruff, since E501 is delegated to black) - Sync "kilm import" help text across __init__.py/main.py with the vendor list already in the command docstring (UltraLibrarian, SnapMagic) - Pin COLUMNS=200 in tests so Rich's terminal-width auto-detection can't wrap output differently across environments and break substring assertions --- .../commands/import_zip/__init__.py | 2 +- .../commands/import_zip/command.py | 30 +++++++++++++++---- kicad_lib_manager/main.py | 2 +- tests/test_import_zip_command.py | 12 +++++++- 4 files changed, 37 insertions(+), 9 deletions(-) diff --git a/kicad_lib_manager/commands/import_zip/__init__.py b/kicad_lib_manager/commands/import_zip/__init__.py index 534f76a..9e09d4f 100644 --- a/kicad_lib_manager/commands/import_zip/__init__.py +++ b/kicad_lib_manager/commands/import_zip/__init__.py @@ -4,7 +4,7 @@ import_zip_app = typer.Typer( name="import", - help="Import SamacSys/Mouser KiCad ZIP(s) into the configured library", + help="Import SamacSys/Mouser/UltraLibrarian/SnapMagic KiCad ZIP(s) into the configured library", rich_markup_mode="rich", callback=import_zip, invoke_without_command=True, diff --git a/kicad_lib_manager/commands/import_zip/command.py b/kicad_lib_manager/commands/import_zip/command.py index f2f5c94..5cbf754 100644 --- a/kicad_lib_manager/commands/import_zip/command.py +++ b/kicad_lib_manager/commands/import_zip/command.py @@ -135,15 +135,27 @@ def _upgrade_fp(path: Path, kicad_cli: Optional[Path]) -> None: shutil.copy2(path, in_pretty / path.name) out_dir = Path(tmp) / "out" - cmd = _build_kicad_cli_cmd(kicad_cli, "fp", "upgrade", "--force", "--output", str(out_dir), str(in_pretty)) + cmd = _build_kicad_cli_cmd( + kicad_cli, + "fp", + "upgrade", + "--force", + "--output", + str(out_dir), + str(in_pretty), + ) try: subprocess.run(cmd, capture_output=True, text=True, check=True, timeout=30) except subprocess.TimeoutExpired: - console.print(f"[yellow] warn: kicad-cli fp upgrade timed out for {path.name}, skipping[/yellow]") + console.print( + f"[yellow] warn: kicad-cli fp upgrade timed out for {path.name}, skipping[/yellow]" + ) return except subprocess.CalledProcessError as exc: - console.print(f"[yellow] warn: kicad-cli fp upgrade failed for {path.name}: {exc.stderr.strip()}[/yellow]") + console.print( + f"[yellow] warn: kicad-cli fp upgrade failed for {path.name}: {exc.stderr.strip()}[/yellow]" + ) return upgraded = out_dir / path.name if upgraded.exists(): @@ -158,9 +170,13 @@ def _upgrade_sym(sym_file: Path, kicad_cli: Optional[Path]) -> None: try: subprocess.run(cmd, capture_output=True, text=True, check=True, timeout=30) except subprocess.TimeoutExpired: - console.print(f"[yellow] warn: kicad-cli sym upgrade timed out for {sym_file.name}, skipping[/yellow]") + console.print( + f"[yellow] warn: kicad-cli sym upgrade timed out for {sym_file.name}, skipping[/yellow]" + ) except subprocess.CalledProcessError as exc: - console.print(f"[yellow] warn: kicad-cli sym upgrade failed for {sym_file.name}: {exc.stderr.strip()}[/yellow]") + console.print( + f"[yellow] warn: kicad-cli sym upgrade failed for {sym_file.name}: {exc.stderr.strip()}[/yellow]" + ) def _build_kicad_cli_cmd(kicad_cli: Path, *args: str) -> list[str]: @@ -182,7 +198,9 @@ def _safe_extractall(zf: zipfile.ZipFile, dest: Path) -> None: dest_resolved = dest.resolve() for member in zf.namelist(): member_path = (dest / member).resolve() - if member_path != dest_resolved and not member_path.is_relative_to(dest_resolved): + if member_path != dest_resolved and not member_path.is_relative_to( + dest_resolved + ): raise ValueError(f"Unsafe ZIP entry rejected: {member!r}") zf.extractall(dest) diff --git a/kicad_lib_manager/main.py b/kicad_lib_manager/main.py index d3ce059..f711179 100644 --- a/kicad_lib_manager/main.py +++ b/kicad_lib_manager/main.py @@ -163,7 +163,7 @@ def main( app.add_typer( import_zip_app, name="import", - help="Import SamacSys/Mouser KiCad ZIP(s) into the library", + help="Import SamacSys/Mouser/UltraLibrarian/SnapMagic KiCad ZIP(s) into the library", ) diff --git a/tests/test_import_zip_command.py b/tests/test_import_zip_command.py index 247a577..381d362 100644 --- a/tests/test_import_zip_command.py +++ b/tests/test_import_zip_command.py @@ -21,6 +21,13 @@ runner = CliRunner() + +@pytest.fixture(autouse=True) +def _fixed_console_width(monkeypatch: pytest.MonkeyPatch) -> None: + """Pin terminal width so Rich doesn't wrap output lines differently per host.""" + monkeypatch.setenv("COLUMNS", "200") + + # ── Unit tests for helpers ──────────────────────────────────────────────────── SAMPLE_SYM_LIB = """\ @@ -82,7 +89,10 @@ def test_safe_extractall_rejects_zip_slip(tmp_path: Path): zf.writestr("../../outside.txt", "malicious content") dest = tmp_path / "extract" dest.mkdir() - with zipfile.ZipFile(zip_path) as zf, pytest.raises(ValueError, match="Unsafe ZIP entry"): + with ( + zipfile.ZipFile(zip_path) as zf, + pytest.raises(ValueError, match="Unsafe ZIP entry"), + ): _safe_extractall(zf, dest) assert not (tmp_path / "outside.txt").exists()