feat: reintroduce nix flake #1683
Conversation
This exposes the `cargo pgrx` binary as a flake package called `cargo-pgrx`. It can be built using `nix build .#cargo-pgrx`. The systems specified in this flake were chosen from the project's README of tested systems. The flake inputs were chosen to minimize the maintenance burden. Since the build runs tests in order to succeed, but the tests are performed at build time for nix, the `test_parse_managed_postmasters` test has been skipped. Since pgrx already has a test suite that runs outside of nix, this should be a good compromise to allow the build to work, but also keep the majority of the tests to ensure that nix users are aware when they are broken. In addition to this, the `PGRX_HOME` variable must be set for tests, so it is assigned to a tmpdir before running the tests.
This script can be ran to update the rust-toolchain that is used by nix builds. Since `fenix` is a flake input end-users are able to override it themselves to whichever version they please, however it is useful to have the provided version up to date, particularly if the project ends up having an MSRV. We could likely change this script to be one that updates all flake inputs, however I find it useful to be able to update the rust toolchain separately from the other dependencies and have chosen to provide a script soley for that. This matches the existing `rustup.sh` that is used to update the rustup toolchain version. If you wish to make this file a generic `flake.lock` updater, remove `fenix` from the call to `nix flake update` and it will update all inputs.
This function can be used by external nix flakes to build an extension using pgrx. The caller must provide the postgres package, the rust toolchain, extension source code, and flake system that they are building for. Most of this configuration has been converted from the previous nix flake exposed from pgrx. I've made every attempt to trim away parts that don't serve a purpose, but there's a chance that this could be simplified slightly by someone who has more knowledge of pgrx than I do.
|
I've opened this as a draft PR so it can gather attention while I try to see if I can somehow test on |
| pname = "${name}-pg${postgresMajor}"; | ||
| nativeBuildInputs = [ | ||
| pkgs.pkg-config | ||
| pkgs.rustPlatform.bindgenHook |
There was a problem hiding this comment.
bindgenHook is a handy package I found that sets BINDGEN_EXTRA_CLANG_ARGS, which was previously being done manually in the build step. See https://github.com/symphorien/nixpkgs/blob/master/pkgs/build-support/rust/hooks/rust-bindgen-hook.sh
| PGRX_PG_SYS_SKIP_BINDING_REWRITE = "1"; | ||
| CARGO = "${rustToolchain}/bin/cargo"; | ||
| CARGO_BUILD_INCREMENTAL = "false"; | ||
| RUST_BACKTRACE = "full"; |
There was a problem hiding this comment.
These get set as environment variables for the build
|
Thanks @justinrubek. If you will, give us a bit of time to digest this. A quick comment is I think we'd want to see some kind of documentation in our fledgling "book". Probably from two perspectives. One being general Nix users, and the other being me and @workingjubilee and @thomcc and @NotGyro so we know how to update the things that might need updating in the future. Additionally, I'm sure we'll require a separate CI job that tests a nix build of just the simple |
|
Absolutely take your time, I'm in no hurry. I'm very responsive to GitHub notifications, and I'll do my best to address everything. I can get started on those pieces and anything that is further requested as it comes in. |
overview
This reintroduces the nix code after it was removed in #1682. It has been refactored to live in a single file and many parts of it have been removed because they weren't necessary. I will attempt to provide an overview of everything that is taking place in this configuration and why I've chosen to write things the way I have. Additionally, my commit messages have context if you expand them.
The previous flake did the following things:
cargo-pgrxbinaryI've chosen to focus on the first two options in the above list. There is no need to verify formatting using nix since this is done through GitHub Actions. I could feasibly make an example, but I'll only do that once I'm sure that this PR can be accepted (and if it is requested). For now, as part of my testing, I've taken the arrays example from the
pgrx-examplesdirectory and made a separate repo showcasing how to use this new flake https://github.com/justinrubek/pgrx-nix-exampleflake inputs
Here is what each input does and why we're using them:
cargo-pgrxand also to call pgrx when building an extensionperSystemfunction inflake.nix.gitdirectory from being copied into buildsFor the most part we should be fine to leave these dependencies locked to their current version in
flake.lock. However, it isn't a bad idea to try to keep them up to date. I've provided a shell script that will update thefenixinput to retrieve newer rust toolchains, although I could see that it may better suit the project to have the script update every input so I will change it if asked.flake outputs
This flake exposes two outputs:
packages.${system}.cargo-pgrxandlib.buildPgrxExtensioncargo-pgrx
This is the pgrx binary, built using the source code from this repo. It's build is a pretty standard cargo build, but there are a few extra things happening.
As part of the build step the tests are ran. However, pgrx's tests appear to have two requirements that won't be met by default:
PGRX_HOMEenvironment variable must be settest_parse_managed_postmasters,cargo pgrx initmust have been ranFor 1. the solution is easy: the
preCheckblock is ran which sets the file to a temporary directory. However, 2. is not as simple because we don't have the pgrx binary available yet due to this being the build step for it. The compromise is to skip any tests that require this to be ran. The rest of the tests are still ran, so we can be fairly sure the nix build isn't broken. And since the pgrx test suite is not being ran through nix any issues with this test will be detected anyway.This should require minimal maintenance as building a cargo package is fairly trivial. The only exceptions are:
buildInputsto contain the package corresponding to the new dependency. For a list of packages, see https://search.nixos.org/packages?channel=unstablebuildPgrxExtension
This is the more complicated piece, but it is also the larger value-add for nix users. I've taken most of the logic from the previous configuration, distilling it down by removing extra code and simplifying the rest. I believe I've gotten it nearly as simple as it can be, but there may be some further room for improvement.
The function is responsible for setting up postgres in a way that stops
pgrxfrom attempting to download it. This is because flake builds are executed in a sandbox without network access. Consuming flakes must provide the postgres package they wish to use in order for this to work. The function will determine the major version to set up the proper feature flags as well as put postgres in a subdirectory ofPGRX_HOMEaccording to its version (this is thepreBuildAndTestblock).Apart from that, there is some file cleanup that takes place in the
preFixupandpostInstallsteps, primarily to ensure that only the minimum things required end up in the final build (if I understanding right, the--out-dirflag frompgrxis placing things there that are not desired in the final build, so I chose to delete them).Testing performed
I have tested this all using
x86_64-linux. I am unable to testaarch64-linux, although I may be able to tryaarch64-darwin. I suspect that it will work onaarch64-linuxanyway.I've verified that I can build extensions and load them into postgres. See justinrubek/pgrx-nix-example#1 (comment). I've also back-ported this change to pgrx v0.11 and tested using pgx_ulid.
further work
I don't have a wide variety of extensions to test against. I can imagine the need for some tweaks depending on any issues encountered by people using this flake, but I'm not sure what those would be at this time. I am willing and able to look into this should any come up, so please route these to me so I can help.
Beyond that, I have a few small changes that may be good:
buildPgrxExtensionswhy this is important for nix users
There is a
cargo-pgrxbinary andbuildPgrxExtensionfunction in nixpkgs, but that is not sufficient due to thepgrxbinary version needing to match the cargo dependency. If a consumer wishes to use a newer version of pgrx, they will have to wait for it to make it into nixpkgs. On the other side of things, if they wish to remain on an older version of pgrx they may have to pin an old nixpkgs. By keeping the flake in this repo consumers are able to easily use the latest version of pgrx without any difficulty. Here's an example of the supabase folks dealing with a glibc issue where they pray updatingcargo-pgrxwill resolve the issue supabase/nix-postgres#28 (comment), which is precisely the kind of thing I'd like to avoid.