feat(nix): Replace Fenix and Naersk with my-nix-lib #20

Merged
martin merged 2 commits from my-nix-lib into master 2026-08-01 15:03:50 +00:00
Owner
No description provided.
martin self-assigned this 2026-08-01 11:16:10 +00:00
feat(nix): Replace Fenix and Naersk with my-nix-lib
All checks were successful
ci/woodpecker/push/verify Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
63237bf5b7
Collaborator

Review Summary

Solid, focused migration. This PR replaces the repo-local fenix+naersk wiring with shared my-nix-lib helpers (mkRustPlatform/mkDevShell) and makes scripts/codegen.sh env-driven so codegen can run inside the Nix build. .nix/pkgs.nix gets much leaner, flake.nix drops local boilerplate, and the new CODEGEN_SOURCE_ROOT/GENERATED_DIR overrides 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:

  1. Offline lockfile generation is fragile (.nix/pkgs.nix preBuild). cargo generate-lockfile --offline runs in a sandbox with no network, so it only works while the committed Cargo.lock already covers every dependency of the freshly generated target/generated/server crate. It currently does (the committed lockfile lists the server package 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.
  2. Package metadata regressed: pname/version and the whole meta block (description, license, mainProgram) were dropped, degrading nix run, nix flake show, and nixpkgs metadata. Restore them unless mkRustPlatform already derives equivalents from Cargo.toml.
  3. Rust toolchain is now transitive: pinned implicitly by 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.
  4. Minor: the three independently overridable paths in codegen.sh are easy to set partially (e.g. only GENERATED_DIR), which would make inputs resolve against the store copy of the script where openapi.yaml is absent; safe today because the Nix preBuild sets 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

# Review Summary Solid, focused migration. This PR replaces the repo-local fenix+naersk wiring with shared `my-nix-lib` helpers (`mkRustPlatform`/`mkDevShell`) and makes `scripts/codegen.sh` env-driven so codegen can run inside the Nix build. `.nix/pkgs.nix` gets much leaner, `flake.nix` drops local boilerplate, and the new `CODEGEN_SOURCE_ROOT`/`GENERATED_DIR` overrides 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: 1. **Offline lockfile generation is fragile** (`.nix/pkgs.nix` preBuild). `cargo generate-lockfile --offline` runs in a sandbox with no network, so it only works while the committed `Cargo.lock` already covers every dependency of the freshly generated `target/generated/server` crate. It currently does (the committed lockfile lists the `server` package 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. 2. **Package metadata regressed**: `pname`/`version` and the whole `meta` block (description, license, `mainProgram`) were dropped, degrading `nix run`, `nix flake show`, and nixpkgs metadata. Restore them unless `mkRustPlatform` already derives equivalents from Cargo.toml. 3. **Rust toolchain is now transitive**: pinned implicitly by `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. 4. Minor: the three independently overridable paths in `codegen.sh` are easy to set partially (e.g. only `GENERATED_DIR`), which would make inputs resolve against the store copy of the script where `openapi.yaml` is absent; safe today because the Nix `preBuild` sets 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**
clank-bot left a comment

Code review findings from opencode.

Code review findings from opencode.
@ -8,4 +7,1 @@
cargoToml = lib.trivial.importTOML ../Cargo.toml;
inherit (cargoToml.package) name version description;
in
rustPlatform.buildPackage {
Collaborator

[style] This PR drops pname/version and the whole meta block (description, license, mainProgram) from the derivation. That loses the package description and license for nix flake show/nixpkgs consumers, and mainProgram matters for nix run/nix run .#recurring-events. Restore meta (and pname/version if mkRustPlatform doesn't already derive them from Cargo.toml, as the previous code did via lib.trivial.importTOML).

[style] This PR drops `pname`/`version` and the whole `meta` block (description, license, `mainProgram`) from the derivation. That loses the package description and license for `nix flake show`/`nixpkgs` consumers, and `mainProgram` matters for `nix run`/`nix run .#recurring-events`. Restore `meta` (and `pname`/`version` if `mkRustPlatform` doesn't already derive them from Cargo.toml, as the previous code did via `lib.trivial.importTOML`).
.nix/pkgs.nix Outdated
@ -28,0 +20,4 @@
CODEGEN_SOURCE_ROOT=${lib.cleanSource ../.} \
GENERATED_DIR="$PWD/target/generated" \
bash ${../scripts/codegen.sh}
cargo generate-lockfile --offline
Collaborator

[bug] cargo generate-lockfile --offline runs inside the Nix build sandbox, which has no network, so offline resolution only succeeds while the committed Cargo.lock already pins every dependency the freshly generated target/generated/server crate needs (it currently does — the committed lockfile lists the server package 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 make generate-lockfile --offline fail 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.

[bug] `cargo generate-lockfile --offline` runs inside the Nix build sandbox, which has no network, so offline resolution only succeeds while the committed `Cargo.lock` already pins every dependency the freshly generated `target/generated/server` crate needs (it currently does — the committed lockfile lists the `server` package 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 make `generate-lockfile --offline` fail 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;
Collaborator

[style] The Rust toolchain is now pinned transitively by my-nix-lib (its internal fenix), instead of being declared explicitly in this flake as before. Toolchain/edition drift (Cargo.toml uses edition = "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.

[style] The Rust toolchain is now pinned transitively by `my-nix-lib` (its internal `fenix`), instead of being declared explicitly in this flake as before. Toolchain/edition drift (Cargo.toml uses `edition = "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}"
Collaborator

[style] With REPO_ROOT, SOURCE_ROOT, and GENERATED_DIR independently overridable, partially setting them (e.g. only GENERATED_DIR) makes inputs resolve against REPO_ROOT, which — when the script runs from a Nix store copy via bash ${../scripts/codegen.sh} — points at a store dir that does not contain openapi.yaml/Cargo.toml. Safe today because the Nix preBuild always sets both CODEGEN_SOURCE_ROOT and GENERATED_DIR, but deriving defaults from a single anchor or validating that the pair is set together would be more robust.

[style] With `REPO_ROOT`, `SOURCE_ROOT`, and `GENERATED_DIR` independently overridable, partially setting them (e.g. only `GENERATED_DIR`) makes inputs resolve against `REPO_ROOT`, which — when the script runs from a Nix store copy via `bash ${../scripts/codegen.sh}` — points at a store dir that does not contain `openapi.yaml`/`Cargo.toml`. Safe today because the Nix `preBuild` always sets both `CODEGEN_SOURCE_ROOT` and `GENERATED_DIR`, but deriving defaults from a single anchor or validating that the pair is set together would be more robust.
fix(nix): use of cargo generate-lockfile in prebuild
All checks were successful
ci/woodpecker/push/verify Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
14a2f0c87f
Collaborator

Review Summary

Overall this is a well-structured migration from the fenix/naersk setup to the author's own my-nix-lib (Crane-backed mkRustPlatform/mkDevShell). The scripts/codegen.sh refactor is the strongest part: splitting REPO_ROOT from SOURCE_ROOT/GENERATED_DIR and running codegen with CODEGEN_SOURCE_ROOT=${lib.cleanSource ../.} + GENERATED_DIR="$PWD/target/generated" is a good fix (the old nix preBuild resolved REPO_ROOT from $0 inside the store and would write into the read-only store path). Docs and lockfile were updated consistently, and Cargo.toml (0.3.3) now matches Cargo.lock.

Two things need attention before merge:

  1. Bootstrap-stub/lockfile sync under Crane's --locked default. Crane's buildPackage (crane v0.23.4) defaults to cargoExtraArgs = "--locked" and derives both the vendored deps (cargoVendorDir) and the deps-only buildDepsOnly artifact derivation from src as committed — i.e. before preBuild regenerates target/generated/server. That means the committed bootstrap stub target/generated/server/Cargo.toml must exactly match what codegen produces (same version and dependency set), or cargo build --locked fails in the sandbox. This diff bumps server to 0.3.3 in Cargo.lock but does not touch the committed stub; if the stub is still on 0.3.0 (or a stale dep list), nix build will break. Worth a local nix build / nix flake check verification.

  2. Loss of package metadata. The meta block (description, license = mit, mainProgram) was dropped. nix run .#default still works via the single-binary fallback and the container image hardcodes /bin/recurring-events, so this is cosmetic, but license/description metadata is a small regression and cheap to restore.

Minor notes: the my flake input is unpinned (only the lock pins it to refs/heads/main); everything else (devshell, formatter, image, CI) looks consistent.


*🤖 Review by opencode (opencode/deepseek-v4-flash-free) — verdict: changes

# Review Summary Overall this is a well-structured migration from the fenix/naersk setup to the author's own `my-nix-lib` (Crane-backed `mkRustPlatform`/`mkDevShell`). The `scripts/codegen.sh` refactor is the strongest part: splitting `REPO_ROOT` from `SOURCE_ROOT`/`GENERATED_DIR` and running codegen with `CODEGEN_SOURCE_ROOT=${lib.cleanSource ../.}` + `GENERATED_DIR="$PWD/target/generated"` is a good fix (the old nix `preBuild` resolved `REPO_ROOT` from `$0` inside the store and would write into the read-only store path). Docs and lockfile were updated consistently, and `Cargo.toml` (0.3.3) now matches `Cargo.lock`. Two things need attention before merge: 1. **Bootstrap-stub/lockfile sync under Crane's `--locked` default.** Crane's `buildPackage` (crane v0.23.4) defaults to `cargoExtraArgs = "--locked"` and derives both the vendored deps (`cargoVendorDir`) and the deps-only `buildDepsOnly` artifact derivation from `src` as committed — i.e. *before* `preBuild` regenerates `target/generated/server`. That means the committed bootstrap stub `target/generated/server/Cargo.toml` must exactly match what codegen produces (same version and dependency set), or `cargo build --locked` fails in the sandbox. This diff bumps `server` to 0.3.3 in `Cargo.lock` but does not touch the committed stub; if the stub is still on 0.3.0 (or a stale dep list), `nix build` will break. Worth a local `nix build` / `nix flake check` verification. 2. **Loss of package metadata.** The `meta` block (`description`, `license = mit`, `mainProgram`) was dropped. `nix run .#default` still works via the single-binary fallback and the container image hardcodes `/bin/recurring-events`, so this is cosmetic, but `license`/`description` metadata is a small regression and cheap to restore. Minor notes: the `my` flake input is unpinned (only the lock pins it to `refs/heads/main`); everything else (devshell, formatter, image, CI) looks consistent. --- *🤖 Review by opencode (opencode/deepseek-v4-flash-free) — verdict: **changes**
clank-bot left a comment

Code review findings from opencode.

Code review findings from opencode.
@ -25,3 +18,3 @@
];
preBuild = ''
bash scripts/codegen.sh
CODEGEN_SOURCE_ROOT=${lib.cleanSource ../.} \
Collaborator

[bug] Crane's buildPackage defaults to cargoExtraArgs = "--locked" and evaluates its cargoVendorDir (vendor FOD) and deps-only buildDepsOnly derivation from src as committed — before this preBuild regenerates target/generated/server. Correctness therefore depends on the committed bootstrap stub target/generated/server/Cargo.toml exactly matching what codegen produces (version AND dependency list). This PR bumps server to 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 thus nix build) will fail. Please confirm the stub is in sync and that nix build / nix flake check pass.

[bug] Crane's buildPackage defaults to `cargoExtraArgs = "--locked"` and evaluates its `cargoVendorDir` (vendor FOD) and deps-only `buildDepsOnly` derivation from `src` as committed — before this `preBuild` regenerates `target/generated/server`. Correctness therefore depends on the committed bootstrap stub `target/generated/server/Cargo.toml` exactly matching what codegen produces (version AND dependency list). This PR bumps `server` to 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 thus `nix build`) will fail. Please confirm the stub is in sync and that `nix build` / `nix flake check` pass.
@ -28,1 +20,4 @@
CODEGEN_SOURCE_ROOT=${lib.cleanSource ../.} \
GENERATED_DIR="$PWD/target/generated" \
bash ${../scripts/codegen.sh}
'';
Collaborator

[style] The previous meta block (description, license = lib.licenses.mit, mainProgram) was removed. nix run .#default still works via the single-binary fallback and the image uses a hardcoded entrypoint, so this is not a functional break, but losing license/description metadata is a small regression. Consider restoring a minimal meta (at minimum license and mainProgram = "recurring-events").

[style] The previous `meta` block (description, license = lib.licenses.mit, mainProgram) was removed. `nix run .#default` still works via the single-binary fallback and the image uses a hardcoded entrypoint, so this is not a functional break, but losing `license`/`description` metadata is a small regression. Consider restoring a minimal `meta` (at minimum `license` and `mainProgram = "recurring-events"`).
@ -10,2 +6,2 @@
naersk = {
url = "github:nix-community/naersk";
my = {
url = "git+https://code.martials.no/Personal/my-nix-lib";
Collaborator

[style] The my input URL (git+https://code.martials.no/Personal/my-nix-lib) has no ?ref=/rev= pin; reproducibility currently relies solely on flake.lock pinning refs/heads/main. Consider pinning explicitly (e.g. git+https://...?ref=main) so nix flake update doesn't silently track new main commits.

[style] The `my` input URL (`git+https://code.martials.no/Personal/my-nix-lib`) has no `?ref=`/`rev=` pin; reproducibility currently relies solely on flake.lock pinning `refs/heads/main`. Consider pinning explicitly (e.g. `git+https://...?ref=main`) so `nix flake update` doesn't silently track new main commits.
martin merged commit 3c49e6a1e0 into master 2026-08-01 15:03:50 +00:00
martin deleted branch my-nix-lib 2026-08-01 15:03:50 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
martin/recurring-events!20
No description provided.