feat: support nix flake and minimize rust binary size#127
Open
thinkgos wants to merge 1 commit into
Open
Conversation
Reviewer's GuideAdds Nix flake-based packaging and development environment, and tightens Rust release build settings to minimize the compiled binary size. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
[profile.release]changes (especiallypanic = "abort",lto = true, andcodegen-units = 1) globally affect all release builds and can make debugging and integration with other tooling harder; consider moving these to an opt-in profile or gating them behind a Cargo feature to avoid surprising consumers. - In
flake.nix, you define bothrustToolchainand a separaterustPlatformusingrust-bin.stable.latest.minimal; you could simplify by derivingrustPlatformfromrustToolchain(or vice versa) to avoid duplicating version/toolchain configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `[profile.release]` changes (especially `panic = "abort"`, `lto = true`, and `codegen-units = 1`) globally affect all release builds and can make debugging and integration with other tooling harder; consider moving these to an opt-in profile or gating them behind a Cargo feature to avoid surprising consumers.
- In `flake.nix`, you define both `rustToolchain` and a separate `rustPlatform` using `rust-bin.stable.latest.minimal`; you could simplify by deriving `rustPlatform` from `rustToolchain` (or vice versa) to avoid duplicating version/toolchain configuration.
## Individual Comments
### Comment 1
<location path="flake.nix" line_range="62-53" />
<code_context>
+ {
+ # overlay
+ # https://flake.parts/overlays.html#consuming-an-overlay
+ _module.args.pkgs = import inputs.nixpkgs {
+ inherit system;
+ overlays = [ (import inputs.rust-overlay) ];
</code_context>
<issue_to_address>
**issue (bug_risk):** The `pkgs` used in `let` bindings likely doesn't include the `rust-overlay`, which can break `pkgs.rust-bin.*` and `makeRustPlatform`.
In this `perSystem` block, the `pkgs` in the `let` come from the un-overlaid argument and are only later replaced via `_module.args.pkgs = import inputs.nixpkgs { ... overlays = [ (import inputs.rust-overlay) ]; };`. As a result, `rustToolchain` / `rustPlatform` are likely defined from a `pkgs` without `rust-overlay`, so `pkgs.rust-bin.*` may be missing or inconsistent.
To avoid this ordering issue, import `nixpkgs` with the overlay into a dedicated binding and use it everywhere, for example:
```nix
let
pkgsWithRust = import inputs.nixpkgs {
inherit system;
overlays = [ (import inputs.rust-overlay) ];
};
rustToolchain = pkgsWithRust.rust-bin.stable.latest.default.override { ... };
rustPlatform = pkgsWithRust.makeRustPlatform { ... };
cargoToml = fromTOML (builtins.readFile ./Cargo.toml);
in {
_module.args.pkgs = pkgsWithRust;
...
}
```
This keeps the overlayed `pkgs` consistent and avoids subtle breakage if the `pkgs` argument changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I use it on Nix, but the upstream repository doesn't support it yet. I've added Nix flake support in this PR and using it in my home-manager configuration.
Additionally, minimize rust binary size.
Summary by Sourcery
Add Nix flake support and tune the Rust release build profile for smaller binaries.
New Features:
Enhancements: