Copilot/combine red tape with blueprints#148
Conversation
Removed details about the rpi-configuration.nix and its usage.
Refactor package handling and improve system argument management in default.nix.
Add homesGeneric function to handle user entries and configurations.
…rt mkFlake Agent-Logs-Url: https://github.com/leporuid/blueprint/sessions/79bd4f0c-0f48-474b-9a3b-9689dbd4230d Co-authored-by: leporuid <98006840+leporuid@users.noreply.github.com>
Agent-Logs-Url: https://github.com/leporuid/blueprint/sessions/e2443ba5-256f-4c95-a67d-589b7cb1ff70 Co-authored-by: leporuid <98006840+leporuid@users.noreply.github.com>
Agent-Logs-Url: https://github.com/leporuid/blueprint/sessions/e2c160ab-cf01-4678-82fa-265f379ee20a Co-authored-by: leporuid <98006840+leporuid@users.noreply.github.com>
…-optimizations Copilot/research lib default nix optimizations
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c3c01115c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| in | ||
| { | ||
| class = "nixos"; | ||
| class = "nixos-raspberrypi"; |
There was a problem hiding this comment.
Keep RPi hosts in a supported class
Returning class = "nixos-raspberrypi" for rpi-configuration.nix makes these hosts fail later in hostsByCategory, which only recognizes nixos, nix-darwin, system-manager, robotnix, and nix-on-droid. Any project still using hosts/<name>/rpi-configuration.nix now errors during evaluation instead of producing nixosConfigurations.<name>.
Useful? React with 👍 / 👎.
| inherit hostName; | ||
| }; | ||
| ] ++ mkHomeUsersModule hostname home-manager.nixosModules.default; | ||
| inherit specialArgs; |
There was a problem hiding this comment.
Continue injecting hostName into host modules
loadNixOS/loadNixDarwin now pass plain specialArgs (and loadSystemManager passes hostname), so hostName is no longer provided to host configuration modules. Existing configs that declare { hostName, ... } will fail to evaluate with a missing argument, which is a breaking change from previous behavior.
Useful? React with 👍 / 👎.
| pname: { type, path }: import path { | ||
| inherit | ||
| pname | ||
| flake | ||
| inputs |
There was a problem hiding this comment.
Restore perSystem args for checks imports
checks/<name>.nix used to receive systemArgs.${system} (including perSystem and scope helpers), but this import now forwards only a fixed subset of arguments. Checks that rely on the per-system interface now fail at evaluation time, e.g. when referencing perSystem.<input>.<pkg>.
Useful? React with 👍 / 👎.
| }; | ||
| }; | ||
|
|
||
| outputs = inputs: (inputs.blueprint { inherit inputs; src = ./.; }) // { |
There was a problem hiding this comment.
Drop unsupported src argument in red-tape template
The template calls inputs.blueprint { inherit inputs; src = ./.; }, but blueprint’s functor in this repo does not accept a src parameter (it takes inputs, prefix, nixpkgs, and systems). Initializing from this template will fail with an unexpected-argument error.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds red-tape integration alongside Blueprint and updates Blueprint internals/templates/docs to support the combined workflow.
Changes:
- Add
red-tapeas a flake input and re-exportmkFlakefrom this flake’s outputs. - Introduce a new
templates/red-tapetemplate and update docs around folder structure. - Refactor
lib/default.nixsystem/home/check module wiring and adjust the formatter script.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/red-tape/flake.nix | New template flake wiring Blueprint + red-tape |
| templates/red-tape/devshell.nix | New devshell stub for the red-tape template |
| lib/default.nix | Refactors per-system args injection, nixpkgs config module, hosts/homes handling, and checks import args |
| formatter.nix | Switch nixfmt package and adjust file traversal pipeline |
| flake.nix | Adds red-tape input and re-exports mkFlake |
| flake.lock | Locks red-tape and transitive inputs; updates nixpkgs revision |
| docs/content/getting-started/folder_structure.md | Updates documented host file layout and other folder-structure guidance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| deadnix --no-lambda-pattern-names --edit "$@" | ||
|
|
||
| # Use git to traverse since nixfmt doesn't have good traversal | ||
| git ls-files -z "$@" | grep --null '\.nix$' | xargs --null --no-run-if-empty nixfmt | ||
| git ls-files "$@" | grep --null '\.nix$' | xargs --no-run-if-empty nixfmt | ||
| ''; |
There was a problem hiding this comment.
The formatting pipeline is inconsistent: git ls-files is now newline-delimited, but grep --null expects NUL-delimited input, and xargs is no longer using NUL mode. This will break formatting for paths with spaces/newlines and may fail outright depending on grep/xargs behavior. Prefer restoring a NUL-delimited pipeline (git ls-files -z + grep -z/--null-data + xargs -0) or remove all NUL-specific flags consistently.
| outputs = inputs: (inputs.blueprint { inherit inputs; src = ./.; }) // { | ||
| # Re-export red-tape's mkFlake for direct access | ||
| inherit (inputs.red-tape) mkFlake; |
There was a problem hiding this comment.
inputs.blueprint (this repo) is called with an unexpected src argument here. mkBlueprint currently accepts inputs, prefix, nixpkgs, and systems (no src), so this template will fail with an "unexpected argument 'src'" error. Remove src or pass the directory via prefix if needed.
| loadNixOS = hostname: path: { | ||
| class = "nixos"; | ||
| value = inputs.nixpkgs.lib.nixosSystem { | ||
| modules = [ | ||
| nixpkgsConfigModule | ||
| perSystemModule | ||
| path | ||
| ] ++ mkHomeUsersModule hostName home-manager.nixosModules.default; | ||
| specialArgs = specialArgs // { | ||
| inherit hostName; | ||
| }; | ||
| ] ++ mkHomeUsersModule hostname home-manager.nixosModules.default; | ||
| inherit specialArgs; | ||
| }; |
There was a problem hiding this comment.
loadNixOS no longer passes the hostname into module arguments (previously via specialArgs // { inherit hostName; }). The docs still describe hostName being provided to configuration.nix, and existing host modules may rely on it. Consider restoring hostName (or a consistent hostname) in specialArgs/_module.args to avoid a breaking change.
| { | ||
| class = "nix-darwin"; | ||
| value = nix-darwin.lib.darwinSystem { | ||
| modules = [ | ||
| nixpkgsConfigModule | ||
| perSystemModule | ||
| path | ||
| ] ++ mkHomeUsersModule hostName home-manager.darwinModules.default; | ||
| specialArgs = specialArgs // { | ||
| inherit hostName; | ||
| }; | ||
| ] ++ mkHomeUsersModule hostname home-manager.darwinModules.default; | ||
| inherit specialArgs; | ||
| }; |
There was a problem hiding this comment.
loadNixDarwin similarly no longer passes the hostname into module arguments. If modules (or docs) expect hostName, this is a breaking change. Consider restoring hostName/hostname in specialArgs for darwin systems as well.
| value = system-manager.lib.makeSystemConfig { | ||
| modules = [ | ||
| perSystemSMModule | ||
| perSystemModule | ||
| path | ||
| ]; | ||
| extraSpecialArgs = specialArgs // { | ||
| inherit hostName; | ||
| inherit hostname; | ||
| }; |
There was a problem hiding this comment.
loadSystemManager now injects hostname into extraSpecialArgs, but other system loaders historically provided hostName (and the docs still reference hostName). Consider keeping the argument name consistent (e.g., continue providing hostName, optionally aliasing hostname) to avoid breaking downstream configs.
| { | ||
| class = "nixos"; | ||
| class = "nixos-raspberrypi"; | ||
| value = nixos-raspberrypi.lib.nixosSystem { | ||
| modules = [ | ||
| nixpkgsConfigModule | ||
| perSystemModule | ||
| path | ||
| ] | ||
| ++ mkHomeUsersModule hostName home-manager.nixosModules.default; | ||
| specialArgs = specialArgs // { | ||
| inherit hostName; | ||
| nixos-raspberrypi = inputs.nixos-raspberrypi; | ||
| }; | ||
| }; | ||
| ] ++ mkHomeUsersModule hostname home-manager.nixosModules.default; | ||
| inherit specialArgs; | ||
| }; |
There was a problem hiding this comment.
loadNixOSRPi now returns class = "nixos-raspberrypi", but hostsByCategory only supports grouping for class "nixos" (plus darwin/system-manager/etc.). As-is, any rpi host will hit the "class not supported" throw during grouping. Either keep the class as "nixos" or add a grouping case for "nixos-raspberrypi" (likely mapping to nixosConfigurations).
| ## **Hosts** for machine configurations | ||
|
|
||
| ## `hosts/<hostname>/(default.nix|configuration.nix|darwin-configuration.nix,system-configuration.nix,rpi-configuration.nix)` | ||
| ## `hosts/<hostname>/(default.nix|configuration.nix|darwin-configuration.nix,system-configuration.nix)` | ||
|
|
There was a problem hiding this comment.
Docs no longer list rpi-configuration.nix as a supported host file, but lib/default.nix still loads hosts/<hostname>/rpi-configuration.nix. Either document this option again (including required inputs.nixos-raspberrypi) or remove the loader to keep docs and behavior consistent.
| importChecksFn = lib.mapAttrs ( | ||
| pname: { type, path }: import path (systemArgs.${system} // { inherit pname; }) | ||
| pname: { type, path }: import path { | ||
| inherit | ||
| pname | ||
| flake | ||
| inputs | ||
| system | ||
| pkgs | ||
| ; | ||
| } |
There was a problem hiding this comment.
checks/<pname> imports are now called with only { pname, flake, inputs, system, pkgs }, but the documentation states checks receive the full per-system argument set (including perSystem). This is a behavioral breaking change for existing checks/ entries that expect perSystem or other per-system args. Consider passing perSystem (and/or restoring systemArgs.${system} // { inherit pname; }) to match the documented interface.
No description provided.