Feature/crane refactor lattice #1
Conversation
WalkthroughAdds a modular Crane-based Rust workspace system: new crane-lattice and crane-lib modules for Cargo.toml/lock parsing, dependency resolution, source preparation, per-crate builders, workspace checks/packages, and architectural documentation aligning components to an eight-layer Bott framework. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Orchestrator as crane-lattice
participant TOML as CargoTomlParser
participant LOCK as CargoLockParser
participant Dep as DependencyResolver
participant Clean as mkCleanSrc
participant Builder as buildWorkspaceMember
participant Checks as workspace-checks
participant Packages as workspace-packages
User->>Orchestrator: mkCraneLattice({ root, gitRev, ... })
Orchestrator->>TOML: readMemberCargoTomls()
TOML-->>Orchestrator: allCargoTomls
Orchestrator->>LOCK: normalizedCargoLock / getCargoLockPackageEntry
LOCK-->>Dep: package entries
Orchestrator->>Dep: getMemberDeps()/getAllDeps()
Dep-->>Orchestrator: transitive deps
Orchestrator->>Clean: mkCleanSrc({ workspaceMembers, extraIncludes, cargoToml, cargoLock })
Clean-->>Orchestrator: cargoWorkspaceSrc
Orchestrator->>Builder: buildWorkspaceMember({...})
Builder-->>Orchestrator: crate derivations / cargoArtifacts
Orchestrator->>Checks: wire cargoWorkspaceAttrs, cargoArtifacts
Checks-->>Orchestrator: clippy/test/doc derivations
Orchestrator->>Packages: expose rust-docs, rust-lib, scripts
Orchestrator-->>User: { cargoWorkspaceSrc, cargoArtifacts, checks, packages, craneLib }
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||||
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
🧹 Nitpick comments (7)
tools/rust/common-rust-deps.nix (1)
4-14: Harden macOS support and surface PKG_CONFIG_PATH ergonomically
- Add libiconv on Darwin; many openssl consumers need it at link time.
- Optionally export pkgConfigPaths as a list for easy env wiring.
{ pkgs, lib, ... }: let - commonBuildInputs = [ - pkgs.pkg-config - pkgs.openssl - ] ++ (lib.optionals pkgs.stdenv.isDarwin [ pkgs.darwin.apple_sdk.frameworks.Security ]); + darwinExtras = + lib.optionals pkgs.stdenv.isDarwin [ + pkgs.darwin.apple_sdk.frameworks.Security + pkgs.libiconv + ]; + commonBuildInputs = [ pkgs.pkg-config pkgs.openssl ] ++ darwinExtras; - pkgConfigPath = "${pkgs.openssl.dev}/lib/pkgconfig"; + pkgConfigPath = "${pkgs.openssl.dev}/lib/pkgconfig"; + pkgConfigPaths = [ pkgConfigPath ]; in { - inherit commonBuildInputs pkgConfigPath; + inherit commonBuildInputs pkgConfigPath pkgConfigPaths; }Please confirm whether consumers set PKG_CONFIG_PATH from pkgConfigPath/pkgConfigPaths (e.g., in devShells or build env).
docs/crqs/CRQ_036_Bott_Universal_Architectural_Framework_Alignment.md (1)
1-20: Polish: capitalize “Bott” and add a pragmatic TL;DR
- Line 1: “bott” → “Bott”.
- Consider adding a short, practical summary mapping modules → responsibilities before the philosophical section.
-The modular structure of `crane-lattice.nix` and its constituent `crane-lib` modules is designed to explicitly align with the **bott Universal Architectural Framework**, +The modular structure of `crane-lattice.nix` and its constituent `crane-lib` modules is designed to explicitly align with the **Bott Universal Architectural Framework**, + +TL;DR (practical): +- utils/common-rust-deps: shared helpers and native inputs +- cargo‑toml/lock parsing: extract workspace/member metadata +- dependency‑resolution: compute transitive local path deps +- mkCleanSrc: filtered workspace source +- buildWorkspaceMember: per‑crate builds +- workspace‑helpers/checks/packages: compose src, checks, and build outputstools/rust/crane-lib/workspace-helpers.nix (1)
18-24: Guard missing workspace members and remove editorial comment
- Use a safe default for
workspace.members.- Drop “Corrected:” inline comment.
- allCargoTomls = builtins.listToAttrs ( - map - ( - dep: lib'.nameValuePair dep (crateCargoToml dep) # Corrected: use crateCargoToml directly - ) - workspaceCargoToml.workspace.members - ); + members = workspaceCargoToml.workspace.members or []; + allCargoTomls = builtins.listToAttrs ( + map (dep: lib'.nameValuePair dep (crateCargoToml dep)) members + );tools/rust/crane-lib/mkCleanSrc.nix (1)
31-52: Fileset filters to only .rs; likely missing build.rs, .proto, .c/.h, .metal, etc.Crates often need non-Rust sources (build scripts, codegen inputs). Consider broadening includes or parameterizing extensions to avoid build breakage.
- (fs.intersection (fs.unions (mkRootPaths workspaceMembers)) ( - fs.fileFilter (file: (builtins.any file.hasExt [ "rs" ])) root - )) + (fs.intersection (fs.unions (mkRootPaths workspaceMembers)) ( + fs.fileFilter (file: + builtins.any file.hasExt [ + "rs" "toml" "lock" "c" "cc" "cpp" "h" "hpp" "proto" "sql" + "json" "yaml" "yml" "metal" "wgsl" "glsl" + "rlib" "S" "s" + ]) root + ))If you prefer strictness, add a param like
extraMemberGlobsand union it into the fileset.tools/rust/crane-lib/cargo-lock-parsing.nix (1)
3-5: Optional: assert Cargo.lock version.You have a TODO for version 4. Add a quick assertion to fail fast on unexpected formats.
let - # TODO: Assert version = 4; + lockToml = lib'.importTOML (root + "/Cargo.lock"); + _assertV4 = assert (lockToml.version or 4) == 4; true; + # lock version asserted above normalizedCargoLock = builtins.foldl' (acc: p: lib'.recursiveUpdate acc p) { } ( - map + map (package: {Then replace the use of
lib'.importTOML (root + "/Cargo.lock")).packagewithlockToml.package.tools/rust/crane-lib/workspace-components.nix (1)
90-103: Remove duplicatepkgs.perlinbuildInputs.Minor cleanup.
buildInputs = [ pkgs.pkg-config - pkgs.perl pkgs.openssl pkgs.protobuf - pkgs.perl pkgs.gnumake pkgs.systemd ] ++ (pkgs.lib.optionals pkgs.stdenv.isDarwin [ pkgs.darwin.apple_sdk.frameworks.Security ]);tools/rust/crane-lib/workspace-checks.nix (1)
28-39: Optional: cap feature combinations to avoid exponential blow‑ups.Consider a guard (e.g., only default/all-features or a max feature count), otherwise evaluation can become huge.
- allFeatureCombinations = pkgs.lib.concatLists ( - builtins.genList (subsets features) (dbg (builtins.length (dbg features) + 1)) - ); + allFeatureCombinations = + let n = builtins.length features; + in if n > 8 then [ features ] # cap for CI sanity + else pkgs.lib.concatLists (builtins.genList (subsets features) (n + 1));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
logs/telemetry.logis excluded by!**/*.log
📒 Files selected for processing (15)
docs/crqs/CRQ_036_Bott_Universal_Architectural_Framework_Alignment.md(1 hunks)tools/rust/README.md(1 hunks)tools/rust/common-rust-deps.nix(1 hunks)tools/rust/crane-architecture.nix(1 hunks)tools/rust/crane-lattice.nix(1 hunks)tools/rust/crane-lib/buildWorkspaceMember.nix(1 hunks)tools/rust/crane-lib/cargo-lock-parsing.nix(1 hunks)tools/rust/crane-lib/cargo-toml-parsing.nix(1 hunks)tools/rust/crane-lib/dependency-resolution.nix(1 hunks)tools/rust/crane-lib/mkCleanSrc.nix(1 hunks)tools/rust/crane-lib/utils.nix(1 hunks)tools/rust/crane-lib/workspace-checks.nix(1 hunks)tools/rust/crane-lib/workspace-components.nix(1 hunks)tools/rust/crane-lib/workspace-helpers.nix(1 hunks)tools/rust/crane-lib/workspace-packages.nix(1 hunks)
🔇 Additional comments (1)
tools/rust/crane-lib/workspace-packages.nix (1)
29-40: RUSTDOCFLAGS use unstable options; ensure nightly toolchain.
-Z unstable-optionsrequires nightly. Confirmcrane.libhere is alreadyoverrideToolchain rust.toolchains.nightly; otherwise docs will fail.Do you want me to wire
crane.lib = crane.lib.overrideToolchain rust.toolchains.nightlyhere explicitly for the doc derivation?
| { root | ||
| , gitRev | ||
| , # Pass all necessary arguments for crane-lattice.nix | ||
| inherit inputs pkgs lib system args rust dbg; | ||
| }: | ||
| let | ||
| # Import crane-lattice.nix | ||
| craneLattice = import ./crane-lattice.nix { inherit inputs; }; | ||
|
|
||
| # Instantiate the crane-lattice module | ||
| # This will effectively build the crane functionality based on the composed modules | ||
| crane = craneLattice.perSystem { | ||
| inherit pkgs lib system args rust dbg gitRev; | ||
| inputs = { | ||
| inherit inputs; # Pass all original inputs | ||
| self = inputs.self; # Ensure self is passed correctly | ||
| }; | ||
| }; |
There was a problem hiding this comment.
root argument is dropped; forward it to crane-lattice.
mkArchitecturalCrane accepts root but doesn’t pass it to craneLattice.perSystem, breaking consumers like utils/mkCleanSrc expecting root.
- crane = craneLattice.perSystem {
- inherit pkgs lib system args rust dbg gitRev;
+ crane = craneLattice.perSystem {
+ inherit pkgs lib system args rust dbg gitRev root;
inputs = {
inherit inputs; # Pass all original inputs
self = inputs.self; # Ensure self is passed correctly
};
};🤖 Prompt for AI Agents
In tools/rust/crane-architecture.nix around lines 80 to 97, the
mkArchitecturalCrane function accepts a root argument but does not forward it
into craneLattice.perSystem; update the crane = craneLattice.perSystem call to
include root (e.g., add root to the list of inherited top-level parameters
alongside pkgs lib system args rust dbg gitRev) and/or ensure inputs passed into
perSystem include root (for example inputs = { inherit inputs root; self =
inputs.self; }), so consumers like utils/mkCleanSrc continue to receive root.
| cargoTomlParsing = import ./crane-lib/cargo-toml-parsing.nix { inherit pkgs lib' root; }; | ||
| inherit (cargoTomlParsing) getCraneMetadata getExtraIncludes getIncludes readMemberCargoTomls crateCargoToml workspaceCargoToml; | ||
|
|
||
| cargoLockParsing = import ./crane-lib/cargo-lock-parsing.nix { inherit pkgs lib' root; }; | ||
| inherit (cargoLockParsing) normalizedCargoLock getCargoLockPackageEntry getAllPackageDependencies cleanCargoLock; | ||
|
|
||
| dependencyResolution = import ./crane-lib/dependency-resolution.nix { inherit lib' crateCargoToml workspaceCargoToml; }; | ||
| inherit (dependencyResolution) getMemberDeps getAllDeps; | ||
|
|
||
| mkCleanSrcModule = import ./crane-lib/mkCleanSrc.nix { inherit pkgs lib' fs mkRootPaths isListOf writeTOML readMemberCargoTomls; }; | ||
| mkCleanSrc = mkCleanSrcModule.mkCleanSrc; | ||
|
|
||
| buildWorkspaceMemberModule = import ./crane-lib/buildWorkspaceMember.nix { | ||
| inherit pkgs lib' rust inputs craneLib dbg gitRev | ||
| isListOf crateCargoToml workspaceCargoToml getMemberDeps getAllDeps | ||
| cleanCargoLock writeTOML readMemberCargoTomls mkCleanSrc | ||
| getIncludes getExtraIncludes; | ||
| root = null; # root will be passed to mkCraneLattice | ||
| }; | ||
| buildWorkspaceMember = buildWorkspaceMemberModule.buildWorkspaceMember; | ||
|
|
There was a problem hiding this comment.
root is undefined for these imports; move them inside mkCraneLattice where root is available.
Current imports of cargo-toml-parsing.nix and cargo-lock-parsing.nix require root. Evaluate them after root is known.
- cargoTomlParsing = import ./crane-lib/cargo-toml-parsing.nix { inherit pkgs lib' root; };
- inherit (cargoTomlParsing) getCraneMetadata getExtraIncludes getIncludes readMemberCargoTomls crateCargoToml workspaceCargoToml;
-
- cargoLockParsing = import ./crane-lib/cargo-lock-parsing.nix { inherit pkgs lib' root; };
- inherit (cargoLockParsing) normalizedCargoLock getCargoLockPackageEntry getAllPackageDependencies cleanCargoLock;
-
- dependencyResolution = import ./crane-lib/dependency-resolution.nix { inherit lib' crateCargoToml workspaceCargoToml; };
- inherit (dependencyResolution) getMemberDeps getAllDeps;
-
- mkCleanSrcModule = import ./crane-lib/mkCleanSrc.nix { inherit pkgs lib' fs mkRootPaths isListOf writeTOML readMemberCargoTomls; };
- mkCleanSrc = mkCleanSrcModule.mkCleanSrc;
-
- buildWorkspaceMemberModule = import ./crane-lib/buildWorkspaceMember.nix {
- inherit pkgs lib' rust inputs craneLib dbg gitRev
- isListOf crateCargoToml workspaceCargoToml getMemberDeps getAllDeps
- cleanCargoLock writeTOML readMemberCargoTomls mkCleanSrc
- getIncludes getExtraIncludes;
- root = null; # root will be passed to mkCraneLattice
- };
- buildWorkspaceMember = buildWorkspaceMemberModule.buildWorkspaceMember;
+ # Defer root-dependent imports to mkCraneLatticeInside mkCraneLattice’s let, add:
+ cargoTomlParsing = import ./crane-lib/cargo-toml-parsing.nix { inherit pkgs lib'; root = root; };
+ inherit (cargoTomlParsing) getCraneMetadata getExtraIncludes getIncludes readMemberCargoTomls crateCargoToml workspaceCargoToml;
+
+ cargoLockParsing = import ./crane-lib/cargo-lock-parsing.nix { inherit pkgs lib'; root = root; };
+ inherit (cargoLockParsing) normalizedCargoLock getCargoLockPackageEntry getAllPackageDependencies cleanCargoLock;
+
+ dependencyResolution = import ./crane-lib/dependency-resolution.nix { inherit lib' crateCargoToml workspaceCargoToml; };
+ inherit (dependencyResolution) getMemberDeps getAllDeps;
+
+ mkCleanSrcModule = import ./crane-lib/mkCleanSrc.nix { inherit pkgs lib' fs mkRootPaths isListOf writeTOML readMemberCargoTomls; };
+ mkCleanSrc = mkCleanSrcModule.mkCleanSrc;
+
+ buildWorkspaceMemberModule = import ./crane-lib/buildWorkspaceMember.nix {
+ inherit pkgs lib' rust inputs craneLib dbg gitRev
+ isListOf crateCargoToml workspaceCargoToml getMemberDeps getAllDeps
+ cleanCargoLock writeTOML readMemberCargoTomls mkCleanSrc
+ getIncludes getExtraIncludes;
+ root = root;
+ };
+ buildWorkspaceMember = buildWorkspaceMemberModule.buildWorkspaceMember;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In tools/rust/crane-lattice.nix around lines 25 to 45, the imports of
cargo-toml-parsing.nix and cargo-lock-parsing.nix (and any downstream imports
that depend on them) are using `root` which is not defined at top-level; move
those imports into the body of mkCraneLattice where `root` is provided, remove
the top-level imports and their `inherit` lines, and instead import them inside
mkCraneLattice’s let with `inherit pkgs lib' root` (and likewise import
dependency-resolution, mkCleanSrc.nix and buildWorkspaceMember.nix there if they
also need root), then adjust the local `inherit (...)` statements to reference
the results from the newly scoped imports so all functions that require `root`
are evaluated after root is known.
| mkCraneLattice = | ||
| { root | ||
| , gitRev | ||
| , | ||
| }: | ||
| let |
There was a problem hiding this comment.
Syntax error: extra comma in argument set.
{ root, gitRev, , } is invalid.
- mkCraneLattice =
- { root
- , gitRev
- ,
- }:
+ mkCraneLattice =
+ { root
+ , gitRev
+ }:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mkCraneLattice = | |
| { root | |
| , gitRev | |
| , | |
| }: | |
| let | |
| mkCraneLattice = | |
| { root | |
| , gitRev | |
| }: | |
| let |
🤖 Prompt for AI Agents
In tools/rust/crane-lattice.nix around lines 47 to 52, the function argument set
contains an extra trailing comma `{ root, gitRev, , }` which is a syntax error;
remove the stray comma so the argument set is `{ root, gitRev }` (or if an
optional argument was intended, add its name instead of the comma), then run
nix-instantiate or nix-build to verify the file parses.
| checks = import ./crane-lib/workspace-checks.nix { | ||
| inherit pkgs lib' crane cargoWorkspaceAttrs cargoArtifacts allCargoTomls dbg; | ||
| }; | ||
|
|
||
| packages = import ./crane-lib/workspace-packages.nix { | ||
| inherit pkgs lib' crane cargoWorkspaceAttrs cargoArtifacts workspaceCargoVendorDir inputs mkCraneLattice; | ||
| }; | ||
| }; |
There was a problem hiding this comment.
Export wiring uses undefined locals; inherit from crane.
cargoWorkspaceAttrs, cargoArtifacts, allCargoTomls, and workspaceCargoVendorDir live on the crane value returned by mkCraneLattice.
- checks = import ./crane-lib/workspace-checks.nix {
- inherit pkgs lib' crane cargoWorkspaceAttrs cargoArtifacts allCargoTomls dbg;
- };
+ checks = import ./crane-lib/workspace-checks.nix {
+ inherit pkgs lib' crane dbg;
+ inherit (crane) cargoWorkspaceAttrs cargoArtifacts allCargoTomls;
+ };
- packages = import ./crane-lib/workspace-packages.nix {
- inherit pkgs lib' crane cargoWorkspaceAttrs cargoArtifacts workspaceCargoVendorDir inputs mkCraneLattice;
- };
+ packages = import ./crane-lib/workspace-packages.nix {
+ inherit pkgs lib' crane inputs mkCraneLattice;
+ inherit (crane) cargoWorkspaceAttrs cargoArtifacts workspaceCargoVendorDir;
+ };| cargoVendorDir = craneLib.vendorMultipleCargoDeps { | ||
| inherit (craneLib.findCargoFiles crateRepoSource) cargoConfigs; | ||
| cargoLockList = lib'.optionalAttrs (buildStdTarget != null) [ | ||
| ./rust-std-Cargo.lock | ||
| ]; | ||
| cargoLockParsedList = [ |
There was a problem hiding this comment.
Use optionals (list) not optionalAttrs (attrset) for cargoLockList.
cargoLockList expects a list. optionalAttrs returns an attrset and will break evaluation.
- cargoLockList = lib'.optionalAttrs (buildStdTarget != null) [
- ./rust-std-Cargo.lock
- ];
+ cargoLockList = lib'.optionals (buildStdTarget != null) [
+ ./rust-std-Cargo.lock
+ ];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cargoVendorDir = craneLib.vendorMultipleCargoDeps { | |
| inherit (craneLib.findCargoFiles crateRepoSource) cargoConfigs; | |
| cargoLockList = lib'.optionalAttrs (buildStdTarget != null) [ | |
| ./rust-std-Cargo.lock | |
| ]; | |
| cargoLockParsedList = [ | |
| cargoVendorDir = craneLib.vendorMultipleCargoDeps { | |
| inherit (craneLib.findCargoFiles crateRepoSource) cargoConfigs; | |
| cargoLockList = lib'.optionals (buildStdTarget != null) [ | |
| ./rust-std-Cargo.lock | |
| ]; | |
| cargoLockParsedList = [ |
🤖 Prompt for AI Agents
In tools/rust/crane-lib/buildWorkspaceMember.nix around lines 199 to 204, the
code uses lib'.optionalAttrs to build cargoLockList but that returns an
attribute set; replace it with lib.optionals (or lib.optional for a single item)
so cargoLockList is a proper list. Concretely, use lib.optionals (buildStdTarget
!= null) [ ./rust-std-Cargo.lock ] (or lib.optional (buildStdTarget != null)
./rust-std-Cargo.lock) to produce a list instead of an attrset.
| workspaceCargoVendorDir = crane.lib.vendorCargoDeps { | ||
| src = crane.cargoWorkspaceSrc; | ||
| overrideVendorGitCheckout = | ||
| ps: drv: | ||
| if pkgs.lib.any (p: (pkgs.lib.hasInfix "zhiburt/tabled" p.source)) ps then | ||
| drv.overrideAttrs | ||
| (_old: { | ||
| postPatch = '' |
There was a problem hiding this comment.
Vendoring src should use cargoWorkspaceSrc, not crane.cargoWorkspaceSrc.
crane doesn’t expose cargoWorkspaceSrc here; you computed it in this file.
- workspaceCargoVendorDir = crane.lib.vendorCargoDeps {
- src = crane.cargoWorkspaceSrc;
+ workspaceCargoVendorDir = crane.lib.vendorCargoDeps {
+ src = cargoWorkspaceSrc;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| workspaceCargoVendorDir = crane.lib.vendorCargoDeps { | |
| src = crane.cargoWorkspaceSrc; | |
| overrideVendorGitCheckout = | |
| ps: drv: | |
| if pkgs.lib.any (p: (pkgs.lib.hasInfix "zhiburt/tabled" p.source)) ps then | |
| drv.overrideAttrs | |
| (_old: { | |
| postPatch = '' | |
| workspaceCargoVendorDir = crane.lib.vendorCargoDeps { | |
| src = cargoWorkspaceSrc; | |
| overrideVendorGitCheckout = | |
| ps: drv: | |
| if pkgs.lib.any (p: (pkgs.lib.hasInfix "zhiburt/tabled" p.source)) ps then | |
| drv.overrideAttrs | |
| (_old: { | |
| postPatch = '' |
🤖 Prompt for AI Agents
In tools/rust/crane-lib/workspace-components.nix around lines 56 to 63, the
vendoring call sets src = crane.cargoWorkspaceSrc but that symbol is not
exported from crane; replace it with the locally computed cargoWorkspaceSrc
(i.e., set src = cargoWorkspaceSrc) so the vendorCargoDeps uses the correct
workspace source; confirm cargoWorkspaceSrc is defined earlier in the file and
adjust any references accordingly.
| cargoWorkspaceSrc = mkCleanSrc { | ||
| workspaceMembers = workspaceCargoToml.workspace.members; | ||
| extraIncludes = (getIncludes allCargoTomls) ++ (getExtraIncludes allCargoTomls); | ||
| cargoToml = /${inputs.self}/Cargo.toml; # Assuming inputs.self is the root of the repo | ||
| cargoLock = /${inputs.self}/Cargo.lock; # Assuming inputs.self is the root of the repo | ||
| dontRemoveDevDeps = true; | ||
| root = inputs.self; # Pass root to mkCleanSrc | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Safer Cargo.toml/lock path construction
Leading / before interpolation is redundant and can produce //... paths. Prefer simple interpolation.
- cargoToml = /${inputs.self}/Cargo.toml; # Assuming inputs.self is the root of the repo
- cargoLock = /${inputs.self}/Cargo.lock; # Assuming inputs.self is the root of the repo
+ cargoToml = "${inputs.self}/Cargo.toml";
+ cargoLock = "${inputs.self}/Cargo.lock";Please confirm that inputs.self in your call site is a path-like (coercible) value; otherwise pass self.outPath.
I'll verify this review comment by examining the codebase. Let me start by checking the actual file and understanding the context around path handling in Nix.
🏁 Script executed:
#!/bin/bash
# Check the actual file content around the specified lines
find . -name "workspace-helpers.nix" -type f 2>/dev/null | head -20Length of output: 44
🏁 Script executed:
#!/bin/bash
# Look at the workspace-helpers.nix file to see current state
cat tools/rust/crane-lib/workspace-helpers.nix 2>/dev/null | head -50Length of output: 1075
🏁 Script executed:
#!/bin/bash
# Search for mkCleanSrc definition to understand its function signature and expectations
rg -n "mkCleanSrc" tools/rust/crane-lib/ -A 10 -B 2Length of output: 7889
🏁 Script executed:
#!/bin/bash
# Search for how inputs.self is used elsewhere in the codebase
rg -n "inputs\.self" tools/rust/crane-lib/ -B 2 -A 2Length of output: 1955
🏁 Script executed:
#!/bin/bash
# Check for any flake.nix or other files that define inputs.self
find tools/rust/crane-lib -name "*.nix" -type f -exec grep -l "inputs" {} \;Length of output: 178
I need to understand more about how inputs.self is used in Nix flakes and whether the path construction is problematic. Let me search for additional context.
🏁 Script executed:
#!/bin/bash
# Find flake.nix files to understand inputs.self context
find . -name "flake.nix" -type f 2>/dev/nullLength of output: 11
🏁 Script executed:
#!/bin/bash
# Check for any documentation or examples in the repo about path construction
rg -n "outPath" tools/rust/crane-lib/ -B 2 -A 2Length of output: 0
🏁 Script executed:
#!/bin/bash
# Look at how mkCleanSrc actually uses cargoToml/cargoLock parameters
cat tools/rust/crane-lib/mkCleanSrc.nixLength of output: 2388
🌐 Web query:
Nix flakes inputs.self path construction best practices
💡 Result:
Short best-practices cheat‑sheet for constructing paths with flakes (inputs.self)
-
Prefer using self (self.outPath / toString self) instead of ./. when referencing the flake source inside your outputs — it avoids double-copying and makes evaluations cheaper and cleaner. [1][3]
-
When declaring local path inputs in inputs.*, use explicit path: or ./ syntax:
- Inside flake.nix a relative path must start with ./ and is resolved relative to that flake’s directory. Don’t rely on ../ across unrelated trees (illegal in some cases). Use absolute path:… for other cases. [1][2]
-
If consumers of your flake need Git submodules or LFS to be fetched automatically, declare it with inputs.self.submodules = true and/or inputs.self.lfs = true (so callers need not add fetch flags). [1][4]
-
Don’t expose the entire project root as a derivation source unfiltered. Prefer:
- exporting a specific output (e.g. self.packages, self.sourcesFiltered) or
- produce a controlled source with builtins.path, fetchTree, or a staging/filter step to avoid needless rebuilds on unrelated file edits. This reduces rebuild churn and store bloat. [3][2]
-
Use path: URL form when you need determinism or special characters (path:… encodes and escapes correctly). If the path points into a git repo, Nix may treat it as git+file:. Prefer using git+file:?ref=rev when you want a specific commit. [2][5]
-
For transitive overrides: use inputs..follows to make dependency inputs align (avoid duplicated nixpkgs etc.). Use overrides rather than copying inputs around. [1]
If you want, paste your flake.nix (or the snippet that’s confusing you) and I’ll give targeted corrections.
Sources: Nix flake manual and reference (path & self semantics), man pages, and Nix community discussion. [1] [2] [3] [4]
Let me check the flake.nix to understand the inputs.self context, then verify both affected files.
🏁 Script executed:
#!/bin/bash
# Check flake.nix to understand inputs.self
head -100 flake.nixLength of output: 3294
🏁 Script executed:
#!/bin/bash
# Verify both files have the same issue
echo "=== workspace-helpers.nix ===" && grep -n "cargoToml\|cargoLock" tools/rust/crane-lib/workspace-helpers.nix && echo -e "\n=== workspace-components.nix ===" && grep -n "cargoToml\|cargoLock" tools/rust/crane-lib/workspace-components.nixLength of output: 439
Fix Cargo.toml/lock path construction in both workspace files—use string interpolation instead of path literal syntax
The review comment is correct. The path construction using /${inputs.self}/... is less idiomatic in Nix than string interpolation. The suggested fix applies, but note that this issue exists in both files:
tools/rust/crane-lib/workspace-helpers.nix(lines 29-30)tools/rust/crane-lib/workspace-components.nix(lines 31-32)
Use string interpolation in both locations:
- cargoToml = /${inputs.self}/Cargo.toml;
- cargoLock = /${inputs.self}/Cargo.lock;
+ cargoToml = "${inputs.self}/Cargo.toml";
+ cargoLock = "${inputs.self}/Cargo.lock";This aligns with Nix flake best practices for handling self-references and is clearer about intent.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| cargoWorkspaceSrc = mkCleanSrc { | |
| workspaceMembers = workspaceCargoToml.workspace.members; | |
| extraIncludes = (getIncludes allCargoTomls) ++ (getExtraIncludes allCargoTomls); | |
| cargoToml = /${inputs.self}/Cargo.toml; # Assuming inputs.self is the root of the repo | |
| cargoLock = /${inputs.self}/Cargo.lock; # Assuming inputs.self is the root of the repo | |
| dontRemoveDevDeps = true; | |
| root = inputs.self; # Pass root to mkCleanSrc | |
| }; | |
| cargoWorkspaceSrc = mkCleanSrc { | |
| workspaceMembers = workspaceCargoToml.workspace.members; | |
| extraIncludes = (getIncludes allCargoTomls) ++ (getExtraIncludes allCargoTomls); | |
| cargoToml = "${inputs.self}/Cargo.toml"; | |
| cargoLock = "${inputs.self}/Cargo.lock"; | |
| dontRemoveDevDeps = true; | |
| root = inputs.self; # Pass root to mkCleanSrc | |
| }; |
🤖 Prompt for AI Agents
In tools/rust/crane-lib/workspace-helpers.nix around lines 26 to 33 (and also
update tools/rust/crane-lib/workspace-components.nix at the analogous lines
~31-32), the Cargo.toml and Cargo.lock paths are constructed using the Nix path
literal syntax (/${inputs.self}/...), which is non-idiomatic and can break
self-references; replace those path literals with string interpolation
referencing inputs.self (e.g. "${inputs.self}/Cargo.toml" and
"${inputs.self}/Cargo.lock") in both files so the paths are formed correctly and
follow flake/self best practices.
| buildWasmContract = import ../buildWasmContract.nix { | ||
| # Relative path to original | ||
| inherit | ||
| buildWorkspaceMember | ||
| crateCargoToml | ||
| pkgs | ||
| lib' | ||
| rust | ||
| craneLib | ||
| dbg | ||
| gitRev | ||
| ; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify relative import path and tighten inputs
- Confirm
tools/rust/buildWasmContract.nixexists relative to this file. - If it’s optional, gate the import or expose a stub.
🏁 Script executed:
#!/bin/bash
# Check that the file exists and show its first lines
fd -t f buildWasmContract.nix -a
rg -n -m1 '^{' tools/rust/buildWasmContract.nix || trueLength of output: 119
The file does exist. Let me verify the import is correctly structured and check whether it should be conditional:Now let me verify the file structure and check if the inputs are tightened appropriately:
---
🏁 Script executed:
```shell
#!/bin/bash
# View the buildWasmContract.nix file structure
cat tools/rust/buildWasmContract.nix
Length of output: 3048
I found a critical issue. The buildWasmContract.nix file expects a parameter named lib, but the import in the review snippet passes lib' (with a quote). Let me verify this in the full context:
---
🏁 Script executed:
```shell
#!/bin/bash
# View workspace-helpers.nix around the import statement
cat tools/rust/crane-lib/workspace-helpers.nix
Length of output: 1138
Fix parameter name mismatch in import statement
The import passes lib' but buildWasmContract.nix expects lib. Line 37 must rename the parameter:
buildWasmContract = import ../buildWasmContract.nix {
inherit
buildWorkspaceMember
crateCargoToml
pkgs
rust
craneLib
dbg
gitRev
;
lib = lib';
};The inherit lib' statement creates { lib' = ... } but the module declares { ..., lib, ... }, causing the lib parameter to be undefined.
🤖 Prompt for AI Agents
In tools/rust/crane-lib/workspace-helpers.nix around lines 35 to 47, the import
passes a parameter named lib' while buildWasmContract.nix expects lib; update
the import so lib is provided by mapping lib = lib' (remove or stop inheriting
lib') and keep the rest of the inherited parameters unchanged so the called
module receives the expected lib parameter.
| check-all-workspace-members-individually = pkgs.writeShellApplication { | ||
| name = "check-all-workspace-members-individually"; | ||
| text = '' | ||
| cargo metadata --no-deps | jq '.workspace_members[]' -r | xargs -I{} cargo check -p {} | ||
| ''; | ||
| }; |
There was a problem hiding this comment.
Script lacks runtime dependencies and uses PackageId instead of package name.
- Add
runtimeInputssocargo,jq, andxargsare available. cargo metadata .workspace_members[]yields PackageId;cargo -pexpects a name (or name@version). Extract names from.packages[]filtered byworkspace_members.
check-all-workspace-members-individually = pkgs.writeShellApplication {
name = "check-all-workspace-members-individually";
+ runtimeInputs = [ pkgs.cargo pkgs.jq pkgs.findutils ];
text = ''
- cargo metadata --no-deps | jq '.workspace_members[]' -r | xargs -I{} cargo check -p {}
+ cargo metadata --no-deps --format-version 1 \
+ | jq -r '
+ .workspace_members as $m
+ | .packages[]
+ | select(.id as $id | any($m[]; . == $id))
+ | .name
+ ' \
+ | xargs -I{} cargo check -p {}
'';
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| check-all-workspace-members-individually = pkgs.writeShellApplication { | |
| name = "check-all-workspace-members-individually"; | |
| text = '' | |
| cargo metadata --no-deps | jq '.workspace_members[]' -r | xargs -I{} cargo check -p {} | |
| ''; | |
| }; | |
| check-all-workspace-members-individually = pkgs.writeShellApplication { | |
| name = "check-all-workspace-members-individually"; | |
| runtimeInputs = [ pkgs.cargo pkgs.jq pkgs.findutils ]; | |
| text = '' | |
| cargo metadata --no-deps --format-version 1 \ | |
| | jq -r ' | |
| .workspace_members as $m | |
| | .packages[] | |
| | select(.id as $id | any($m[]; . == $id)) | |
| | .name | |
| ' \ | |
| | xargs -I{} cargo check -p {} | |
| ''; | |
| }; |
| ```nix | ||
| # In your main flake.nix | ||
| inputs = { | ||
| # ... other inputs | ||
| craneLattice = { | ||
| url = "./vendor/nix/union/tools/rust/crane-lattice.nix"; # Adjust path as needed | ||
| inputs.nixpkgs.follows = "nixpkgs"; | ||
| inputs.crane.follows = "crane"; # Assuming crane is also an input | ||
| inputs.rust-overlay.follows = "rust-overlay"; # Assuming rust-overlay is also an input | ||
| # ... other inputs required by crane-lattice.nix | ||
| }; | ||
| }; | ||
|
|
||
| outputs = { self, nixpkgs, craneLattice, ... }: { | ||
| # ... | ||
| perSystem = { pkgs, lib, system, ... }: | ||
| let | ||
| # Instantiate the craneLattice module | ||
| crane = craneLattice.perSystem { | ||
| inherit pkgs lib system; | ||
| # Pass other necessary arguments like rust, dbg, gitRev, etc. | ||
| # These would typically come from the perSystem scope of your main flake. | ||
| rust = inputs.rust-overlay.packages.${system}.rust-bin.stable.latest.default; # Example | ||
| dbg = lib.debug.trace; # Example | ||
| gitRev = self.rev or "dirty"; # Example | ||
| inputs = { | ||
| inherit self nixpkgs craneLattice; | ||
| crane = inputs.crane; | ||
| # ... other inputs that crane-lattice needs | ||
| }; | ||
| }; | ||
| in | ||
| { | ||
| # Now you can use the 'crane' object provided by crane-lattice | ||
| # For example, to access checks or packages: | ||
| checks = crane.checks; | ||
| packages = crane.packages; | ||
| devShells.default = pkgs.mkShell { | ||
| buildInputs = [ crane.packages.rust-lib ]; # Example | ||
| }; | ||
| }; | ||
| }; | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
Fix flake example: avoid shadowing inputs and use import or path: for local file
- Using a single .nix file as a flake input via
url = "./.../crane-lattice.nix"won’t work unless that path is a flake. Useimporton a local path, orurl = "path:./tools/rust"if that directory is a flake. - The attrset field
inputs = { ... }shadows the flake’sinputs, breakinginputs.rust-overlay...references. - Provide a minimal, working snippet.
- inputs = {
- # ... other inputs
- craneLattice = {
- url = "./vendor/nix/union/tools/rust/crane-lattice.nix"; # Adjust path as needed
- inputs.nixpkgs.follows = "nixpkgs";
- inputs.crane.follows = "crane"; # Assuming crane is also an input
- inputs.rust-overlay.follows = "rust-overlay"; # Assuming rust-overlay is also an input
- # ... other inputs required by crane-lattice.nix
- };
- };
+ # No separate flake input needed for a local .nix module.
- outputs = { self, nixpkgs, craneLattice, ... }: {
+ outputs = { self, nixpkgs, rust-overlay, crane, ... }: {
# ...
perSystem = { pkgs, lib, system, ... }:
let
- # Instantiate the craneLattice module
- crane = craneLattice.perSystem {
+ craneLattice = import ./tools/rust/crane-lattice.nix;
+ crane = craneLattice.perSystem {
inherit pkgs lib system;
- # Pass other necessary arguments like rust, dbg, gitRev, etc.
- # These would typically come from the perSystem scope of your main flake.
- rust = inputs.rust-overlay.packages.${system}.rust-bin.stable.latest.default; # Example
- dbg = lib.debug.trace; # Example
+ rust = rust-overlay.packages.${system}.rust-bin.stable.latest.default; # Example
+ dbg = lib.debug.traceVal; # Less noisy than trace
gitRev = self.rev or "dirty"; # Example
- inputs = {
- inherit self nixpkgs craneLattice;
- crane = inputs.crane;
- # ... other inputs that crane-lattice needs
- };
+ inputs = { inherit self nixpkgs crane; };
};
in
{
checks = crane.checks;
packages = crane.packages;
devShells.default = pkgs.mkShell {
- buildInputs = [ crane.packages.rust-lib ]; # Example
+ buildInputs = [ ]; # Fill with actual packages exposed by the lattice
};
};
};- Which public packages does your lattice export? Does
rust-libexist? If not, please update the example accordingly.
I need to gather information about the actual codebase structure and the Nix flake patterns being used. Let me investigate.
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the crane-lattice file and examine its structure
fd "crane-lattice" --type f -x cat {}
Length of output: 5555
🏁 Script executed:
#!/bin/bash
# Search for flake.nix files to understand actual usage patterns
fd "flake.nix" --type f -x head -50 {}Length of output: 1852
🏁 Script executed:
#!/bin/bash
# Look for crane, rust-overlay, or related module exports
rg -n "packages\s*=" --type nix --max-count=20 -A3 -B1Length of output: 10836
🏁 Script executed:
#!/bin/bash
# Check the tools/rust directory structure
fd --type f --base-directory tools/rust 2>/dev/null || echo "Directory not found"Length of output: 445
I need to check what the workspace-packages.nix file exports to validate the package exports claim.
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine what packages are exported by crane-lattice
cat tools/rust/crane-lib/workspace-packages.nix
Length of output: 3507
Fix flake example: correct URL syntax and inputs shadowing, and confirm rust-lib exists
The README documentation example has two real issues that need correcting:
- URL syntax error: Local
.nixfiles cannot be used as flake inputs viaurl = "...". Useimportinstead. - inputs shadowing bug: The let-binding
inputs = { ... }masks the function parameterinputs, breaking the reference toinputs.craneandinputs.rust-overlay.
However, rust-lib does exist: It is exported by workspace-packages.nix (defined as pkgs.mkRootDrv "rust-lib") and is available when crane-lattice is properly imported.
The corrected example should:
- Remove the flake input declaration for craneLattice
- Use
import ./tools/rust/crane-lattice.nixinstead - Accept
rust-overlayandcraneas top-level flake inputs - Pass them directly without shadowing via a separate
inputslet-binding - Reference exported packages from the lattice correctly
- inputs = {
- # ... other inputs
- craneLattice = {
- url = "./vendor/nix/union/tools/rust/crane-lattice.nix"; # Adjust path as needed
- inputs.nixpkgs.follows = "nixpkgs";
- inputs.crane.follows = "crane"; # Assuming crane is also an input
- inputs.rust-overlay.follows = "rust-overlay"; # Assuming rust-overlay is also an input
- # ... other inputs required by crane-lattice.nix
- };
- };
+ # No separate flake input needed for a local .nix module.
- outputs = { self, nixpkgs, craneLattice, ... }: {
+ outputs = { self, nixpkgs, rust-overlay, crane, ... }: {
# ...
perSystem = { pkgs, lib, system, ... }:
let
- # Instantiate the craneLattice module
- crane = craneLattice.perSystem {
+ craneLattice = import ./tools/rust/crane-lattice.nix;
+ crane = craneLattice.perSystem {
inherit pkgs lib system;
- # Pass other necessary arguments like rust, dbg, gitRev, etc.
- # These would typically come from the perSystem scope of your main flake.
- rust = inputs.rust-overlay.packages.${system}.rust-bin.stable.latest.default; # Example
- dbg = lib.debug.trace; # Example
+ rust = rust-overlay.packages.${system}.rust-bin.stable.latest.default; # Example
+ dbg = lib.debug.traceVal; # Less noisy than trace
gitRev = self.rev or "dirty"; # Example
- inputs = {
- inherit self nixpkgs craneLattice;
- crane = inputs.crane;
- # ... other inputs that crane-lattice needs
- };
+ inputs = { inherit self nixpkgs crane; };
};
in
{
checks = crane.checks;
packages = crane.packages;
devShells.default = pkgs.mkShell {
- buildInputs = [ crane.packages.rust-lib ]; # Example
+ buildInputs = [ crane.packages.rust-lib ]; # rust-lib is exported
};
};
};🤖 Prompt for AI Agents
In tools/rust/README.md around lines 16 to 58, the flake example uses an invalid
local-flake URL and masks the top-level `inputs` by declaring a local `inputs`
let-binding; to fix it, remove the `craneLattice` flake input and import the
local file with `import ./tools/rust/crane-lattice.nix` from the outputs scope,
accept `crane` and `rust-overlay` as top-level flake inputs in the example, and
when instantiating the crane lattice pass the original top-level `inputs` (or
explicit `crane` and `rust-overlay`) into the `craneLattice.perSystem` call
instead of redefining `inputs`; also update the example to reference the
exported package name (rust-lib) from the instantiated `crane` object (e.g.,
crane.packages.rust-lib) so it matches the module export.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tools/rust/common-rust-deps.nix (2)
11-11: Consider simplifying the alias.The
pkgConfigPathalias simply renamesopensslPkgConfigPathwithout adding logic or transformation. You could either exportopensslPkgConfigPathdirectly or document why the renaming improves the interface.Apply this diff to remove the indirection:
- pkgConfigPath = opensslPkgConfigPath;And update the export:
{ - inherit commonBuildInputs pkgConfigPath; + inherit commonBuildInputs; + pkgConfigPath = opensslPkgConfigPath; }Or if the renaming is intentional for API consistency, keep as-is.
1-16: Consider naming scope alignment.The file is named
common-rust-deps.nixbut currently only aggregates OpenSSL-related dependencies. While this supports future extensibility, the name might suggest a broader scope than currently implemented. If other common Rust dependencies (like additional build tools or libraries) will be added, the current structure is appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
tools/rust/README.md(1 hunks)tools/rust/common-rust-deps.nix(1 hunks)tools/rust/openssl-deps.nix(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/rust/README.md
🔇 Additional comments (2)
tools/rust/openssl-deps.nix (1)
1-17: LGTM! Clean modular OpenSSL dependency definition.The implementation correctly:
- Handles Darwin-specific Security framework with conditional inclusion
- Exposes both build inputs and pkg-config path
- Follows standard Nix conventions
tools/rust/common-rust-deps.nix (1)
4-9: LGTM! Correct import and augmentation of build inputs.The file properly imports OpenSSL dependencies and augments them with
pkg-config, creating a clean interface for downstream consumers.
PR Type
Enhancement
Description
Modularized crane Rust build system into 8-layer architectural framework
Extracted common Rust dependencies into reusable shared module
Formalized prime-chunk based architecture with explicit layer mapping
Added comprehensive documentation for Bott Universal Framework alignment
Diagram Walkthrough
File Walkthrough
13 files
Extracted common Rust build dependencies moduleFormalized 8-layer prime-chunk architectural frameworkCore orchestrator importing and composing modular layersUtility functions for path handling and TOML generationParse and extract metadata from Cargo.toml filesNormalize and resolve Cargo.lock dependency informationRecursively resolve local and workspace dependenciesFilter and prepare clean source for compilationBuild individual workspace members with customizationCompose workspace source and WASM contract buildingDefine workspace-level clippy, test, and doc checksDefine buildable packages and documentation outputsCompose workspace components and build artifacts2 files
Document 8-layer architecture aligned with Bott periodicityAdd usage guide for modularized crane-lattice systemSummary by CodeRabbit
Documentation
New Features
Chores