feat: Added nix support#29
Conversation
- Added nix derivation to package software - Lets you try the program without installing it via `nix run` - Allows NixOS users to install and run the program :] - Updated README with details on how to use it
📝 WalkthroughWalkthroughAdds Nix Flakes support and direnv integration: new Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Direnv as direnv
participant Nix as Nix Flakes
participant Crane as crane
participant Rust as rust-toolchain
Dev->>Direnv: cd into repo
Direnv->>Nix: evaluate `use flake` (reads `flake.nix`)
Nix->>Crane: instantiate crane package / build or devShell
Crane->>Rust: determine/provide rust-toolchain (local toml or nightly)
Crane-->>Nix: produce package & devShell (including `libsteam_api` placement)
Nix-->>Direnv: activate shell with inputs (RUST_SRC_PATH, deps)
Direnv-->>Dev: shell environment ready
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Let me know if there's anything I can expand on! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
README.md (1)
27-39: Consider noting that Nix flakes must be enabled.Flakes are still an experimental feature on some Nix installations. Users on older setups may need
--experimental-features 'nix-command flakes'or the equivalentnix.confsetting. A brief note would prevent confusion for those users.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 27 - 39, The Nix instructions assume flakes are enabled but don't mention that flakes are experimental on some installs; update the "Nix" section in README.md (the paragraph and the `nix run github:unhappychoice/steamfetch` examples) to add a brief note that Nix flakes must be enabled and include the two common options to enable them: run with `--experimental-features 'nix-command flakes'` or set `experimental-features = nix-command flakes` in nix.conf, and show that the same flag can be used when passing arguments (i.e., include the flag before `run` or document adjusting nix.conf). Ensure the note is short and placed immediately above or below the existing example code blocks so users see it before trying `nix run`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flake.nix`:
- Line 2: The flake's description string is a leftover personal template; update
the description attribute (the description = "..." value) to a project-specific
sentence that names the project and its intent (e.g., "<project-name> — Nix
flake for Rust development and CI" or similar). Edit the description value in
flake.nix (the description attribute) to a concise, accurate project-specific
phrase that reflects the repository's purpose.
- Around line 30-40: The rust-src extension is commented out in the
rust-toolchain fallback which breaks RUST_SRC_PATH in the devShell and causes
rust-analyzer issues; update the rust-toolchain fallback (the expression
assigning rust-toolchain) to enable the rust-src extension (uncomment or add
"extensions = [ \"rust-src\" ]" inside toolchain.default.override) so the
selected nightly includes source files, and also add guidance to require an
explicit rust-toolchain.toml or rust-version in Cargo.toml to avoid always using
the latest nightly (reference: rust-toolchain, rust-src, RUST_SRC_PATH,
devShell, rust-toolchain.toml, Cargo.toml).
---
Nitpick comments:
In `@README.md`:
- Around line 27-39: The Nix instructions assume flakes are enabled but don't
mention that flakes are experimental on some installs; update the "Nix" section
in README.md (the paragraph and the `nix run github:unhappychoice/steamfetch`
examples) to add a brief note that Nix flakes must be enabled and include the
two common options to enable them: run with `--experimental-features
'nix-command flakes'` or set `experimental-features = nix-command flakes` in
nix.conf, and show that the same flag can be used when passing arguments (i.e.,
include the flag before `run` or document adjusting nix.conf). Ensure the note
is short and placed immediately above or below the existing example code blocks
so users see it before trying `nix run`.
| # Builds the rust components from the toolchain file, or defaults back to the latest nightly build | ||
| rust-toolchain = | ||
| if builtins.pathExists ./rust-toolchain.toml then | ||
| pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml | ||
| else | ||
| pkgs.rust-bin.selectLatestNightlyWith ( | ||
| toolchain: | ||
| toolchain.default.override { | ||
| # extensions = [ "rust-src" ]; | ||
| } | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if rust-toolchain.toml exists in the repo and what channel it specifies
echo "=== Checking for rust-toolchain.toml ==="
fd -t f 'rust-toolchain' --max-depth 1
echo "---"
if [ -f rust-toolchain.toml ]; then
echo "Contents:"
cat rust-toolchain.toml
elif [ -f rust-toolchain ]; then
echo "Contents of rust-toolchain:"
cat rust-toolchain
else
echo "No rust-toolchain file found - nightly fallback WILL be used"
fi
echo ""
echo "=== Checking Cargo.toml for rust-version (MSRV) ==="
rg -n 'rust-version' Cargo.toml || echo "No rust-version field found"Repository: unhappychoice/steamfetch
Length of output: 254
🏁 Script executed:
#!/bin/bash
# Verify the flake.nix devShell configuration around line 88
echo "=== Checking flake.nix lines 80-95 for RUST_SRC_PATH ==="
sed -n '80,95p' flake.nixRepository: unhappychoice/steamfetch
Length of output: 542
🏁 Script executed:
#!/bin/bash
# Verify lines 30-40 in flake.nix to confirm rust-src is commented out
echo "=== Checking flake.nix lines 28-42 for rust-src status ==="
sed -n '28,42p' flake.nixRepository: unhappychoice/steamfetch
Length of output: 671
Uncomment rust-src extension; nightly fallback will cause issues.
Two confirmed problems:
-
Line 38:
rust-srcis commented out, but the devShell (line 87) setsRUST_SRC_PATH = "${rust-toolchain}/lib/rustlib/src/rust/library"assuming it exists. Sincerust-toolchain.tomlis absent, the nightly fallback will be used and won't includerust-src, causingRUST_SRC_PATHto point to a nonexistent path and breaking rust-analyzer. -
With no
rust-toolchain.tomland norust-versioninCargo.toml, the project will always use the latest nightly Rust. This is likely unintended and may cause unexpected build failures or compatibility issues.
Proposed fix
rust-toolchain =
if builtins.pathExists ./rust-toolchain.toml then
pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml
else
- pkgs.rust-bin.selectLatestNightlyWith (
- toolchain:
- toolchain.default.override {
- # extensions = [ "rust-src" ];
- }
- );
+ pkgs.rust-bin.stable.latest.default.override {
+ extensions = [ "rust-src" ];
+ };📝 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.
| # Builds the rust components from the toolchain file, or defaults back to the latest nightly build | |
| rust-toolchain = | |
| if builtins.pathExists ./rust-toolchain.toml then | |
| pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml | |
| else | |
| pkgs.rust-bin.selectLatestNightlyWith ( | |
| toolchain: | |
| toolchain.default.override { | |
| # extensions = [ "rust-src" ]; | |
| } | |
| ); | |
| # Builds the rust components from the toolchain file, or defaults back to the latest nightly build | |
| rust-toolchain = | |
| if builtins.pathExists ./rust-toolchain.toml then | |
| pkgs.rust-bin.fromRustupToolchainFile ./rust-toolchain.toml | |
| else | |
| pkgs.rust-bin.stable.latest.default.override { | |
| extensions = [ "rust-src" ]; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` around lines 30 - 40, The rust-src extension is commented out in
the rust-toolchain fallback which breaks RUST_SRC_PATH in the devShell and
causes rust-analyzer issues; update the rust-toolchain fallback (the expression
assigning rust-toolchain) to enable the rust-src extension (uncomment or add
"extensions = [ \"rust-src\" ]" inside toolchain.default.override) so the
selected nightly includes source files, and also add guidance to require an
explicit rust-toolchain.toml or rust-version in Cargo.toml to avoid always using
the latest nightly (reference: rust-toolchain, rust-src, RUST_SRC_PATH,
devShell, rust-toolchain.toml, Cargo.toml).
There was a problem hiding this comment.
It did catch the flake description, so I'll change that. I assumed it was alright to use the latest nightly build and have it be pinned via flake.lock, but if you think it should be the default stable toolchain instead LMK!
There was a problem hiding this comment.
@of-the-stars You're absolutely right about flake.lock pinning! I overlooked that detail. The nightly version will indeed be locked until you explicitly run nix flake update, so it won't cause unexpected changes. That's a perfectly reasonable approach for Nix users.
The main issue is still the commented-out rust-src extension on line 38. Since the devShell references RUST_SRC_PATH on line 87, you'll need to uncomment that line:
extensions = [ "rust-src" ];As for nightly vs stable, that's your call! If the project builds fine with nightly and you prefer it, go ahead and keep it. Just make sure rust-src is included so rust-analyzer works properly in the dev shell. 🙂
Followed coderabbit's suggestions - Changed description - Uncommented rust-src extension
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
flake.nix (2)
81-84: Nit: remove emptypackageslist andshellHook.Both are no-ops. Removing them reduces noise and leaves room that's clearly intentional when something is added later.
♻️ Proposed cleanup
devShells.default = pkgs.mkShell { # Inherits buildInputs from crane-package inputsFrom = [ crane-package ]; - - # Additional packages for the dev environment - packages = with pkgs; [ - ]; - - shellHook = ""; - env = { # Needed for rust-analyzer RUST_SRC_PATH = "${rust-toolchain}/lib/rustlib/src/rust/library"; }; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` around lines 81 - 84, Remove the no-op entries in flake.nix by deleting the empty packages block ("packages = with pkgs; [ ];") and the empty shellHook assignment ("shellHook = \"\";") so the flake file contains only meaningful configuration; locate and remove those two symbols (packages and shellHook) from the current flake.nix stanza to reduce noise and leave a clean placeholder for future additions.
46-46:pnameis computed but never consumed.The extracted
pnameis never passed tobuildPackage,buildDepsOnly, orcommonArgs, so it has no effect on the derivation name. Either pass it or remove it.♻️ Proposed fix
- pname = craneLib.crateNameFromCargoToml { cargoToml = ./Cargo.toml; }.pname; - # Common arguments shared between buildPackage and buildDepsOnly commonArgs = { inherit src; + pname = craneLib.crateNameFromCargoToml { cargoToml = ./Cargo.toml; }.pname; strictDeps = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` at line 46, The variable pname is computed but never used; either remove the computation or propagate it into the package derivation so it affects the output name—update the call sites that assemble the Nix derivation (e.g., pass pname into commonArgs or into buildPackage/buildDepsOnly) so the resulting derivation uses pname (for example add pname = pname to the attribute set passed into commonArgs or include pname as an argument when calling buildPackage/buildDepsOnly); if you prefer to drop it, simply remove the pname = craneLib.crateNameFromCargoToml ... expression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flake.nix`:
- Line 94: The lambda passed to eachDefaultSystem is returning a double-nested
key by using formatter.${system} and nixpkgs.legacyPackages.${system}; change
the lambda to return a plain formatter attribute (so eachDefaultSystem will add
the per-system key) and reference the already-in-scope pkgs for the formatter
binary (use pkgs.nixfmt-tree or equivalent) instead of
nixpkgs.legacyPackages.${system}.nixfmt-tree; update the entry that currently
mentions formatter.${system} to simply formatter = ... and use pkgs for the
package lookup.
---
Duplicate comments:
In `@flake.nix`:
- Line 2: The flake.nix description has been updated to a project-specific value
(the description field) and the rust-src extension has been uncommented so
RUST_SRC_PATH points to a valid path; no further code changes are required—leave
the description = "Steamfetch nix flake" entry and the rust-src extension as-is
to keep RUST_SRC_PATH correct.
---
Nitpick comments:
In `@flake.nix`:
- Around line 81-84: Remove the no-op entries in flake.nix by deleting the empty
packages block ("packages = with pkgs; [ ];") and the empty shellHook assignment
("shellHook = \"\";") so the flake file contains only meaningful configuration;
locate and remove those two symbols (packages and shellHook) from the current
flake.nix stanza to reduce noise and leave a clean placeholder for future
additions.
- Line 46: The variable pname is computed but never used; either remove the
computation or propagate it into the package derivation so it affects the output
name—update the call sites that assemble the Nix derivation (e.g., pass pname
into commonArgs or into buildPackage/buildDepsOnly) so the resulting derivation
uses pname (for example add pname = pname to the attribute set passed into
commonArgs or include pname as an argument when calling
buildPackage/buildDepsOnly); if you prefer to drop it, simply remove the pname =
craneLib.crateNameFromCargoToml ... expression.
Man AI is better than I thought it was. I wonder where it's getting all of this.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
flake.nix (3)
46-46:pnameis computed but never referenced — dead code.
pnameis derived fromCargo.tomlbut never passed tocommonArgsorcrane-package. Crane inferspnameautomatically fromCargo.tomlanyway.♻️ Proposed fix
- pname = craneLib.crateNameFromCargoToml { cargoToml = ./Cargo.toml; }.pname; - # Common arguments shared between buildPackage and buildDepsOnly🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` at line 46, The variable pname assigned via craneLib.crateNameFromCargoToml (pname = craneLib.crateNameFromCargoToml { cargoToml = ./Cargo.toml; }.pname) is unused dead code; remove that assignment from the flake and rely on crane's automatic crate name inference instead, so delete the pname line and ensure neither commonArgs nor crane-package expect an explicit pname elsewhere.
7-7: Addinputs.nixpkgs.followstocraneto avoid a duplicate nixpkgs instance.
rust-overlaycorrectly followsnixpkgs, butcranedoes not. This causes Nix to evaluate two separate nixpkgs instances, increasing evaluation time and potentially causing subtle mismatches.♻️ Proposed fix
- crane.url = "github:ipetkov/crane"; + crane = { + url = "github:ipetkov/crane"; + inputs.nixpkgs.follows = "nixpkgs"; + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` at line 7, The flake declares crane without having it follow the main nixpkgs which causes a second nixpkgs instance; add an inputs follow so crane uses the same nixpkgs as rust-overlay by setting the crane input to follow nixpkgs (i.e., add inputs.crane.follows = "nixpkgs") so both crane and rust-overlay share the same nixpkgs instance.
81-82:with pkgs;on an empty list is a no-op.The
with pkgs;scope binding has no effect on an empty list. Either populate the list or drop thewithexpression.♻️ Proposed fix
- packages = with pkgs; [ - ]; + packages = [ ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` around lines 81 - 82, The packages attribute currently uses the redundant scope binding "with pkgs;" on an empty list (packages = with pkgs; [ ];), which is a no-op; either remove the "with pkgs;" to make it simply packages = [ ]; or populate the list with package identifiers that rely on the with-binding — update the packages expression accordingly and ensure any package names reference pkgs explicitly if you remove the with (e.g., pkgs.somePackage).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@flake.nix`:
- Line 46: The variable pname assigned via craneLib.crateNameFromCargoToml
(pname = craneLib.crateNameFromCargoToml { cargoToml = ./Cargo.toml; }.pname) is
unused dead code; remove that assignment from the flake and rely on crane's
automatic crate name inference instead, so delete the pname line and ensure
neither commonArgs nor crane-package expect an explicit pname elsewhere.
- Line 7: The flake declares crane without having it follow the main nixpkgs
which causes a second nixpkgs instance; add an inputs follow so crane uses the
same nixpkgs as rust-overlay by setting the crane input to follow nixpkgs (i.e.,
add inputs.crane.follows = "nixpkgs") so both crane and rust-overlay share the
same nixpkgs instance.
- Around line 81-82: The packages attribute currently uses the redundant scope
binding "with pkgs;" on an empty list (packages = with pkgs; [ ];), which is a
no-op; either remove the "with pkgs;" to make it simply packages = [ ]; or
populate the list with package identifiers that rely on the with-binding —
update the packages expression accordingly and ensure any package names
reference pkgs explicitly if you remove the with (e.g., pkgs.somePackage).
|
the pname is there as QoL to tell nix what the program's name is outside of the cargo derivation for possible use in nixpkgs. the |
of-the-stars
force-pushed
the
feature/nix-support
branch
from
f2eb9d8 to
196e8db
Compare
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@unhappychoice I think the PR is ready to merge. What do you think? |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
flake.nix (1)
84-87: Consider removing empty declarations.The empty
packageslist andshellHookcould be removed to reduce noise. However, if keepingpackagesas a placeholder for future additions is intentional (as mentioned in PR discussion), that's reasonable.♻️ Optional cleanup
# Additional packages for the dev environment - packages = with pkgs; [ - ]; - - shellHook = "";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@flake.nix` around lines 84 - 87, Remove the noisy empty declarations by deleting the empty packages = with pkgs; [ ]; and shellHook = "" entries; if you intend to keep packages as a deliberate placeholder, leave packages but replace the empty list with a short comment (e.g., packages = with pkgs; [ /* future packages */ ];) and remove shellHook instead; update the flake.nix block containing the packages and shellHook symbols accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flake.nix`:
- Around line 71-74: The postInstall step uses a glob to copy
./target/release/libsteam_api.* into $out/bin but will fail when those files are
not produced in the Nix build; update the postInstall in flake.nix to either (1)
add the Steam SDK as a buildInput so build.rs in steamworks-sys can produce
libsteam_api, or (2) make the copy conditional/error-tolerant (e.g., check for
files or redirect errors) so cp doesn't fail the build, and add a short comment
documenting that $out/bin is intentionally used for the binary's rpath; refer to
the postInstall block, the cp ./target/release/libsteam_api.* pattern, build.rs
in steamworks-sys, and $out/bin to locate where to change.
---
Nitpick comments:
In `@flake.nix`:
- Around line 84-87: Remove the noisy empty declarations by deleting the empty
packages = with pkgs; [ ]; and shellHook = "" entries; if you intend to keep
packages as a deliberate placeholder, leave packages but replace the empty list
with a short comment (e.g., packages = with pkgs; [ /* future packages */ ];)
and remove shellHook instead; update the flake.nix block containing the packages
and shellHook symbols accordingly.
unhappychoice
left a comment
unhappychoice
left a comment
There was a problem hiding this comment.
LGTM! Well-structured Nix flake using Crane + rust-overlay. All major issues from CodeRabbit reviews have been addressed (description updated, rust-src extension uncommented, formatter double-nesting fixed, crane follows nixpkgs).
Minor nitpicks (non-blocking):
pnameis computed but unused (Crane infers it automatically)- Empty
packageslist andshellHookare no-ops (template leftovers)
Thanks for the contribution!
2 participants
nix runSummary by CodeRabbit
New Features
Documentation
Chores