Add flags to tune cores & jobs in build#42
Conversation
📝 WalkthroughWalkthroughAdds two optional CLI options ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
RossComputerGuy
temporarily deployed
to
release
GitHub Actions
Inactive
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cmd/build.rs`:
- Around line 99-118: The verbose flag is ineffective because nix_command
currently runs via .output() which captures stdout/stderr so --print-build-logs
is discarded; update the call site in build.rs where args is built (variables:
cores, max_jobs, args, drv, verbose) to call the variant of nix_command that
streams or pipes output to the terminal when verbose is true (use the "piped" or
"spawn/stream" variant) and retain the current captured/no-output variant when
verbose is false, ensuring nix_command is invoked accordingly and error
handling/wrap_err("failed to build derivation") remains intact.
- Around line 26-32: The flags build_jobs and build_cores currently have
defaults that always pass --max-jobs/--cores and override nix.conf; change their
types to Option<usize> (update the struct fields build_jobs and build_cores and
their CLI arg definitions to be optional—remove default_value) and only push
"--max-jobs" and "--cores" into the nix args when the Option is Some (e.g., if
let Some(max_jobs) = self.build_jobs { ... } and if let Some(cores) =
self.build_cores { ... }). Also switch the invocation that currently uses
nix_command (which calls .output()) to the variant that inherits stderr
(nix_command_pipe_with_output or equivalent) when handling the
--print-build-logs path so build logs are streamed to the console instead of
being captured.
src/cmd/build.rs
Outdated
| /// Number of concurrent Nix builds | ||
| #[arg(short = 'j', long, default_value = "1", env = "FLAKE_ITER_BUILD_JOBS")] | ||
| build_jobs: usize, | ||
|
|
||
| /// Number of cores to utilize per job | ||
| #[arg(short = 'c', long, default_value = "0", env = "FLAKE_ITER_BUILD_CORES")] | ||
| build_cores: usize, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /repo && find . -name "build.rs" -type f | head -20Repository: DeterminateSystems/flake-iter
Length of output: 128
🏁 Script executed:
cd /repo && cat -n src/cmd/build.rs | head -50Repository: DeterminateSystems/flake-iter
Length of output: 128
🏁 Script executed:
cd /repo && cat -n src/cmd/build.rs | sed -n '20,40p'Repository: DeterminateSystems/flake-iter
Length of output: 128
🏁 Script executed:
cd /repo && cat -n src/cmd/build.rs | sed -n '95,125p'Repository: DeterminateSystems/flake-iter
Length of output: 128
🌐 Web query:
Nix build --max-jobs -j flag semantics default value auto
💡 Result:
nix build -j N/--max-jobs Nsets the maximum number of derivations (build jobs) Nix will build in parallel on the local machine. (releases.nixos.org)-j automeans use the number of CPUs on the system. (releases.nixos.org)- If you don’t pass
-j/--max-jobs, the value comes from themax-jobsconfiguration setting (same name innix.conf, and every config setting can also be set via a corresponding CLI flag). (releases.nixos.org) - The
max-jobssetting defaults to1if not configured. (releases.nixos.org) -j 0means do not build locally (useful if you want builds to happen only on remote builders). (releases.nixos.org)
Citations:
- 1: https://releases.nixos.org/nix/nix-2.24.0/manual/command-ref/nix-build.html?utm_source=openai
- 2: https://releases.nixos.org/nix/nix-2.24.0/manual/command-ref/nix-build.html?utm_source=openai
- 3: https://releases.nixos.org/nix/nix-2.24.0/manual/command-ref/nix-build.html?utm_source=openai
- 4: https://releases.nixos.org/nix/nix-2.24.0/manual/command-ref/nix-build.html?utm_source=openai
- 5: https://releases.nixos.org/nix/nix-2.24.0/manual/command-ref/nix-build.html?utm_source=openai
🌐 Web query:
Nix build --cores flag default semantics meaning zero
💡 Result:
nix build --cores N controls per-derivation build parallelism by setting the environment variable NIX_BUILD_CORES for builders (e.g., used like make -jN when a package enables parallel building). It is not the number of simultaneous derivations (that’s --max-jobs / -j). [1][2]
Default: --cores defaults to the Nix config setting cores (aka nix.settings.cores on NixOS); if that isn’t set, it falls back to 1. [1]
Meaning of 0: --cores 0 means “use all available CPU cores on the system” (i.e., NIX_BUILD_CORES becomes the machine’s core count). [1][2]
Note (newer Nix): as of Nix 2.31, when the config build-cores is set to 0, Nix auto-detects cores and passes the detected number via NIX_BUILD_CORES (instead of passing literal 0). [3]
Sources:
[1] Nix Reference Manual (nix-build / --cores) https://releases.nixos.org/nix/nix-2.24.0/manual/command-ref/nix-build.html
[2] Nix Reference Manual (“Tuning Cores and Jobs”) https://releases.nixos.org/nix/nix-2.28.1/manual/advanced-topics/cores-vs-jobs.html
[3] Nix 2.31 release notes (Determinate Systems manual) https://manual.determinate.systems/release-notes/rl-2.31.html
🏁 Script executed:
wc -l src/cmd/build.rsRepository: DeterminateSystems/flake-iter
Length of output: 93
🏁 Script executed:
cat -n src/cmd/build.rs | sed -n '1,50p'Repository: DeterminateSystems/flake-iter
Length of output: 2086
🏁 Script executed:
cat -n src/cmd/build.rs | sed -n '20,45p'Repository: DeterminateSystems/flake-iter
Length of output: 1196
🏁 Script executed:
cat -n src/cmd/build.rs | sed -n '90,125p'Repository: DeterminateSystems/flake-iter
Length of output: 1492
🏁 Script executed:
cat -n src/cmd/mod.rs | head -100Repository: DeterminateSystems/flake-iter
Length of output: 3393
🏁 Script executed:
grep -n "fn nix_command" src/cmd/mod.rsRepository: DeterminateSystems/flake-iter
Length of output: 393
🏁 Script executed:
grep -A 20 "fn nix_command\(" src/cmd/mod.rsRepository: DeterminateSystems/flake-iter
Length of output: 96
🏁 Script executed:
sed -n '210,235p' src/cmd/mod.rsRepository: DeterminateSystems/flake-iter
Length of output: 819
🏁 Script executed:
sed -n '223,250p' src/cmd/mod.rsRepository: DeterminateSystems/flake-iter
Length of output: 955
Address the verbose build log visibility issue and clarify Nix flag behavior.
The --max-jobs 1 default matches Nix's own configuration default, but always passing it overrides a user's nix.conf if they've set a higher value. The --cores 0 flag has special semantics in Nix—it instructs the build system to use all available CPU cores—rather than being an "unset" behavior. However, both flags will suppress user configuration in nix.conf when not explicitly intended.
More critically, the --print-build-logs flag (line 112) won't display build logs to users because nix_command captures all output via .output() (line 212 in src/cmd/mod.rs). To fix this, use nix_command_pipe_with_output or similar that inherits stderr, so build logs appear on the console.
Consider making build_jobs and build_cores optional (Option<usize>) to preserve user settings from nix.conf when not explicitly specified:
♻️ Suggested refactor for optional flags
- #[arg(short = 'j', long, default_value = "1", env = "FLAKE_ITER_BUILD_JOBS")]
- build_jobs: usize,
+ #[arg(short = 'j', long, env = "FLAKE_ITER_BUILD_JOBS")]
+ build_jobs: Option<usize>,
- #[arg(short = 'c', long, default_value = "0", env = "FLAKE_ITER_BUILD_CORES")]
- build_cores: usize,
+ #[arg(short = 'c', long, env = "FLAKE_ITER_BUILD_CORES")]
+ build_cores: Option<usize>,Then only append flags when explicitly set:
if let Some(cores) = self.build_cores {
args.extend(["--cores", &cores.to_string()]);
}
if let Some(max_jobs) = self.build_jobs {
args.extend(["--max-jobs", &max_jobs.to_string()]);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cmd/build.rs` around lines 26 - 32, The flags build_jobs and build_cores
currently have defaults that always pass --max-jobs/--cores and override
nix.conf; change their types to Option<usize> (update the struct fields
build_jobs and build_cores and their CLI arg definitions to be optional—remove
default_value) and only push "--max-jobs" and "--cores" into the nix args when
the Option is Some (e.g., if let Some(max_jobs) = self.build_jobs { ... } and if
let Some(cores) = self.build_cores { ... }). Also switch the invocation that
currently uses nix_command (which calls .output()) to the variant that inherits
stderr (nix_command_pipe_with_output or equivalent) when handling the
--print-build-logs path so build logs are streamed to the console instead of
being captured.
15a69cf to
409fe8b
Compare
RossComputerGuy
temporarily deployed
to
release
GitHub Actions
Inactive
Flake lock file updates:
• Updated input 'fenix':
'https://api.flakehub.com/f/pinned/nix-community/fenix/0.1.2471%2Brev-df3c2e78ec13418f85c1f26e77a50f865ec57d38/019ad905-65fc-7ba6-8d92-a0b2cf1c6ea6/source.tar.gz' (2025-12-01)
→ 'https://api.flakehub.com/f/pinned/nix-community/fenix/0.1.2542%2Brev-78d518f5ca32a86dc767de481160dbae640c70cf/019c1851-0ed1-7f81-b175-8e8bbad651e6/source.tar.gz' (2026-02-01)
• Updated input 'fenix/rust-analyzer-src':
'github:rust-lang/rust-analyzer/d646b23' (2025-11-30)
→ 'github:rust-lang/rust-analyzer/17304e9' (2026-01-31)
• Updated input 'nixpkgs':
'https://api.flakehub.com/f/pinned/NixOS/nixpkgs/0.2511.903561%2Brev-c8cfcd6ccd422e41cc631a0b73ed4d5a925c393d/019b231b-8461-7fef-af86-fa7662ea16ad/source.tar.gz' (2025-12-15)
→ 'https://api.flakehub.com/f/pinned/NixOS/nixpkgs/0.2511.908322%2Brev-e764fc9a405871f1f6ca3d1394fb422e0a0c3951/019c90c7-b2fa-759d-838f-611ca6eed88b/source.tar.gz' (2026-02-24)
RossComputerGuy
force-pushed
the
RossComputerGuy/build-tuning
branch
from
409fe8b to
abb12e7
Compare
RossComputerGuy
temporarily deployed
to
release
GitHub Actions
Inactive
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cmd/build.rs (1)
99-120: Consider pre-building the static arg prefix outside the loop.
build_cores,build_jobs, andverboseare loop-invariant; onlydrvchanges. Recomputing the conditional prefix every iteration adds noise without benefit. Hoisting it out and cloning per iteration is cleaner. Theargsshadow on line 120 also goes away.♻️ Proposed refactor
+ let mut base_args: Vec<String> = + vec![String::from("build"), String::from("--print-out-paths")]; + + if let Some(cores) = &self.build_cores { + base_args.push(String::from("--cores")); + base_args.push(cores.to_string()); + } + + if let Some(max_jobs) = &self.build_jobs { + base_args.push(String::from("--max-jobs")); + base_args.push(max_jobs.to_string()); + } + + if verbose { + base_args.push(String::from("--print-build-logs")); + } + for (n, (drv, (outputs, output_paths))) in derivations.into_iter().enumerate() { let drv = format!("{}^{}", drv.display(), outputs.join(",")); // ... cache-check block unchanged ... - let mut args: Vec<String> = - vec![String::from("build"), String::from("--print-out-paths")]; - - if let Some(cores) = &self.build_cores { - let cores = cores.to_string(); - args.push(String::from("--cores")); - args.push(cores); - } - - if let Some(max_jobs) = &self.build_jobs { - let max_jobs = max_jobs.to_string(); - args.push(String::from("--max-jobs")); - args.push(max_jobs); - } - - if verbose { - args.push(String::from("--print-build-logs")); - } - - args.push(drv); - - let args: Vec<&str> = args.iter().map(String::as_str).collect(); + let mut args = base_args.clone(); + args.push(drv); + let arg_strs: Vec<&str> = args.iter().map(String::as_str).collect(); info!("Building derivation {} of {num}", n + 1); if verbose { - nix_command_pipe_no_output(&args).wrap_err("failed to build derivation")?; + nix_command_pipe_no_output(&arg_strs).wrap_err("failed to build derivation")?; } else { - nix_command(&args).wrap_err("failed to build derivation")?; + nix_command(&arg_strs).wrap_err("failed to build derivation")?; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cmd/build.rs` around lines 99 - 120, Hoist construction of the loop-invariant prefix for the build command out of the loop: create a base Vec<String> (e.g. base_args) that contains "build", "--print-out-paths" and conditionally the "--cores"/build_cores, "--max-jobs"/build_jobs and "--print-build-logs" when verbose is true; then inside the loop clone base_args to a per-iteration args vector, push the changing drv value, and only then map to Vec<&str> if needed—this removes repeated work and the args shadowing around the current args variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cmd/build.rs`:
- Around line 99-120: Hoist construction of the loop-invariant prefix for the
build command out of the loop: create a base Vec<String> (e.g. base_args) that
contains "build", "--print-out-paths" and conditionally the
"--cores"/build_cores, "--max-jobs"/build_jobs and "--print-build-logs" when
verbose is true; then inside the loop clone base_args to a per-iteration args
vector, push the changing drv value, and only then map to Vec<&str> if
needed—this removes repeated work and the args shadowing around the current args
variable.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
src/cmd/build.rs
2 participants
Adds the ability to tune the build's cores & jobs. This is useful so runners with limited resources can be tuned, also helps prevent flakiness for some builds.
Summary by CodeRabbit
New Features
Improvements