Skip to content

fix(catalog): resolve Debian/Ubuntu package naming mismatches#39

Merged
CybotTM merged 2 commits into
mainfrom
fix/debian-package-naming
Feb 25, 2026
Merged

fix(catalog): resolve Debian/Ubuntu package naming mismatches#39
CybotTM merged 2 commits into
mainfrom
fix/debian-package-naming

Conversation

@CybotTM
Copy link
Copy Markdown
Member

@CybotTM CybotTM commented Feb 25, 2026

Summary

  • Add candidates field to bat.json (batcat) and fd.json (fdfind) for Debian binary 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
  • Wire resolver into upgrade.py so apt-cache policy uses correct Debian package names

Closes #35

Test plan

  • 17 new tests (6 catalog + 8 resolver + 3 integration)
  • TestCatalogDebianNaming: verifies candidates and apt methods in catalog JSON
  • TestAptPackageNameResolver: covers fd, bat, delta, ripgrep, unknown tools, legacy fields, malformed JSON
  • TestCheckUpgradeAvailableAptResolved: verifies apt-cache policy receives resolved package name
  • All 509 tests pass

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 <info@sebastianmendel.de>
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

Copy link
Copy Markdown
Member Author

@CybotTM CybotTM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: fix(catalog): resolve Debian/Ubuntu package naming mismatches

What was done well

  • The catalog JSON changes (bat.json, fd.json, delta.json) are all factually correct. Verified against the actual apt repository: bat package is bat, fd package is fd-find, delta package is git-delta.
  • The resolve_apt_package_name() function has a clean design with proper fallback chain: available_methods (modern) -> package_managers (legacy) -> packages (legacy) -> tool_name (default).
  • The candidates additions to bat.json and fd.json correctly address binary name detection on Debian.
  • The delta.json changes (adding available_methods, changing install_method to auto) are well-structured and consistent with other catalog entries like bat.json, fd.json, and ripgrep.json.
  • The integration tests in TestCheckUpgradeAvailableAptResolved are well-written -- they mock subprocess.run and verify the exact command-line arguments, which directly validates the fix.
  • CI passes on Ubuntu and macOS (Windows and lint still pending at time of review).

Issues requiring changes

Important: Two tests are no-ops (2 of claimed 17 tests)

The PR description claims 17 tests, but two of them do not test their stated behavior:

  1. test_resolve_apt_package_name_with_legacy_packages_field (tests/test_upgrade.py): The mock setup exits scope before assertions. The final assertion assert result != "php" or result == "php" is a tautology (always True). This test verifies nothing about legacy field handling.

  2. test_resolve_apt_package_name_with_malformed_json (tests/test_upgrade.py): Same pattern -- mock exits scope, then tests a nonexistent tool (the "file not found" path) instead of the "malformed JSON" path. The json.JSONDecodeError handler in resolve_apt_package_name() is never exercised by any test.

These should either be fixed to properly test their stated scenarios or removed. See inline comments for suggested fixes.

Suggestion: Scope clarification

The original issue (#35) identifies three affected code paths: upgrade.py, installer.py, and install_plan.py. This PR only fixes upgrade.py (the apt-cache policy query path). The PR description should note that installer/install_plan fixes are deferred.

Minor observations

  • The except (json.JSONDecodeError, KeyError) in resolve_apt_package_name() includes KeyError which is unreachable given that all dictionary accesses use .get() with defaults. See inline comment.
  • The resolver reads catalog JSON from disk on every call rather than reusing the existing ToolCatalog instance. Not a blocker but worth considering for efficiency in bulk operations.
  • The ctags tool (packages.apt: "universal-ctags") and sponge (packages.apt: "moreutils") also have name mismatches but are package_manager install method tools, so they go through a different code path and are not affected by this specific fix. Good that the scope is contained.

Verdict

The core fix is correct and well-targeted. The two no-op tests need to be fixed before merge -- they give a false sense of coverage for the legacy field and malformed JSON error paths.

Comment thread cli_audit/catalog.py
Comment thread tests/test_upgrade.py Outdated
Comment thread tests/test_upgrade.py Outdated
Comment thread cli_audit/upgrade.py
Comment thread cli_audit/catalog.py Outdated
- 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 <info@sebastianmendel.de>
@CybotTM CybotTM merged commit 7faa758 into main Feb 25, 2026
11 checks passed
@CybotTM CybotTM deleted the fix/debian-package-naming branch March 16, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(catalog): Debian/Ubuntu package naming mismatches break detection and installation

1 participant