From d8ed5dc6ba2b073b978e0b38cb5964a3445dc724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20de=20Kok?= Date: Fri, 28 Aug 2026 08:01:16 +0000 Subject: [PATCH 1/4] kernels-data: make the version length a type parameter `Version` stored versions in a dynamic-length `Vec` and would trim any trailing zeros (e.g. `13.0` -> `13`). This leads to issues when we re-serialize `build.toml` files where we actually meant `13.0` (e.g. for Nix version comparisons, `13.0` and `13` are different). We could simply remove the zero-trimming logic, but this also does not seem correct, because a user could e.g. use 13.0.0 for a CUDA version, but this breaks some Nix logic as well and that is not the grannularity we want to use for e.g. CUDA and Torch versions. So instead, add the version length as a type parameter, e.g. `Version<2>`. This ensures that we are always serializing correctly and catches erroneous version use by the user (e.g `13` or `13.0.0` when `N=2`). Also add some tests. --- kernels-data/src/config/mod.rs | 16 +++--- kernels-data/src/config/v2.rs | 10 ++-- kernels-data/src/config/v3.rs | 10 ++-- kernels-data/src/config/v4.rs | 12 ++-- kernels-data/src/config/v5.rs | 10 ++-- kernels-data/src/version.rs | 102 +++++++++++++++++++++++++-------- 6 files changed, 108 insertions(+), 52 deletions(-) diff --git a/kernels-data/src/config/mod.rs b/kernels-data/src/config/mod.rs index b4b0265fe..80774aa58 100644 --- a/kernels-data/src/config/mod.rs +++ b/kernels-data/src/config/mod.rs @@ -209,8 +209,8 @@ impl General { } pub struct CudaGeneral { - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub kernel_depends: Option>, pub python_depends: Option>, } @@ -236,8 +236,8 @@ pub struct Hub { pub struct Torch { pub include: Option>, - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub pyext: Option>, pub src: Vec, pub stable_abi: Option, @@ -249,14 +249,14 @@ pub struct Torch { #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(untagged)] pub enum TorchAbi { - All(Version), - Mapping(BTreeMap), + All(Version<2>), + Mapping(BTreeMap>), } impl TorchAbi { /// Stable ABI version to target for `backend`, or `None` if it should be /// built without the stable ABI. - pub fn for_backend(&self, backend: Backend) -> Option<&Version> { + pub fn for_backend(&self, backend: Backend) -> Option<&Version<2>> { match self { TorchAbi::All(version) => Some(version), TorchAbi::Mapping(mapping) => mapping.get(&backend), @@ -327,7 +327,7 @@ pub enum Kernel { Cuda { cuda_capabilities: Option>, cuda_flags: Option>, - cuda_minver: Option, + cuda_minver: Option>, cxx_flags: Option>, depends: Vec, include: Option>, diff --git a/kernels-data/src/config/v2.rs b/kernels-data/src/config/v2.rs index f79204a8b..345369ba7 100644 --- a/kernels-data/src/config/v2.rs +++ b/kernels-data/src/config/v2.rs @@ -27,9 +27,9 @@ pub struct General { #[serde(default)] pub universal: bool, - pub cuda_maxver: Option, + pub cuda_maxver: Option>, - pub cuda_minver: Option, + pub cuda_minver: Option>, pub hub: Option, @@ -63,8 +63,8 @@ impl Display for PythonDependency { #[serde(deny_unknown_fields)] pub struct Torch { pub include: Option>, - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub pyext: Option>, #[serde(default)] @@ -85,7 +85,7 @@ pub enum Kernel { Cuda { cuda_capabilities: Option>, cuda_flags: Option>, - cuda_minver: Option, + cuda_minver: Option>, cxx_flags: Option>, depends: Vec, include: Option>, diff --git a/kernels-data/src/config/v3.rs b/kernels-data/src/config/v3.rs index a77fe7bd1..0672b4b96 100644 --- a/kernels-data/src/config/v3.rs +++ b/kernels-data/src/config/v3.rs @@ -56,8 +56,8 @@ pub struct General { #[derive(Debug, Deserialize, Serialize)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct CudaGeneral { - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub python_depends: Option>, } @@ -84,8 +84,8 @@ pub struct Hub { #[serde(deny_unknown_fields)] pub struct Torch { pub include: Option>, - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub pyext: Option>, #[serde(default)] @@ -114,7 +114,7 @@ pub enum Kernel { Cuda { cuda_capabilities: Option>, cuda_flags: Option>, - cuda_minver: Option, + cuda_minver: Option>, cxx_flags: Option>, depends: Vec, include: Option>, diff --git a/kernels-data/src/config/v4.rs b/kernels-data/src/config/v4.rs index 418e9add8..08b8f7bf5 100644 --- a/kernels-data/src/config/v4.rs +++ b/kernels-data/src/config/v4.rs @@ -55,8 +55,8 @@ pub struct General { #[derive(Debug, Deserialize, Serialize)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct CudaGeneral { - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub python_depends: Option>, } @@ -83,14 +83,14 @@ pub struct Hub { #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct Torch { pub include: Option>, - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub pyext: Option>, #[serde(default)] pub src: Vec, - pub stable_abi: Option, + pub stable_abi: Option>, pub cxx_flags: Option>, } @@ -130,7 +130,7 @@ pub enum Kernel { Cuda { cuda_capabilities: Option>, cuda_flags: Option>, - cuda_minver: Option, + cuda_minver: Option>, cxx_flags: Option>, depends: Vec, include: Option>, diff --git a/kernels-data/src/config/v5.rs b/kernels-data/src/config/v5.rs index 314289a85..dc1078ee2 100644 --- a/kernels-data/src/config/v5.rs +++ b/kernels-data/src/config/v5.rs @@ -72,8 +72,8 @@ pub struct General { #[derive(Debug, Deserialize, Serialize)] #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct CudaGeneral { - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub kernel_depends: Option>, pub python_depends: Option>, } @@ -109,8 +109,8 @@ pub struct Hub { #[serde(deny_unknown_fields, rename_all = "kebab-case")] pub struct Torch { pub include: Option>, - pub minver: Option, - pub maxver: Option, + pub minver: Option>, + pub maxver: Option>, pub pyext: Option>, #[serde(default)] @@ -157,7 +157,7 @@ pub enum Kernel { Cuda { cuda_capabilities: Option>, cuda_flags: Option>, - cuda_minver: Option, + cuda_minver: Option>, cxx_flags: Option>, depends: Vec, include: Option>, diff --git a/kernels-data/src/version.rs b/kernels-data/src/version.rs index e3ccac7f3..b4da26046 100644 --- a/kernels-data/src/version.rs +++ b/kernels-data/src/version.rs @@ -6,9 +6,9 @@ use itertools::Itertools; use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] -pub struct Version(Vec); +pub struct Version([usize; N]); -impl<'de> Deserialize<'de> for Version { +impl<'de, const N: usize> Deserialize<'de> for Version { fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, @@ -18,26 +18,22 @@ impl<'de> Deserialize<'de> for Version { } } -impl Serialize for Version { +impl Serialize for Version { fn serialize(&self, serializer: S) -> Result where S: Serializer, { - serializer.serialize_str(&self.0.iter().map(|v| v.to_string()).join(".")) + serializer.collect_str(self) } } -impl Display for Version { +impl Display for Version { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{}", - itertools::join(self.0.iter().map(|v| v.to_string()), ".") - ) + write!(f, "{}", self.0.iter().map(|v| v.to_string()).join(".")) } } -impl Deref for Version { +impl Deref for Version { type Target = [usize]; #[inline] @@ -46,24 +42,27 @@ impl Deref for Version { } } -impl From> for Version { - fn from(value: Vec) -> Self { +impl TryFrom> for Version { + type Error = eyre::Report; + + fn try_from(value: Vec) -> Result { // Remove trailing zeros for normalization. - let mut normalized = value - .into_iter() - .rev() - .skip_while(|&x| x == 0) - .collect::>(); - normalized.reverse(); - Version(normalized) + let significant = value.len() - value.iter().rev().take_while(|&&x| x == 0).count(); + ensure!( + significant <= N, + "Version has {significant} components, expected at most {N}" + ); + let mut parts = [0; N]; + parts[..significant].copy_from_slice(&value[..significant]); + Ok(Version(parts)) } } -impl FromStr for Version { +impl FromStr for Version { type Err = eyre::Report; fn from_str(version: &str) -> Result { - let version = version.trim().to_owned(); + let version = version.trim(); ensure!(!version.is_empty(), "Empty version string"); let mut version_parts = Vec::new(); for part in version.split('.') { @@ -73,6 +72,63 @@ impl FromStr for Version { version_parts.push(version_part); } - Ok(Version::from(version_parts)) + Version::try_from(version_parts) + } +} + +#[cfg(test)] +mod tests { + use std::cmp::Ordering; + use std::str::FromStr; + + use super::Version; + + #[test] + fn trailing_zeros_are_insignificant() { + assert_eq!( + Version::<2>::from_str("5").unwrap(), + Version::from_str("5.0").unwrap() + ); + } + + #[test] + fn version_ord() { + assert_eq!( + Version::<2>::from_str("5.0") + .unwrap() + .cmp(&Version::from_str("5").unwrap()), + Ordering::Equal + ); + assert_eq!( + Version::<2>::from_str("5.0") + .unwrap() + .cmp(&Version::from_str("5.1").unwrap()), + Ordering::Less + ); + assert_eq!( + Version::<2>::from_str("5.1") + .unwrap() + .cmp(&Version::from_str("5.0").unwrap()), + Ordering::Greater + ); + } + + #[test] + fn too_many_significant_components_is_an_error() { + assert!(Version::<2>::from_str("1.2.3").is_err()); + } + + #[test] + fn display_keeps_all_components() { + assert_eq!(Version::<2>::from_str("12").unwrap().to_string(), "12.0"); + } + + #[test] + fn serde_roundtrip() { + let version = Version::<2>::from_str("12.8").unwrap(); + let serialized = serde_json::to_string(&version).unwrap(); + assert_eq!(serialized, "\"12.8\""); + let deserialized: Version<2> = serde_json::from_str(&serialized).unwrap(); + assert_eq!(version, deserialized); } } From 1c46293759dc2f69cf0d66f4b10604a9ffd8ae87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20de=20Kok?= Date: Fri, 28 Aug 2026 08:19:29 +0000 Subject: [PATCH 2/4] `PyVersion`: keep old version behavior --- kernels-data/bindings/python/src/lib.rs | 31 +----------- kernels-data/bindings/python/src/version.rs | 54 +++++++++++++++++++++ 2 files changed, 56 insertions(+), 29 deletions(-) create mode 100644 kernels-data/bindings/python/src/version.rs diff --git a/kernels-data/bindings/python/src/lib.rs b/kernels-data/bindings/python/src/lib.rs index 5ea8ce836..746cb84fd 100644 --- a/kernels-data/bindings/python/src/lib.rs +++ b/kernels-data/bindings/python/src/lib.rs @@ -8,44 +8,17 @@ use kernels_data::config::{Backend, KernelDependency, KernelName, KernelVersion} use kernels_data::digest::{Digest, DigestAlgorithm, DigestViolation}; use kernels_data::git::{GitStatus, Oid}; use kernels_data::metadata::{BackendInfo, KernelBuilderVersion, Metadata, Provenance}; -use kernels_data::version::Version; use pyo3::Bound as PyBound; use pyo3::exceptions::{PyException, PyOSError, PyRuntimeError, PyValueError}; use pyo3::prelude::*; mod config; mod lock; +mod version; use config::{PyBuild, PyGeneral}; use lock::{PyKernelLock, PyKernelLocks, PyKernelPaths, PyNixKernelLock, PyNixKernelLocks}; - -/// A dotted numeric version (e.g. `12.8.0`). Trailing zeros are stripped -/// during normalization. -#[pyclass(name = "Version", frozen, eq, ord, hash)] -#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] -struct PyVersion { - inner: Version, -} - -#[pymethods] -impl PyVersion { - /// Parse a version string of the form `X`, `X.Y`, `X.Y.Z`, ... - #[staticmethod] - #[pyo3(name = "from_str")] - fn py_from_str(s: &str) -> PyResult { - Version::from_str(s) - .map(|inner| Self { inner }) - .map_err(|err| PyValueError::new_err(format!("Cannot parse version `{s}`: {err}"))) - } - - fn __str__(&self) -> String { - self.inner.to_string() - } - - fn __repr__(&self) -> String { - format!("Version('{}')", self.inner) - } -} +use version::PyVersion; /// A validated kernel name matching `^[a-z][-a-z0-9]*[a-z0-9]$`. #[pyclass(name = "KernelName", frozen, eq, hash)] diff --git a/kernels-data/bindings/python/src/version.rs b/kernels-data/bindings/python/src/version.rs new file mode 100644 index 000000000..ee6c7f089 --- /dev/null +++ b/kernels-data/bindings/python/src/version.rs @@ -0,0 +1,54 @@ +use pyo3::exceptions::PyValueError; +use pyo3::prelude::*; + +/// A dotted numeric version (e.g. `12.8.0`). Trailing zeros are stripped +/// during normalization. +#[pyclass(name = "Version", frozen, eq, ord, hash)] +#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +pub(crate) struct PyVersion { + // We are storing this as a `Box<[usize]>` to allow storing kernels-data + // `Version` in directly in `PyVersion` as well. + inner: Box<[usize]>, +} + +#[pymethods] +impl PyVersion { + /// Parse a version string of the form `X`, `X.Y`, `X.Y.Z`, ... + #[staticmethod] + #[pyo3(name = "from_str")] + fn py_from_str(s: &str) -> PyResult { + let version = s.trim(); + if version.is_empty() { + return Err(PyValueError::new_err(format!( + "Cannot parse version `{s}`: empty version string" + ))); + } + let mut parts = Vec::new(); + for part in version.split('.') { + let component: usize = part.parse().map_err(|_| { + PyValueError::new_err(format!( + "Cannot parse version `{s}`: version must consist of numbers" + )) + })?; + parts.push(component); + } + // Remove trailing zeros for normalization. + let trailing_zeros = parts.iter().rev().take_while(|&&x| x == 0).count(); + parts.truncate(parts.len() - trailing_zeros); + Ok(Self { + inner: parts.into_boxed_slice(), + }) + } + + fn __str__(&self) -> String { + self.inner + .iter() + .map(|v| v.to_string()) + .collect::>() + .join(".") + } + + fn __repr__(&self) -> String { + format!("Version('{}')", self.__str__()) + } +} From b8951f27517cd6e6a88b384023e972fdfa75c334 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20de=20Kok?= Date: Fri, 28 Aug 2026 08:28:56 +0000 Subject: [PATCH 3/4] Move around version tests --- .../python/tests/test_kernels_data.py | 24 ----------------- .../bindings/python/tests/test_version.py | 26 +++++++++++++++++++ 2 files changed, 26 insertions(+), 24 deletions(-) create mode 100644 kernels-data/bindings/python/tests/test_version.py diff --git a/kernels-data/bindings/python/tests/test_kernels_data.py b/kernels-data/bindings/python/tests/test_kernels_data.py index 92f09c237..cca9b474f 100644 --- a/kernels-data/bindings/python/tests/test_kernels_data.py +++ b/kernels-data/bindings/python/tests/test_kernels_data.py @@ -8,7 +8,6 @@ KernelName, KernelVersion, Metadata, - Version, ) @@ -18,29 +17,6 @@ def _write_metadata(path, **fields): return path -def test_version_parse_and_normalize(): - assert str(Version.from_str("12.8.0")) == "12.8" - assert str(Version.from_str("1")) == "1" - assert str(Version.from_str("1.2.3")) == "1.2.3" - - -def test_version_ordering_and_hash(): - v1 = Version.from_str("1.2") - v2 = Version.from_str("1.2.0") - v3 = Version.from_str("1.3") - assert v1 == v2 - assert v1 < v3 - assert hash(v1) == hash(v2) - assert {v1, v2, v3} == {v1, v3} - - -def test_version_invalid(): - with pytest.raises(ValueError): - Version.from_str("abc") - with pytest.raises(ValueError): - Version.from_str("") - - def test_kernel_name_valid(): n = KernelName("my-kernel") assert str(n) == "my-kernel" diff --git a/kernels-data/bindings/python/tests/test_version.py b/kernels-data/bindings/python/tests/test_version.py new file mode 100644 index 000000000..c91400f01 --- /dev/null +++ b/kernels-data/bindings/python/tests/test_version.py @@ -0,0 +1,26 @@ +import pytest + +from kernels_data import Version + + +def test_version_parse_and_normalize(): + assert str(Version.from_str("12.8.0")) == "12.8" + assert str(Version.from_str("1")) == "1" + assert str(Version.from_str("1.2.3")) == "1.2.3" + + +def test_version_ordering_and_hash(): + v1 = Version.from_str("1.2") + v2 = Version.from_str("1.2.0") + v3 = Version.from_str("1.3") + assert v1 == v2 + assert v1 < v3 + assert hash(v1) == hash(v2) + assert {v1, v2, v3} == {v1, v3} + + +def test_version_invalid(): + with pytest.raises(ValueError): + Version.from_str("abc") + with pytest.raises(ValueError): + Version.from_str("") From 28b4105f2bf3db11b142cbaff71b8d821a73eb82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20de=20Kok?= Date: Fri, 28 Aug 2026 09:10:41 +0000 Subject: [PATCH 4/4] Make parsing/from Vec conversion strict as well Reject bad data. --- kernels-data/src/version.rs | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/kernels-data/src/version.rs b/kernels-data/src/version.rs index b4da26046..90908a85a 100644 --- a/kernels-data/src/version.rs +++ b/kernels-data/src/version.rs @@ -46,14 +46,13 @@ impl TryFrom> for Version { type Error = eyre::Report; fn try_from(value: Vec) -> Result { - // Remove trailing zeros for normalization. - let significant = value.len() - value.iter().rev().take_while(|&&x| x == 0).count(); ensure!( - significant <= N, - "Version has {significant} components, expected at most {N}" + value.len() == N, + "Version has {} components, expected {N}", + value.len() ); let mut parts = [0; N]; - parts[..significant].copy_from_slice(&value[..significant]); + parts.copy_from_slice(&value); Ok(Version(parts)) } } @@ -84,11 +83,10 @@ mod tests { use super::Version; #[test] - fn trailing_zeros_are_insignificant() { - assert_eq!( - Version::<2>::from_str("5").unwrap(), - Version::from_str("5.0").unwrap() - ); + fn versions_must_have_exactly_n_components() { + assert!(Version::<2>::from_str("12.8").is_ok()); + assert!(Version::<2>::from_str("13").is_err()); + assert!(Version::<2>::from_str("12.8.0").is_err()); } #[test] @@ -96,7 +94,7 @@ mod tests { assert_eq!( Version::<2>::from_str("5.0") .unwrap() - .cmp(&Version::from_str("5").unwrap()), + .cmp(&Version::from_str("5.0").unwrap()), Ordering::Equal ); assert_eq!( @@ -113,14 +111,9 @@ mod tests { ); } - #[test] - fn too_many_significant_components_is_an_error() { - assert!(Version::<2>::from_str("1.2.3").is_err()); - } - #[test] fn display_keeps_all_components() { - assert_eq!(Version::<2>::from_str("12").unwrap().to_string(), "12.0"); + assert_eq!(Version::<2>::from_str("12.0").unwrap().to_string(), "12.0"); } #[test]