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__()) + } +} 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("") 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..90908a85a 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,26 @@ impl Deref for Version { } } -impl From> for Version { - fn from(value: Vec) -> Self { - // Remove trailing zeros for normalization. - let mut normalized = value - .into_iter() - .rev() - .skip_while(|&x| x == 0) - .collect::>(); - normalized.reverse(); - Version(normalized) +impl TryFrom> for Version { + type Error = eyre::Report; + + fn try_from(value: Vec) -> Result { + ensure!( + value.len() == N, + "Version has {} components, expected {N}", + value.len() + ); + let mut parts = [0; N]; + parts.copy_from_slice(&value); + 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 +71,57 @@ 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 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] + fn version_ord() { + assert_eq!( + Version::<2>::from_str("5.0") + .unwrap() + .cmp(&Version::from_str("5.0").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 display_keeps_all_components() { + assert_eq!(Version::<2>::from_str("12.0").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); } }