Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions nix-bindings-store/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,20 @@ impl Store {
r
}

#[doc(alias = "nix_store_get_version")]
pub fn get_version(&mut self) -> Result<String> {
let mut r = result_string_init!();
unsafe {
check_call!(raw::store_get_version(
&mut self.context,
self.inner.ptr(),
Some(callback_get_result_string),
callback_get_result_string_data(&mut r)
))
}?;
r
}
Comment on lines +275 to +287
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

nix_store_get_version Nix C API introduction version minimum

💡 Result:

nix_store_get_version() is part of the libstore C bindings (src/libstore/c/nix_api_store.h). In the published external C-API docs for Nix 2.17.0, nix_api_store.h exists and documents nix_store_get_version(), so the minimum Nix version you can rely on for this function is Nix 2.17.0 (2023-07-24). [1][2]

Sources:
[1] Nix 2.17.0 external C-API docs file list (shows nix_api_store.h) — https://pub.yori.cc/nix-c-api-docs/files.html
[2] nix_api_store.h / libstore docs (shows nix_store_get_version() signature) — https://pub.yori.cc/nix-c-api-docs/group__libstore.html


🏁 Script executed:

# Check the crate structure and minimum Nix version requirement
git ls-files | head -20

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 403


🏁 Script executed:

# Look for version gating patterns and Cargo.toml
fd "Cargo.toml" -x cat {}

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 9296


🏁 Script executed:

# Search for existing cfg(nix_at_least) or similar version gating
rg "nix_at_least|cfg.*nix" --type rust

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 3995


🏁 Script executed:

# Check if there are any version requirements or comments about Nix versions
rg "version|nix.*version" nix-bindings-store/src/ -i --type rust | head -20

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 852


🏁 Script executed:

# Check the exact context around get_version function
sed -n '270,290p' nix-bindings-store/src/store.rs

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 665


🏁 Script executed:

# Check the build.rs to see the full version cfg setup
cat nix-bindings-store/build.rs

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 305


🏁 Script executed:

# Look for any comments or documentation about minimum Nix version
rg "minimum.*nix|nix.*version.*require|MSRV" -i nix-bindings-store/ nix-bindings-expr/ nix-bindings-flake/

Repository: DeterminateSystems/nix-bindings-rust

Length of output: 62


Add version gate to get_version(); nix_store_get_version requires Nix 2.17.0+.

The get_version() function calls nix_store_get_version(), which was introduced in Nix 2.17.0 (2023-07-24). Without a version guard, builds against older Nix versions will fail at link time. Your build.rs currently emits cfg only for ["2.26", "2.31", "2.33.0pre", "2.33"], leaving this function unguarded. Add #[cfg(nix_at_least = "2.17")] to the function, or extend the version list in build.rs to include "2.17".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@nix-bindings-store/src/store.rs` around lines 275 - 287, The get_version()
wrapper calls raw::store_get_version which is only present in Nix >= 2.17, so
add a compile-time guard: annotate the get_version function with
#[cfg(nix_at_least = "2.17")] (and #[cfg_attr(docsrs, doc(cfg(nix_at_least =
"2.17")))] if you want docs visibility) so it is only compiled when that feature
exists; alternatively, update build.rs to emit cfg for "2.17" in addition to the
existing versions so the nix_at_least macro will enable get_version for that
release. Ensure the change references the get_version function and the
raw::store_get_version call.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The minimum version is basically 2.26, at least for the fetchers it is. It may be possible to use this with < 2.17 but afaik, it's not tested nor really supported. Most people are going to be on 2.26 or newer.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RossComputerGuy, thanks for clarifying! Since the project's minimum supported version is 2.26, I'd suggest adding #[cfg(nix_at_least = "2.26")] to get_version() for consistency with other version-specific functions like get_storedir() (line 259). This ensures the function is only available when building against supported Nix versions.

#[cfg(nix_at_least = "2.26")]
#[doc(alias = "nix_store_get_version")]
pub fn get_version(&mut self) -> Result<String> {
    // ... existing implementation
}

✏️ Learnings added
Learnt from: RossComputerGuy
Repo: DeterminateSystems/nix-bindings-rust PR: 12
File: nix-bindings-store/src/store.rs:275-287
Timestamp: 2026-02-23T23:54:54.401Z
Learning: The nix-bindings-rust project's minimum supported Nix version is 2.26, particularly for the fetchers. While some functionality may work with earlier versions, they are not tested or officially supported.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No like, I am pretty sure nix-bindings-rust won't even compile even if we do this. This function has been around for long enough that version compatibility isn't a problem.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RossComputerGuy, understood! You're right—if the project already requires Nix 2.26+ to compile and nix_store_get_version() existed well before that, then adding a version gate here is unnecessary. I appreciate the clarification. The implementation looks good as-is.


🧠 Learnings used
Learnt from: RossComputerGuy
Repo: DeterminateSystems/nix-bindings-rust PR: 12
File: nix-bindings-store/src/store.rs:275-287
Timestamp: 2026-02-23T23:54:54.401Z
Learning: Enforce and document the minimum Nix version (2.26) for fetcher-related code in this project. Ensure tests and CI assume Nix 2.26+ and clearly note compatibility in the code comments or README. If a fetcher path or related logic exists elsewhere, consider applying the same version requirement there or referencing a central compatibility document to avoid drift.


#[doc(alias = "nix_store_parse_path")]
pub fn parse_store_path(&mut self, path: &str) -> Result<StorePath> {
let path = CString::new(path)?;
Expand Down Expand Up @@ -612,6 +626,15 @@ mod tests {
}
}

#[test]
fn version() {
let mut store = crate::store::Store::open(Some("dummy://"), []).unwrap();
assert_eq!(store.get_version().unwrap(), String::new());

let mut store = crate::store::Store::open(None, []).unwrap();
assert_ne!(store.get_version().unwrap(), String::new());
}

#[test]
fn weak_ref() {
let mut store = Store::open(None, HashMap::new()).unwrap();
Expand Down
39 changes: 39 additions & 0 deletions nix-bindings-util/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use crate::raw_sys as raw;
use anyhow::Result;
use std::ffi::{c_char, CStr};

pub mod context;
pub mod settings;
#[macro_use]
Expand All @@ -6,3 +10,38 @@ pub mod nix_version;

// Re-export for use in macros
pub use nix_bindings_util_sys as raw_sys;

#[doc(alias = "nix_libutil_init")]
pub fn init() -> Result<()> {
let mut ctx = context::Context::new();
unsafe {
check_call!(raw::libutil_init(&mut ctx))?;
}
Ok(())
}

#[doc(alias = "nix_version_get")]
pub fn get_version() -> Result<&'static str> {
let c_str = unsafe {
let ptr = raw::version_get();
CStr::from_ptr(ptr as *const c_char)
};

Ok(c_str.to_str()?)
}

#[cfg(test)]
mod tests {
use super::*;
use crate::nix_version::parse_version;

#[test]
fn init() {
super::init().unwrap();
}

#[test]
fn version() {
assert!(parse_version(get_version().unwrap()) > (0, 0, 0));
}
}
7 changes: 1 addition & 6 deletions nix-bindings-util/src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,11 @@ pub fn get(key: &str) -> Result<String> {

#[cfg(test)]
mod tests {
use crate::check_call;

use super::*;

#[ctor::ctor]
fn setup() {
let mut ctx = context::Context::new();
unsafe {
check_call!(nix_bindings_util_sys::libutil_init(&mut ctx)).unwrap();
}
crate::init().unwrap();
}

#[test]
Expand Down