Sync with upstream (2026-04-07)#18
Conversation
Flake lock file updates:
• Updated input 'flake-parts':
'github:hercules-ci/flake-parts/80daad04eddbbf5a4d883996a73f3f542fa437ac?narHash=sha256-PVvu7OqHBGWN16zSi6tEmPwwHQ4rLPU9Plvs8/1TUBY%3D' (2026-01-11)
→ 'github:hercules-ci/flake-parts/57928607ea566b5db3ad13af0e57e921e6b12381?narHash=sha256-AnYjnFWgS49RlqX7LrC4uA%2BsCCDBj0Ry/WOJ5XWAsa0%3D' (2026-02-02)
• Updated input 'flake-parts/nixpkgs-lib':
'github:nix-community/nixpkgs.lib/2075416fcb47225d9b68ac469a5c4801a9c4dd85?narHash=sha256-k00uTP4JNfmejrCLJOwdObYC9jHRrr/5M/a/8L2EIdo%3D' (2025-12-14)
→ 'github:nix-community/nixpkgs.lib/72716169fe93074c333e8d0173151350670b824c?narHash=sha256-cBEymOf4/o3FD5AZnzC3J9hLbiZ%2BQDT/KDuyHXVJOpM%3D' (2026-02-01)
• Updated input 'nix':
'github:NixOS/nix/77b6b01b727f0cd1324e431a32a8854768b957ef?narHash=sha256-vpI7XEfX5zeCVRANUzhMNsZfrMWuN0rwNenQ3z0rJNo%3D' (2026-01-30)
→ 'github:NixOS/nix/0acd0566e85e4597269482824711bcde7b518600?narHash=sha256-jJIlRLPPVYu860MVFx4gsRx3sskmLDSRWXXue5tYncw%3D' (2026-02-27)
• Updated input 'nix-cargo-integration':
'github:90-008/nix-cargo-integration/6d583e2098fa3df490c2597df06386e3efcc39b6?narHash=sha256-bjtDp0NHjLjDOjklQVHCDCVM5q39zDzuwenNri0p4Ys%3D' (2026-01-31)
→ 'github:90-008/nix-cargo-integration/c783c5dff02c06f2af6226d4dd4d494542d0a4d2?narHash=sha256-NaUqM0i6XIGdgRNxxQ9sfgCAVeE2Ko9rz7e19RsNUKw%3D' (2026-02-28)
• Updated input 'nix-cargo-integration/parts':
'github:hercules-ci/flake-parts/80daad04eddbbf5a4d883996a73f3f542fa437ac?narHash=sha256-PVvu7OqHBGWN16zSi6tEmPwwHQ4rLPU9Plvs8/1TUBY%3D' (2026-01-11)
→ 'github:hercules-ci/flake-parts/57928607ea566b5db3ad13af0e57e921e6b12381?narHash=sha256-AnYjnFWgS49RlqX7LrC4uA%2BsCCDBj0Ry/WOJ5XWAsa0%3D' (2026-02-02)
• Updated input 'nix-cargo-integration/rust-overlay':
'github:oxalica/rust-overlay/a1d32c90c8a4ea43e9586b7e5894c179d5747425?narHash=sha256-zmnvRUm15QrlKH0V1BZoiT3U%2BQ%2Btr%2BP5Osi8qgtL9fY%3D' (2026-01-31)
→ 'github:oxalica/rust-overlay/a1ab5e89ab12e1a37c0b264af6386a7472d68a15?narHash=sha256-x6IFQ9bL7YYfW2m2z8D3Em2YtAA3HE8kiCFwai2fwrw%3D' (2026-02-28)
• Updated input 'nix-cargo-integration/treefmt':
'github:numtide/treefmt-nix/28b19c5844cc6e2257801d43f2772a4b4c050a1b?narHash=sha256-8aAYwyVzSSwIhP2glDhw/G0i5%2BwOrren3v6WmxkVonM%3D' (2026-01-29)
→ 'github:numtide/treefmt-nix/337a4fe074be1042a35086f15481d763b8ddc0e7?narHash=sha256-wQ6NJSuFqAEmIg2VMnLdCnUc0b7vslUohqqGGD%2BFyxk%3D' (2026-02-04)
• Updated input 'nixpkgs':
'github:NixOS/nixpkgs/bfc1b8a4574108ceef22f02bafcf6611380c100d?narHash=sha256-msG8SU5WsBUfVVa/9RPLaymvi5bI8edTavbIq3vRlhI%3D' (2026-01-26)
→ 'github:NixOS/nixpkgs/dd9b079222d43e1943b6ebd802f04fd959dc8e61?narHash=sha256-I45esRSssFtJ8p/gLHUZ1OUaaTaVLluNkABkk6arQwE%3D' (2026-02-27)
Flake lock file updates:
• Updated input 'hercules-ci-effects':
'github:hercules-ci/hercules-ci-effects/c19e263e6e22ec7379d972f19e6a322f943c73fb?narHash=sha256-V0YOJRum50gtKgwavsAfwXc9%2BXAsJCC7386YZx1sWGQ%3D' (2026-01-15)
→ 'github:hercules-ci/hercules-ci-effects/0b152e0f7c5cc265a529cd63374b80e2771b207b?narHash=sha256-HPBNYf7HiKtBVy7/69vKpLYHX6wTcUxndxmybzDlXP8%3D' (2026-02-15)
• Updated input 'hercules-ci-effects/flake-parts':
'github:hercules-ci/flake-parts/80daad04eddbbf5a4d883996a73f3f542fa437ac?narHash=sha256-PVvu7OqHBGWN16zSi6tEmPwwHQ4rLPU9Plvs8/1TUBY%3D' (2026-01-11)
→ 'github:hercules-ci/flake-parts/57928607ea566b5db3ad13af0e57e921e6b12381?narHash=sha256-AnYjnFWgS49RlqX7LrC4uA%2BsCCDBj0Ry/WOJ5XWAsa0%3D' (2026-02-02)
• Updated input 'hercules-ci-effects/nixpkgs':
'github:NixOS/nixpkgs/1412caf7bf9e660f2f962917c14b1ea1c3bc695e?narHash=sha256-AIdl6WAn9aymeaH/NvBj0H9qM%2BXuAuYbGMZaP0zcXAQ%3D' (2026-01-13)
→ 'github:NixOS/nixpkgs/a82ccc39b39b621151d6732718e3e250109076fa?narHash=sha256-gf2AmWVTs8lEq7z/3ZAsgnZDhWIckkb%2BZnAo5RzSxJg%3D' (2026-02-13)
• Updated input 'pre-commit-hooks-nix':
'github:cachix/pre-commit-hooks.nix/a1ef738813b15cf8ec759bdff5761b027e3e1d23?narHash=sha256-Efs3VUPelRduf3PpfPP2ovEB4CXT7vHf8W%2Bxc49RL/U%3D' (2026-01-22)
→ 'github:cachix/pre-commit-hooks.nix/6e34e97ed9788b17796ee43ccdbaf871a5c2b476?narHash=sha256-%2BeXlIc4/7dE6EcPs9a2DaSY3fTA9AE526hGqkNID3Wg%3D' (2026-02-25)
• Updated input 'treefmt-nix':
'github:numtide/treefmt-nix/28b19c5844cc6e2257801d43f2772a4b4c050a1b?narHash=sha256-8aAYwyVzSSwIhP2glDhw/G0i5%2BwOrren3v6WmxkVonM%3D' (2026-01-29)
→ 'github:numtide/treefmt-nix/337a4fe074be1042a35086f15481d763b8ddc0e7?narHash=sha256-wQ6NJSuFqAEmIg2VMnLdCnUc0b7vslUohqqGGD%2BFyxk%3D' (2026-02-04)
…tem builds Nix changed "required system or feature not available" to "platform mismatch".
… errors Nix 2.34 memoizes primop errors by default. RecoverableError uses NIX_ERR_RECOVERABLE so transient errors allow the thunk to be retried.
`flake.lock`: Update
📝 WalkthroughWalkthroughThis PR introduces support for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 `@nix-bindings-expr/src/eval_state.rs`:
- Around line 2947-2977: The test eval_state_primop_recoverable_error_in_chain
currently returns Err(primop::RecoverableError::new(...).into()) which produces
a top-level error without a source chain; update the closure passed to
new_value_thunk to return a wrapper error type that implements std::error::Error
with source() returning the primop::RecoverableError (or otherwise use
anyhow::Error::context to attach the RecoverableError as a source), then return
that wrapper converted into the expected Error (i.e. wrapper.into()) so the code
path that searches the error chain will find RecoverableError; locate the
closure in new_value_thunk inside eval_state_primop_recoverable_error_in_chain
and replace the direct conversion with the chained-wrapper conversion while
keeping the second branch returning es.new_value_int(42).
In `@nix-bindings-expr/src/primop.rs`:
- Around line 172-179: The current error handling only calls error_code(e) on
the top-level error, missing wrapped RecoverableError instances; update the
logic in the catch arm that builds err_str/err_code (the closure using err_str,
err_code, CString::new, and raw_util::set_err_msg) to walk the error chain
(using e.source() / std::error::Error::chain or similar) and call error_code on
each cause until a non-unknown code (or a RecoverableError) is found, then use
that code for raw_util::set_err_msg; alternatively refactor/extend error_code to
inspect the full error chain and return the first matching code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2b7f940-ab10-439d-8aab-371b8565f32c
⛔ Files ignored due to path filters (2)
dev/flake.lockis excluded by!**/*.lockflake.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
CHANGELOG.mdnix-bindings-expr/build.rsnix-bindings-expr/src/eval_state.rsnix-bindings-expr/src/primop.rsnix-bindings-store/src/store.rs
| fn eval_state_primop_recoverable_error_in_chain() { | ||
| gc_registering_current_thread(|| { | ||
| let store = Store::open(None, []).unwrap(); | ||
| let mut es = EvalState::new(store, []).unwrap(); | ||
|
|
||
| let call_count = std::cell::Cell::new(0u32); | ||
| let v = es | ||
| .new_value_thunk( | ||
| "recoverable_chain_test", | ||
| Box::new(move |es: &mut EvalState| { | ||
| let count = call_count.get(); | ||
| call_count.set(count + 1); | ||
| if count == 0 { | ||
| // Wrap RecoverableError in .context(), pushing it down the chain | ||
| Err(primop::RecoverableError::new("transient failure").into()) | ||
| } else { | ||
| es.new_value_int(42) | ||
| } | ||
| }), | ||
| ) | ||
| .unwrap(); | ||
|
|
||
| // First force should fail | ||
| let r = es.force(&v); | ||
| assert!(r.is_err()); | ||
|
|
||
| // Second force should succeed if RecoverableError is found in the chain | ||
| es.force(&v).unwrap(); | ||
| let i = es.require_int(&v).unwrap(); | ||
| assert_eq!(i, 42); | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
git ls-files | grep -E "eval_state\.rs$"Repository: DeterminateSystems/nix-bindings-rust
Length of output: 115
🏁 Script executed:
wc -l nix-bindings-expr/src/eval_state.rsRepository: DeterminateSystems/nix-bindings-rust
Length of output: 120
🏁 Script executed:
sed -n '2940,2980p' nix-bindings-expr/src/eval_state.rsRepository: DeterminateSystems/nix-bindings-rust
Length of output: 1485
🏁 Script executed:
# Let's also search for the RecoverableError definition to understand its structure
rg "struct RecoverableError|impl.*RecoverableError" --type rust -A 10Repository: DeterminateSystems/nix-bindings-rust
Length of output: 1659
🏁 Script executed:
# Let's look for the test function names to ensure we're looking at the right code
rg "fn eval_state_primop_recoverable" nix-bindings-expr/src/eval_state.rs -A 2Repository: DeterminateSystems/nix-bindings-rust
Length of output: 383
Test doesn't actually verify error chain behavior.
The closure returns Err(primop::RecoverableError::new(...).into()), which is the same direct conversion as the preceding test. Since RecoverableError implements Error with no source() method, this produces a top-level error with no chain—not the error-chain scenario the test name suggests.
To match the test's intent, introduce a wrapper error that chains RecoverableError via source():
Suggested test adjustment
#[test]
#[cfg(nix_at_least = "2.34.0pre")]
fn eval_state_primop_recoverable_error_in_chain() {
gc_registering_current_thread(|| {
+ #[derive(Debug)]
+ struct WrappedRecoverable(primop::RecoverableError);
+ impl std::fmt::Display for WrappedRecoverable {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ write!(f, "wrapped recoverable")
+ }
+ }
+ impl std::error::Error for WrappedRecoverable {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+ Some(&self.0)
+ }
+ }
+
let store = Store::open(None, []).unwrap();
let mut es = EvalState::new(store, []).unwrap();
@@
call_count.set(count + 1);
if count == 0 {
- // Wrap RecoverableError in .context(), pushing it down the chain
- Err(primop::RecoverableError::new("transient failure").into())
+ Err(Box::new(WrappedRecoverable(
+ primop::RecoverableError::new("transient failure"),
+ )))
} else {
es.new_value_int(42)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nix-bindings-expr/src/eval_state.rs` around lines 2947 - 2977, The test
eval_state_primop_recoverable_error_in_chain currently returns
Err(primop::RecoverableError::new(...).into()) which produces a top-level error
without a source chain; update the closure passed to new_value_thunk to return a
wrapper error type that implements std::error::Error with source() returning the
primop::RecoverableError (or otherwise use anyhow::Error::context to attach the
RecoverableError as a source), then return that wrapper converted into the
expected Error (i.e. wrapper.into()) so the code path that searches the error
chain will find RecoverableError; locate the closure in new_value_thunk inside
eval_state_primop_recoverable_error_in_chain and replace the direct conversion
with the chained-wrapper conversion while keeping the second branch returning
es.new_value_int(42).
| let err_str = e.to_string(); | ||
| let err_code = error_code(e); | ||
| let cstr = CString::new(err_str).unwrap_or_else(|_e| { | ||
| CString::new("<rust nix-expr application error message contained null byte>") | ||
| .unwrap() | ||
| }); | ||
| raw_util::set_err_msg(context_out, raw_util::err_NIX_ERR_UNKNOWN, cstr.as_ptr()); | ||
| raw_util::set_err_msg(context_out, err_code, cstr.as_ptr()); | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current implementation only checks top-level error type and does not walk source() chain.
rg -n -C3 'fn error_code|downcast_ref::<RecoverableError>|source\(' nix-bindings-expr/src/primop.rsRepository: DeterminateSystems/nix-bindings-rust
Length of output: 415
🏁 Script executed:
#!/bin/bash
# 1. Search for RecoverableError definition and usage
rg -n 'RecoverableError' nix-bindings-expr/src/ -B2 -A5Repository: DeterminateSystems/nix-bindings-rust
Length of output: 5570
🏁 Script executed:
#!/bin/bash
# 2. Check how errors are created/wrapped in function_adapter
rg -n 'function_adapter|Box::new.*Error|anyhow::anyhow' nix-bindings-expr/src/primop.rs -B3 -A3Repository: DeterminateSystems/nix-bindings-rust
Length of output: 426
🏁 Script executed:
#!/bin/bash
# 3. Verify Error::source() method and error chain patterns
rg -n 'Error|source|chain' nix-bindings-expr/src/primop.rs | head -30Repository: DeterminateSystems/nix-bindings-rust
Length of output: 1008
🏁 Script executed:
#!/bin/bash
# 4. Check the full error_code function and surrounding context
sed -n '170,195p' nix-bindings-expr/src/primop.rsRepository: DeterminateSystems/nix-bindings-rust
Length of output: 866
🏁 Script executed:
#!/bin/bash
# Check if Error::source() is used anywhere in the codebase
rg -n 'source\(\)' nix-bindings-expr/src/Repository: DeterminateSystems/nix-bindings-rust
Length of output: 62
🏁 Script executed:
#!/bin/bash
# Look at the test more carefully - specifically the wrapped error case
sed -n '2950,2980p' nix-bindings-expr/src/eval_state.rsRepository: DeterminateSystems/nix-bindings-rust
Length of output: 1211
🏁 Script executed:
#!/bin/bash
# Check what the .context() method does and what error type chain it creates
rg -n 'use.*anyhow|use.*error|Context' nix-bindings-expr/src/primop.rs | head -20Repository: DeterminateSystems/nix-bindings-rust
Length of output: 344
🏁 Script executed:
#!/bin/bash
# Check if anyhow is being used and how errors are wrapped
rg -n 'anyhow|chain|context|source' nix-bindings-expr/src/ -i | head -30Repository: DeterminateSystems/nix-bindings-rust
Length of output: 2952
🏁 Script executed:
#!/bin/bash
# Check the imports and dependencies
cat nix-bindings-expr/Cargo.toml | grep -A 20 '\[dependencies\]'Repository: DeterminateSystems/nix-bindings-rust
Length of output: 823
🏁 Script executed:
#!/bin/bash
# Look at actual primop implementations to see error patterns
rg -n 'Err\(' nix-bindings-expr/src/ -B1 -A1 | head -50Repository: DeterminateSystems/nix-bindings-rust
Length of output: 3248
Recoverable-error detection currently misses wrapped errors.
error_code() only checks the top-level error (Line 186), so recoverable errors wrapped in another error type won't be detected and will be reported as err_NIX_ERR_UNKNOWN. This breaks the documented "error chain" behavior and the test expectations for retries on Nix ≥ 2.34.
The test at eval_state.rs:2973 explicitly expects this behavior: "Second force should succeed if RecoverableError is found in the chain," but the current implementation cannot detect wrapped errors.
💡 Proposed fix
- let err_str = e.to_string();
- let err_code = error_code(e);
+ let err_code = error_code(e.as_ref());
+ let err_str = e.to_string();
let cstr = CString::new(err_str).unwrap_or_else(|_e| {
CString::new("<rust nix-expr application error message contained null byte>")
.unwrap()
});
raw_util::set_err_msg(context_out, err_code, cstr.as_ptr());
@@
#[cfg_attr(not(nix_at_least = "2.34.0pre"), allow(unused))]
-fn error_code(e: Box<dyn Error>) -> raw_util::err {
+fn error_code(e: &(dyn Error + 'static)) -> raw_util::err {
#[cfg(nix_at_least = "2.34.0pre")]
- if e.downcast_ref::<RecoverableError>().is_some() {
- return raw_util::err_NIX_ERR_RECOVERABLE;
+ {
+ if e.downcast_ref::<RecoverableError>().is_some() {
+ return raw_util::err_NIX_ERR_RECOVERABLE;
+ }
+ let mut source = e.source();
+ while let Some(err) = source {
+ if err.downcast_ref::<RecoverableError>().is_some() {
+ return raw_util::err_NIX_ERR_RECOVERABLE;
+ }
+ source = err.source();
+ }
}
raw_util::err_NIX_ERR_UNKNOWN
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nix-bindings-expr/src/primop.rs` around lines 172 - 179, The current error
handling only calls error_code(e) on the top-level error, missing wrapped
RecoverableError instances; update the logic in the catch arm that builds
err_str/err_code (the closure using err_str, err_code, CString::new, and
raw_util::set_err_msg) to walk the error chain (using e.source() /
std::error::Error::chain or similar) and call error_code on each cause until a
non-unknown code (or a RecoverableError) is found, then use that code for
raw_util::set_err_msg; alternatively refactor/extend error_code to inspect the
full error chain and return the first matching code.
Naxdy
left a comment
There was a problem hiding this comment.
I pushed a commit that moves the RecoverableError into our Error enum. If there's no issues with it, feel free to merge. The rest LGTM.
|
continued in #19 because GitHub is funny |
Summary by CodeRabbit
New Features
RecoverableErrortype for primop operations, enabling retry on next force evaluation instead of memoizing errors.Tests