Add repository package discovery script#89
Conversation
Docker-based tool that queries pgEdge DNF and APT repositories, diffs discovered packages against catalog.json, and interactively walks through adding new packages to the catalog. Features: - Queries both pgedge and pgedge-noarch EL repos + DEB repo - Shell glob exclusion patterns for non-user-facing packages - PG-version detection (standalone vs versioned) - Meta-package membership from DEB deps (authoritative source) - Interactive walkthrough with category/name/description prompts - Self-test suite (5 checks, no Docker required) - Flags: --discover-only, --verbose, --el-only, --deb-only Run: ./scripts/discover-repo-packages.sh --discover-only Test: ./scripts/discover-repo-packages.sh --self-test
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
Deploying pgedge-docs with
|
| Latest commit: |
3c542e9
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d746cd2a.pgedge-docs.pages.dev |
| Branch Preview URL: | https://feat-repo-discovery-script.pgedge-docs.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
scripts/discover-repo-packages.sh (3)
1027-1027: Stale TODO marker — confirm intent before merging.
# Additional tests added in later tasksreads like a hand-off note from PR drafting. If no follow-up tests are planned for this PR, remove it; otherwise capture the work in a tracking issue so it doesn't linger.Would you like me to open a follow-up issue describing the remaining self-test coverage (e.g., catalog version-expansion, exclusion-glob edge cases) so this comment can be removed?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/discover-repo-packages.sh` at line 1027, The inline TODO comment "# Additional tests added in later tasks" is stale—either remove it or replace it with a tracked work item; confirm whether more tests will be added for self-test coverage (catalog version-expansion, exclusion-glob edge cases) and if not delete the line from scripts/discover-repo-packages.sh, otherwise create a follow-up issue and replace the comment with the issue number or URL so the intent is captured and won't linger; look for the exact string "# Additional tests added in later tasks" to locate and update the comment.
668-669: 💤 Low valueSC2188: use
: > fileinstead of a bare redirection.
> "$file"with no command is a quirk that works in bash but is flagged by shellcheck and isn't portable to other POSIX shells. Replace with the standard: > "$file"idiom in bothdiscover_elanddiscover_deb.♻️ Proposed fix (apply at both locations)
- > "$output_dir/rocky9-packages.txt" - > "$output_dir/rocky9-metapkg-deps.txt" + : > "$output_dir/rocky9-packages.txt" + : > "$output_dir/rocky9-metapkg-deps.txt"- > "$output_dir/debian12-packages.txt" - > "$output_dir/debian12-metapkg-deps.txt" + : > "$output_dir/debian12-packages.txt" + : > "$output_dir/debian12-metapkg-deps.txt"Also applies to: 717-718
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/discover-repo-packages.sh` around lines 668 - 669, The script uses bare redirections like > "$output_dir/rocky9-packages.txt" (and similarly for "$output_dir/rocky9-metapkg-deps.txt") inside the discover_el and discover_deb logic, which ShellCheck SC2188 flags as non-portable; replace each bare redirection with the portable no-op truncation form : > "$output_dir/rocky9-packages.txt" and : > "$output_dir/rocky9-metapkg-deps.txt" (and the analogous two occurrences around lines noted for discover_deb) so files are created/truncated portably.
458-458: ⚡ Quick winHardcoded
18will silently regress when a new PG major ships.
detect_pg_versionsalready enumerates("16" "17" "18")(lines 89, 160), the metapkgrepoquery/apt-cache dependscalls target…_18/…-18directly, and the{ver}expansion ininteractive_walkthroughuses18as the canonical resolution. The moment PG 19 lands, all three places will need to be updated in lockstep or membership detection will quietly miss the new variants.Centralize this — for example a single
DEFAULT_PG_VERSIONconstant near the top (sourced fromdefault_pg_versionin catalog.json if available), and reference it indiscover_el,discover_deb, andinteractive_walkthrough.Also applies to: 660-662, 709-711
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/discover-repo-packages.sh` at line 458, Replace the hardcoded "18" by introducing a single DEFAULT_PG_VERSION variable near the top (set it from catalog.json's default_pg_version if present, otherwise derive the latest entry from detect_pg_versions), then update usage sites: change the rpm_name expansion that sets check_name (where rpm_name//\{ver\}/18), the metapkg repoquery/apt-cache depends constructions in discover_el and discover_deb, and the {ver} resolution in interactive_walkthrough to use ${DEFAULT_PG_VERSION}; keep detect_pg_versions unchanged as the authoritative list but reference DEFAULT_PG_VERSION for canonical resolution so new PG majors don’t require multiple edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/discover-repo-packages.sh`:
- Around line 202-211: The pkg_in_metapkg function (and the same loose check
used in interactive_walkthrough for in_full/in_minimal classification) currently
treats any dependency containing $pkg as a match, causing false positives;
change the matching logic to only accept exact tokens or clearly delimited
variants (e.g., match "$dep" == "$pkg" or "$dep" == "${pkg}_*" or split dep
tokens and compare each token exactly) so versioned or suffixed names like
pkg_... are matched intentionally and not by arbitrary substring containment;
update pkg_in_metapkg and the corresponding checks in interactive_walkthrough to
use this stricter comparison.
- Around line 53-58: The load_exclusions loop currently uses line="${line// /}"
which removes all internal spaces and ignores tabs, breaking patterns with
leading tabs; change the trimming logic in load_exclusions so you only strip
leading and trailing whitespace (spaces and tabs) and preserve internal spaces
when populating EXCLUSION_PATTERNS. Replace the single global-space removal with
a two-step trim using POSIX parameter expansion (remove leading whitespace, then
trailing whitespace) before the empty-check and appending to EXCLUSION_PATTERNS;
keep the existing comment-stripping step (line="${line%%#*}") and the rest of
the loop unchanged so is_excluded can match correctly.
---
Nitpick comments:
In `@scripts/discover-repo-packages.sh`:
- Line 1027: The inline TODO comment "# Additional tests added in later tasks"
is stale—either remove it or replace it with a tracked work item; confirm
whether more tests will be added for self-test coverage (catalog
version-expansion, exclusion-glob edge cases) and if not delete the line from
scripts/discover-repo-packages.sh, otherwise create a follow-up issue and
replace the comment with the issue number or URL so the intent is captured and
won't linger; look for the exact string "# Additional tests added in later
tasks" to locate and update the comment.
- Around line 668-669: The script uses bare redirections like >
"$output_dir/rocky9-packages.txt" (and similarly for
"$output_dir/rocky9-metapkg-deps.txt") inside the discover_el and discover_deb
logic, which ShellCheck SC2188 flags as non-portable; replace each bare
redirection with the portable no-op truncation form : >
"$output_dir/rocky9-packages.txt" and : > "$output_dir/rocky9-metapkg-deps.txt"
(and the analogous two occurrences around lines noted for discover_deb) so files
are created/truncated portably.
- Line 458: Replace the hardcoded "18" by introducing a single
DEFAULT_PG_VERSION variable near the top (set it from catalog.json's
default_pg_version if present, otherwise derive the latest entry from
detect_pg_versions), then update usage sites: change the rpm_name expansion that
sets check_name (where rpm_name//\{ver\}/18), the metapkg repoquery/apt-cache
depends constructions in discover_el and discover_deb, and the {ver} resolution
in interactive_walkthrough to use ${DEFAULT_PG_VERSION}; keep detect_pg_versions
unchanged as the authoritative list but reference DEFAULT_PG_VERSION for
canonical resolution so new PG majors don’t require multiple edits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aac0f51a-6f47-41ac-9b44-7a05e09693fa
📒 Files selected for processing (10)
.gitignorescripts/discover-repo-packages.shscripts/excluded-packages.txtscripts/test-fixtures/catalog-test.jsonscripts/test-fixtures/deb-metapkg-deps.txtscripts/test-fixtures/deb-packages.txtscripts/test-fixtures/el-metapkg-deps.txtscripts/test-fixtures/el-packages.txtscripts/test-fixtures/excluded-test.txtscripts/test-fixtures/expected-output.json
- load_exclusions: replace global space removal with POSIX trim that handles leading/trailing spaces and tabs without stripping internal spaces from patterns - pkg_in_metapkg: remove substring fallback that caused false positives (e.g. pgedge-pg matching pgedge-pgadmin4), use exact match only
1 participant
Summary
catalog.jsonand reports NEW, MATCH, MISSING, EXCLUDEDUsage
Test plan
./scripts/discover-repo-packages.sh --self-test— all 5 checks pass./scripts/discover-repo-packages.sh --discover-only— exits 0, shows MATCH/EXCLUDED with no unexpected NEW./scripts/discover-repo-packages.sh --discover-only --verbose— excluded packages listed individuallyscripts/output/is gitignored and not committedSummary by CodeRabbit