From 1bb8a23faf15c2be8caef4d3fbe0577cf97db89a Mon Sep 17 00:00:00 2001 From: David Holtz Date: Mon, 31 Aug 2026 12:05:47 -0400 Subject: [PATCH 1/7] feat: add minver to kernels-data --- kernels-data/bindings/python/kernels_data.pyi | 2 ++ kernels-data/bindings/python/src/lib.rs | 10 +++++- .../python/tests/test_kernels_data.py | 3 ++ kernels-data/src/config/mod.rs | 4 +++ kernels-data/src/config/v3.rs | 1 + kernels-data/src/config/v4.rs | 1 + kernels-data/src/config/v5.rs | 4 +++ kernels-data/src/metadata.rs | 32 +++++++++++++++++++ 8 files changed, 56 insertions(+), 1 deletion(-) diff --git a/kernels-data/bindings/python/kernels_data.pyi b/kernels-data/bindings/python/kernels_data.pyi index 53c29b7c5..363d5f87b 100644 --- a/kernels-data/bindings/python/kernels_data.pyi +++ b/kernels-data/bindings/python/kernels_data.pyi @@ -659,6 +659,8 @@ class Metadata: @property def version(self) -> Optional[int]: ... @property + def minver(self) -> Optional[str]: ... + @property def license(self) -> Optional[str]: ... @property def upstream(self) -> Optional[str]: ... diff --git a/kernels-data/bindings/python/src/lib.rs b/kernels-data/bindings/python/src/lib.rs index 746cb84fd..62241d08d 100644 --- a/kernels-data/bindings/python/src/lib.rs +++ b/kernels-data/bindings/python/src/lib.rs @@ -393,6 +393,7 @@ struct PyMetadata { id: String, name: PyKernelName, version: usize, + minver: Option, license: String, upstream: Option, source: Option, @@ -409,6 +410,7 @@ impl From for PyMetadata { id: m.id, name: PyKernelName { inner: m.name }, version: m.version, + minver: m.minver.map(|v| v.to_string()), license: m.license, upstream: m.upstream.map(|u| u.as_url().to_string()), source: m.source.map(|u| u.as_url().to_string()), @@ -465,6 +467,11 @@ impl PyMetadata { self.version } + #[getter] + fn minver(&self) -> Option<&str> { + self.minver.as_deref() + } + #[getter] fn license(&self) -> &str { &self.license @@ -507,10 +514,11 @@ impl PyMetadata { fn __repr__(&self) -> String { format!( - "Metadata(id={}, name={:?}, version={:?}, license={:?}, upstream={:?}, source={:?}, python_depends={:?}, kernel_depends={:?}, backend={}, digest={}, provenance={})", + "Metadata(id={}, name={:?}, version={:?}, minver={:?}, license={:?}, upstream={:?}, source={:?}, python_depends={:?}, kernel_depends={:?}, backend={}, digest={}, provenance={})", self.id, self.name, self.version, + self.minver, self.license, self.upstream, self.source, diff --git a/kernels-data/bindings/python/tests/test_kernels_data.py b/kernels-data/bindings/python/tests/test_kernels_data.py index cca9b474f..5c79c4d45 100644 --- a/kernels-data/bindings/python/tests/test_kernels_data.py +++ b/kernels-data/bindings/python/tests/test_kernels_data.py @@ -80,6 +80,7 @@ def test_metadata_load_full(tmp_path): "version": 1, "name": "my-kernel", "license": "Apache-2.0", + "minver": "0.11.0", "upstream": "https://github.com/example/kernel", "source": "https://github.com/example/kernel-builder", "python-depends": ["torch"], @@ -91,6 +92,7 @@ def test_metadata_load_full(tmp_path): assert m.id == "_my_kernel_8a3be8f" assert m.name == KernelName("my-kernel") assert m.version == 1 + assert m.minver == "0.11.0" assert m.license == "Apache-2.0" assert m.upstream == "https://github.com/example/kernel" assert m.source == "https://github.com/example/kernel-builder" @@ -115,6 +117,7 @@ def test_metadata_load_minimal(tmp_path): ) m = Metadata.read_from_file(path) assert m.version == 1 + assert m.minver is None assert m.license == "Apache-2.0" assert m.upstream is None assert m.source is None diff --git a/kernels-data/src/config/mod.rs b/kernels-data/src/config/mod.rs index 80774aa58..2e3587bfc 100644 --- a/kernels-data/src/config/mod.rs +++ b/kernels-data/src/config/mod.rs @@ -117,6 +117,9 @@ pub struct General { /// Kernel-builder formatted source repository (must contain build.toml and flake.nix). pub source: Option, + /// Minimum version of the `kernels` Python library required to load the kernel. + pub minver: Option>, + pub backends: Vec, pub hub: Option, pub kernel_depends: Option>, @@ -506,6 +509,7 @@ mod tests { license: "apache-2.0".to_string(), upstream: None, source: None, + minver: None, backends: vec![Backend::Tpu], hub: None, kernel_depends: None, diff --git a/kernels-data/src/config/v3.rs b/kernels-data/src/config/v3.rs index 0672b4b96..51d8cbf69 100644 --- a/kernels-data/src/config/v3.rs +++ b/kernels-data/src/config/v3.rs @@ -201,6 +201,7 @@ impl TryFrom for super::General { license, upstream: general.upstream, source: None, + minver: None, backends: general.backends.into_iter().map(Into::into).collect(), cuda: general.cuda.map(Into::into), hub: general.hub.map(Into::into), diff --git a/kernels-data/src/config/v4.rs b/kernels-data/src/config/v4.rs index 08b8f7bf5..1f1a3250b 100644 --- a/kernels-data/src/config/v4.rs +++ b/kernels-data/src/config/v4.rs @@ -199,6 +199,7 @@ impl From for super::General { license: general.license, upstream: general.upstream, source: general.source, + minver: None, backends: general.backends.into_iter().map(Into::into).collect(), cuda: general.cuda.map(Into::into), hub: general.hub.map(Into::into), diff --git a/kernels-data/src/config/v5.rs b/kernels-data/src/config/v5.rs index dc1078ee2..a65c5e4af 100644 --- a/kernels-data/src/config/v5.rs +++ b/kernels-data/src/config/v5.rs @@ -52,6 +52,8 @@ pub struct General { pub source: Option, + pub minver: Option>, + pub backends: Vec, pub cuda: Option, @@ -226,6 +228,7 @@ impl From for super::General { license: general.license, upstream: general.upstream, source: general.source, + minver: general.minver, backends: general.backends.into_iter().map(Into::into).collect(), cuda: general.cuda.map(Into::into), hub: general.hub.map(Into::into), @@ -443,6 +446,7 @@ impl From for General { license: general.license, upstream: general.upstream, source: general.source, + minver: general.minver, backends: general.backends.into_iter().map(Into::into).collect(), cuda: general.cuda.map(Into::into), hub: general.hub.map(Into::into), diff --git a/kernels-data/src/metadata.rs b/kernels-data/src/metadata.rs index 0eb5a0449..20619da5d 100644 --- a/kernels-data/src/metadata.rs +++ b/kernels-data/src/metadata.rs @@ -6,6 +6,7 @@ use serde::{Deserialize, Deserializer, Serialize}; use crate::config::{Backend, Build, GitUrl, KernelDependency, KernelName}; use crate::digest::Digest; use crate::git::{GitStatus, Oid}; +use crate::version::Version; #[derive(Debug, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] @@ -93,6 +94,8 @@ pub struct Metadata { pub name: KernelName, pub id: String, pub version: usize, + #[serde(skip_serializing_if = "Option::is_none")] + pub minver: Option>, pub license: String, #[serde(skip_serializing_if = "Option::is_none")] pub upstream: Option, @@ -125,6 +128,7 @@ impl Metadata { id, name: build.general.name.clone(), version: build.general.version, + minver: build.general.minver.clone(), license: build.general.license.clone(), upstream: build.general.upstream.clone(), source: build.general.source.clone(), @@ -165,6 +169,7 @@ mod tests { use crate::config::{Backend, Build, Framework, General, KernelName, TorchNoarch, TvmFfi}; use crate::git::{GitStatus, Oid}; + use crate::version::Version; use super::{KernelBuilderVersion, Metadata, Provenance}; @@ -203,6 +208,7 @@ mod tests { license: "apache-2.0".to_string(), upstream: None, source: None, + minver: None, backends: vec![Backend::Cuda, Backend::Rocm, Backend::Cpu], hub: None, kernel_depends: None, @@ -260,6 +266,7 @@ mod tests { license: "apache-2.0".to_string(), upstream: None, source: None, + minver: None, backends: vec![Backend::Cuda], hub: None, kernel_depends: None, @@ -303,6 +310,31 @@ mod tests { assert!(parsed.provenance.is_none()); } + #[test] + fn metadata_without_minver_serializes_without_key() { + let build = torch_noarch_build(); + let metadata = Metadata::for_backend(&build, "test-id".to_string(), Backend::Cuda).unwrap(); + + let json = serde_json::to_string(&metadata).unwrap(); + assert!(!json.contains("minver")); + + let parsed: Metadata = serde_json::from_str(&json).unwrap(); + assert!(parsed.minver.is_none()); + } + + #[test] + fn metadata_round_trips_minver() { + let mut build = torch_noarch_build(); + build.general.minver = Some(Version::from_str("0.11.0").unwrap()); + let metadata = Metadata::for_backend(&build, "test-id".to_string(), Backend::Cuda).unwrap(); + + let json = serde_json::to_string(&metadata).unwrap(); + assert!(json.contains(r#""minver":"0.11.0""#)); + + let parsed: Metadata = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed.minver, Some(Version::from_str("0.11.0").unwrap())); + } + #[test] fn metadata_always_emits_kernel_depends() { let build = torch_noarch_build(); From 910e029b65e5e3d1f68733a3db78cf0d4d3906ae Mon Sep 17 00:00:00 2001 From: David Holtz Date: Mon, 31 Aug 2026 12:07:12 -0400 Subject: [PATCH 2/7] feat: check minver in client --- kernels/src/kernels/cli/info.py | 2 ++ kernels/src/kernels/importer.py | 25 +++++++++++++ kernels/tests/test_minver.py | 62 +++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 kernels/tests/test_minver.py diff --git a/kernels/src/kernels/cli/info.py b/kernels/src/kernels/cli/info.py index 6619ab50c..71f467e57 100644 --- a/kernels/src/kernels/cli/info.py +++ b/kernels/src/kernels/cli/info.py @@ -120,6 +120,7 @@ def _metadata_info(metadata: Metadata | None, variants: list[Variant]) -> dict: { "name": str(metadata.name), "version": metadata.version, + "minver": metadata.minver, "license": metadata.license, "upstream": metadata.upstream, "source": metadata.source, @@ -144,6 +145,7 @@ def value(v) -> str: print(f"Name: {value(info.get('name'))}") print(f"Version: {value(info.get('version'))}") + print(f"Minimum kernels version: {value(info.get('minver'))}") print(f"License: {value(info.get('license'))}") print(f"Upstream: {value(info.get('upstream'))}") print(f"Source: {value(info.get('source'))}") diff --git a/kernels/src/kernels/importer.py b/kernels/src/kernels/importer.py index 4b39498cb..1bf81fe49 100644 --- a/kernels/src/kernels/importer.py +++ b/kernels/src/kernels/importer.py @@ -6,6 +6,7 @@ from types import ModuleType from kernels_data import Metadata +from packaging.version import parse from kernels.hf_hub import RepoInfo @@ -27,6 +28,8 @@ class LoadedKernel: - `id` (`str`): kernel identifier that is unique to the kernel version + backend. - `name` (`str`): the name of the kernel. - `version` (`int`): the version of the kernel. + - `minver` (`str | None`): the minimum `kernels` library version + required to load the kernel. - `license` (`str`): the license of the kernel. - `upstream` (`str | None`): the original upstream repository of the kernel. - `source` (`str | None`): the kernel-builder formatted source repository. @@ -91,6 +94,27 @@ def _warn_if_dirty(metadata: Metadata, variant_str: str) -> None: ) +def _warn_if_below_minver(metadata: Metadata, variant_str: str) -> None: + """Warn when the installed `kernels` library is older than the minimum + version required by a kernel. + """ + minver = metadata.minver + if minver is None: + return + + # Avoid an import cycle. + from kernels import __version__ + + if parse(__version__) < parse(minver): + warnings.warn( + f"Kernel '{metadata.name}' variant '{variant_str}' requires " + f"kernels>={minver}, but version {__version__} is installed. " + "The kernel may not load or work correctly; upgrade with: " + "pip install --upgrade kernels", + stacklevel=3, + ) + + def _import_from_path( variant_path: Path, deps: dict[str, ModuleType], @@ -101,6 +125,7 @@ def _import_from_path( metadata = Metadata.read_from_file(variant_path / "metadata.json") _warn_if_dirty(metadata, variant_path.name) + _warn_if_below_minver(metadata, variant_path.name) module_name = metadata.name.python_name file_path = variant_path / "__init__.py" diff --git a/kernels/tests/test_minver.py b/kernels/tests/test_minver.py new file mode 100644 index 000000000..e40a805e3 --- /dev/null +++ b/kernels/tests/test_minver.py @@ -0,0 +1,62 @@ +import json + +import pytest +from kernels_data import Metadata + +from kernels import __version__ +from kernels.importer import ( + _import_from_path, + _loaded_kernels, + _warn_if_below_minver, +) + + +def _write_variant(tmp_path, minver): + variant_dir = tmp_path / "build" / "torch28-cxx11-cu128-x86_64-linux" + variant_dir.mkdir(parents=True) + metadata = { + "id": "activation_1_cuda", + "name": "activation", + "version": 1, + "license": "Apache-2.0", + "python-depends": ["torch"], + "backend": {"type": "cuda"}, + } + if minver is not None: + metadata["minver"] = minver + (variant_dir / "metadata.json").write_text(json.dumps(metadata)) + return variant_dir + + +@pytest.mark.parametrize("minver", [None, "0.0.1"]) +def test_no_warning_when_minver_met(tmp_path, recwarn, minver): + variant_dir = _write_variant(tmp_path, minver) + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + _warn_if_below_minver(metadata, variant_dir.name) + assert len(recwarn) == 0 + + +def test_warns_when_minver_not_met(tmp_path): + variant_dir = _write_variant(tmp_path, "999.0.0") + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + with pytest.warns(UserWarning, match="requires kernels>=999.0.0"): + _warn_if_below_minver(metadata, variant_dir.name) + + +def test_warning_mentions_installed_version(tmp_path): + variant_dir = _write_variant(tmp_path, "999.0.0") + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + with pytest.warns(UserWarning, match=f"version {__version__} is installed"): + _warn_if_below_minver(metadata, variant_dir.name) + + +def test_import_from_path_warns_on_unmet_minver(tmp_path): + variant_dir = _write_variant(tmp_path, "999.0.0") + (variant_dir / "__init__.py").write_text("value = 42\n") + _loaded_kernels.pop(variant_dir, None) + try: + with pytest.warns(UserWarning, match="requires kernels>=999.0.0"): + module = _import_from_path(variant_dir, deps={}) + assert module.value == 42 + finally: + _loaded_kernels.pop(variant_dir, None) From dd9924efcdd03f4e554bd9d2962792fa6eeb92eb Mon Sep 17 00:00:00 2001 From: David Holtz Date: Mon, 31 Aug 2026 15:24:39 -0400 Subject: [PATCH 3/7] feat: add minver to docs --- docs/source/builder/writing-kernels.md | 4 ++++ docs/source/kernel-requirements.md | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/docs/source/builder/writing-kernels.md b/docs/source/builder/writing-kernels.md index eb36c60f8..6de8b4775 100644 --- a/docs/source/builder/writing-kernels.md +++ b/docs/source/builder/writing-kernels.md @@ -208,6 +208,10 @@ The following sections enumerate all supported options for `build.toml`. - `source`: Git-compatible URL (passable to `git clone`) of the kernel-builder formatted source repository. This repository must contain a `build.toml` and `flake.nix` so that it can be pulled and built with the kernel builder. +- `minver`: the minimum version of the `kernels` Python library required + to load the kernel (e.g. `"0.11.0"`). The version is written to the kernel's + `metadata.json` and the `kernels` library warns when it loads a kernel that + requires a newer version than the one that is installed. - `python-depends` (**experimental**): a list of additional Python dependencies that the kernel requires. The only supported dependencies are `einops`, `helion`, and `nvidia-cutlass-dsl`. diff --git a/docs/source/kernel-requirements.md b/docs/source/kernel-requirements.md index 4309f0c00..028aa97a0 100644 --- a/docs/source/kernel-requirements.md +++ b/docs/source/kernel-requirements.md @@ -69,6 +69,10 @@ metadata. Currently the following top-level keys are supported: - `name` (`str`, required): then name of the kernel. Replacing dashes by underscores should result in the module name of the kernel. - `version` (`int`, required): the kernel version number. +- `minver` (`str`, optional): the minimum version of the `kernels` + Python library required to load the kernel (e.g. `"0.11.0"`). The + `kernels` library warns when it loads a kernel that requires a newer + version than the one that is installed. - `license` (`str`, required): the kernel license in. Refer to the list of [supported license identifiers](https://huggingface.co/docs/hub/repositories-licenses). - `upstream` (`str`, optional): Git-compatible URL (passable to `git clone`) From c235f23517ad602bcc74ac139978880f7d0b9f18 Mon Sep 17 00:00:00 2001 From: David Holtz Date: Tue, 1 Sep 2026 10:13:24 -0400 Subject: [PATCH 4/7] fix: improve naming and only allow builder to set minver --- docs/source/builder/writing-kernels.md | 4 -- docs/source/kernel-requirements.md | 9 +-- kernels-data/bindings/python/kernels_data.pyi | 7 ++- kernels-data/bindings/python/src/lib.rs | 12 ++-- kernels-data/bindings/python/src/version.rs | 26 ++++++-- .../python/tests/test_kernels_data.py | 7 ++- kernels-data/src/config/mod.rs | 50 +++++++++++++-- kernels-data/src/config/v3.rs | 1 - kernels-data/src/config/v4.rs | 1 - kernels-data/src/config/v5.rs | 4 -- kernels-data/src/metadata.rs | 43 ++++++++----- kernels-data/src/version.rs | 7 +++ kernels/src/kernels/cli/info.py | 4 +- kernels/src/kernels/importer.py | 40 +++++++++--- kernels/tests/test_minver.py | 62 ++++++++++++++++--- 15 files changed, 210 insertions(+), 67 deletions(-) diff --git a/docs/source/builder/writing-kernels.md b/docs/source/builder/writing-kernels.md index 6de8b4775..eb36c60f8 100644 --- a/docs/source/builder/writing-kernels.md +++ b/docs/source/builder/writing-kernels.md @@ -208,10 +208,6 @@ The following sections enumerate all supported options for `build.toml`. - `source`: Git-compatible URL (passable to `git clone`) of the kernel-builder formatted source repository. This repository must contain a `build.toml` and `flake.nix` so that it can be pulled and built with the kernel builder. -- `minver`: the minimum version of the `kernels` Python library required - to load the kernel (e.g. `"0.11.0"`). The version is written to the kernel's - `metadata.json` and the `kernels` library warns when it loads a kernel that - requires a newer version than the one that is installed. - `python-depends` (**experimental**): a list of additional Python dependencies that the kernel requires. The only supported dependencies are `einops`, `helion`, and `nvidia-cutlass-dsl`. diff --git a/docs/source/kernel-requirements.md b/docs/source/kernel-requirements.md index 028aa97a0..4c720022d 100644 --- a/docs/source/kernel-requirements.md +++ b/docs/source/kernel-requirements.md @@ -69,10 +69,11 @@ metadata. Currently the following top-level keys are supported: - `name` (`str`, required): then name of the kernel. Replacing dashes by underscores should result in the module name of the kernel. - `version` (`int`, required): the kernel version number. -- `minver` (`str`, optional): the minimum version of the `kernels` - Python library required to load the kernel (e.g. `"0.11.0"`). The - `kernels` library warns when it loads a kernel that requires a newer - version than the one that is installed. +- `kernels-minver` (`str`, optional): the minimum version of the `kernels` + Python library required to load the kernel (e.g. `"0.17.0"`). This key is + determined by the kernel builder from the features that the kernel uses, + it is not set by kernel authors. The `kernels` library warns when it loads + a kernel that requires a newer version than the one that is installed. - `license` (`str`, required): the kernel license in. Refer to the list of [supported license identifiers](https://huggingface.co/docs/hub/repositories-licenses). - `upstream` (`str`, optional): Git-compatible URL (passable to `git clone`) diff --git a/kernels-data/bindings/python/kernels_data.pyi b/kernels-data/bindings/python/kernels_data.pyi index 363d5f87b..314b07190 100644 --- a/kernels-data/bindings/python/kernels_data.pyi +++ b/kernels-data/bindings/python/kernels_data.pyi @@ -659,7 +659,12 @@ class Metadata: @property def version(self) -> Optional[int]: ... @property - def minver(self) -> Optional[str]: ... + def kernels_minver(self) -> Optional[Version]: + """Minimum `kernels` library version required to load this variant. + + `None` for metadata written before this key existed. + """ + ... @property def license(self) -> Optional[str]: ... @property diff --git a/kernels-data/bindings/python/src/lib.rs b/kernels-data/bindings/python/src/lib.rs index 62241d08d..315c71332 100644 --- a/kernels-data/bindings/python/src/lib.rs +++ b/kernels-data/bindings/python/src/lib.rs @@ -393,7 +393,7 @@ struct PyMetadata { id: String, name: PyKernelName, version: usize, - minver: Option, + kernels_minver: Option, license: String, upstream: Option, source: Option, @@ -410,7 +410,7 @@ impl From for PyMetadata { id: m.id, name: PyKernelName { inner: m.name }, version: m.version, - minver: m.minver.map(|v| v.to_string()), + kernels_minver: m.kernels_minver.map(Into::into), license: m.license, upstream: m.upstream.map(|u| u.as_url().to_string()), source: m.source.map(|u| u.as_url().to_string()), @@ -468,8 +468,8 @@ impl PyMetadata { } #[getter] - fn minver(&self) -> Option<&str> { - self.minver.as_deref() + fn kernels_minver(&self) -> Option { + self.kernels_minver.clone() } #[getter] @@ -514,11 +514,11 @@ impl PyMetadata { fn __repr__(&self) -> String { format!( - "Metadata(id={}, name={:?}, version={:?}, minver={:?}, license={:?}, upstream={:?}, source={:?}, python_depends={:?}, kernel_depends={:?}, backend={}, digest={}, provenance={})", + "Metadata(id={}, name={:?}, version={:?}, kernels_minver={:?}, license={:?}, upstream={:?}, source={:?}, python_depends={:?}, kernel_depends={:?}, backend={}, digest={}, provenance={})", self.id, self.name, self.version, - self.minver, + self.kernels_minver, self.license, self.upstream, self.source, diff --git a/kernels-data/bindings/python/src/version.rs b/kernels-data/bindings/python/src/version.rs index ee6c7f089..5a69163be 100644 --- a/kernels-data/bindings/python/src/version.rs +++ b/kernels-data/bindings/python/src/version.rs @@ -1,3 +1,4 @@ +use kernels_data::version::Version; use pyo3::exceptions::PyValueError; use pyo3::prelude::*; @@ -11,6 +12,24 @@ pub(crate) struct PyVersion { inner: Box<[usize]>, } +impl PyVersion { + /// Construct a version from its components, stripping trailing zeros so + /// that versions that only differ in trailing zeros compare as equal. + fn normalized(mut parts: Vec) -> Self { + let trailing_zeros = parts.iter().rev().take_while(|&&x| x == 0).count(); + parts.truncate(parts.len() - trailing_zeros); + Self { + inner: parts.into_boxed_slice(), + } + } +} + +impl From> for PyVersion { + fn from(version: Version) -> Self { + PyVersion::normalized(version.to_vec()) + } +} + #[pymethods] impl PyVersion { /// Parse a version string of the form `X`, `X.Y`, `X.Y.Z`, ... @@ -32,12 +51,7 @@ impl PyVersion { })?; 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(), - }) + Ok(PyVersion::normalized(parts)) } fn __str__(&self) -> String { diff --git a/kernels-data/bindings/python/tests/test_kernels_data.py b/kernels-data/bindings/python/tests/test_kernels_data.py index 5c79c4d45..b80d9f182 100644 --- a/kernels-data/bindings/python/tests/test_kernels_data.py +++ b/kernels-data/bindings/python/tests/test_kernels_data.py @@ -8,6 +8,7 @@ KernelName, KernelVersion, Metadata, + Version, ) @@ -80,7 +81,7 @@ def test_metadata_load_full(tmp_path): "version": 1, "name": "my-kernel", "license": "Apache-2.0", - "minver": "0.11.0", + "kernels-minver": "0.17.0", "upstream": "https://github.com/example/kernel", "source": "https://github.com/example/kernel-builder", "python-depends": ["torch"], @@ -92,7 +93,7 @@ def test_metadata_load_full(tmp_path): assert m.id == "_my_kernel_8a3be8f" assert m.name == KernelName("my-kernel") assert m.version == 1 - assert m.minver == "0.11.0" + assert m.kernels_minver == Version.from_str("0.17.0") assert m.license == "Apache-2.0" assert m.upstream == "https://github.com/example/kernel" assert m.source == "https://github.com/example/kernel-builder" @@ -117,7 +118,7 @@ def test_metadata_load_minimal(tmp_path): ) m = Metadata.read_from_file(path) assert m.version == 1 - assert m.minver is None + assert m.kernels_minver is None assert m.license == "Apache-2.0" assert m.upstream is None assert m.source is None diff --git a/kernels-data/src/config/mod.rs b/kernels-data/src/config/mod.rs index 2e3587bfc..d8d2888a3 100644 --- a/kernels-data/src/config/mod.rs +++ b/kernels-data/src/config/mod.rs @@ -37,6 +37,13 @@ use crate::version::Version; pub type CurrentConfig = v5::Build; pub const CURRENT_EDITION: usize = 5; +/// Baseline `kernels` version that can load kernels built with the current +/// metadata format. +const KERNELS_VERSION_BASELINE: Version<3> = Version::new([0, 14, 0]); + +/// First `kernels` version that can resolve kernel dependencies. +const KERNELS_VERSION_KERNEL_DEPENDS: Version<3> = Version::new([0, 17, 0]); + pub struct Build { pub general: General, pub kernels: HashMap, @@ -60,6 +67,24 @@ impl Build { pub fn repo_id(&self) -> Option<&str> { self.general.hub.as_ref().and_then(|h| h.repo_id.as_deref()) } + + /// Minimum version of the `kernels` Python library required to load a + /// build variant for `backend`. + /// + /// The version is derived from the features that the kernel uses, rather + /// than being specified by the kernel author, who would have to keep track + /// of which `kernels` version introduced support for which feature. + pub fn required_kernels_version(&self, backend: Backend) -> Version<3> { + let mut required = KERNELS_VERSION_BASELINE; + + // Kernel dependencies are only fetched by newer versions of `kernels`, + // older versions would fail to import the kernel. + if !self.general.all_kernel_depends(backend).is_empty() { + required = required.max(KERNELS_VERSION_KERNEL_DEPENDS); + } + + required + } } pub enum Framework { @@ -117,9 +142,6 @@ pub struct General { /// Kernel-builder formatted source repository (must contain build.toml and flake.nix). pub source: Option, - /// Minimum version of the `kernels` Python library required to load the kernel. - pub minver: Option>, - pub backends: Vec, pub hub: Option, pub kernel_depends: Option>, @@ -509,7 +531,6 @@ mod tests { license: "apache-2.0".to_string(), upstream: None, source: None, - minver: None, backends: vec![Backend::Tpu], hub: None, kernel_depends: None, @@ -549,4 +570,25 @@ mod tests { .is_none() ); } + + /// The minimum `kernels` version is derived from the features that a + /// kernel uses, so kernel authors must not be able to set it themselves. + #[test] + fn general_minver_is_rejected() { + let toml = r#" + [general] + name = "test-kernel" + version = 1 + edition = 5 + license = "apache-2.0" + backends = ["cuda"] + minver = "0.11.0" + + [torch] + src = [] + "#; + + let err = toml::from_str::(toml).unwrap_err().to_string(); + assert!(err.contains("unknown field `minver`"), "{err}"); + } } diff --git a/kernels-data/src/config/v3.rs b/kernels-data/src/config/v3.rs index 51d8cbf69..0672b4b96 100644 --- a/kernels-data/src/config/v3.rs +++ b/kernels-data/src/config/v3.rs @@ -201,7 +201,6 @@ impl TryFrom for super::General { license, upstream: general.upstream, source: None, - minver: None, backends: general.backends.into_iter().map(Into::into).collect(), cuda: general.cuda.map(Into::into), hub: general.hub.map(Into::into), diff --git a/kernels-data/src/config/v4.rs b/kernels-data/src/config/v4.rs index 1f1a3250b..08b8f7bf5 100644 --- a/kernels-data/src/config/v4.rs +++ b/kernels-data/src/config/v4.rs @@ -199,7 +199,6 @@ impl From for super::General { license: general.license, upstream: general.upstream, source: general.source, - minver: None, backends: general.backends.into_iter().map(Into::into).collect(), cuda: general.cuda.map(Into::into), hub: general.hub.map(Into::into), diff --git a/kernels-data/src/config/v5.rs b/kernels-data/src/config/v5.rs index a65c5e4af..dc1078ee2 100644 --- a/kernels-data/src/config/v5.rs +++ b/kernels-data/src/config/v5.rs @@ -52,8 +52,6 @@ pub struct General { pub source: Option, - pub minver: Option>, - pub backends: Vec, pub cuda: Option, @@ -228,7 +226,6 @@ impl From for super::General { license: general.license, upstream: general.upstream, source: general.source, - minver: general.minver, backends: general.backends.into_iter().map(Into::into).collect(), cuda: general.cuda.map(Into::into), hub: general.hub.map(Into::into), @@ -446,7 +443,6 @@ impl From for General { license: general.license, upstream: general.upstream, source: general.source, - minver: general.minver, backends: general.backends.into_iter().map(Into::into).collect(), cuda: general.cuda.map(Into::into), hub: general.hub.map(Into::into), diff --git a/kernels-data/src/metadata.rs b/kernels-data/src/metadata.rs index 20619da5d..cbf4564e8 100644 --- a/kernels-data/src/metadata.rs +++ b/kernels-data/src/metadata.rs @@ -94,8 +94,10 @@ pub struct Metadata { pub name: KernelName, pub id: String, pub version: usize, + /// Minimum version of the `kernels` Python library required to load this + /// build variant. Absent in metadata written before this key existed. #[serde(skip_serializing_if = "Option::is_none")] - pub minver: Option>, + pub kernels_minver: Option>, pub license: String, #[serde(skip_serializing_if = "Option::is_none")] pub upstream: Option, @@ -128,7 +130,7 @@ impl Metadata { id, name: build.general.name.clone(), version: build.general.version, - minver: build.general.minver.clone(), + kernels_minver: Some(build.required_kernels_version(backend)), license: build.general.license.clone(), upstream: build.general.upstream.clone(), source: build.general.source.clone(), @@ -167,7 +169,10 @@ mod tests { use std::collections::HashMap; use std::str::FromStr; - use crate::config::{Backend, Build, Framework, General, KernelName, TorchNoarch, TvmFfi}; + use crate::config::{ + Backend, Build, Framework, General, KernelDependency, KernelName, KernelVersion, + TorchNoarch, TvmFfi, + }; use crate::git::{GitStatus, Oid}; use crate::version::Version; @@ -208,7 +213,6 @@ mod tests { license: "apache-2.0".to_string(), upstream: None, source: None, - minver: None, backends: vec![Backend::Cuda, Backend::Rocm, Backend::Cpu], hub: None, kernel_depends: None, @@ -266,7 +270,6 @@ mod tests { license: "apache-2.0".to_string(), upstream: None, source: None, - minver: None, backends: vec![Backend::Cuda], hub: None, kernel_depends: None, @@ -311,28 +314,40 @@ mod tests { } #[test] - fn metadata_without_minver_serializes_without_key() { + fn metadata_kernels_minver_defaults_to_baseline() { let build = torch_noarch_build(); let metadata = Metadata::for_backend(&build, "test-id".to_string(), Backend::Cuda).unwrap(); let json = serde_json::to_string(&metadata).unwrap(); - assert!(!json.contains("minver")); + assert!(json.contains(r#""kernels-minver":"0.14.0""#)); let parsed: Metadata = serde_json::from_str(&json).unwrap(); - assert!(parsed.minver.is_none()); + assert_eq!( + parsed.kernels_minver, + Some(Version::from_str("0.14.0").unwrap()) + ); } #[test] - fn metadata_round_trips_minver() { + fn metadata_kernels_minver_bumped_for_kernel_depends() { let mut build = torch_noarch_build(); - build.general.minver = Some(Version::from_str("0.11.0").unwrap()); + build.general.kernel_depends = Some(vec![KernelDependency { + repo_id: "kernels-community/activation".to_string(), + version: KernelVersion::Version(1), + }]); let metadata = Metadata::for_backend(&build, "test-id".to_string(), Backend::Cuda).unwrap(); - let json = serde_json::to_string(&metadata).unwrap(); - assert!(json.contains(r#""minver":"0.11.0""#)); + assert_eq!( + metadata.kernels_minver, + Some(Version::from_str("0.17.0").unwrap()) + ); + } - let parsed: Metadata = serde_json::from_str(&json).unwrap(); - assert_eq!(parsed.minver, Some(Version::from_str("0.11.0").unwrap())); + #[test] + fn metadata_without_kernels_minver_parses() { + let json = r#"{"name":"test-kernel","id":"test-id","version":1,"license":"apache-2.0","python-depends":[],"backend":{"type":"cuda"}}"#; + let parsed: Metadata = serde_json::from_str(json).unwrap(); + assert!(parsed.kernels_minver.is_none()); } #[test] diff --git a/kernels-data/src/version.rs b/kernels-data/src/version.rs index 90908a85a..e9e8db30b 100644 --- a/kernels-data/src/version.rs +++ b/kernels-data/src/version.rs @@ -8,6 +8,13 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer, de}; #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub struct Version([usize; N]); +impl Version { + /// Construct a version from its components. + pub const fn new(parts: [usize; N]) -> Self { + Version(parts) + } +} + impl<'de, const N: usize> Deserialize<'de> for Version { fn deserialize(deserializer: D) -> Result where diff --git a/kernels/src/kernels/cli/info.py b/kernels/src/kernels/cli/info.py index 71f467e57..cfa020025 100644 --- a/kernels/src/kernels/cli/info.py +++ b/kernels/src/kernels/cli/info.py @@ -120,7 +120,7 @@ def _metadata_info(metadata: Metadata | None, variants: list[Variant]) -> dict: { "name": str(metadata.name), "version": metadata.version, - "minver": metadata.minver, + "kernels_minver": (str(metadata.kernels_minver) if metadata.kernels_minver is not None else None), "license": metadata.license, "upstream": metadata.upstream, "source": metadata.source, @@ -145,7 +145,7 @@ def value(v) -> str: print(f"Name: {value(info.get('name'))}") print(f"Version: {value(info.get('version'))}") - print(f"Minimum kernels version: {value(info.get('minver'))}") + print(f"Minimum kernels version: {value(info.get('kernels_minver'))}") print(f"License: {value(info.get('license'))}") print(f"Upstream: {value(info.get('upstream'))}") print(f"Source: {value(info.get('source'))}") diff --git a/kernels/src/kernels/importer.py b/kernels/src/kernels/importer.py index 1bf81fe49..efb5bf2f0 100644 --- a/kernels/src/kernels/importer.py +++ b/kernels/src/kernels/importer.py @@ -5,8 +5,8 @@ from pathlib import Path from types import ModuleType -from kernels_data import Metadata -from packaging.version import parse +from kernels_data import Metadata, Version +from packaging.version import InvalidVersion, parse from kernels.hf_hub import RepoInfo @@ -28,8 +28,8 @@ class LoadedKernel: - `id` (`str`): kernel identifier that is unique to the kernel version + backend. - `name` (`str`): the name of the kernel. - `version` (`int`): the version of the kernel. - - `minver` (`str | None`): the minimum `kernels` library version - required to load the kernel. + - `kernels_minver` (`Version | None`): the minimum `kernels` library + version required to load the kernel. - `license` (`str`): the license of the kernel. - `upstream` (`str | None`): the original upstream repository of the kernel. - `source` (`str | None`): the kernel-builder formatted source repository. @@ -94,18 +94,42 @@ def _warn_if_dirty(metadata: Metadata, variant_str: str) -> None: ) +def _installed_version() -> Version | None: + """The installed `kernels` version as a numeric version. + + Pre-release and development suffixes are stripped, since kernel metadata + only records release versions. Without stripping, a development version + like `0.17.0.dev0` would compare as older than the `0.17.0` it implements. + + Returns `None` if the installed version is not a PEP 440 version, which + can happen for versions derived from a VCS checkout. Such a version cannot + be compared, and this check must never make a kernel fail to load. + """ + # Avoid an import cycle. + from kernels import __version__ + + try: + release = parse(__version__).release + except InvalidVersion: + return None + + return Version.from_str(".".join(str(part) for part in release)) + + def _warn_if_below_minver(metadata: Metadata, variant_str: str) -> None: """Warn when the installed `kernels` library is older than the minimum version required by a kernel. """ - minver = metadata.minver + minver = metadata.kernels_minver if minver is None: return - # Avoid an import cycle. - from kernels import __version__ + installed = _installed_version() + if installed is not None and installed < minver: + # Report the verbatim installed version, not the normalized one used + # for comparison, so the message matches what `pip show` reports. + from kernels import __version__ - if parse(__version__) < parse(minver): warnings.warn( f"Kernel '{metadata.name}' variant '{variant_str}' requires " f"kernels>={minver}, but version {__version__} is installed. " diff --git a/kernels/tests/test_minver.py b/kernels/tests/test_minver.py index e40a805e3..c7a75ff96 100644 --- a/kernels/tests/test_minver.py +++ b/kernels/tests/test_minver.py @@ -1,11 +1,13 @@ import json +import re import pytest -from kernels_data import Metadata +from kernels_data import Metadata, Version -from kernels import __version__ +import kernels from kernels.importer import ( _import_from_path, + _installed_version, _loaded_kernels, _warn_if_below_minver, ) @@ -23,7 +25,7 @@ def _write_variant(tmp_path, minver): "backend": {"type": "cuda"}, } if minver is not None: - metadata["minver"] = minver + metadata["kernels-minver"] = minver (variant_dir / "metadata.json").write_text(json.dumps(metadata)) return variant_dir @@ -36,26 +38,68 @@ def test_no_warning_when_minver_met(tmp_path, recwarn, minver): assert len(recwarn) == 0 +def test_no_warning_for_dev_version_of_required_release(tmp_path, recwarn, monkeypatch): + # A development version implements the release it leads up to, so + # `0.17.0.dev0` must satisfy a `0.17.0` requirement. + monkeypatch.setattr(kernels, "__version__", "0.17.0.dev0") + variant_dir = _write_variant(tmp_path, "0.17.0") + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + _warn_if_below_minver(metadata, variant_dir.name) + assert len(recwarn) == 0 + + +@pytest.mark.parametrize( + "installed", + ["0.17.0.dev0", "0.17.0rc1", "0.17.0.post1", "0.17.0+cu121", "0.17"], +) +def test_installed_version_uses_release_segment(monkeypatch, installed): + monkeypatch.setattr(kernels, "__version__", installed) + assert _installed_version() == Version.from_str("0.17.0") + + +def test_installed_version_is_none_for_non_pep440_version(monkeypatch): + monkeypatch.setattr(kernels, "__version__", "0.17.0-dirty") + assert _installed_version() is None + + +def test_unparseable_installed_version_does_not_fail_load(tmp_path, recwarn, monkeypatch): + # A version that cannot be compared must not turn this advisory check into + # a hard failure. + monkeypatch.setattr(kernels, "__version__", "0.17.0-dirty") + variant_dir = _write_variant(tmp_path, "999.1.0") + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + _warn_if_below_minver(metadata, variant_dir.name) + assert len(recwarn) == 0 + + +def test_version_ordering_is_numeric_not_lexicographic(): + # `0.9 < 0.10` only holds for numeric comparison; string comparison would + # get this backwards. + assert Version.from_str("0.9") < Version.from_str("0.10") + assert Version.from_str("0.14") == Version.from_str("0.14.0") + assert Version.from_str("0.14.0") < Version.from_str("0.14.1") + + def test_warns_when_minver_not_met(tmp_path): - variant_dir = _write_variant(tmp_path, "999.0.0") + variant_dir = _write_variant(tmp_path, "999.1.0") metadata = Metadata.read_from_file(variant_dir / "metadata.json") - with pytest.warns(UserWarning, match="requires kernels>=999.0.0"): + with pytest.warns(UserWarning, match="requires kernels>=999.1"): _warn_if_below_minver(metadata, variant_dir.name) def test_warning_mentions_installed_version(tmp_path): - variant_dir = _write_variant(tmp_path, "999.0.0") + variant_dir = _write_variant(tmp_path, "999.1.0") metadata = Metadata.read_from_file(variant_dir / "metadata.json") - with pytest.warns(UserWarning, match=f"version {__version__} is installed"): + with pytest.warns(UserWarning, match=f"version {re.escape(kernels.__version__)} is installed"): _warn_if_below_minver(metadata, variant_dir.name) def test_import_from_path_warns_on_unmet_minver(tmp_path): - variant_dir = _write_variant(tmp_path, "999.0.0") + variant_dir = _write_variant(tmp_path, "999.1.0") (variant_dir / "__init__.py").write_text("value = 42\n") _loaded_kernels.pop(variant_dir, None) try: - with pytest.warns(UserWarning, match="requires kernels>=999.0.0"): + with pytest.warns(UserWarning, match="requires kernels>=999.1"): module = _import_from_path(variant_dir, deps={}) assert module.value == 42 finally: From 1a717bb4ce9db1d5c36e36a09f43fc5cd1cf157b Mon Sep 17 00:00:00 2001 From: David Holtz Date: Tue, 1 Sep 2026 11:28:46 -0400 Subject: [PATCH 5/7] fix: simplify versions --- kernels-data/bindings/python/kernels_data.pyi | 13 ++++--- kernels-data/bindings/python/src/version.rs | 34 +++++++++---------- .../python/tests/test_kernels_data.py | 2 +- .../bindings/python/tests/test_version.py | 25 +++++++++----- kernels/src/kernels/importer.py | 8 ++++- kernels/tests/test_minver.py | 7 ++-- 6 files changed, 49 insertions(+), 40 deletions(-) diff --git a/kernels-data/bindings/python/kernels_data.pyi b/kernels-data/bindings/python/kernels_data.pyi index 314b07190..58794fa21 100644 --- a/kernels-data/bindings/python/kernels_data.pyi +++ b/kernels-data/bindings/python/kernels_data.pyi @@ -133,17 +133,16 @@ class Provenance: @final class Version: - """A dotted numeric version (e.g. `12.8.0`). - - Trailing zeros are stripped during normalization. - """ + """A dotted numeric version (e.g. `12.8.0`).""" @staticmethod - def from_str(s: str) -> "Version": - """Parse a version string of the form `X`, `X.Y`, `X.Y.Z`, ... + def from_str(s: str, n_components: int) -> "Version": + """Parse a version string with exactly `n_components` dotted + components (e.g. `12.8.0` for `n_components=3`). Raises: - ValueError: If the string is empty or contains non-numeric parts. + ValueError: If the string is empty, contains non-numeric parts, + or does not have exactly `n_components` components. """ ... diff --git a/kernels-data/bindings/python/src/version.rs b/kernels-data/bindings/python/src/version.rs index 5a69163be..016900319 100644 --- a/kernels-data/bindings/python/src/version.rs +++ b/kernels-data/bindings/python/src/version.rs @@ -2,8 +2,7 @@ use kernels_data::version::Version; use pyo3::exceptions::PyValueError; use pyo3::prelude::*; -/// A dotted numeric version (e.g. `12.8.0`). Trailing zeros are stripped -/// during normalization. +/// A dotted numeric version (e.g. `12.8.0`). #[pyclass(name = "Version", frozen, eq, ord, hash)] #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub(crate) struct PyVersion { @@ -12,30 +11,21 @@ pub(crate) struct PyVersion { inner: Box<[usize]>, } -impl PyVersion { - /// Construct a version from its components, stripping trailing zeros so - /// that versions that only differ in trailing zeros compare as equal. - fn normalized(mut parts: Vec) -> Self { - let trailing_zeros = parts.iter().rev().take_while(|&&x| x == 0).count(); - parts.truncate(parts.len() - trailing_zeros); - Self { - inner: parts.into_boxed_slice(), - } - } -} - impl From> for PyVersion { fn from(version: Version) -> Self { - PyVersion::normalized(version.to_vec()) + Self { + inner: Box::from(&*version), + } } } #[pymethods] impl PyVersion { - /// Parse a version string of the form `X`, `X.Y`, `X.Y.Z`, ... + /// Parse a version string with exactly `n_components` dotted components + /// (e.g. `12.8.0` for `n_components=3`). #[staticmethod] #[pyo3(name = "from_str")] - fn py_from_str(s: &str) -> PyResult { + fn py_from_str(s: &str, n_components: usize) -> PyResult { let version = s.trim(); if version.is_empty() { return Err(PyValueError::new_err(format!( @@ -51,7 +41,15 @@ impl PyVersion { })?; parts.push(component); } - Ok(PyVersion::normalized(parts)) + if parts.len() != n_components { + return Err(PyValueError::new_err(format!( + "Version `{s}` has {} components, expected {n_components}", + parts.len() + ))); + } + Ok(PyVersion { + inner: parts.into_boxed_slice(), + }) } fn __str__(&self) -> String { diff --git a/kernels-data/bindings/python/tests/test_kernels_data.py b/kernels-data/bindings/python/tests/test_kernels_data.py index b80d9f182..2e1957fde 100644 --- a/kernels-data/bindings/python/tests/test_kernels_data.py +++ b/kernels-data/bindings/python/tests/test_kernels_data.py @@ -93,7 +93,7 @@ def test_metadata_load_full(tmp_path): assert m.id == "_my_kernel_8a3be8f" assert m.name == KernelName("my-kernel") assert m.version == 1 - assert m.kernels_minver == Version.from_str("0.17.0") + assert m.kernels_minver == Version.from_str("0.17.0", 3) assert m.license == "Apache-2.0" assert m.upstream == "https://github.com/example/kernel" assert m.source == "https://github.com/example/kernel-builder" diff --git a/kernels-data/bindings/python/tests/test_version.py b/kernels-data/bindings/python/tests/test_version.py index c91400f01..0579be77e 100644 --- a/kernels-data/bindings/python/tests/test_version.py +++ b/kernels-data/bindings/python/tests/test_version.py @@ -3,16 +3,16 @@ 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_parse(): + assert str(Version.from_str("12.8.0", 3)) == "12.8.0" + assert str(Version.from_str("1", 1)) == "1" + assert str(Version.from_str("1.2.3", 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") + v1 = Version.from_str("1.2.0", 3) + v2 = Version.from_str("1.2.0", 3) + v3 = Version.from_str("1.3.0", 3) assert v1 == v2 assert v1 < v3 assert hash(v1) == hash(v2) @@ -21,6 +21,13 @@ def test_version_ordering_and_hash(): def test_version_invalid(): with pytest.raises(ValueError): - Version.from_str("abc") + Version.from_str("abc", 1) with pytest.raises(ValueError): - Version.from_str("") + Version.from_str("", 1) + + +def test_version_must_have_expected_components(): + with pytest.raises(ValueError, match="has 2 components, expected 3"): + Version.from_str("1.2", 3) + with pytest.raises(ValueError, match="has 3 components, expected 2"): + Version.from_str("1.2.0", 2) diff --git a/kernels/src/kernels/importer.py b/kernels/src/kernels/importer.py index efb5bf2f0..9adb18ab9 100644 --- a/kernels/src/kernels/importer.py +++ b/kernels/src/kernels/importer.py @@ -113,7 +113,13 @@ def _installed_version() -> Version | None: except InvalidVersion: return None - return Version.from_str(".".join(str(part) for part in release)) + # `kernels_minver` always has three components, so bring the release + # segment to the same shape: pad shorter releases with zeros (PEP 440 + # treats `0.17` and `0.17.0` as the same version) and drop any component + # beyond the third (a `X.Y.Z.W` release is never older than `X.Y.Z`). + parts = (release + (0, 0, 0))[:3] + + return Version.from_str(".".join(str(part) for part in parts), 3) def _warn_if_below_minver(metadata: Metadata, variant_str: str) -> None: diff --git a/kernels/tests/test_minver.py b/kernels/tests/test_minver.py index c7a75ff96..c34352b7e 100644 --- a/kernels/tests/test_minver.py +++ b/kernels/tests/test_minver.py @@ -54,7 +54,7 @@ def test_no_warning_for_dev_version_of_required_release(tmp_path, recwarn, monke ) def test_installed_version_uses_release_segment(monkeypatch, installed): monkeypatch.setattr(kernels, "__version__", installed) - assert _installed_version() == Version.from_str("0.17.0") + assert _installed_version() == Version.from_str("0.17.0", 3) def test_installed_version_is_none_for_non_pep440_version(monkeypatch): @@ -75,9 +75,8 @@ def test_unparseable_installed_version_does_not_fail_load(tmp_path, recwarn, mon def test_version_ordering_is_numeric_not_lexicographic(): # `0.9 < 0.10` only holds for numeric comparison; string comparison would # get this backwards. - assert Version.from_str("0.9") < Version.from_str("0.10") - assert Version.from_str("0.14") == Version.from_str("0.14.0") - assert Version.from_str("0.14.0") < Version.from_str("0.14.1") + assert Version.from_str("0.9.0", 3) < Version.from_str("0.10.0", 3) + assert Version.from_str("0.14.0", 3) < Version.from_str("0.14.1", 3) def test_warns_when_minver_not_met(tmp_path): From 970224763f988a49cc8fa354b58922017ed1c280 Mon Sep 17 00:00:00 2001 From: David Holtz Date: Tue, 1 Sep 2026 11:48:40 -0400 Subject: [PATCH 6/7] fix: prefer handling missing version components in comparison traits --- kernels-data/bindings/python/kernels_data.pyi | 13 ++-- kernels-data/bindings/python/src/version.rs | 61 +++++++++++++++---- .../python/tests/test_kernels_data.py | 2 +- .../bindings/python/tests/test_version.py | 29 +++++---- kernels/src/kernels/importer.py | 8 +-- kernels/tests/test_minver.py | 7 ++- 6 files changed, 77 insertions(+), 43 deletions(-) diff --git a/kernels-data/bindings/python/kernels_data.pyi b/kernels-data/bindings/python/kernels_data.pyi index 58794fa21..6acab2dfa 100644 --- a/kernels-data/bindings/python/kernels_data.pyi +++ b/kernels-data/bindings/python/kernels_data.pyi @@ -133,16 +133,17 @@ class Provenance: @final class Version: - """A dotted numeric version (e.g. `12.8.0`).""" + """A dotted numeric version (e.g. `12.8.0`). + + Versions that only differ in trailing zeros compare as equal. + """ @staticmethod - def from_str(s: str, n_components: int) -> "Version": - """Parse a version string with exactly `n_components` dotted - components (e.g. `12.8.0` for `n_components=3`). + def from_str(s: str) -> "Version": + """Parse a version string of the form `X`, `X.Y`, `X.Y.Z`, ... Raises: - ValueError: If the string is empty, contains non-numeric parts, - or does not have exactly `n_components` components. + ValueError: If the string is empty or contains non-numeric parts. """ ... diff --git a/kernels-data/bindings/python/src/version.rs b/kernels-data/bindings/python/src/version.rs index 016900319..1d3c832eb 100644 --- a/kernels-data/bindings/python/src/version.rs +++ b/kernels-data/bindings/python/src/version.rs @@ -1,10 +1,14 @@ +use std::cmp::Ordering; +use std::hash::{Hash, Hasher}; + use kernels_data::version::Version; use pyo3::exceptions::PyValueError; use pyo3::prelude::*; -/// A dotted numeric version (e.g. `12.8.0`). +/// A dotted numeric version (e.g. `12.8.0`). Versions that only differ in +/// trailing zeros compare as equal. #[pyclass(name = "Version", frozen, eq, ord, hash)] -#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] +#[derive(Clone, Debug)] pub(crate) struct PyVersion { // We are storing this as a `Box<[usize]>` to allow storing kernels-data // `Version` in directly in `PyVersion` as well. @@ -19,13 +23,54 @@ impl From> for PyVersion { } } +// A parsed version string does not carry `Version`'s guarantee that both +// sides of a comparison have the same number of components, so compare +// component-wise with missing components counting as zero (`0.17` and +// `0.17.0` are the same version). +impl Ord for PyVersion { + fn cmp(&self, other: &Self) -> Ordering { + let n = self.inner.len().max(other.inner.len()); + for i in 0..n { + let lhs = self.inner.get(i).copied().unwrap_or(0); + let rhs = other.inner.get(i).copied().unwrap_or(0); + match lhs.cmp(&rhs) { + Ordering::Equal => (), + ordering => return ordering, + } + } + Ordering::Equal + } +} + +impl PartialOrd for PyVersion { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +impl PartialEq for PyVersion { + fn eq(&self, other: &Self) -> bool { + self.cmp(other) == Ordering::Equal + } +} + +impl Eq for PyVersion {} + +impl Hash for PyVersion { + fn hash(&self, state: &mut H) { + // Skip trailing zeros so that versions that compare as equal hash + // identically. + let trailing_zeros = self.inner.iter().rev().take_while(|&&x| x == 0).count(); + self.inner[..self.inner.len() - trailing_zeros].hash(state); + } +} + #[pymethods] impl PyVersion { - /// Parse a version string with exactly `n_components` dotted components - /// (e.g. `12.8.0` for `n_components=3`). + /// Parse a version string of the form `X`, `X.Y`, `X.Y.Z`, ... #[staticmethod] #[pyo3(name = "from_str")] - fn py_from_str(s: &str, n_components: usize) -> PyResult { + fn py_from_str(s: &str) -> PyResult { let version = s.trim(); if version.is_empty() { return Err(PyValueError::new_err(format!( @@ -41,12 +86,6 @@ impl PyVersion { })?; parts.push(component); } - if parts.len() != n_components { - return Err(PyValueError::new_err(format!( - "Version `{s}` has {} components, expected {n_components}", - parts.len() - ))); - } Ok(PyVersion { inner: parts.into_boxed_slice(), }) diff --git a/kernels-data/bindings/python/tests/test_kernels_data.py b/kernels-data/bindings/python/tests/test_kernels_data.py index 2e1957fde..b80d9f182 100644 --- a/kernels-data/bindings/python/tests/test_kernels_data.py +++ b/kernels-data/bindings/python/tests/test_kernels_data.py @@ -93,7 +93,7 @@ def test_metadata_load_full(tmp_path): assert m.id == "_my_kernel_8a3be8f" assert m.name == KernelName("my-kernel") assert m.version == 1 - assert m.kernels_minver == Version.from_str("0.17.0", 3) + assert m.kernels_minver == Version.from_str("0.17.0") assert m.license == "Apache-2.0" assert m.upstream == "https://github.com/example/kernel" assert m.source == "https://github.com/example/kernel-builder" diff --git a/kernels-data/bindings/python/tests/test_version.py b/kernels-data/bindings/python/tests/test_version.py index 0579be77e..1f153402f 100644 --- a/kernels-data/bindings/python/tests/test_version.py +++ b/kernels-data/bindings/python/tests/test_version.py @@ -4,30 +4,29 @@ def test_version_parse(): - assert str(Version.from_str("12.8.0", 3)) == "12.8.0" - assert str(Version.from_str("1", 1)) == "1" - assert str(Version.from_str("1.2.3", 3)) == "1.2.3" + assert str(Version.from_str("12.8.0")) == "12.8.0" + 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.0", 3) - v2 = Version.from_str("1.2.0", 3) - v3 = Version.from_str("1.3.0", 3) + 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_trailing_zeros_compare_equal(): + assert Version.from_str("0.17") == Version.from_str("0.17.0") + assert Version.from_str("0.17") < Version.from_str("0.17.1") + assert Version.from_str("0.17.0.1") > Version.from_str("0.17") + + def test_version_invalid(): with pytest.raises(ValueError): - Version.from_str("abc", 1) + Version.from_str("abc") with pytest.raises(ValueError): - Version.from_str("", 1) - - -def test_version_must_have_expected_components(): - with pytest.raises(ValueError, match="has 2 components, expected 3"): - Version.from_str("1.2", 3) - with pytest.raises(ValueError, match="has 3 components, expected 2"): - Version.from_str("1.2.0", 2) + Version.from_str("") diff --git a/kernels/src/kernels/importer.py b/kernels/src/kernels/importer.py index 9adb18ab9..efb5bf2f0 100644 --- a/kernels/src/kernels/importer.py +++ b/kernels/src/kernels/importer.py @@ -113,13 +113,7 @@ def _installed_version() -> Version | None: except InvalidVersion: return None - # `kernels_minver` always has three components, so bring the release - # segment to the same shape: pad shorter releases with zeros (PEP 440 - # treats `0.17` and `0.17.0` as the same version) and drop any component - # beyond the third (a `X.Y.Z.W` release is never older than `X.Y.Z`). - parts = (release + (0, 0, 0))[:3] - - return Version.from_str(".".join(str(part) for part in parts), 3) + return Version.from_str(".".join(str(part) for part in release)) def _warn_if_below_minver(metadata: Metadata, variant_str: str) -> None: diff --git a/kernels/tests/test_minver.py b/kernels/tests/test_minver.py index c34352b7e..c7a75ff96 100644 --- a/kernels/tests/test_minver.py +++ b/kernels/tests/test_minver.py @@ -54,7 +54,7 @@ def test_no_warning_for_dev_version_of_required_release(tmp_path, recwarn, monke ) def test_installed_version_uses_release_segment(monkeypatch, installed): monkeypatch.setattr(kernels, "__version__", installed) - assert _installed_version() == Version.from_str("0.17.0", 3) + assert _installed_version() == Version.from_str("0.17.0") def test_installed_version_is_none_for_non_pep440_version(monkeypatch): @@ -75,8 +75,9 @@ def test_unparseable_installed_version_does_not_fail_load(tmp_path, recwarn, mon def test_version_ordering_is_numeric_not_lexicographic(): # `0.9 < 0.10` only holds for numeric comparison; string comparison would # get this backwards. - assert Version.from_str("0.9.0", 3) < Version.from_str("0.10.0", 3) - assert Version.from_str("0.14.0", 3) < Version.from_str("0.14.1", 3) + assert Version.from_str("0.9") < Version.from_str("0.10") + assert Version.from_str("0.14") == Version.from_str("0.14.0") + assert Version.from_str("0.14.0") < Version.from_str("0.14.1") def test_warns_when_minver_not_met(tmp_path): From 74806977ef9cb8a5fa07057a2d82c78b073011e4 Mon Sep 17 00:00:00 2001 From: David Holtz Date: Tue, 1 Sep 2026 12:08:26 -0400 Subject: [PATCH 7/7] fix: add guard for older packaging version --- kernels/src/kernels/importer.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/kernels/src/kernels/importer.py b/kernels/src/kernels/importer.py index efb5bf2f0..8fa184700 100644 --- a/kernels/src/kernels/importer.py +++ b/kernels/src/kernels/importer.py @@ -113,6 +113,11 @@ def _installed_version() -> Version | None: except InvalidVersion: return None + # packaging < 22 returns a LegacyVersion with `release = None` for + # non-PEP 440 versions instead of raising `InvalidVersion`. + if release is None: + return None + return Version.from_str(".".join(str(part) for part in release))