feat(nix): Replace Fenix and Naersk with my-nix-lib #20
Loading…
Reference in a new issue
No description provided.
Delete branch "my-nix-lib"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Review Summary
Solid, focused migration. This PR replaces the repo-local fenix+naersk wiring with shared
my-nix-libhelpers (mkRustPlatform/mkDevShell) and makesscripts/codegen.shenv-driven so codegen can run inside the Nix build..nix/pkgs.nixgets much leaner,flake.nixdrops local boilerplate, and the newCODEGEN_SOURCE_ROOT/GENERATED_DIRoverrides correctly keep generated output in the writable build dir ($PWD/target/generated) instead of a read-only store path. Docs (AGENTS.md) match the actual CI versions (v7.24.0 / rust:1.97-bookworm), and the flake.lock refresh is consistent.Main points:
.nix/pkgs.nixpreBuild).cargo generate-lockfile --offlineruns in a sandbox with no network, so it only works while the committedCargo.lockalready covers every dependency of the freshly generatedtarget/generated/servercrate. It currently does (the committed lockfile lists theserverpackage with its full dep set), but any generator/template change that introduces a new dependency will fail resolution offline with no way to fetch. Worth verifying on a clean sandbox and documenting the assumption.pname/versionand the wholemetablock (description, license,mainProgram) were dropped, degradingnix run,nix flake show, and nixpkgs metadata. Restore them unlessmkRustPlatformalready derives equivalents from Cargo.toml.my-nix-lib's internal fenix rather than in this repo, so toolchain/edition drift is no longer visible here. Consider pinning or documenting the expected toolchain.codegen.share easy to set partially (e.g. onlyGENERATED_DIR), which would make inputs resolve against the store copy of the script whereopenapi.yamlis absent; safe today because the NixpreBuildsets both, but a single anchor would be sturdier.No issues in the Rust application code; the nix-only changes are the review focus.
*🤖 Review by opencode (opencode/deepseek-v4-flash-free) — verdict: changes
Code review findings from opencode.
@ -8,4 +7,1 @@cargoToml = lib.trivial.importTOML ../Cargo.toml;inherit (cargoToml.package) name version description;inrustPlatform.buildPackage {[style] This PR drops
pname/versionand the wholemetablock (description, license,mainProgram) from the derivation. That loses the package description and license fornix flake show/nixpkgsconsumers, andmainProgrammatters fornix run/nix run .#recurring-events. Restoremeta(andpname/versionifmkRustPlatformdoesn't already derive them from Cargo.toml, as the previous code did vialib.trivial.importTOML).@ -28,0 +20,4 @@CODEGEN_SOURCE_ROOT=${lib.cleanSource ../.} \GENERATED_DIR="$PWD/target/generated" \bash ${../scripts/codegen.sh}cargo generate-lockfile --offline[bug]
cargo generate-lockfile --offlineruns inside the Nix build sandbox, which has no network, so offline resolution only succeeds while the committedCargo.lockalready pins every dependency the freshly generatedtarget/generated/servercrate needs (it currently does — the committed lockfile lists theserverpackage and its full dep set). Any change that adds a new dependency to the generated crate (an openapi-generator-cli/template bump, or a spec change that introduces a codegen-time dependency) will makegenerate-lockfile --offlinefail with a "no matching package" error and no way to fetch. Please verify this on a clean sandbox and document the assumption (or vendor/fetch deps up front); note the committed lockfile is also stale (server/recurring-events at 0.3.0 vs Cargo.toml 0.3.3), so a lockfile rewrite is guaranteed each build.@ -48,3 +36,1 @@cargo = rustToolchain;rustc = rustToolchain;};inherit (my.lib.${system}.rust) mkRustPlatform mkDevShell;[style] The Rust toolchain is now pinned transitively by
my-nix-lib(its internalfenix), instead of being declared explicitly in this flake as before. Toolchain/edition drift (Cargo.toml usesedition = "2024") is no longer visible or controllable from this repo. Consider pinning the expected toolchain explicitly or at least documenting the minimum required version in AGENTS.md so MSRV/build-version drift is reproducible.@ -4,2 +4,2 @@REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"GENERATED_DIR="$REPO_ROOT/target/generated"REPO_ROOT="${REPO_ROOT:-$(cd "$(dirname "$0")/.." && pwd)}"SOURCE_ROOT="${CODEGEN_SOURCE_ROOT:-$REPO_ROOT}"[style] With
REPO_ROOT,SOURCE_ROOT, andGENERATED_DIRindependently overridable, partially setting them (e.g. onlyGENERATED_DIR) makes inputs resolve againstREPO_ROOT, which — when the script runs from a Nix store copy viabash ${../scripts/codegen.sh}— points at a store dir that does not containopenapi.yaml/Cargo.toml. Safe today because the NixpreBuildalways sets bothCODEGEN_SOURCE_ROOTandGENERATED_DIR, but deriving defaults from a single anchor or validating that the pair is set together would be more robust.Review Summary
Overall this is a well-structured migration from the fenix/naersk setup to the author's own
my-nix-lib(Crane-backedmkRustPlatform/mkDevShell). Thescripts/codegen.shrefactor is the strongest part: splittingREPO_ROOTfromSOURCE_ROOT/GENERATED_DIRand running codegen withCODEGEN_SOURCE_ROOT=${lib.cleanSource ../.}+GENERATED_DIR="$PWD/target/generated"is a good fix (the old nixpreBuildresolvedREPO_ROOTfrom$0inside the store and would write into the read-only store path). Docs and lockfile were updated consistently, andCargo.toml(0.3.3) now matchesCargo.lock.Two things need attention before merge:
Bootstrap-stub/lockfile sync under Crane's
--lockeddefault. Crane'sbuildPackage(crane v0.23.4) defaults tocargoExtraArgs = "--locked"and derives both the vendored deps (cargoVendorDir) and the deps-onlybuildDepsOnlyartifact derivation fromsrcas committed — i.e. beforepreBuildregeneratestarget/generated/server. That means the committed bootstrap stubtarget/generated/server/Cargo.tomlmust exactly match what codegen produces (same version and dependency set), orcargo build --lockedfails in the sandbox. This diff bumpsserverto 0.3.3 inCargo.lockbut does not touch the committed stub; if the stub is still on 0.3.0 (or a stale dep list),nix buildwill break. Worth a localnix build/nix flake checkverification.Loss of package metadata. The
metablock (description,license = mit,mainProgram) was dropped.nix run .#defaultstill works via the single-binary fallback and the container image hardcodes/bin/recurring-events, so this is cosmetic, butlicense/descriptionmetadata is a small regression and cheap to restore.Minor notes: the
myflake input is unpinned (only the lock pins it torefs/heads/main); everything else (devshell, formatter, image, CI) looks consistent.*🤖 Review by opencode (opencode/deepseek-v4-flash-free) — verdict: changes
Code review findings from opencode.
@ -25,3 +18,3 @@];preBuild = ''bash scripts/codegen.shCODEGEN_SOURCE_ROOT=${lib.cleanSource ../.} \[bug] Crane's buildPackage defaults to
cargoExtraArgs = "--locked"and evaluates itscargoVendorDir(vendor FOD) and deps-onlybuildDepsOnlyderivation fromsrcas committed — before thispreBuildregeneratestarget/generated/server. Correctness therefore depends on the committed bootstrap stubtarget/generated/server/Cargo.tomlexactly matching what codegen produces (version AND dependency list). This PR bumpsserverto 0.3.3 in Cargo.lock but does not update that committed stub; if the stub still says 0.3.0 or has a stale dep list,cargo build --locked(and thusnix build) will fail. Please confirm the stub is in sync and thatnix build/nix flake checkpass.@ -28,1 +20,4 @@CODEGEN_SOURCE_ROOT=${lib.cleanSource ../.} \GENERATED_DIR="$PWD/target/generated" \bash ${../scripts/codegen.sh}'';[style] The previous
metablock (description, license = lib.licenses.mit, mainProgram) was removed.nix run .#defaultstill works via the single-binary fallback and the image uses a hardcoded entrypoint, so this is not a functional break, but losinglicense/descriptionmetadata is a small regression. Consider restoring a minimalmeta(at minimumlicenseandmainProgram = "recurring-events").@ -10,2 +6,2 @@naersk = {url = "github:nix-community/naersk";my = {url = "git+https://code.martials.no/Personal/my-nix-lib";[style] The
myinput URL (git+https://code.martials.no/Personal/my-nix-lib) has no?ref=/rev=pin; reproducibility currently relies solely on flake.lock pinningrefs/heads/main. Consider pinning explicitly (e.g.git+https://...?ref=main) sonix flake updatedoesn't silently track new main commits.