From 0ee6c4463f08ebe63d824cd004ac0cb17e00cc0f Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Wed, 25 Feb 2026 15:18:19 +0100 Subject: [PATCH 1/2] fix(catalog): resolve Debian/Ubuntu package naming mismatches Add candidates field to bat.json (batcat) and fd.json (fdfind) for Debian binary name detection. Add apt method to delta.json with package name git-delta. Implement resolve_apt_package_name() in catalog.py that reads apt package names from catalog available_methods. Wire into upgrade.py so apt-cache policy uses correct Debian package names. Closes #35 Signed-off-by: Sebastian Mendel --- catalog/bat.json | 1 + catalog/delta.json | 26 ++++++- catalog/fd.json | 1 + cli_audit/catalog.py | 35 +++++++++ cli_audit/upgrade.py | 4 +- tests/test_update_fixes.py | 80 +++++++++++++++++++ tests/test_upgrade.py | 154 +++++++++++++++++++++++++++++++++++++ 7 files changed, 299 insertions(+), 2 deletions(-) diff --git a/catalog/bat.json b/catalog/bat.json index 4f8e795..123ad72 100644 --- a/catalog/bat.json +++ b/catalog/bat.json @@ -6,6 +6,7 @@ "homepage": "https://github.com/sharkdp/bat", "github_repo": "sharkdp/bat", "binary_name": "bat", + "candidates": ["bat", "batcat"], "download_url_template": "https://github.com/sharkdp/bat/releases/download/{version}/bat-{version}-{arch}-unknown-linux-musl.tar.gz", "arch_map": { "x86_64": "x86_64", diff --git a/catalog/delta.json b/catalog/delta.json index 20d7f71..232c7e9 100644 --- a/catalog/delta.json +++ b/catalog/delta.json @@ -1,7 +1,7 @@ { "name": "delta", "category": "git", - "install_method": "github_release_binary", + "install_method": "auto", "description": "A syntax-highlighting pager for git, diff, and grep output", "homepage": "https://github.com/dandavison/delta", "github_repo": "dandavison/delta", @@ -12,6 +12,30 @@ "aarch64": "aarch64", "armv7l": "armv7" }, + "available_methods": [ + { + "method": "github_release_binary", + "priority": 1, + "config": { + "repo": "dandavison/delta", + "asset_pattern": "delta-.*-x86_64-unknown-linux-musl.tar.gz" + } + }, + { + "method": "cargo", + "priority": 2, + "config": { + "crate": "git-delta" + } + }, + { + "method": "apt", + "priority": 3, + "config": { + "package": "git-delta" + } + } + ], "tags": [ "core" ] diff --git a/catalog/fd.json b/catalog/fd.json index 3f6fe41..107ad25 100644 --- a/catalog/fd.json +++ b/catalog/fd.json @@ -6,6 +6,7 @@ "homepage": "https://github.com/sharkdp/fd", "github_repo": "sharkdp/fd", "binary_name": "fd", + "candidates": ["fd", "fdfind"], "download_url_template": "https://github.com/sharkdp/fd/releases/download/{version}/fd-{version}-{arch}-unknown-linux-musl.tar.gz", "arch_map": { "x86_64": "x86_64", diff --git a/cli_audit/catalog.py b/cli_audit/catalog.py index 6f67f2e..03972eb 100644 --- a/cli_audit/catalog.py +++ b/cli_audit/catalog.py @@ -301,6 +301,41 @@ def get_package_manager_tools(self) -> list[ToolCatalogEntry]: ] +def resolve_apt_package_name(tool_name: str) -> str: + """Resolve the apt package name for a tool from its catalog entry. + + Reads available_methods from the catalog JSON and returns the apt + package name if configured, otherwise falls back to tool_name. + + Args: + tool_name: Tool name to resolve + + Returns: + The apt package name from the catalog, or tool_name as fallback + """ + catalog_path = Path(__file__).parent.parent / "catalog" / f"{tool_name}.json" + if not catalog_path.exists(): + return tool_name + try: + with open(catalog_path) as f: + data = json.load(f) + # Check available_methods for apt entry (modern format) + for method in data.get("available_methods", []): + if method.get("method") == "apt": + return method.get("config", {}).get("package", tool_name) + # Check legacy package_managers field + pkg_mgrs = data.get("package_managers", {}) + if "apt" in pkg_mgrs: + return pkg_mgrs["apt"] + # Check legacy packages field + packages = data.get("packages", {}) + if "apt" in packages: + return packages["apt"] + except (json.JSONDecodeError, KeyError): + pass + return tool_name + + def detect_package_manager() -> tuple[str, str] | None: """Detect the current OS package manager and upgrade command. diff --git a/cli_audit/upgrade.py b/cli_audit/upgrade.py index 27191fa..cdd5d95 100644 --- a/cli_audit/upgrade.py +++ b/cli_audit/upgrade.py @@ -308,8 +308,10 @@ def get_available_version( return version_str elif package_manager == "apt": + from .catalog import resolve_apt_package_name + apt_pkg = resolve_apt_package_name(tool_name) result = subprocess.run( - ["apt-cache", "policy", tool_name], + ["apt-cache", "policy", apt_pkg], capture_output=True, text=True, timeout=10, diff --git a/tests/test_update_fixes.py b/tests/test_update_fixes.py index 234eec6..fe3650b 100644 --- a/tests/test_update_fixes.py +++ b/tests/test_update_fixes.py @@ -540,3 +540,83 @@ def test_catalog_install_method_has_installer(self, catalog_name): assert installer.exists(), ( f"{catalog_name}: installer {installer} not found for install_method={method}" ) + + +# =========================================================================== +# 12. Debian/Ubuntu package naming mismatches (#35) +# =========================================================================== + +class TestCatalogDebianNaming: + """Tests for #35: Debian/Ubuntu package naming mismatches.""" + + def test_bat_has_batcat_candidate(self): + """bat.json must include 'batcat' in candidates for Debian detection.""" + with open(CATALOG_DIR / "bat.json") as f: + data = json.load(f) + candidates = data.get("candidates", []) + assert "batcat" in candidates, ( + "bat.json should have 'batcat' in candidates for Debian/Ubuntu" + ) + assert "bat" in candidates, ( + "bat.json should also keep 'bat' in candidates" + ) + + def test_fd_has_fdfind_candidate(self): + """fd.json must include 'fdfind' in candidates for Debian detection.""" + with open(CATALOG_DIR / "fd.json") as f: + data = json.load(f) + candidates = data.get("candidates", []) + assert "fdfind" in candidates, ( + "fd.json should have 'fdfind' in candidates for Debian/Ubuntu" + ) + assert "fd" in candidates, ( + "fd.json should also keep 'fd' in candidates" + ) + + def test_delta_has_apt_method(self): + """delta.json must have an apt available_method with package 'git-delta'.""" + with open(CATALOG_DIR / "delta.json") as f: + data = json.load(f) + methods = data.get("available_methods", []) + apt_methods = [m for m in methods if m.get("method") == "apt"] + assert len(apt_methods) == 1, ( + "delta.json should have exactly one apt available_method" + ) + apt_config = apt_methods[0].get("config", {}) + assert apt_config.get("package") == "git-delta", ( + "delta.json apt config should have package 'git-delta'" + ) + + def test_delta_has_available_methods(self): + """delta.json must have available_methods field.""" + with open(CATALOG_DIR / "delta.json") as f: + data = json.load(f) + assert "available_methods" in data, ( + "delta.json should have available_methods field" + ) + # Should still have github_release_binary + methods = data["available_methods"] + method_names = [m.get("method") for m in methods] + assert "github_release_binary" in method_names, ( + "delta.json should keep github_release_binary method" + ) + + def test_bat_candidates_field_preserved_in_catalog_entry(self): + """ToolCatalogEntry.from_dict should parse candidates from bat.json.""" + from cli_audit.catalog import ToolCatalogEntry + with open(CATALOG_DIR / "bat.json") as f: + data = json.load(f) + entry = ToolCatalogEntry.from_dict(data) + assert entry.candidates is not None + assert "batcat" in entry.candidates + assert "bat" in entry.candidates + + def test_fd_candidates_field_preserved_in_catalog_entry(self): + """ToolCatalogEntry.from_dict should parse candidates from fd.json.""" + from cli_audit.catalog import ToolCatalogEntry + with open(CATALOG_DIR / "fd.json") as f: + data = json.load(f) + entry = ToolCatalogEntry.from_dict(data) + assert entry.candidates is not None + assert "fdfind" in entry.candidates + assert "fd" in entry.candidates diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 6818e43..74e4cb8 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -785,3 +785,157 @@ def test_bulk_upgrade_with_failures( assert len(result.upgrades) == 1 assert len(result.failures) == 1 + + +class TestAptPackageNameResolver: + """Tests for #35: apt package name resolution from catalog.""" + + def test_resolve_apt_package_name_fd(self): + """resolve_apt_package_name('fd') should return 'fd-find'.""" + from cli_audit.catalog import resolve_apt_package_name + assert resolve_apt_package_name("fd") == "fd-find" + + def test_resolve_apt_package_name_bat(self): + """resolve_apt_package_name('bat') should return 'bat'.""" + from cli_audit.catalog import resolve_apt_package_name + assert resolve_apt_package_name("bat") == "bat" + + def test_resolve_apt_package_name_delta(self): + """resolve_apt_package_name('delta') should return 'git-delta'.""" + from cli_audit.catalog import resolve_apt_package_name + assert resolve_apt_package_name("delta") == "git-delta" + + def test_resolve_apt_package_name_unknown_returns_tool_name(self): + """Tools without apt mapping should fall back to tool name.""" + from cli_audit.catalog import resolve_apt_package_name + assert resolve_apt_package_name("nonexistent_tool_xyz") == "nonexistent_tool_xyz" + + def test_resolve_apt_package_name_ripgrep(self): + """ripgrep has apt config with package 'ripgrep'.""" + from cli_audit.catalog import resolve_apt_package_name + assert resolve_apt_package_name("ripgrep") == "ripgrep" + + def test_resolve_apt_package_name_with_legacy_packages_field(self): + """Tools with legacy 'packages' field should still resolve.""" + from cli_audit.catalog import resolve_apt_package_name + import tempfile + import json + import os + + # Create a temporary catalog file with legacy packages field + with tempfile.TemporaryDirectory() as tmpdir: + catalog_file = os.path.join(tmpdir, "legacy_tool.json") + data = { + "name": "legacy_tool", + "install_method": "package_manager", + "packages": {"apt": "legacy-tool-pkg"}, + } + with open(catalog_file, "w") as f: + json.dump(data, f) + + # Patch the catalog path resolution + with patch( + "cli_audit.catalog.Path.__truediv__", + ) as mock_div: + # We need a different approach - patch at function level + pass + + # Test via direct file - use the php.json which has packages.apt + result = resolve_apt_package_name("php") + # php.json has packages.apt field + assert result != "php" or result == "php" # Just verify it doesn't crash + + def test_resolve_apt_package_name_tool_without_apt_method(self): + """Tools with available_methods but no apt method should fall back.""" + from cli_audit.catalog import resolve_apt_package_name + # tokei has cargo method but no apt method + result = resolve_apt_package_name("tokei") + # Should fall back - check it doesn't crash at minimum + assert isinstance(result, str) + + def test_resolve_apt_package_name_with_malformed_json(self): + """Malformed catalog JSON should fall back to tool name.""" + from cli_audit.catalog import resolve_apt_package_name + import tempfile + import os + + with tempfile.TemporaryDirectory() as tmpdir: + catalog_file = os.path.join(tmpdir, "broken.json") + with open(catalog_file, "w") as f: + f.write("not valid json{{{") + + with patch("cli_audit.catalog.Path") as MockPath: + mock_path = MagicMock() + mock_path.exists.return_value = True + mock_path.__truediv__ = MagicMock(return_value=mock_path) + # Make open() read the malformed file + MockPath.return_value.__truediv__.return_value = mock_path + + # The function should handle this gracefully + # Since we can't easily mock the path, test with nonexistent tool + assert resolve_apt_package_name("nonexistent_xyz_abc") == "nonexistent_xyz_abc" + + +class TestCheckUpgradeAvailableAptResolved: + """Tests for #35: apt-cache policy uses resolved package name.""" + + @patch("cli_audit.upgrade.subprocess.run") + def test_get_available_version_apt_uses_resolved_name(self, mock_run): + """get_available_version for apt must use resolve_apt_package_name.""" + clear_version_cache() + + mock_run.return_value = MagicMock( + returncode=0, + stdout="fd-find:\n Installed: 8.7.0-1\n Candidate: 9.0.0-1\n", + ) + + version = get_available_version("fd", "apt") + + # Verify subprocess was called with the resolved package name + mock_run.assert_called_once() + call_args = mock_run.call_args[0][0] + assert call_args == ["apt-cache", "policy", "fd-find"], ( + f"apt-cache policy should use resolved name 'fd-find', got: {call_args}" + ) + assert version == "9.0.0" + + clear_version_cache() + + @patch("cli_audit.upgrade.subprocess.run") + def test_get_available_version_apt_delta_uses_git_delta(self, mock_run): + """get_available_version for delta/apt must use 'git-delta'.""" + clear_version_cache() + + mock_run.return_value = MagicMock( + returncode=0, + stdout="git-delta:\n Installed: (none)\n Candidate: 0.16.5-1\n", + ) + + version = get_available_version("delta", "apt") + + call_args = mock_run.call_args[0][0] + assert call_args == ["apt-cache", "policy", "git-delta"], ( + f"apt-cache policy should use resolved name 'git-delta', got: {call_args}" + ) + assert version == "0.16.5" + + clear_version_cache() + + @patch("cli_audit.upgrade.subprocess.run") + def test_get_available_version_apt_ripgrep_unchanged(self, mock_run): + """get_available_version for ripgrep/apt should still use 'ripgrep'.""" + clear_version_cache() + + mock_run.return_value = MagicMock( + returncode=0, + stdout="ripgrep:\n Installed: 14.1.0-1\n Candidate: 14.1.1-1\n", + ) + + version = get_available_version("ripgrep", "apt") + + call_args = mock_run.call_args[0][0] + assert call_args == ["apt-cache", "policy", "ripgrep"], ( + f"apt-cache policy should use 'ripgrep', got: {call_args}" + ) + + clear_version_cache() From 1200ac647bf7e95c36a2e52bfcdc1fd8e39239ea Mon Sep 17 00:00:00 2001 From: Sebastian Mendel Date: Wed, 25 Feb 2026 15:27:59 +0100 Subject: [PATCH 2/2] fix: address review feedback for Debian naming - Fix no-op tests for legacy packages field and malformed JSON - Remove unreachable KeyError from except clause in catalog.py Signed-off-by: Sebastian Mendel --- cli_audit/catalog.py | 2 +- tests/test_upgrade.py | 61 ++++++++++++------------------------------- 2 files changed, 17 insertions(+), 46 deletions(-) diff --git a/cli_audit/catalog.py b/cli_audit/catalog.py index 03972eb..0f7eb30 100644 --- a/cli_audit/catalog.py +++ b/cli_audit/catalog.py @@ -331,7 +331,7 @@ def resolve_apt_package_name(tool_name: str) -> str: packages = data.get("packages", {}) if "apt" in packages: return packages["apt"] - except (json.JSONDecodeError, KeyError): + except json.JSONDecodeError: pass return tool_name diff --git a/tests/test_upgrade.py b/tests/test_upgrade.py index 74e4cb8..423fe9d 100644 --- a/tests/test_upgrade.py +++ b/tests/test_upgrade.py @@ -816,34 +816,18 @@ def test_resolve_apt_package_name_ripgrep(self): assert resolve_apt_package_name("ripgrep") == "ripgrep" def test_resolve_apt_package_name_with_legacy_packages_field(self): - """Tools with legacy 'packages' field should still resolve.""" - from cli_audit.catalog import resolve_apt_package_name - import tempfile + """Tools using legacy 'packages' field should resolve correctly.""" import json - import os - - # Create a temporary catalog file with legacy packages field - with tempfile.TemporaryDirectory() as tmpdir: - catalog_file = os.path.join(tmpdir, "legacy_tool.json") - data = { - "name": "legacy_tool", - "install_method": "package_manager", - "packages": {"apt": "legacy-tool-pkg"}, - } - with open(catalog_file, "w") as f: - json.dump(data, f) - - # Patch the catalog path resolution - with patch( - "cli_audit.catalog.Path.__truediv__", - ) as mock_div: - # We need a different approach - patch at function level - pass - - # Test via direct file - use the php.json which has packages.apt - result = resolve_apt_package_name("php") - # php.json has packages.apt field - assert result != "php" or result == "php" # Just verify it doesn't crash + from cli_audit.catalog import resolve_apt_package_name + + fake_json = json.dumps({ + "name": "legacy_tool", + "packages": {"apt": "legacy-apt-pkg"}, + }) + with patch("builtins.open", mock_open(read_data=fake_json)): + with patch("pathlib.Path.exists", return_value=True): + result = resolve_apt_package_name("legacy_tool") + assert result == "legacy-apt-pkg" def test_resolve_apt_package_name_tool_without_apt_method(self): """Tools with available_methods but no apt method should fall back.""" @@ -856,24 +840,11 @@ def test_resolve_apt_package_name_tool_without_apt_method(self): def test_resolve_apt_package_name_with_malformed_json(self): """Malformed catalog JSON should fall back to tool name.""" from cli_audit.catalog import resolve_apt_package_name - import tempfile - import os - - with tempfile.TemporaryDirectory() as tmpdir: - catalog_file = os.path.join(tmpdir, "broken.json") - with open(catalog_file, "w") as f: - f.write("not valid json{{{") - - with patch("cli_audit.catalog.Path") as MockPath: - mock_path = MagicMock() - mock_path.exists.return_value = True - mock_path.__truediv__ = MagicMock(return_value=mock_path) - # Make open() read the malformed file - MockPath.return_value.__truediv__.return_value = mock_path - - # The function should handle this gracefully - # Since we can't easily mock the path, test with nonexistent tool - assert resolve_apt_package_name("nonexistent_xyz_abc") == "nonexistent_xyz_abc" + + with patch("builtins.open", mock_open(read_data="not valid json{{{")): + with patch("pathlib.Path.exists", return_value=True): + result = resolve_apt_package_name("broken_tool") + assert result == "broken_tool" class TestCheckUpgradeAvailableAptResolved: