-
Notifications
You must be signed in to change notification settings - Fork 105
feat: append builder info to detect builds made with uncommitted changes in the builder #666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
0379597
a6fa638
b638689
cd94930
27a2009
b6e800e
2f18556
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,52 @@ | ||
| use std::process::Command; | ||
|
|
||
| fn main() { | ||
| minijinja_embed::embed_templates!("src/pyproject/templates"); | ||
| emit_git_info(); | ||
| } | ||
|
|
||
| fn emit_git_info() { | ||
| let sha = env_var("KERNEL_BUILDER_GIT_SHA").or_else(|| git_output(&["rev-parse", "HEAD"])); | ||
| if let Some(sha) = sha { | ||
| println!("cargo:rustc-env=KERNEL_BUILDER_GIT_SHA={sha}"); | ||
| } | ||
|
|
||
| let dirty = match env_var("KERNEL_BUILDER_GIT_DIRTY") { | ||
| Some(value) => is_truthy(&value), | ||
| // Only consider tracked files; untracked files (e.g. generated build | ||
| // artifacts) should not mark the build as dirty. | ||
| None => git_output(&["status", "--porcelain", "--untracked-files=no"]) | ||
| .map(|out| !out.is_empty()) | ||
| .unwrap_or(false), | ||
| }; | ||
| println!( | ||
| "cargo:rustc-env=KERNEL_BUILDER_GIT_DIRTY={}", | ||
| if dirty { "1" } else { "0" } | ||
| ); | ||
|
|
||
| println!("cargo:rerun-if-env-changed=KERNEL_BUILDER_GIT_SHA"); | ||
| println!("cargo:rerun-if-env-changed=KERNEL_BUILDER_GIT_DIRTY"); | ||
| println!("cargo:rerun-if-changed=../.git/HEAD"); | ||
| println!("cargo:rerun-if-changed=../.git/index"); | ||
| } | ||
|
|
||
| fn env_var(name: &str) -> Option<String> { | ||
| std::env::var(name).ok().filter(|s| !s.is_empty()) | ||
| } | ||
|
|
||
| fn is_truthy(value: &str) -> bool { | ||
| value == "1" || value.eq_ignore_ascii_case("true") | ||
| } | ||
|
|
||
| fn git_output(args: &[&str]) -> Option<String> { | ||
| let output = Command::new("git").args(args).output().ok()?; | ||
| if !output.status.success() { | ||
| return None; | ||
| } | ||
| let s = String::from_utf8_lossy(&output.stdout).trim().to_string(); | ||
| if s.is_empty() { | ||
| None | ||
| } else { | ||
| Some(s) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,7 @@ use eyre::Result; | |
| use itertools::Itertools; | ||
|
|
||
| use kernels_data::config::{Backend, General}; | ||
| use kernels_data::metadata::{BackendInfo, Metadata}; | ||
| use kernels_data::metadata::{BackendInfo, BuildInfo, KernelBuilderInfo, Metadata}; | ||
|
|
||
| use crate::pyproject::ops_identifier::KernelIdentifier; | ||
| use crate::pyproject::FileSet; | ||
|
|
@@ -20,11 +20,30 @@ pub fn write_compat_py(file_set: &mut FileSet) -> Result<()> { | |
| Ok(()) | ||
| } | ||
|
|
||
| fn kernel_builder_info() -> KernelBuilderInfo { | ||
| KernelBuilderInfo { | ||
| version: env!("CARGO_PKG_VERSION").to_owned(), | ||
| sha: option_env!("KERNEL_BUILDER_GIT_SHA").map(str::to_owned), | ||
| dirty: matches!(option_env!("KERNEL_BUILDER_GIT_DIRTY"), Some("1")), | ||
| } | ||
| } | ||
|
|
||
| pub fn write_metadata( | ||
| general: &General, | ||
| kernel_id: &KernelIdentifier, | ||
| file_set: &mut FileSet, | ||
| ) -> Result<()> { | ||
| // Prefer externally-provided `kernel-builder` provenance (e.g. from Nix), | ||
| // falling back to the provenance baked in at compile time. | ||
| let kernel_builder = kernel_id | ||
| .kernel_builder() | ||
| .cloned() | ||
| .unwrap_or_else(kernel_builder_info); | ||
|
Comment on lines
+36
to
+41
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems the wrong place to do this. The builder's hash/version should be burned into the builder, so we should adjust the derivation to give the build access to it. |
||
| let build_info = BuildInfo { | ||
| kernel_builder: Some(kernel_builder), | ||
| kernel: kernel_id.git_info().cloned(), | ||
| }; | ||
|
|
||
| for backend in &Backend::all() { | ||
| let writer = file_set.entry(format!("metadata-{backend}.json")); | ||
|
|
||
|
|
@@ -51,6 +70,7 @@ pub fn write_metadata( | |
| backend_type: *backend, | ||
| }, | ||
| digest: None, | ||
| build_info: Some(build_info.clone()), | ||
| }; | ||
|
|
||
| serde_json::to_writer_pretty(writer, &metadata)?; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ use std::{ | |
|
|
||
| use eyre::{bail, Result}; | ||
| use kernels_data::config::{Build, Framework}; | ||
| use kernels_data::metadata::{GitInfo, KernelBuilderInfo}; | ||
| use minijinja::Environment; | ||
|
|
||
| use crate::{ | ||
|
|
@@ -39,16 +40,38 @@ pub fn create_pyproject_file_set(build: Build, kernel_id: &KernelIdentifier) -> | |
| Ok(file_set) | ||
| } | ||
|
|
||
| #[allow(clippy::too_many_arguments)] | ||
| pub fn create_pyproject( | ||
| kernel_dir: Option<PathBuf>, | ||
| target_dir: Option<PathBuf>, | ||
| force: bool, | ||
| unique_id: Option<String>, | ||
| kernel_sha: Option<String>, | ||
| kernel_dirty: bool, | ||
|
Comment on lines
+49
to
+50
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can lead to incorrect metadata in case someone generates This is currently also an issue for the sha hash we use in the ops name. We are generating it when making the project files, but it should probably be done inside CMake (but would better be a separate PR to avoid PRs becoming too big). |
||
| kernel_builder_sha: Option<String>, | ||
| kernel_builder_dirty: bool, | ||
| ) -> Result<()> { | ||
| let kernel_dir = check_or_infer_kernel_dir(kernel_dir)?; | ||
| let target_dir = check_or_infer_target_dir(&kernel_dir, target_dir)?; | ||
| let build = parse_build(&kernel_dir)?; | ||
| let kernel_id = KernelIdentifier::new(&kernel_dir, build.general.name.python_name(), unique_id); | ||
| let git_override = kernel_sha.map(|sha| GitInfo { | ||
| sha, | ||
| dirty: kernel_dirty, | ||
| }); | ||
| // The version is always that of the running `kernel-builder`; only the | ||
| // git provenance is supplied externally (e.g. by Nix). | ||
| let kernel_builder_override = kernel_builder_sha.map(|sha| KernelBuilderInfo { | ||
| version: env!("CARGO_PKG_VERSION").to_owned(), | ||
| sha: Some(sha), | ||
| dirty: kernel_builder_dirty, | ||
| }); | ||
| let kernel_id = KernelIdentifier::new( | ||
| &kernel_dir, | ||
| build.general.name.python_name(), | ||
| unique_id, | ||
| git_override, | ||
| kernel_builder_override, | ||
| ); | ||
| let file_set = create_pyproject_file_set(build, &kernel_id)?; | ||
| file_set.write(&target_dir, force)?; | ||
|
|
||
|
|
@@ -65,7 +88,14 @@ pub fn clean_pyproject( | |
| let kernel_dir = check_or_infer_kernel_dir(kernel_dir)?; | ||
| let target_dir = check_or_infer_target_dir(&kernel_dir, target_dir)?; | ||
| let build = parse_build(&kernel_dir)?; | ||
| let kernel_id = KernelIdentifier::new(&kernel_dir, build.general.name.python_name(), unique_id); | ||
| // Provenance is irrelevant when computing the set of files to clean. | ||
| let kernel_id = KernelIdentifier::new( | ||
| &kernel_dir, | ||
| build.general.name.python_name(), | ||
| unique_id, | ||
| None, | ||
| None, | ||
| ); | ||
|
|
||
| let generated_files = create_pyproject_file_set(build, &kernel_id)?.into_names(); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth using
builtover our own custom code:https://crates.io/crates/built
Also uses
git2rather than shelling out, which is generally more reliable than shelling out (e.g.gitmay not be available in a build sandbox).Also seems to be used by at least one very high-profile crate (rav1e).