diff --git a/docs/source/kernel-requirements.md b/docs/source/kernel-requirements.md index 4309f0c0..4c720022 100644 --- a/docs/source/kernel-requirements.md +++ b/docs/source/kernel-requirements.md @@ -69,6 +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. +- `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 53c29b7c..6acab2df 100644 --- a/kernels-data/bindings/python/kernels_data.pyi +++ b/kernels-data/bindings/python/kernels_data.pyi @@ -135,7 +135,7 @@ class Provenance: class Version: """A dotted numeric version (e.g. `12.8.0`). - Trailing zeros are stripped during normalization. + Versions that only differ in trailing zeros compare as equal. """ @staticmethod @@ -659,6 +659,13 @@ class Metadata: @property def version(self) -> Optional[int]: ... @property + 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 def upstream(self) -> Optional[str]: ... diff --git a/kernels-data/bindings/python/src/lib.rs b/kernels-data/bindings/python/src/lib.rs index 746cb84f..315c7133 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, + kernels_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, + 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()), @@ -465,6 +467,11 @@ impl PyMetadata { self.version } + #[getter] + fn kernels_minver(&self) -> Option { + self.kernels_minver.clone() + } + #[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={:?}, kernels_minver={:?}, license={:?}, upstream={:?}, source={:?}, python_depends={:?}, kernel_depends={:?}, backend={}, digest={}, provenance={})", self.id, self.name, self.version, + 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 ee6c7f08..1d3c832e 100644 --- a/kernels-data/bindings/python/src/version.rs +++ b/kernels-data/bindings/python/src/version.rs @@ -1,16 +1,70 @@ +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`). Trailing zeros are stripped -/// during normalization. +/// 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. inner: Box<[usize]>, } +impl From> for PyVersion { + fn from(version: Version) -> Self { + Self { + inner: Box::from(&*version), + } + } +} + +// 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 of the form `X`, `X.Y`, `X.Y.Z`, ... @@ -32,10 +86,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 { + 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 cca9b474..b80d9f18 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,6 +81,7 @@ def test_metadata_load_full(tmp_path): "version": 1, "name": "my-kernel", "license": "Apache-2.0", + "kernels-minver": "0.17.0", "upstream": "https://github.com/example/kernel", "source": "https://github.com/example/kernel-builder", "python-depends": ["torch"], @@ -91,6 +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.license == "Apache-2.0" assert m.upstream == "https://github.com/example/kernel" assert m.source == "https://github.com/example/kernel-builder" @@ -115,6 +118,7 @@ def test_metadata_load_minimal(tmp_path): ) m = Metadata.read_from_file(path) assert m.version == 1 + 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/bindings/python/tests/test_version.py b/kernels-data/bindings/python/tests/test_version.py index c91400f0..1f153402 100644 --- a/kernels-data/bindings/python/tests/test_version.py +++ b/kernels-data/bindings/python/tests/test_version.py @@ -3,8 +3,8 @@ from kernels_data import Version -def test_version_parse_and_normalize(): - assert str(Version.from_str("12.8.0")) == "12.8" +def test_version_parse(): + 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" @@ -19,6 +19,12 @@ def test_version_ordering_and_hash(): 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") diff --git a/kernels-data/src/config/mod.rs b/kernels-data/src/config/mod.rs index 80774aa5..d8d2888a 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 { @@ -545,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/metadata.rs b/kernels-data/src/metadata.rs index 0eb5a044..cbf4564e 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,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 kernels_minver: Option>, pub license: String, #[serde(skip_serializing_if = "Option::is_none")] pub upstream: Option, @@ -125,6 +130,7 @@ impl Metadata { id, name: build.general.name.clone(), version: build.general.version, + kernels_minver: Some(build.required_kernels_version(backend)), license: build.general.license.clone(), upstream: build.general.upstream.clone(), source: build.general.source.clone(), @@ -163,8 +169,12 @@ 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; use super::{KernelBuilderVersion, Metadata, Provenance}; @@ -303,6 +313,43 @@ mod tests { assert!(parsed.provenance.is_none()); } + #[test] + 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(r#""kernels-minver":"0.14.0""#)); + + let parsed: Metadata = serde_json::from_str(&json).unwrap(); + assert_eq!( + parsed.kernels_minver, + Some(Version::from_str("0.14.0").unwrap()) + ); + } + + #[test] + fn metadata_kernels_minver_bumped_for_kernel_depends() { + let mut build = torch_noarch_build(); + 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(); + + assert_eq!( + metadata.kernels_minver, + Some(Version::from_str("0.17.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] fn metadata_always_emits_kernel_depends() { let build = torch_noarch_build(); diff --git a/kernels-data/src/version.rs b/kernels-data/src/version.rs index 90908a85..e9e8db30 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 6619ab50..cfa02002 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, + "kernels_minver": (str(metadata.kernels_minver) if metadata.kernels_minver is not None else None), "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('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 4b39498c..8fa18470 100644 --- a/kernels/src/kernels/importer.py +++ b/kernels/src/kernels/importer.py @@ -5,7 +5,8 @@ from pathlib import Path from types import ModuleType -from kernels_data import Metadata +from kernels_data import Metadata, Version +from packaging.version import InvalidVersion, 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. + - `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. @@ -91,6 +94,56 @@ 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 + + # 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)) + + +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.kernels_minver + if minver is None: + return + + 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__ + + 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 +154,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 00000000..c7a75ff9 --- /dev/null +++ b/kernels/tests/test_minver.py @@ -0,0 +1,106 @@ +import json +import re + +import pytest +from kernels_data import Metadata, Version + +import kernels +from kernels.importer import ( + _import_from_path, + _installed_version, + _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["kernels-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_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.1.0") + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + 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.1.0") + metadata = Metadata.read_from_file(variant_dir / "metadata.json") + 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.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.1"): + module = _import_from_path(variant_dir, deps={}) + assert module.value == 42 + finally: + _loaded_kernels.pop(variant_dir, None)