Conversation
Greptile SummaryThis PR adds Nix packaging support ( Key findings:
Confidence Score: 4/5Not safe to merge as-is — nix build will fail for all users until the vendorHash is properly computed. One P1 issue blocks the primary use-case: the vendorHash in nix/pgschema.nix appears to be an unverified placeholder, meaning nix build will hard-fail immediately. All other findings are P2 style/improvement suggestions that don't affect correctness. Resolving the vendorHash issue would raise this to 5/5. nix/pgschema.nix — the vendorHash must be computed and verified before the package is usable. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([User runs nix build / nix run]) --> B{Flake or legacy?}
B -- flake --> C[flake.nix\nresolves nixpkgs from flake.lock]
B -- legacy --> D[default.nix\nimports nixpkgs from channel]
C --> E[callPackage ./nix/pgschema.nix]
D --> E
E --> F[Read internal/version/VERSION\nfor version string]
E --> G[lib.cleanSource ..\nfilters repo source]
F --> H[buildGoModule]
G --> H
H --> I{proxyVendor = true\nFetch Go modules via proxy}
I --> J{Verify vendorHash}
J -- hash matches --> K[go build -ldflags '-s -w'\noutput: bin/pgschema]
J -- hash mismatch --> L([Build fails ❌\nnix prints correct hash])
K --> M([result/bin/pgschema ✅])
Reviews (1): Last reviewed commit: "feat: add nix flake/package" | Re-trigger Greptile |
nix/pgschema.nix
Outdated
| # Replace with the real hash from `nix build` output. | ||
| vendorHash = "sha256-3nV7AEsWyEvIbxHetoEsA8PPXJ6ENvU/sz7Wn5aysss="; |
There was a problem hiding this comment.
Unverified placeholder
vendorHash will break the build
The inline comment — "Replace with the real hash from nix build output" — strongly indicates this hash was never actually computed from the live dependency set. With proxyVendor = true, Nix fetches all Go modules from the proxy and checks them against vendorHash; a wrong hash causes an immediate, hard build failure:
error: hash mismatch in fixed-output derivation
wanted: sha256-3nV7AEsWyEvIbxHetoEsA8PPXJ6ENvU/sz7Wn5aysss=
got: sha256-<actual>
The standard Nix workflow is to set vendorHash = lib.fakeHash; (or ""), run nix build, then replace it with the hash printed in the error. That step appears not to have been done here, so nix build will fail out of the box for every user.
nix/pgschema.nix
Outdated
| # go_1_24 is not available in some nixpkgs revisions; use the closest newer toolchain. | ||
| go = pkgs.go_1_25; |
There was a problem hiding this comment.
Go toolchain version overrides
go.mod toolchain directive
go.mod declares go 1.24.0 (minimum) and toolchain go1.24.7 (preferred). Pinning go = pkgs.go_1_25 overrides that toolchain directive entirely. Go 1.25 is backward-compatible, so this won't break compilation in practice, but it silently ignores the upstream-pinned toolchain.
A more future-proof option is to omit the go attribute entirely and let buildGoModule use nixpkgs' default Go toolchain, or check for pkgs.go_1_24 first:
go = pkgs.go_1_24 or pkgs.go_1_25;This way the derivation respects the author-specified toolchain when available and only falls back when it isn't.
| ldflags = [ | ||
| "-s" | ||
| "-w" | ||
| ]; |
There was a problem hiding this comment.
Missing version-injection ldflags
According to CLAUDE.md, the canonical build injects GitCommit and BuildDate into the binary:
-X github.com/pgplex/pgschema/cmd.GitCommit=...
-X 'github.com/pgplex/pgschema/cmd.BuildDate=...'
The Nix derivation only passes -s -w. As a result, pgschema version (or any version display) will show empty values for commit and build date. Nix exposes the revision via self.rev (or self.shortRev) in the flake, which can be threaded through to the derivation, e.g.:
# In flake.nix, pass rev to the package:
pgschema = pkgs.callPackage ./nix/pgschema.nix { rev = self.shortRev or "dirty"; };
# In nix/pgschema.nix, receive and use it:
{ pkgs ? import <nixpkgs> {}, rev ? "unknown" }:
...
ldflags = [
"-s" "-w"
"-X github.com/pgplex/pgschema/cmd.GitCommit=${rev}"
];
flake.nix
Outdated
| description = "pgschema"; | ||
|
|
||
| inputs = { | ||
| nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; |
There was a problem hiding this comment.
Tracking
nixos-unstable may cause silent regressions on nix flake update
The lock file pins the flake to a specific nixos-unstable commit today, so the build is reproducible now. However, every nix flake update will roll to the latest nixos-unstable, which can change the Go toolchain, library versions, or even remove go_1_25, causing unexpected build breaks for downstream users.
Consider targeting a stable channel (nixos-24.11 or nixos-25.05) in the inputs URL to make future updates more predictable:
| nixpkgs.url = "github:NixOS/nixpkgs/nixos-unstable"; | |
| nixpkgs.url = "github:NixOS/nixpkgs/nixos-25.05"; |
| ### Nix (flake) | ||
|
|
There was a problem hiding this comment.
nix run without argument separator won't forward CLI flags
nix run by itself runs the binary with no arguments. To pass arguments to pgschema (e.g. pgschema plan), users need to separate nix flags from program flags with --. The example could be more helpful:
| ### Nix (flake) | |
| nix run . -- plan |
or at minimum a note like:
Use
nix run . -- <pgschema args>to pass arguments to the binary (e.g.nix run . -- plan).
There was a problem hiding this comment.
Pull request overview
Adds first-class Nix packaging support for pgschema (Issue #333), enabling users to build/run the CLI via Nix flakes or legacy default.nix.
Changes:
- Introduces a
buildGoModulederivation forpgschemaundernix/. - Adds
flake.nix/flake.lockto expose Nixpackagesandappsfor common systems. - Documents basic
nix build/nix runusage in the README.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| nix/pgschema.nix | Adds Nix derivation for building the Go CLI with ldflags metadata. |
| flake.nix | Adds flake outputs for packages/apps across Linux/Darwin systems. |
| flake.lock | Pins nixpkgs input revision used by the flake. |
| default.nix | Enables non-flake Nix builds via callPackage. |
| README.md | Documents Nix flake usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Prefer Go 1.24 when available; fall back to the closest newer toolchain. | ||
| go = if pkgs ? go_1_24 then pkgs.go_1_24 else pkgs.go_1_25; | ||
| subPackages = [ "." ]; |
There was a problem hiding this comment.
The Go toolchain fallback isn’t safe: if pkgs doesn’t provide go_1_24, this expression unconditionally references pkgs.go_1_25, which may not exist in many nixpkgs revisions (especially when using default.nix with an arbitrary <nixpkgs>), causing an evaluation failure. Consider falling back to pkgs.go (or checking pkgs ? go_1_25 as well), and ensure the selected Go version is compatible with go.mod’s go 1.24/toolchain go1.24.7 settings (often also requiring GOTOOLCHAIN=local to prevent network toolchain downloads in sandboxed builds).
|
|
||
| env = { | ||
| CGO_ENABLED = "0"; | ||
| }; |
There was a problem hiding this comment.
buildGoModule runs go test ./... by default, and this repo has many integration tests that start embedded Postgres unless testing.Short() is enabled. In Nix sandbox builds this can be very slow and may fail (e.g., due to embedded-postgres downloading binaries). Consider setting checkFlags = [ "-short" ] (preferred to still run unit tests) or doCheck = false to make nix build reliable.
| }; | |
| }; | |
| # Run Go tests in short mode to avoid slow or flaky integration tests | |
| # (e.g., those that start embedded Postgres) during Nix builds. | |
| checkFlags = [ "-short" ]; |
Closes #333
Summary: