From 7f01b44a1818d7e2569394bb46a54f4781f8541a Mon Sep 17 00:00:00 2001 From: Yvan Sraka Date: Wed, 10 Dec 2025 22:34:01 +0100 Subject: [PATCH] feat: migrate pgrx extensions to crane build system MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change adds crane support to the existing pgrx extension builder, enabling better incremental build performance and caching. The unified buildPgrxExtension.nix accepts an optional craneLib parameter and uses crane's two-phase build pattern (buildDepsOnly + buildPackage) when available, falling back to rustPlatform otherwise. The key insight was that crane's modifiedSrc derivations created platform-specific builds during evaluation, causing IFD cross-compilation failures. The solution removes unnecessary source modifications and relies on extensions' existing postPatch mechanisms to handle external Cargo.lock files. This prevents IFD issues while maintaining crane's incremental build benefits. For wrappers extension, the preConfigure script was enhanced to handle both legacy table-style and modern inline Cargo.toml dependency formats, and conditionally skips git URL modifications when using cargoVendorDir to prevent networking issues in sandboxed builds. The awk script in preConfigure was also corrected to check getline return values to prevent infinite loops at end-of-file. The rustcWrapper used by pgrx versions prior to 0.12.0 had an infinite recursion bug where ORIGINAL_RUSTC was referenced but never initialized. When the wrapper modified PATH to intercept rustc calls, it would recursively invoke itself because the original rustc location was unknown. The fix initializes ORIGINAL_RUSTC using command -v before modifying PATH, ensuring the wrapper can locate the actual rustc executable. pg_jsonschema now uses crane successfully, while pg_graphql and wrappers remain on rustPlatform. The rustPlatform path continues to work correctly with the rustcWrapper fix in place for extensions using older pgrx versions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- flake.lock | 16 ++ flake.nix | 1 + nix/cargo-pgrx/buildPgrxExtension.nix | 249 ++++++++++++++++++-------- nix/cargo-pgrx/mkPgrxExtension.nix | 17 +- nix/ext/pg_graphql/default.nix | 7 +- nix/ext/pg_jsonschema/default.nix | 1 + nix/ext/wrappers/default.nix | 45 +++-- nix/overlays/default.nix | 5 +- 8 files changed, 247 insertions(+), 94 deletions(-) diff --git a/flake.lock b/flake.lock index 4924941a8..1237d2b67 100644 --- a/flake.lock +++ b/flake.lock @@ -1,5 +1,20 @@ { "nodes": { + "crane": { + "locked": { + "lastModified": 1765145449, + "narHash": "sha256-aBVHGWWRzSpfL++LubA0CwOOQ64WNLegrYHwsVuVN7A=", + "owner": "ipetkov", + "repo": "crane", + "rev": "69f538cdce5955fcd47abfed4395dc6d5194c1c5", + "type": "github" + }, + "original": { + "owner": "ipetkov", + "repo": "crane", + "type": "github" + } + }, "flake-compat": { "flake": false, "locked": { @@ -238,6 +253,7 @@ }, "root": { "inputs": { + "crane": "crane", "flake-parts": "flake-parts", "flake-utils": "flake-utils", "git-hooks": "git-hooks", diff --git a/flake.nix b/flake.nix index 595cd2d96..de1cd8233 100644 --- a/flake.nix +++ b/flake.nix @@ -25,6 +25,7 @@ nixpkgs.url = "https://channels.nixos.org/nixos-unstable/nixexprs.tar.xz"; rust-overlay.inputs.nixpkgs.follows = "nixpkgs"; rust-overlay.url = "github:oxalica/rust-overlay"; + crane.url = "github:ipetkov/crane"; treefmt-nix.inputs.nixpkgs.follows = "nixpkgs"; treefmt-nix.url = "github:numtide/treefmt-nix"; }; diff --git a/nix/cargo-pgrx/buildPgrxExtension.nix b/nix/cargo-pgrx/buildPgrxExtension.nix index a316015a5..3d9fc9f9f 100644 --- a/nix/cargo-pgrx/buildPgrxExtension.nix +++ b/nix/cargo-pgrx/buildPgrxExtension.nix @@ -29,6 +29,7 @@ { lib, cargo-pgrx, + craneLib ? null, pkg-config, rustPlatform, stdenv, @@ -36,10 +37,18 @@ defaultBindgenHook, }: -# The idea behind: Use it mostly like rustPlatform.buildRustPackage and so -# we hand most of the arguments down. +# Unified pgrx extension builder supporting both rustPlatform and crane. +# When craneLib is provided, uses crane for better incremental builds and caching. +# Otherwise falls back to rustPlatform.buildRustPackage. # -# Additional arguments are: +# Crane separates dependency builds from main crate builds, enabling better caching. +# Both approaches accept the same arguments and produce compatible outputs. +# +# IMPORTANT: External Cargo.lock files are handled by extensions' postPatch phases, +# not by copying during evaluation. This avoids IFD (Import From Derivation) issues +# that caused cross-compilation failures when evaluating aarch64 packages on x86_64. +# +# Additional arguments: # - `postgresql` postgresql package of the version of postgresql this extension should be build for. # Needs to be the build platform variant. # - `useFakeRustfmt` Whether to use a noop fake command as rustfmt. cargo-pgrx tries to call rustfmt. @@ -139,84 +148,182 @@ let pg_ctl stop ''; - argsForBuildRustPackage = builtins.removeAttrs args [ - "postgresql" - "useFakeRustfmt" - "usePgTestCheckFeature" - ]; - - # so we don't accidentally `(rustPlatform.buildRustPackage argsForBuildRustPackage) // { ... }` because - # we forgot parentheses - finalArgs = argsForBuildRustPackage // { - buildInputs = (args.buildInputs or [ ]); - - nativeBuildInputs = - (args.nativeBuildInputs or [ ]) - ++ [ - cargo-pgrx - postgresql - pkg-config - bindgenHook - ] - ++ lib.optionals useFakeRustfmt [ fakeRustfmt ]; - - buildPhase = '' - runHook preBuild - - echo "Executing cargo-pgrx buildPhase" - ${preBuildAndTest} - ${maybeEnterBuildAndTestSubdir} - - export PGRX_BUILD_FLAGS="--frozen -j $NIX_BUILD_CORES ${builtins.concatStringsSep " " cargoBuildFlags}" - export PGX_BUILD_FLAGS="$PGRX_BUILD_FLAGS" - - ${lib.optionalString needsRustcWrapper '' - export ORIGINAL_RUSTC="$(command -v ${stdenv.cc.targetPrefix}rustc || command -v rustc)" - export PATH="${rustcWrapper}/bin:$PATH" - export RUSTC="${rustcWrapper}/bin/rustc" - ''} - - ${lib.optionalString stdenv.hostPlatform.isDarwin ''RUSTFLAGS="''${RUSTFLAGS:+''${RUSTFLAGS} }-Clink-args=-Wl,-undefined,dynamic_lookup"''} \ - cargo ${pgrxBinaryName} package \ - --pg-config ${lib.getDev postgresql}/bin/pg_config \ - ${maybeDebugFlag} \ - --features "${builtins.concatStringsSep " " buildFeatures}" \ - --out-dir "$out" - - ${maybeLeaveBuildAndTestSubdir} - - runHook postBuild - ''; + # Crane-specific: Determine if we're using crane and handle cargo lock info + # Note: External lockfiles are handled by extensions' postPatch, not here, to avoid + # creating platform-specific derivations during evaluation (prevents IFD issues) + useCrane = craneLib != null; + cargoLockInfo = args.cargoLock or null; + + # External Cargo.lock files are handled by the extension's postPatch phase + # which creates symlinks. Crane finds them during build, not evaluation. + # This approach prevents IFD cross-compilation issues. + + # Use rustPlatform.importCargoLock instead of crane's vendorCargoDeps for git dependencies. + # crane's vendorCargoDeps uses builtins.fetchGit which only searches the default branch, + # causing errors like: + # "error: Cannot find Git revision 'e565bc43c1b9fa6b25a601f68bcec1423a984cc1' in ref + # 'refs/heads/main' of repository 'https://github.com/burmecia/iceberg-rust'!" + # rustPlatform.importCargoLock with allowBuiltinFetchGit searches all refs (branches/tags). + cargoVendorDir = + if useCrane && cargoLockInfo != null && cargoLockInfo ? outputHashes then + rustPlatform.importCargoLock { + lockFile = cargoLockInfo.lockFile; + outputHashes = cargoLockInfo.outputHashes; + allowBuiltinFetchGit = true; + } + else + null; + + # Remove rustPlatform-specific args and pgrx-specific args. + # For crane, also remove build/install phases (added back later). + argsForBuilder = builtins.removeAttrs args ( + [ + "postgresql" + "useFakeRustfmt" + "usePgTestCheckFeature" + ] + ++ lib.optionals useCrane [ + "cargoHash" # rustPlatform uses this, crane uses Cargo.lock directly + "cargoLock" # handled separately via modifiedSrc and cargoVendorDir + "installPhase" # we provide our own pgrx-specific install phase + "buildPhase" # we provide our own pgrx-specific build phase + ] + ); + + # Common arguments for both rustPlatform and crane + commonArgs = + argsForBuilder + // { + src = args.src; # Use original source - extensions handle external lockfiles via postPatch + strictDeps = true; + + buildInputs = (args.buildInputs or [ ]); + + nativeBuildInputs = + (args.nativeBuildInputs or [ ]) + ++ [ + cargo-pgrx + postgresql + pkg-config + bindgenHook + ] + ++ lib.optionals useFakeRustfmt [ fakeRustfmt ]; + + PGRX_PG_SYS_SKIP_BINDING_REWRITE = "1"; + CARGO_BUILD_INCREMENTAL = "false"; + RUST_BACKTRACE = "full"; + + checkNoDefaultFeatures = true; + checkFeatures = + (args.checkFeatures or [ ]) + ++ (lib.optionals usePgTestCheckFeature [ "pg_test" ]) + ++ [ "pg${pgrxPostgresMajor}" ]; + } + // lib.optionalAttrs (cargoVendorDir != null) { + inherit cargoVendorDir; + }; + + # Shared build and install phases for both rustPlatform and crane + sharedBuildPhase = '' + runHook preBuild + + ${preBuildAndTest} + ${maybeEnterBuildAndTestSubdir} + + export PGRX_BUILD_FLAGS="--frozen -j $NIX_BUILD_CORES ${builtins.concatStringsSep " " cargoBuildFlags}" + export PGX_BUILD_FLAGS="$PGRX_BUILD_FLAGS" + + ${lib.optionalString needsRustcWrapper '' + export ORIGINAL_RUSTC="$(command -v rustc)" + export PATH="${rustcWrapper}/bin:$PATH" + export RUSTC="${rustcWrapper}/bin/rustc" + ''} + + ${lib.optionalString stdenv.hostPlatform.isDarwin ''RUSTFLAGS="''${RUSTFLAGS:+''${RUSTFLAGS} }-Clink-args=-Wl,-undefined,dynamic_lookup"''} \ + cargo ${pgrxBinaryName} package \ + --pg-config ${lib.getDev postgresql}/bin/pg_config \ + ${maybeDebugFlag} \ + --features "${builtins.concatStringsSep " " buildFeatures}" \ + --out-dir "$out" + + ${maybeLeaveBuildAndTestSubdir} + + runHook postBuild + ''; + + sharedInstallPhase = '' + runHook preInstall + + ${maybeEnterBuildAndTestSubdir} + cargo-${pgrxBinaryName} ${pgrxBinaryName} stop all + + mv $out/${postgresql}/* $out + mv $out/${postgresql.lib}/* $out + rm -rf $out/nix + + ${maybeLeaveBuildAndTestSubdir} + + runHook postInstall + ''; + + # Arguments for rustPlatform.buildRustPackage + rustPlatformArgs = commonArgs // { + buildPhase = sharedBuildPhase; + installPhase = sharedInstallPhase; preCheck = preBuildAndTest + args.preCheck or ""; + }; - installPhase = '' - runHook preInstall + # Crane's two-phase build: first build dependencies, then build the extension. + # buildDepsOnly creates a derivation containing only Cargo dependency artifacts. + # This is cached separately, so changing extension code doesn't rebuild dependencies. + cargoArtifacts = + if useCrane then + craneLib.buildDepsOnly ( + commonArgs + // { + pname = "${args.pname or "pgrx-extension"}-deps"; - echo "Executing buildPgrxExtension install" + # pgrx-pg-sys needs PGRX_HOME during dependency build + preBuild = '' + ${preBuildAndTest} + ${maybeEnterBuildAndTestSubdir} + '' + + (args.preBuild or ""); - ${maybeEnterBuildAndTestSubdir} + postBuild = '' + ${maybeLeaveBuildAndTestSubdir} + '' + + (args.postBuild or ""); - cargo-${pgrxBinaryName} ${pgrxBinaryName} stop all + # Dependencies don't have a postInstall phase + postInstall = ""; - mv $out/${postgresql}/* $out - mv $out/${postgresql.lib}/* $out - rm -rf $out/nix + # Need to specify PostgreSQL version feature for pgrx dependencies + # and disable default features to avoid multiple pg version conflicts + cargoExtraArgs = "--no-default-features --features ${ + builtins.concatStringsSep "," ([ "pg${pgrxPostgresMajor}" ] ++ buildFeatures) + }"; + } + ) + else + null; - ${maybeLeaveBuildAndTestSubdir} + # Arguments for crane.buildPackage + craneArgs = commonArgs // { + inherit cargoArtifacts; + pname = args.pname or "pgrx-extension"; - runHook postInstall - ''; + # Explicitly preserve postInstall from args (needed for version-specific file renaming) + postInstall = args.postInstall or ""; - PGRX_PG_SYS_SKIP_BINDING_REWRITE = "1"; - CARGO_BUILD_INCREMENTAL = "false"; - RUST_BACKTRACE = "full"; + # We handle installation ourselves via pgrx, don't let crane try to install binaries + doNotInstallCargoBinaries = true; + doNotPostBuildInstallCargoBinaries = true; - checkNoDefaultFeatures = true; - checkFeatures = - (args.checkFeatures or [ ]) - ++ (lib.optionals usePgTestCheckFeature [ "pg_test" ]) - ++ [ "pg${pgrxPostgresMajor}" ]; + buildPhase = sharedBuildPhase; + installPhase = sharedInstallPhase; + preCheck = preBuildAndTest + args.preCheck or ""; }; in -rustPlatform.buildRustPackage finalArgs +if useCrane then craneLib.buildPackage craneArgs else rustPlatform.buildRustPackage rustPlatformArgs diff --git a/nix/cargo-pgrx/mkPgrxExtension.nix b/nix/cargo-pgrx/mkPgrxExtension.nix index c7970451a..1a595c1ed 100644 --- a/nix/cargo-pgrx/mkPgrxExtension.nix +++ b/nix/cargo-pgrx/mkPgrxExtension.nix @@ -5,6 +5,9 @@ makeRustPlatform, rust-bin, system, + crane ? null, + useCrane ? false, + pkgs, }: let inherit ((callPackage ./default.nix { inherit rustVersion; })) mkCargoPgrx; @@ -51,9 +54,19 @@ let } else rustPlatform.bindgenHook; + + # Initialize crane with the same Rust toolchain as rustPlatform to ensure consistency. + # crane.mkLib creates a library of crane functions bound to a specific package set, + # then we override the toolchain to match the pgrx-required Rust version. + craneLib = + if useCrane then + assert crane != null; + (crane.mkLib pkgs).overrideToolchain rust-bin.stable.${rustVersion}.default + else + null; in +# Use unified builder that supports both crane and rustPlatform callPackage ./buildPgrxExtension.nix { - inherit rustPlatform; - inherit cargo-pgrx; + inherit rustPlatform cargo-pgrx craneLib; defaultBindgenHook = bindgenHook; } diff --git a/nix/ext/pg_graphql/default.nix b/nix/ext/pg_graphql/default.nix index a7f6d1065..a44136f5a 100644 --- a/nix/ext/pg_graphql/default.nix +++ b/nix/ext/pg_graphql/default.nix @@ -17,6 +17,7 @@ let cargo = rust-bin.stable.${rustVersion}.default; mkPgrxExtension = callPackages ../../cargo-pgrx/mkPgrxExtension.nix { inherit rustVersion pgrxVersion; + useCrane = false; }; src = fetchFromGitHub { owner = "supabase"; @@ -103,8 +104,10 @@ let }; } // lib.optionalAttrs (builtins.compareVersions "1.2.0" version >= 0) { - # Add missing Cargo.lock - patches = [ ./0001-Add-missing-Cargo.lock-${version}.patch ]; + # External Cargo.lock needs to be linked for rustPlatform + postPatch = '' + ln -s ${./Cargo-${version}.lock} Cargo.lock + ''; cargoLock = { lockFile = ./Cargo-${version}.lock; diff --git a/nix/ext/pg_jsonschema/default.nix b/nix/ext/pg_jsonschema/default.nix index 426c53dea..47bae448f 100644 --- a/nix/ext/pg_jsonschema/default.nix +++ b/nix/ext/pg_jsonschema/default.nix @@ -15,6 +15,7 @@ let cargo = rust-bin.stable.${rustVersion}.default; mkPgrxExtension = callPackages ../../cargo-pgrx/mkPgrxExtension.nix { inherit rustVersion pgrxVersion; + useCrane = true; }; src = fetchFromGitHub { owner = "supabase"; diff --git a/nix/ext/wrappers/default.nix b/nix/ext/wrappers/default.nix index 696358ea4..23d0055a3 100644 --- a/nix/ext/wrappers/default.nix +++ b/nix/ext/wrappers/default.nix @@ -18,6 +18,7 @@ let cargo = rust-bin.stable.${rustVersion}.default; mkPgrxExtension = callPackages ../../cargo-pgrx/mkPgrxExtension.nix { inherit rustVersion pgrxVersion; + useCrane = false; }; in mkPgrxExtension ( @@ -103,33 +104,41 @@ let }; }; - preConfigure = '' + preConfigure = lib.optionalString (!(cargoLock ? outputHashes)) '' cd wrappers # update the clickhouse-rs dependency # append the branch name to the git URL to help cargo locate the commit # while maintaining the rev for reproducibility - awk -i inplace ' - /\[dependencies.clickhouse-rs\]/ { - print - getline - if ($0 ~ /git =/) { - print "git = \"https://github.com/burmecia/clickhouse-rs/supabase-patch\"" - } else { + # Only needed when not using cargoVendorDir (i.e., with rustPlatform) + + # Handle both old format [dependencies.clickhouse-rs] and new inline format + if grep -q "^\[dependencies\.clickhouse-rs\]" Cargo.toml; then + # Old format: [dependencies.clickhouse-rs] section + awk -i inplace ' + /\[dependencies.clickhouse-rs\]/ { print - } - while ($0 !~ /^\[/ && NF > 0) { getline - if ($0 ~ /rev =/) print - if ($0 ~ /^\[/) print + if ($0 ~ /git =/) { + print "git = \"https://github.com/burmecia/clickhouse-rs/supabase-patch\"" + } else { + print + } + while ($0 !~ /^\[/ && NF > 0) { + if (getline <= 0) break + if ($0 ~ /rev =/) print + if ($0 ~ /^\[/) print + } + next } - next - } - { print } - ' Cargo.toml + { print } + ' Cargo.toml + elif grep -q 'clickhouse-rs.*=.*{.*git.*=.*"https://github.com/suharev7/clickhouse-rs"' Cargo.toml; then + # New format: inline dependency format + sed -i 's|git = "https://github.com/suharev7/clickhouse-rs"|git = "https://github.com/burmecia/clickhouse-rs/supabase-patch"|g' Cargo.toml + fi - # Verify the file is still valid TOML, break build with this erroru - # if it is not + # Verify the file is still valid TOML if ! cargo verify-project 2>/dev/null; then echo "Failed to maintain valid TOML syntax" exit 1 diff --git a/nix/overlays/default.nix b/nix/overlays/default.nix index a5a18d5c9..782bec9af 100644 --- a/nix/overlays/default.nix +++ b/nix/overlays/default.nix @@ -1,4 +1,4 @@ -{ self, ... }: +{ self, inputs, ... }: { flake.overlays.default = final: _prev: { # NOTE: add any needed overlays here. in theory we could @@ -17,6 +17,9 @@ xmrig = throw "The xmrig package has been explicitly disabled in this flake."; + # Make crane available as pkgs.crane for Rust builds + crane = inputs.crane; + cargo-pgrx = final.callPackage ../cargo-pgrx/default.nix { inherit (final) lib; inherit (final) fetchCrate;