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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build2cmake/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ impl Build {

pub struct General {
pub name: String,
pub version: Option<usize>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

likely overly pedantic, but maybe we want to be more strict on the version and disallow negative numbers or 0? additionally maybe we want to use a less ambiguous type like u32.

conversely maybe we want a more open ended type like a string? just thinking out loud here..

curious to hear your thoughts on the type?

Copy link
Member Author

@danieldk danieldk Jan 14, 2026

Choose a reason for hiding this comment

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

Certainly not overly pedantic, I changed it a few times while working on this.

but maybe we want to be more strict on the version and disallow negative numbers or 0?

Negative numbers are already disallowed, >= 0 I feel adds the unnecessary complication of having to use NonZero. I think it's also a nice number to show that the kernel is still in the initial stages.

conversely maybe we want a more open ended type like a string?

I find this a hard one. Initially I had a string and some additional validation to check that the string can be parsed as usize. When it was still called a channel, my idea was that you could also make testing and development channels and you would put something like "foo-test" here, it would get uploaded to the channel channel-foo-test.

The primary objections I can think of are: making it a string could confuse people into thinking they could put a full dotted version there and it makes you have to do more parsing and validation. But the objections are not particularly strong.

That said, I think we could easily transition to a string in the future by either changing the type to an enum or by upgrading to version v4 of the configuration. Using an enum could even be semantically clearer: either it is a version or it is something else (that we haven't thought of yet).


/// Hugging Face Hub license identifier.
pub license: Option<String>,
Expand Down
1 change: 1 addition & 0 deletions build2cmake/src/config/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ impl TryFrom<Build> for super::Build {
Ok(Self {
general: super::General {
name: build.general.name,
version: None,
license: None,
backends,
hub: None,
Expand Down
1 change: 1 addition & 0 deletions build2cmake/src/config/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ impl General {

super::General {
name: general.name,
version: None,
license: None,
backends,
cuda,
Expand Down
4 changes: 4 additions & 0 deletions build2cmake/src/config/v3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ pub struct Build {
pub struct General {
pub name: String,

pub version: Option<usize>,

pub license: Option<String>,

pub backends: Vec<Backend>,
Expand Down Expand Up @@ -143,6 +145,7 @@ impl From<General> for super::General {
fn from(general: General) -> Self {
Self {
name: general.name,
version: general.version,
license: general.license,
backends: general.backends.into_iter().map(Into::into).collect(),
cuda: general.cuda.map(Into::into),
Expand Down Expand Up @@ -296,6 +299,7 @@ impl From<super::General> for General {
fn from(general: super::General) -> Self {
Self {
name: general.name,
version: general.version,
license: general.license,
backends: general.backends.into_iter().map(Into::into).collect(),
cuda: general.cuda.map(Into::into),
Expand Down
2 changes: 2 additions & 0 deletions build2cmake/src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ use serde::{Deserialize, Serialize};
#[derive(Debug, Deserialize, Serialize)]
#[serde(deny_unknown_fields, rename_all = "kebab-case")]
pub struct Metadata {
#[serde(skip_serializing_if = "Option::is_none")]
pub version: Option<usize>,
#[serde(skip_serializing_if = "Option::is_none")]
pub license: Option<String>,
pub python_depends: Vec<String>,
Expand Down
3 changes: 2 additions & 1 deletion build2cmake/src/torch/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,12 @@ pub fn write_metadata(backend: Backend, general: &General, file_set: &mut FileSe
.collect::<Result<Vec<_>>>()?;

let metadata = Metadata {
version: general.version,
license: general.license.clone(),
python_depends,
};

serde_json::to_writer(writer, &metadata)?;
serde_json::to_writer_pretty(writer, &metadata)?;

Ok(())
}
8 changes: 6 additions & 2 deletions docs/writing-kernels.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ depends = [ "torch" ]

- `name` (required): the name of the kernel. The Python code for a Torch
extension must be stored in `torch-ext/<name>`.
- `version` (int, **experimental**): the major version of the kernel.
The version is written to the kernel's `metadata.json` and is used
by the `kernels upload` command to upload the kernel to a version
branch named `v<version>`.
- `backends` (required): a list of supported backends. Must be one or
more of `cpu`, `cuda`, `metal`, `rocm`, or `xpu`.
- `python-depends` (**experimental**): a list of additional Python dependencies
Expand Down Expand Up @@ -124,9 +128,9 @@ options:
- `include` (optional): include directories relative to the project root.
Default: `[]`.
- `maxver` (optional): only build for this Torch version and earlier. Use cautiously, since this option produces
non-compliant kernels if the version range does not correspond to the [required variants](build-variants.md).
non-compliant kernels if the version range does not correspond to the [required variants](build-variants.md).
- `minver` (optional): only build for this Torch version and later. Use cautiously, since this option produces
non-compliant kernels if the version range does not correspond to the [required variants](build-variants.md).
non-compliant kernels if the version range does not correspond to the [required variants](build-variants.md).

### `kernel.<name>`

Expand Down
5 changes: 3 additions & 2 deletions lib/gen-flake-outputs.nix
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,13 @@ in
"hub"
"repo-id"
] "kernels-community/${buildToml.general.name}" buildToml;
branch = lib.attrByPath [ "general" "hub" "branch" ] "main" buildToml;
branch = lib.attrByPath [ "general" "hub" "branch" ] null buildToml;
branchOpt = lib.optionalString (branch != null) "--branch ${branch}";
Comment on lines +200 to +201
Copy link
Collaborator

Choose a reason for hiding this comment

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

qq since lib.attrByPath [ "general" "hub" "branch" ] is an integer, for example 1, i'm not sure why this gets uploaded to v1, instead of just 1 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

general.hub.branch is still a stringly-typed branch. If it is not set (the normal case), then the --branch option is not passed, so it will be a regular kernels upload repo_id. In that case, the version upload code in kernels upload will be used:

https://github.com/huggingface/kernels/blob/ff4de4eafdfe5c2346b252cf09c6c412e13f3135/src/kernels/upload.py#L48

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah makes sense! thanks for pointing that out

# `kernels upload` fails when there are no build variants to upload.
# However, we do not want this command to error out in that case, so
# only insert the upload command when there is something to upload.
uploadStr = lib.optionalString (applicableBuildSets != [ ]) ''
${pkgs.python3.pkgs.kernels}/bin/kernels upload --repo-id ${repo_id} --branch ${branch} ${bundle}
${pkgs.python3.pkgs.kernels}/bin/kernels upload --repo-id ${repo_id} ${branchOpt} ${bundle}
'';
in
writeScriptBin "build-and-upload" ''
Expand Down
4 changes: 2 additions & 2 deletions overlay.nix
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,8 @@ in
src = final.fetchFromGitHub {
owner = "huggingface";
repo = "kernels";
tag = "v${version}";
hash = "sha256-kTBGje4oMiRqN/m98rvg3r3gqoV1Tg5APleRZbPlziY=";
rev = "ff4de4eafdfe5c2346b252cf09c6c412e13f3135";
hash = "sha256-hn9PZsGnd2BsBejjDQXVKye8abnS5K9jWga7YVkvXOg=";
};
});

Expand Down
Loading