-
-
Notifications
You must be signed in to change notification settings - Fork 232
slim builds for postgres versions just latest extension #2034
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughWalkthroughThis change set adds a new boolean input Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
nix/ext/plpgsql-check.nix (1)
128-136:⚠️ Potential issue | 🟡 MinorInconsistent use of
versionsvsversionsBuiltfor upgrade scripts.The loop on line 131 uses
versions(all supported versions) instead ofversionsBuilt. WhenlatestOnly=true, this creates empty upgrade scripts for version transitions that aren't included in the slim build.For consistency with the rest of the
latestOnlylogic, consider usingversionsBuilt:🔧 Proposed fix
# Create empty upgrade files between consecutive versions # plpgsql_check ships without upgrade scripts - extensions are backward-compatible previous_version="" - for ver in ${lib.concatStringsSep " " versions}; do + for ver in ${lib.concatStringsSep " " versionsBuilt}; do if [[ -n "$previous_version" ]]; then touch $out/share/postgresql/extension/${pname}--''${previous_version}--''${ver}.sql fi previous_version=$ver donenix/ext/wal2json.nix (1)
89-94:⚠️ Potential issue | 🔴 CriticalUse
versionsBuiltinstead ofversionsfor upgrade script files to match built versions.The loop at line 89 creates upgrade path files using
versions(all supported versions), but only the versions inversionsBuiltare actually compiled. InlatestOnlymode, this creates upgrade files for versions that were never built. For example, if only v1.1 is built, the loop still creates--1.0--1.1.sqleven though v1.0 was never compiled. Change the loop to iterate overversionsBuilt:Suggested change
- for ver in ${lib.concatStringsSep " " versions}; do + for ver in ${lib.concatStringsSep " " versionsBuilt}; doThis same bug exists in
nix/ext/plpgsql-check.nixat line 131.
🧹 Nitpick comments (1)
nix/ext/index_advisor.nix (1)
78-79: Consider propagatinglatestOnlyto hypopg dependency.The hypopg dependency is instantiated without passing
latestOnly. When building index_advisor withlatestOnly=true, hypopg will still build all its versions. For a truly slim build, consider:- paths = packages ++ [ (callPackage ./hypopg.nix { inherit postgresql; }) ]; + paths = packages ++ [ (callPackage ./hypopg.nix { inherit postgresql latestOnly; }) ];If hypopg should always include all versions regardless of index_advisor's setting, this can be left as-is.
|
We won't need this one after all closing |
a build of postgres with just the latest extension version and postgres itself
Summary by CodeRabbit
New Features
Chores