feat(nix): expose per-version extension packages, catalog, and update tools - #2317
feat(nix): expose per-version extension packages, catalog, and update tools#2317brainrake wants to merge 3 commits into
Conversation
PostgreSQL Package Dependency Analysis: PR #2317
SummaryNo packages had MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Dependency ChangesExtracting PostgreSQL 15 dependencies...
Runtime Closure Size
Raw Dependency ClosurePostgreSQL 17 Dependency ChangesExtracting PostgreSQL 17 dependencies...
Runtime Closure Size
Raw Dependency Closure |
PostgreSQL Extension Dependency Analysis: PR #2317
SummaryNo extensions had dependencies with MAJOR version updates. Full Analysis ResultsPostgreSQL 15 Extension DependenciesPostgreSQL 17 Extension DependenciesOrioleDB 17 Extension Dependencies |
d11cd58 to
998a591
Compare
3fe60a2 to
998a591
Compare
d2144e3 to
e64f340
Compare
…ce tools
Adds passthru.perVersion (version -> single-version derivation) to each
multi-version extension (18 files, one line each) -- the attrset already
existed inline as the buildEnv input, this surfaces it instead of
discarding it. No change to any buildEnv output or extension identity.
New flake-parts module nix/packages/extension-catalog.nix, exposing
(packages + legacyPackages):
- site-extensions-catalog-<major> (15/17/orioledb-17): pg-extensions-
catalog.json mapping { ext -> { extversion -> store path } }, plus
bin/{resolve,update} preset with PG_EXTENSIONS_CATALOG. Assembled at
build time from the wrappers' control files, so keys are real
extversions (pgsql-http nix "1.5.0" -> "1.5") and include
sub-extensions (postgis_topology, plcoffee, ...). unsafeDiscardReferences
keeps the closure to just the JSON (6.8 KiB, not 2.4 GiB).
- site-extensions-versions-<major> (legacyPackages): the generic
single-version packages the catalog points at (lean, no multi-version
buildEnv), so the CI matrix builds and caches them.
- Generic tools, one version each (not per-major): site-extensions-resolve
(manifest json -> store paths), download-nix-store-paths (retry/timeout
substitution, not extension-specific), site-extensions-update (chains
both + nix-env --install --remove-all onto PROFILE, atomic).
Part of PSQL-1530.
e64f340 to
5bfbb75
Compare
mmlb
left a comment
There was a problem hiding this comment.
A few nits (bashisms and quotes) that would be nice to fix up but the big one to me is the [[ -n $name ]] || continue that doesn't smell good to me. Also the serial vs parallel download.
| b=$(basename "$cf") | ||
| n="''${b%%--*}" | ||
| v="''${b#*--}"; v="''${v%.control}" | ||
| [ -e "$ext/$n.control" ] && continue |
There was a problem hiding this comment.
[[ ]] is a bash builtin and better than [ command, we should prefer it since its a bash script anyway.
| [ -e "$ext/$n.control" ] && continue | |
| [[ -e "$ext/$n.control" ]] && continue |
| n="''${b%%--*}" | ||
| v="''${b#*--}"; v="''${v%.control}" |
There was a problem hiding this comment.
Don't need to use " in assignment and is easier to read with the nix escaping in there too.
| n="''${b%%--*}" | |
| v="''${b#*--}"; v="''${v%.control}" | |
| n=''${b%%--*} | |
| v=''${b#*--}; v=''${v%.control} |
| suffix=".''${b##*.}" | ||
| core="''${b%.*}" | ||
| base="''${core%-[0-9]*}" | ||
| [ "$base" != "$core" ] || continue | ||
| gen="$base$suffix" | ||
| [ -e "$out/lib/$gen" ] || ln -sfn "$b" "$out/lib/$gen" |
There was a problem hiding this comment.
ditto re " and [[ ]] bashism
| suffix=".''${b##*.}" | |
| core="''${b%.*}" | |
| base="''${core%-[0-9]*}" | |
| [ "$base" != "$core" ] || continue | |
| gen="$base$suffix" | |
| [ -e "$out/lib/$gen" ] || ln -sfn "$b" "$out/lib/$gen" | |
| suffix=.''${b##*.} | |
| core=''${b%.*} | |
| base=''${core%-[0-9]*} | |
| [[ "$base" != "$core" ]] || continue | |
| gen=$base$suffix | |
| [[ -e "$out/lib/$gen" ]] || ln -sfn "$b" "$out/lib/$gen" |
| esac | ||
| done | ||
| libs=$(ls -A "$out/lib" 2>/dev/null | wc -l) | ||
| if [ "$generic" -eq 0 ] && [ "$libs" -eq 0 ]; then |
There was a problem hiding this comment.
bash's math tests are nicer than -eq, -ne ...
| if [ "$generic" -eq 0 ] && [ "$libs" -eq 0 ]; then | |
| if ((generic == 0)) && ((libs == 0)); then |
| name="''${base%.control}" | ||
| ver=$(sed -n "s/^default_version = '\(.*\)'.*/\1/p" "$ctrl" | head -1) | ||
| [ -n "$ver" ] || { echo "no default_version in $ctrl" >&2; exit 1; } |
There was a problem hiding this comment.
same thing re bashism and quotes. The case could be swapped out for [[ $base == *--* ]] instead but meh.
| name="''${base%.control}" | |
| ver=$(sed -n "s/^default_version = '\(.*\)'.*/\1/p" "$ctrl" | head -1) | |
| [ -n "$ver" ] || { echo "no default_version in $ctrl" >&2; exit 1; } | |
| name=''${base%.control} | |
| ver=$(sed -n "s/^default_version = '\(.*\)'.*/\1/p" "$ctrl" | head -1) | |
| [[ -n "$ver" ]] || { echo "no default_version in $ctrl" >&2; exit 1; } |
| : "''${PG_EXTENSIONS_CATALOG:?PG_EXTENSIONS_CATALOG must point at a pg-extensions-catalog.json}" | ||
| manifest="''${1:?Usage: $0 path-to/pg-extensions.json}" | ||
| jq -r 'to_entries[] | "\(.key)=\(.value)"' "$manifest" | while IFS='=' read -r name version; do | ||
| [ -n "$name" ] || continue |
There was a problem hiding this comment.
this seems weird, why would we end up with an empty line? If valid, worth a comment for why we might be seeing empty lines
| ln -s ${mkCatalogFile wrappers}/share/pg-extensions-catalog.json "$out/share/" | ||
| makeWrapper ${self'.packages.site-extensions-resolve}/bin/site-extensions-resolve \ | ||
| "$out/bin/site-extensions-resolve" \ | ||
| --set PG_EXTENSIONS_CATALOG "$out/share/pg-extensions-catalog.json" |
There was a problem hiding this comment.
why not --prefix instead? I'd be nice to not have to specify both an envvar and positional arg to the unwrapped scripts, better to both the same way imo.
| text = '' | ||
| manifest="''${1:-/etc/adminapi/pg-extensions.json}" | ||
| profile="''${PROFILE:-/nix/var/nix/profiles/site-extensions}" | ||
| mapfile -t paths < <(site-extensions-resolve "$manifest") |
There was a problem hiding this comment.
We have 2 readarrays and 0 mapfiles in the tree so good to use the same name, not to mention that readarray is more obvious than mapfile imo.
| mapfile -t paths < <(site-extensions-resolve "$manifest") | |
| readarray -t paths < <(site-extensions-resolve "$manifest") |
| done | ||
| echo "ERROR: failed to realize $path" >&2 | ||
| exit 1 | ||
| done |
There was a problem hiding this comment.
The loop as written makes for serial downloads instead of letting nix download in parallel, if network is flaky this is going to be pretty terrible experience waiting 10s for each path * 3. Maybe makes the happy path a little slower too (but probably not a big deal).
I'd probably do something like
ok=false
for _ in 1 2 3 4; do
timeout -k 10s 120s nix-store -r "$@" || continue
ok=true
break
done
if ! $ok; then
ok=true
for path in "$@"; do
echo timeout -k 10s 120s nix-store -r "$path" >/dev/null
echo "ERROR: nix failed to fetch $path after 5 attempts" >&2
ok=false
done
fi
if ! $ok;
exit 1
fi
Part of PSQL-1530.
Adds
passthru.perVersion(version -> single-version derivation) to each multi-version extension (18 files, one line each).Adds flake module
nix/packages/extension-catalog.nix, exposingpackagesandlegacyPackages:site-extensions-catalog-<major>(15/17/orioledb-17)pg-extensions-catalog.jsonmapping{ ext -> { extversion -> store path } }bin/{resolve,update}that use said jsonsite-extensions-versions-<major>: single-version ext packages the catalog points at (lean, ~30 MiB closures, no multi-version buildEnv), so CI builds and caches them.site-extensions-resolve(manifest json -> store paths, readsPG_EXTENSIONS_CATALOG)download-nix-store-paths(retry/timeout substitution, not extension-specific)site-extensions-update(chains both +nix-env --install --remove-allontoPROFILE, atomic — nix-env only swaps generations on success)Should be safe since it only adds new outputs.