PR #56 Review — Nix derivation, flake, and CI automation

Overall this is well-structured and follows good patterns (the versions.json approach matching nixpkgs' ngrok). Below are findings organized by severity.

Critical Issues

1. default.nix + README: nix-env -iA sql-pipe -f URL will fail

default.nix returns a single derivation directly:

pkgs.callPackage ./packaging/nix/package.nix { }

But the README instructs:

nix-env -iA sql-pipe -f https://github.com/vmvarela/sql-pipe/archive/master.tar.gz

-iA sql-pipe expects the -f expression to evaluate to an attribute set containing a sql-pipe attribute. Since default.nix returns the derivation itself, this will fail with an error like error: attribute 'sql-pipe' missing.

Fix (recommended): Keep default.nix as-is and change the README to use -if (install from file):

nix-env -if https://github.com/vmvarela/sql-pipe/archive/master.tar.gz

Warnings

2. flake.lock is missing

The PR adds flake.nix but no flake.lock. Without a committed flake.lock, every consumer will resolve nixpkgs to the latest revision at evaluation time, making builds non-reproducible. Run nix flake lock and commit the generated flake.lock. This is standard practice for all Nix flakes.

3. CI: GITHUB_TOKEN may not be able to push to master (release.yml:738)

The update-nix job uses the default GITHUB_TOKEN to git push directly to master. If branch protection rules are enabled on master (e.g., required reviews, status checks), this push will be rejected. Consider:

• Using a PAT or GitHub App token with bypass permissions
• Or pushing to a branch and auto-creating/merging a PR

4. CI: sed stripping is a no-op / dead code (release.yml:733)

sed -i 's/^          //' packaging/nix/versions.json

The YAML block scalar run: | already strips the common leading indent (10 spaces) from the script content. After YAML processing, the heredoc body has 0–4 spaces of indentation, not 10. The sed pattern (matching 10 leading spaces) matches nothing — it's a harmless no-op but dead code that adds confusion.

Fix: Remove the sed line entirely.

5. update.sh:2: common-updater-scripts is unused

The nix-shell shebang includes common-updater-scripts but the script never uses any commands from it (like update-source-version). Remove it from the shebang:

#!nix-shell -i bash -p curl jq

6. Hash format: bare hex vs. SRI (versions.json)

versions.json uses bare hex SHA256 hashes. While this works, modern nixpkgs convention prefers SRI format (sha256-BASE64STRING=). Bare hex may produce deprecation warnings in newer Nix versions. Consider converting in both update.sh and the CI job with nix hash to-sri --type sha256 "$HEX_HASH".

Minor / Nits

7. dontFixup = stdenvNoCC.hostPlatform.isLinux — correct, add a comment (package.nix:37)

The logic is right: musl-static Linux binaries need no fixup; macOS needs the fixup phase for codesigning (especially on Apple Silicon). A brief inline comment explaining why would help future maintainers:

# Linux binaries are statically linked (musl) — no fixup needed.
# macOS binaries need the fixup phase for ad-hoc codesigning.
dontFixup = stdenvNoCC.hostPlatform.isLinux;

Verified as Correct

• stdenvNoCC — correct choice for pre-built binary installation
• dontPatchELF = true — correct for musl-static binaries
• lib.importJSON ./versions.json — resolves correctly in both flake and non-flake contexts
• testers.testVersion — verified the Nix source wraps the command with 2>&1, so stderr output from sql-pipe --version is captured correctly
• passthru.updateScript = ./update.sh — follows nixpkgs convention
• meta.platforms = builtins.attrNames versions — clever, dynamically derives platforms from JSON keys
• awk extraction in CI — awk default field separator handles the two-space separator in sha256sum output correctly
• jq interpolation — \($ver) syntax is valid jq string interpolation
• .gitignore entry result — standard for Nix build outputs
• Overlay definition — correctly uses final (not _prev) for callPackage
• forAllSystems + flake structure — follows standard conventions
• SHA256 hashes — all four hashes in versions.json verified against sha256sums.txt from the v0.2.0 release

Summary

The critical issue (#1) should be fixed before merge.