From 0ad397ac896688b863fea35ec7c1bf42eda78a6f Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 11 May 2026 10:57:46 -0400 Subject: [PATCH 01/10] work in progress --- menuinst/api.py | 103 +++++++++++++++++++++++++++++++++++++ menuinst/platforms/base.py | 29 ++++++++++- pyproject.toml | 4 ++ recipe/meta.yaml | 2 + 4 files changed, 137 insertions(+), 1 deletion(-) diff --git a/menuinst/api.py b/menuinst/api.py index a93a2a65..6885a29c 100644 --- a/menuinst/api.py +++ b/menuinst/api.py @@ -10,6 +10,12 @@ from pathlib import Path from typing import Callable, Union +try: + import tomllib +except ImportError: + import tomli as tomllib +import tomli_w + from .platforms import Menu, MenuItem from .utils import ( DEFAULT_BASE_PREFIX, @@ -20,6 +26,7 @@ ) log = getLogger(__name__) +MENUINST_TOML_SCHEMA_VERSION = 1 __all__ = [ @@ -38,6 +45,82 @@ def _maybe_try_user(base_prefix: str, target_prefix: str) -> bool: return Path(base_prefix, ".nonadmin").is_file() +def read_menuinst_toml(prefix: Path) -> dict: + """Read menuinst.toml from prefix, returning empty dict if missing/invalid.""" + toml_path = prefix / "Menu" / "menuinst.toml" + if not toml_path.exists(): + return {} + try: + with open(toml_path, "rb") as f: + data = tomllib.load(f) + version = data.get("schema_version", 1) + if version > MENUINST_TOML_SCHEMA_VERSION: + log.warning( + "menuinst.toml at %s has schema_version %d, " + "but this menuinst only understands version %d", + toml_path, + version, + MENUINST_TOML_SCHEMA_VERSION, + ) + return data + except Exception as e: + log.warning("Failed to read menuinst.toml from %s: %s", toml_path, e) + return {} + + +def write_menuinst_toml(prefix: Path, data: dict) -> None: + """Write menuinst.toml atomically.""" + data.setdefault("schema_version", MENUINST_TOML_SCHEMA_VERSION) + menu_dir = prefix / "Menu" + menu_dir.mkdir(parents=True, exist_ok=True) + toml_path = menu_dir / "menuinst.toml" + tmp_path = menu_dir / "menuinst.toml.tmp" + with open(tmp_path, "wb") as f: + tomli_w.dump(data, f) + tmp_path.replace(toml_path) + + +def record_shortcuts( + prefix: Path, + base_prefix: Path, + source: str, + paths: list[os.PathLike], + distribution_name: str | None = None, +) -> None: + """Record created shortcuts to menuinst.toml.""" + if not paths: + return + + data = read_menuinst_toml(prefix) + + # Write distribution_name only to base prefix, and only if not already set + if prefix.samefile(base_prefix) and distribution_name: + data.setdefault("distribution_name", distribution_name) + + # Append shortcuts + shortcuts = data.setdefault("shortcuts", []) + for path in paths: + shortcuts.append({"source": source, "path": str(path)}) + + write_menuinst_toml(prefix, data) + + +def remove_shortcut_records(prefix: Path, source: str) -> None: + """Remove shortcut entries matching source from menuinst.toml.""" + data = read_menuinst_toml(prefix) + if not data: + return + + shortcuts = data.get("shortcuts", []) + if not shortcuts: + return + + # Filter out entries matching this source + data["shortcuts"] = [s for s in shortcuts if s.get("source") != source] + + write_menuinst_toml(prefix, data) + + def _load( metadata_or_path: os.PathLike | dict, target_prefix: str | None = None, @@ -77,6 +160,19 @@ def install( for menu_item in menu_items: paths += menu_item.create() + # Record shortcuts to menuinst.toml + if isinstance(metadata_or_path, (str, Path)): + source = Path(metadata_or_path).name + else: + source = f"{menu.name}.json" + record_shortcuts( + Path(target_prefix), + Path(base_prefix), + source, + paths, + distribution_name=menu.placeholders.get("DISTRIBUTION_NAME"), + ) + return paths @@ -108,6 +204,13 @@ def remove( paths += menu_item.remove() paths += menu.remove() + # Remove shortcut records from menuinst.toml + if isinstance(metadata_or_path, (str, Path)): + source = Path(metadata_or_path).name + else: + source = f"{menu.name}.json" + remove_shortcut_records(Path(target_prefix), source) + return paths diff --git a/menuinst/platforms/base.py b/menuinst/platforms/base.py index 9a544c5d..3c027b1b 100644 --- a/menuinst/platforms/base.py +++ b/menuinst/platforms/base.py @@ -12,6 +12,11 @@ from tempfile import NamedTemporaryFile from typing import Any, Iterable, Mapping +try: + import tomllib +except ImportError: + import tomli as tomllib + from ..utils import ( DEFAULT_BASE_PREFIX, DEFAULT_PREFIX, @@ -64,6 +69,28 @@ def render(self, value: Any, slug: bool = False, extra: dict | None = None) -> A value = slugify(value) return value + def _get_distribution_name(self) -> str: + """ + Get distribution name with resolution order: + 1. MENUINST_DISTRIBUTION_NAME env var + 2. $BASE_PREFIX/Menu/menuinst.toml + 3. base_prefix.name (fallback) + """ + if name := os.environ.get("MENUINST_DISTRIBUTION_NAME"): + return name + + toml_path = self.base_prefix / "Menu" / "menuinst.toml" + if toml_path.exists(): + try: + with open(toml_path, "rb") as f: + data = tomllib.load(f) + if name := data.get("distribution_name"): + return name + except Exception as e: + log.warning("Failed to read menuinst.toml from %s: %s", toml_path, e) + + return self.base_prefix.name + @property def placeholders(self) -> dict[str, str]: """ @@ -74,7 +101,7 @@ def placeholders(self) -> dict[str, str]: """ return { "BASE_PREFIX": str(self.base_prefix), - "DISTRIBUTION_NAME": self.base_prefix.name, + "DISTRIBUTION_NAME": self._get_distribution_name(), "BASE_PYTHON": str(self.base_prefix / "bin" / "python"), "PREFIX": str(self.prefix), "ENV_NAME": self.env_name, diff --git a/pyproject.toml b/pyproject.toml index c01ba4b3..2d345ac5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -11,6 +11,10 @@ requires-python = ">=3.10" dynamic = [ "version" ] +dependencies = [ + "tomli; python_version < '3.11'", + "tomli-w", +] [project.scripts] menuinst = "menuinst.cli:main" diff --git a/recipe/meta.yaml b/recipe/meta.yaml index 5b6574cd..c4fa26fa 100644 --- a/recipe/meta.yaml +++ b/recipe/meta.yaml @@ -34,6 +34,8 @@ requirements: - setuptools_scm >=6.2 run: - python + - tomli # [py<311] + - tomli-w test: requires: From cb1360d48590dd25429b3e2b41a6ceb5e7919bdc Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 11 May 2026 11:11:08 -0400 Subject: [PATCH 02/10] Add tests --- tests/test_metadata.py | 136 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 136 insertions(+) create mode 100644 tests/test_metadata.py diff --git a/tests/test_metadata.py b/tests/test_metadata.py new file mode 100644 index 00000000..01599c04 --- /dev/null +++ b/tests/test_metadata.py @@ -0,0 +1,136 @@ +"""Tests for distribution_name resolution and menuinst.toml tracking.""" + +from menuinst.api import ( + read_menuinst_toml, + record_shortcuts, + remove_shortcut_records, + write_menuinst_toml, +) +from menuinst.platforms import Menu + +# Placeholder distribution names for tests +DIST_NAME = "Something" +DIST_NAME_ALT = "SomethingElse" + + +class TestGetDistributionName: + """Tests for Menu._get_distribution_name() resolution order.""" + + def test_env_var_takes_priority(self, tmp_path, monkeypatch): + """MENUINST_DISTRIBUTION_NAME env var should be used when set.""" + monkeypatch.setenv("MENUINST_DISTRIBUTION_NAME", DIST_NAME) + menu = Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) + assert menu._get_distribution_name() == DIST_NAME + assert menu.placeholders["DISTRIBUTION_NAME"] == DIST_NAME + + def test_toml_used_when_no_env_var(self, tmp_path, monkeypatch): + """TOML value should be used when env var is not set.""" + monkeypatch.delenv("MENUINST_DISTRIBUTION_NAME", raising=False) + write_menuinst_toml(tmp_path, {"distribution_name": DIST_NAME}) + menu = Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) + assert menu._get_distribution_name() == DIST_NAME + + def test_fallback_to_base_prefix_name(self, tmp_path, monkeypatch): + """Should fall back to base_prefix.name when no env var or TOML.""" + monkeypatch.delenv("MENUINST_DISTRIBUTION_NAME", raising=False) + menu = Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) + assert menu._get_distribution_name() == tmp_path.name + + def test_env_var_overrides_toml(self, tmp_path, monkeypatch): + """Env var should take priority over TOML value.""" + monkeypatch.setenv("MENUINST_DISTRIBUTION_NAME", DIST_NAME) + write_menuinst_toml(tmp_path, {"distribution_name": DIST_NAME_ALT}) + menu = Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) + assert menu._get_distribution_name() == DIST_NAME + + def test_malformed_toml_graceful_fallback(self, tmp_path, monkeypatch, caplog): + """Malformed TOML should log warning and fall back to base_prefix.name.""" + monkeypatch.delenv("MENUINST_DISTRIBUTION_NAME", raising=False) + toml_path = tmp_path / "Menu" / "menuinst.toml" + toml_path.parent.mkdir(parents=True, exist_ok=True) + toml_path.write_text("this is not valid toml {{{{") + menu = Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) + assert menu._get_distribution_name() == tmp_path.name + assert "Failed to read menuinst.toml" in caplog.text + + +class TestShortcutRecording: + """Tests for shortcut recording and removal in menuinst.toml.""" + + def test_install_records_to_toml(self, tmp_path, monkeypatch): + """install() should record shortcuts to menuinst.toml.""" + monkeypatch.delenv("MENUINST_DISTRIBUTION_NAME", raising=False) + base_prefix = tmp_path / "base" + base_prefix.mkdir() + + # Test via record_shortcuts directly + record_shortcuts( + base_prefix, + base_prefix, + "foo.json", + [tmp_path / "foo.lnk", tmp_path / "bar.lnk"], + distribution_name=DIST_NAME, + ) + + data = read_menuinst_toml(base_prefix) + assert data["distribution_name"] == DIST_NAME + assert len(data["shortcuts"]) == 2 + assert data["shortcuts"][0]["source"] == "foo.json" + + def test_remove_cleans_toml(self, tmp_path, monkeypatch): + """remove() should clean up TOML entries.""" + monkeypatch.delenv("MENUINST_DISTRIBUTION_NAME", raising=False) + base_prefix = tmp_path / "base" + base_prefix.mkdir() + + # Pre-populate TOML with shortcuts from two sources + write_menuinst_toml( + base_prefix, + { + "distribution_name": DIST_NAME, + "shortcuts": [ + {"source": "foo.json", "path": "/path/to/foo.lnk"}, + {"source": "foo.json", "path": "/path/to/bar.lnk"}, + {"source": "baz.json", "path": "/path/to/baz.lnk"}, + ], + }, + ) + + # Remove records for foo.json + remove_shortcut_records(base_prefix, "foo.json") + + data = read_menuinst_toml(base_prefix) + assert len(data["shortcuts"]) == 1 + assert data["shortcuts"][0]["source"] == "baz.json" + # distribution_name should be preserved + assert data["distribution_name"] == DIST_NAME + + def test_distribution_name_only_written_to_base_prefix(self, tmp_path): + """distribution_name should only be written when prefix == base_prefix.""" + base_prefix = tmp_path / "base" + env_prefix = tmp_path / "envs" / "foo" + base_prefix.mkdir(parents=True) + env_prefix.mkdir(parents=True) + + # Record to base prefix - should include distribution_name + record_shortcuts( + base_prefix, + base_prefix, + "foo.json", + [tmp_path / "foo.lnk"], + distribution_name=DIST_NAME, + ) + data = read_menuinst_toml(base_prefix) + assert data.get("distribution_name") == DIST_NAME + + # Record to non-base prefix - should NOT include distribution_name + record_shortcuts( + env_prefix, + base_prefix, + "bar.json", + [tmp_path / "bar.lnk"], + distribution_name=DIST_NAME, + ) + data = read_menuinst_toml(env_prefix) + assert "distribution_name" not in data + assert len(data["shortcuts"]) == 1 From 48eb399ed4834151cdf41cdc088ff197304c072a Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 11 May 2026 11:21:10 -0400 Subject: [PATCH 03/10] work in progress --- menuinst/api.py | 35 ++++++----------------------------- menuinst/platforms/base.py | 18 ++++-------------- menuinst/utils.py | 29 +++++++++++++++++++++++++++++ tests/test_metadata.py | 16 ++++++++-------- 4 files changed, 47 insertions(+), 51 deletions(-) diff --git a/menuinst/api.py b/menuinst/api.py index 6885a29c..0262cc83 100644 --- a/menuinst/api.py +++ b/menuinst/api.py @@ -10,23 +10,20 @@ from pathlib import Path from typing import Callable, Union -try: - import tomllib -except ImportError: - import tomli as tomllib import tomli_w from .platforms import Menu, MenuItem from .utils import ( DEFAULT_BASE_PREFIX, DEFAULT_PREFIX, + MENUINST_TOML_SCHEMA_VERSION, _UserOrSystem, elevate_as_needed, + read_menuinst_toml, user_is_admin, ) log = getLogger(__name__) -MENUINST_TOML_SCHEMA_VERSION = 1 __all__ = [ @@ -45,29 +42,6 @@ def _maybe_try_user(base_prefix: str, target_prefix: str) -> bool: return Path(base_prefix, ".nonadmin").is_file() -def read_menuinst_toml(prefix: Path) -> dict: - """Read menuinst.toml from prefix, returning empty dict if missing/invalid.""" - toml_path = prefix / "Menu" / "menuinst.toml" - if not toml_path.exists(): - return {} - try: - with open(toml_path, "rb") as f: - data = tomllib.load(f) - version = data.get("schema_version", 1) - if version > MENUINST_TOML_SCHEMA_VERSION: - log.warning( - "menuinst.toml at %s has schema_version %d, " - "but this menuinst only understands version %d", - toml_path, - version, - MENUINST_TOML_SCHEMA_VERSION, - ) - return data - except Exception as e: - log.warning("Failed to read menuinst.toml from %s: %s", toml_path, e) - return {} - - def write_menuinst_toml(prefix: Path, data: dict) -> None: """Write menuinst.toml atomically.""" data.setdefault("schema_version", MENUINST_TOML_SCHEMA_VERSION) @@ -116,8 +90,11 @@ def remove_shortcut_records(prefix: Path, source: str) -> None: return # Filter out entries matching this source - data["shortcuts"] = [s for s in shortcuts if s.get("source") != source] + filtered = [s for s in shortcuts if s.get("source") != source] + if len(filtered) == len(shortcuts): + return # Nothing was removed + data["shortcuts"] = filtered write_menuinst_toml(prefix, data) diff --git a/menuinst/platforms/base.py b/menuinst/platforms/base.py index 3c027b1b..851a75d8 100644 --- a/menuinst/platforms/base.py +++ b/menuinst/platforms/base.py @@ -12,11 +12,6 @@ from tempfile import NamedTemporaryFile from typing import Any, Iterable, Mapping -try: - import tomllib -except ImportError: - import tomli as tomllib - from ..utils import ( DEFAULT_BASE_PREFIX, DEFAULT_PREFIX, @@ -24,6 +19,7 @@ data_path, deep_update, logged_run, + read_menuinst_toml, slugify, ) @@ -79,15 +75,9 @@ def _get_distribution_name(self) -> str: if name := os.environ.get("MENUINST_DISTRIBUTION_NAME"): return name - toml_path = self.base_prefix / "Menu" / "menuinst.toml" - if toml_path.exists(): - try: - with open(toml_path, "rb") as f: - data = tomllib.load(f) - if name := data.get("distribution_name"): - return name - except Exception as e: - log.warning("Failed to read menuinst.toml from %s: %s", toml_path, e) + data = read_menuinst_toml(self.base_prefix) + if name := data.get("distribution_name"): + return name return self.base_prefix.name diff --git a/menuinst/utils.py b/menuinst/utils.py index 2e25195c..4668c95d 100644 --- a/menuinst/utils.py +++ b/menuinst/utils.py @@ -13,7 +13,13 @@ from typing import Any, Callable, Iterable, Literal, Mapping, Optional, Sequence, Union from unicodedata import normalize +try: + import tomllib +except ImportError: + import tomli as tomllib + logger = getLogger(__name__) +MENUINST_TOML_SCHEMA_VERSION = 1 _TargetOrBase = Union[Literal["target"], Literal["base"]] _UserOrSystem = Union[Literal["user"], Literal["system"]] @@ -70,6 +76,29 @@ def _default_prefix(which: _TargetOrBase = "target") -> str: DEFAULT_BASE_PREFIX = _default_prefix("base") +def read_menuinst_toml(prefix: Path) -> dict: + """Read menuinst.toml from prefix, returning empty dict if missing/invalid.""" + toml_path = prefix / "Menu" / "menuinst.toml" + if not toml_path.exists(): + return {} + try: + with open(toml_path, "rb") as f: + data = tomllib.load(f) + version = data.get("schema_version", 1) + if version > MENUINST_TOML_SCHEMA_VERSION: + logger.warning( + "menuinst.toml at %s has schema_version %d, " + "but this menuinst only understands version %d", + toml_path, + version, + MENUINST_TOML_SCHEMA_VERSION, + ) + return data + except Exception as e: + logger.warning("Failed to read menuinst.toml from %s: %s", toml_path, e) + return {} + + def slugify(text: str): # Adapted from from django.utils.text.slugify # Copyright (c) Django Software Foundation and individual contributors. diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 01599c04..91c3849b 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -1,12 +1,12 @@ -"""Tests for distribution_name resolution and menuinst.toml tracking.""" - -from menuinst.api import ( - read_menuinst_toml, - record_shortcuts, - remove_shortcut_records, - write_menuinst_toml, -) +"""Tests for distribution_name resolution and menuinst.toml tracking. + +TODO: Add integration tests that call install()/remove() with real menu JSON +to verify end-to-end TOML recording through the full shortcut lifecycle. +""" + +from menuinst.api import record_shortcuts, remove_shortcut_records, write_menuinst_toml from menuinst.platforms import Menu +from menuinst.utils import read_menuinst_toml # Placeholder distribution names for tests DIST_NAME = "Something" From 04e17e315a203acabebc2d560a58a22921529d39 Mon Sep 17 00:00:00 2001 From: Robin Date: Mon, 11 May 2026 12:51:16 -0400 Subject: [PATCH 04/10] Add todo --- menuinst/api.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/menuinst/api.py b/menuinst/api.py index 0262cc83..94bbe317 100644 --- a/menuinst/api.py +++ b/menuinst/api.py @@ -80,7 +80,12 @@ def record_shortcuts( def remove_shortcut_records(prefix: Path, source: str) -> None: - """Remove shortcut entries matching source from menuinst.toml.""" + """Remove shortcut entries matching source from menuinst.toml. + + TODO: Use the recorded paths as the source of truth for shortcut removal, + instead of recomputing paths from menu JSON metadata. This would handle + cases where shortcuts were moved or the menu JSON changed. + """ data = read_menuinst_toml(prefix) if not data: return From 606c8bc29b737aba5efc9085e9cf888948796077 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 12 May 2026 10:17:41 -0400 Subject: [PATCH 05/10] Ensure .toml file is written even if no shortcuts --- menuinst/cli/cli.py | 15 ++++++++++++++- tests/test_cli.py | 35 +++++++++++++++++++++++++++++++---- 2 files changed, 45 insertions(+), 5 deletions(-) diff --git a/menuinst/cli/cli.py b/menuinst/cli/cli.py index 133eb854..f6f899da 100644 --- a/menuinst/cli/cli.py +++ b/menuinst/cli/cli.py @@ -1,11 +1,13 @@ from __future__ import annotations import argparse +import os import re import sys from pathlib import Path -from ..api import _install_adapter +from ..api import _install_adapter, write_menuinst_toml +from ..utils import read_menuinst_toml _MENU_RE = re.compile(r"(?:[-\._]menu)?\.json$", re.IGNORECASE) @@ -70,6 +72,17 @@ def install( if root_prefix: root_prefix = str(Path(root_prefix).expanduser().resolve()) + # Persist distribution_name from env var before processing shortcuts. + # The env var allows installers to set distribution_name dynamically at + # install time. We must persist it here because the env var is transient + # and may not be set when packages with shortcuts are installed later. + if env_name := os.environ.get("MENUINST_DISTRIBUTION_NAME"): + base = Path(root_prefix) if root_prefix else prefix + data = read_menuinst_toml(base) + if "distribution_name" not in data: + data["distribution_name"] = env_name + write_menuinst_toml(base, data) + for json_path in (prefix / "Menu").glob("*.json"): if ( packages diff --git a/tests/test_cli.py b/tests/test_cli.py index 2279fd55..abdd028b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,7 +1,7 @@ from __future__ import annotations import json -from typing import TYPE_CHECKING +from pathlib import Path import pytest from conftest import DATA @@ -9,9 +9,6 @@ from menuinst.api import install, remove from menuinst.cli import main as menuinst_main -if TYPE_CHECKING: - from pathlib import Path - def _setup_menu_directory(prefix: Path) -> dict[str, set[Path]]: packages = { @@ -114,3 +111,33 @@ def test_cli_errors(argv: list[str]) -> None: with pytest.raises(SystemExit) as exc: menuinst_main(argv) assert exc.value.code == 2 + + +@pytest.mark.parametrize( + "env_var_set,expect_toml", + [ + pytest.param(True, True, id="env_var_set_creates_toml"), + pytest.param(False, False, id="no_env_var_no_toml"), + ], +) +def test_cli_env_var_persistence( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, env_var_set: bool, expect_toml: bool +) -> None: + """Test that MENUINST_DISTRIBUTION_NAME env var is persisted to menuinst.toml.""" + (tmp_path / ".nonadmin").touch() + (tmp_path / "Menu").mkdir() + + if env_var_set: + monkeypatch.setenv("MENUINST_DISTRIBUTION_NAME", "TestApp") + else: + monkeypatch.delenv("MENUINST_DISTRIBUTION_NAME", raising=False) + + menuinst_main(["--install", "--prefix", str(tmp_path), "--root-prefix", str(tmp_path)]) + + toml_path = tmp_path / "Menu" / "menuinst.toml" + if expect_toml: + assert toml_path.exists(), "menuinst.toml should be created when env var is set" + content = toml_path.read_text() + assert 'distribution_name = "TestApp"' in content + else: + assert not toml_path.exists(), "menuinst.toml should not be created without env var" From a2fbeaba772418683629bf8af5ebd193fc980c0d Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 12 May 2026 15:08:49 -0400 Subject: [PATCH 06/10] Fix bug with source --- menuinst/api.py | 5 +++-- tests/test_metadata.py | 48 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/menuinst/api.py b/menuinst/api.py index 94bbe317..16bcec90 100644 --- a/menuinst/api.py +++ b/menuinst/api.py @@ -283,7 +283,8 @@ def _install_adapter(path: str, remove: bool = False, prefix: str = DEFAULT_PREF kwargs.setdefault("base_prefix", kwargs.pop("root_prefix", DEFAULT_BASE_PREFIX)) if kwargs["base_prefix"] is None: kwargs["base_prefix"] = DEFAULT_BASE_PREFIX + # Pass path so install/remove records the actual filename in menuinst.toml if remove: - _api_remove(metadata, target_prefix=prefix, **kwargs) + _api_remove(json_path, target_prefix=prefix, **kwargs) else: - install(metadata, target_prefix=prefix, **kwargs) + install(json_path, target_prefix=prefix, **kwargs) diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 91c3849b..a7ae2769 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -1,10 +1,13 @@ -"""Tests for distribution_name resolution and menuinst.toml tracking. +"""Tests for distribution_name resolution and menuinst.toml tracking.""" -TODO: Add integration tests that call install()/remove() with real menu JSON -to verify end-to-end TOML recording through the full shortcut lifecycle. -""" +import json -from menuinst.api import record_shortcuts, remove_shortcut_records, write_menuinst_toml +from menuinst.api import ( + _install_adapter, + record_shortcuts, + remove_shortcut_records, + write_menuinst_toml, +) from menuinst.platforms import Menu from menuinst.utils import read_menuinst_toml @@ -134,3 +137,38 @@ def test_distribution_name_only_written_to_base_prefix(self, tmp_path): data = read_menuinst_toml(env_prefix) assert "distribution_name" not in data assert len(data["shortcuts"]) == 1 + + +class TestInstallAdapter: + """Tests for _install_adapter recording correct source filename.""" + + def test_records_actual_filename_not_menu_name(self, tmp_path, monkeypatch): + """_install_adapter should record JSON filename, not rendered menu_name.""" + monkeypatch.delenv("MENUINST_DISTRIBUTION_NAME", raising=False) + (tmp_path / ".nonadmin").touch() + menu_dir = tmp_path / "Menu" + menu_dir.mkdir() + + # Create JSON with menu_name containing placeholder + json_file = menu_dir / "test_shortcut.json" + json_file.write_text( + json.dumps( + { + "$schema": "https://json-schema.org/draft-07/schema", + "menu_name": "{{ DISTRIBUTION_NAME }} Foo Bar", + "menu_items": [ + { + "name": "Foo Bar", + "command": ["echo", "test"], + "platforms": {"linux": {}, "win": {}, "osx": {}}, + } + ], + } + ) + ) + + _install_adapter(str(json_file), prefix=str(tmp_path), root_prefix=str(tmp_path)) + + data = read_menuinst_toml(tmp_path) + # Source should be the filename, not "{{ DISTRIBUTION_NAME }} Foo Bar.json" + assert data["shortcuts"][0]["source"] == "test_shortcut.json" From a2911907a725942c83cd4abb204ca92d720e2218 Mon Sep 17 00:00:00 2001 From: Robin Date: Tue, 12 May 2026 15:17:31 -0400 Subject: [PATCH 07/10] Add activate: False --- tests/test_metadata.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_metadata.py b/tests/test_metadata.py index a7ae2769..f1b5017e 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -160,6 +160,7 @@ def test_records_actual_filename_not_menu_name(self, tmp_path, monkeypatch): { "name": "Foo Bar", "command": ["echo", "test"], + "activate": False, "platforms": {"linux": {}, "win": {}, "osx": {}}, } ], From d5324fb8330ff7fef53419c899a62de2a6ec6c56 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 13 May 2026 13:43:24 -0400 Subject: [PATCH 08/10] Review fixes --- menuinst/cli/cli.py | 4 ++-- menuinst/utils.py | 30 ++++++++++++++++-------------- tests/test_cli.py | 2 +- tests/test_metadata.py | 11 ++++++----- 4 files changed, 25 insertions(+), 22 deletions(-) diff --git a/menuinst/cli/cli.py b/menuinst/cli/cli.py index f6f899da..ebf78918 100644 --- a/menuinst/cli/cli.py +++ b/menuinst/cli/cli.py @@ -76,11 +76,11 @@ def install( # The env var allows installers to set distribution_name dynamically at # install time. We must persist it here because the env var is transient # and may not be set when packages with shortcuts are installed later. - if env_name := os.environ.get("MENUINST_DISTRIBUTION_NAME"): + if distribution_name := os.environ.get("MENUINST_DISTRIBUTION_NAME"): base = Path(root_prefix) if root_prefix else prefix data = read_menuinst_toml(base) if "distribution_name" not in data: - data["distribution_name"] = env_name + data["distribution_name"] = distribution_name write_menuinst_toml(base, data) for json_path in (prefix / "Menu").glob("*.json"): diff --git a/menuinst/utils.py b/menuinst/utils.py index 4668c95d..921d9eb6 100644 --- a/menuinst/utils.py +++ b/menuinst/utils.py @@ -77,26 +77,28 @@ def _default_prefix(which: _TargetOrBase = "target") -> str: def read_menuinst_toml(prefix: Path) -> dict: - """Read menuinst.toml from prefix, returning empty dict if missing/invalid.""" + """Read menuinst.toml from prefix, returning empty dict if missing. + + Raises an exception if the file exists but is invalid. + """ toml_path = prefix / "Menu" / "menuinst.toml" if not toml_path.exists(): return {} try: with open(toml_path, "rb") as f: data = tomllib.load(f) - version = data.get("schema_version", 1) - if version > MENUINST_TOML_SCHEMA_VERSION: - logger.warning( - "menuinst.toml at %s has schema_version %d, " - "but this menuinst only understands version %d", - toml_path, - version, - MENUINST_TOML_SCHEMA_VERSION, - ) - return data - except Exception as e: - logger.warning("Failed to read menuinst.toml from %s: %s", toml_path, e) - return {} + except Exception as exc: + raise ValueError(f"Failed to read {toml_path}") from exc + version = data.get("schema_version", 1) + if version > MENUINST_TOML_SCHEMA_VERSION: + logger.warning( + "menuinst.toml at %s has schema_version %d, " + "but this menuinst only understands version %d", + toml_path, + version, + MENUINST_TOML_SCHEMA_VERSION, + ) + return data def slugify(text: str): diff --git a/tests/test_cli.py b/tests/test_cli.py index abdd028b..24574123 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -123,7 +123,7 @@ def test_cli_errors(argv: list[str]) -> None: def test_cli_env_var_persistence( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, env_var_set: bool, expect_toml: bool ) -> None: - """Test that MENUINST_DISTRIBUTION_NAME env var is persisted to menuinst.toml.""" + """Test that MENUINST_DISTRIBUTION_NAME env var persists in menuinst.toml.""" (tmp_path / ".nonadmin").touch() (tmp_path / "Menu").mkdir() diff --git a/tests/test_metadata.py b/tests/test_metadata.py index f1b5017e..9401f24d 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -2,6 +2,8 @@ import json +import pytest + from menuinst.api import ( _install_adapter, record_shortcuts, @@ -46,15 +48,14 @@ def test_env_var_overrides_toml(self, tmp_path, monkeypatch): menu = Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) assert menu._get_distribution_name() == DIST_NAME - def test_malformed_toml_graceful_fallback(self, tmp_path, monkeypatch, caplog): - """Malformed TOML should log warning and fall back to base_prefix.name.""" + def test_malformed_toml_raises(self, tmp_path, monkeypatch): + """Malformed TOML should raise an exception.""" monkeypatch.delenv("MENUINST_DISTRIBUTION_NAME", raising=False) toml_path = tmp_path / "Menu" / "menuinst.toml" toml_path.parent.mkdir(parents=True, exist_ok=True) toml_path.write_text("this is not valid toml {{{{") - menu = Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) - assert menu._get_distribution_name() == tmp_path.name - assert "Failed to read menuinst.toml" in caplog.text + with pytest.raises(ValueError, match="Failed to read"): + Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) class TestShortcutRecording: From 7aeb8e6a8a7aaa73909104b6d1359fa466773905 Mon Sep 17 00:00:00 2001 From: Robin Date: Wed, 13 May 2026 13:59:47 -0400 Subject: [PATCH 09/10] Update test to account for platform differences --- tests/test_metadata.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 9401f24d..55d1fd84 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -55,7 +55,11 @@ def test_malformed_toml_raises(self, tmp_path, monkeypatch): toml_path.parent.mkdir(parents=True, exist_ok=True) toml_path.write_text("this is not valid toml {{{{") with pytest.raises(ValueError, match="Failed to read"): - Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) + # On Linux, Menu() triggers _get_distribution_name() during __init__, + # but on Windows/macOS it's lazy. Call it explicitly to ensure the + # error is raised on all platforms. + menu = Menu("test", prefix=str(tmp_path), base_prefix=str(tmp_path)) + menu._get_distribution_name() class TestShortcutRecording: From df00f5312adf230cfec53710857ac588d865bf23 Mon Sep 17 00:00:00 2001 From: Robin Date: Thu, 14 May 2026 14:03:05 -0400 Subject: [PATCH 10/10] Add SchemaVer, move function --- menuinst/api.py | 16 +-------------- menuinst/cli/cli.py | 4 ++-- menuinst/utils.py | 44 +++++++++++++++++++++++++++++++++++++----- tests/test_metadata.py | 43 ++++++++++++++++++++++++++++++++++++++++- 4 files changed, 84 insertions(+), 23 deletions(-) diff --git a/menuinst/api.py b/menuinst/api.py index 16bcec90..5481a58f 100644 --- a/menuinst/api.py +++ b/menuinst/api.py @@ -10,17 +10,15 @@ from pathlib import Path from typing import Callable, Union -import tomli_w - from .platforms import Menu, MenuItem from .utils import ( DEFAULT_BASE_PREFIX, DEFAULT_PREFIX, - MENUINST_TOML_SCHEMA_VERSION, _UserOrSystem, elevate_as_needed, read_menuinst_toml, user_is_admin, + write_menuinst_toml, ) log = getLogger(__name__) @@ -42,18 +40,6 @@ def _maybe_try_user(base_prefix: str, target_prefix: str) -> bool: return Path(base_prefix, ".nonadmin").is_file() -def write_menuinst_toml(prefix: Path, data: dict) -> None: - """Write menuinst.toml atomically.""" - data.setdefault("schema_version", MENUINST_TOML_SCHEMA_VERSION) - menu_dir = prefix / "Menu" - menu_dir.mkdir(parents=True, exist_ok=True) - toml_path = menu_dir / "menuinst.toml" - tmp_path = menu_dir / "menuinst.toml.tmp" - with open(tmp_path, "wb") as f: - tomli_w.dump(data, f) - tmp_path.replace(toml_path) - - def record_shortcuts( prefix: Path, base_prefix: Path, diff --git a/menuinst/cli/cli.py b/menuinst/cli/cli.py index ebf78918..07cd973a 100644 --- a/menuinst/cli/cli.py +++ b/menuinst/cli/cli.py @@ -6,8 +6,8 @@ import sys from pathlib import Path -from ..api import _install_adapter, write_menuinst_toml -from ..utils import read_menuinst_toml +from ..api import _install_adapter +from ..utils import read_menuinst_toml, write_menuinst_toml _MENU_RE = re.compile(r"(?:[-\._]menu)?\.json$", re.IGNORECASE) diff --git a/menuinst/utils.py b/menuinst/utils.py index 921d9eb6..afeeac24 100644 --- a/menuinst/utils.py +++ b/menuinst/utils.py @@ -18,9 +18,21 @@ except ImportError: import tomli as tomllib +import tomli_w + logger = getLogger(__name__) -MENUINST_TOML_SCHEMA_VERSION = 1 +MENUINST_TOML_SCHEMA_VERSION = "1-0-0" _TargetOrBase = Union[Literal["target"], Literal["base"]] + + +def parse_schemaver(version: str) -> tuple[int, int, int]: + """Parse SchemaVer string 'MODEL-REVISION-ADDITION' into tuple of ints.""" + parts = version.split("-") + if len(parts) != 3: + raise ValueError(f"Invalid SchemaVer format: {version}") + return int(parts[0]), int(parts[1]), int(parts[2]) + + _UserOrSystem = Union[Literal["user"], Literal["system"]] @@ -89,11 +101,21 @@ def read_menuinst_toml(prefix: Path) -> dict: data = tomllib.load(f) except Exception as exc: raise ValueError(f"Failed to read {toml_path}") from exc - version = data.get("schema_version", 1) - if version > MENUINST_TOML_SCHEMA_VERSION: + version = data.get("schema_version", "1-0-0") + try: + file_ver = parse_schemaver(version) + current_ver = parse_schemaver(MENUINST_TOML_SCHEMA_VERSION) + except ValueError: logger.warning( - "menuinst.toml at %s has schema_version %d, " - "but this menuinst only understands version %d", + "menuinst.toml at %s has invalid schema_version %r", + toml_path, + version, + ) + return data + if file_ver > current_ver: + logger.warning( + "menuinst.toml at %s has schema_version %s, " + "but this menuinst only understands version %s", toml_path, version, MENUINST_TOML_SCHEMA_VERSION, @@ -101,6 +123,18 @@ def read_menuinst_toml(prefix: Path) -> dict: return data +def write_menuinst_toml(prefix: Path, data: dict) -> None: + """Write menuinst.toml atomically.""" + data.setdefault("schema_version", MENUINST_TOML_SCHEMA_VERSION) + menu_dir = prefix / "Menu" + menu_dir.mkdir(parents=True, exist_ok=True) + toml_path = menu_dir / "menuinst.toml" + tmp_path = menu_dir / "menuinst.toml.tmp" + with open(tmp_path, "wb") as f: + tomli_w.dump(data, f) + tmp_path.replace(toml_path) + + def slugify(text: str): # Adapted from from django.utils.text.slugify # Copyright (c) Django Software Foundation and individual contributors. diff --git a/tests/test_metadata.py b/tests/test_metadata.py index 55d1fd84..1f786e83 100644 --- a/tests/test_metadata.py +++ b/tests/test_metadata.py @@ -11,7 +11,7 @@ write_menuinst_toml, ) from menuinst.platforms import Menu -from menuinst.utils import read_menuinst_toml +from menuinst.utils import MENUINST_TOML_SCHEMA_VERSION, parse_schemaver, read_menuinst_toml # Placeholder distribution names for tests DIST_NAME = "Something" @@ -178,3 +178,44 @@ def test_records_actual_filename_not_menu_name(self, tmp_path, monkeypatch): data = read_menuinst_toml(tmp_path) # Source should be the filename, not "{{ DISTRIBUTION_NAME }} Foo Bar.json" assert data["shortcuts"][0]["source"] == "test_shortcut.json" + + +class TestSchemaVersion: + """Tests for SchemaVer parsing and TOML schema version handling.""" + + @pytest.mark.parametrize( + "version,expected", + [ + ("1-0-0", (1, 0, 0)), + ("1-1-3", (1, 1, 3)), + ("2-0-0", (2, 0, 0)), + ("10-20-30", (10, 20, 30)), + ], + ) + def test_parse_schemaver_valid(self, version, expected): + """Valid SchemaVer strings should parse correctly.""" + assert parse_schemaver(version) == expected + + @pytest.mark.parametrize( + "version", + [ + "1", + "1-0", + "1.0.0", + "1-0-0-0", + "a-b-c", + "", + ], + ) + def test_parse_schemaver_invalid(self, version): + """Invalid SchemaVer strings should raise ValueError.""" + with pytest.raises(ValueError): + parse_schemaver(version) + + def test_toml_writes_schemaver_format(self, tmp_path): + """write_menuinst_toml should write schema_version in SchemaVer format.""" + write_menuinst_toml(tmp_path, {"distribution_name": "Test"}) + data = read_menuinst_toml(tmp_path) + assert data["schema_version"] == MENUINST_TOML_SCHEMA_VERSION + # Verify it's a valid SchemaVer string + parse_schemaver(data["schema_version"])