feat: add a toolchains field to checks
commit
e8e1d25feat: add a toolchains field to checks
A check names the toolchains (git-toolchain, refs/meta/toolchains/name) its command needs, validated as ref-path segments the same way other collection keys are; existence of the named ref is left to the worker at job time since it lives in a different namespace the order function has no visibility into.
feat: render a check toolchains list alongside its image and dependencies Assisted-by: Claude:claude-sonnet-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
docs/spec/checks.adoc
@@ -5,8 +5,9 @@
--
The configured checks for a repository MUST be stored at `refs/meta/checks` as
a map from check name to a check body holding an optional shell command, an
-optional sandbox image, and an optional list of dependencies (names of sibling
-checks that must pass first).
+optional sandbox image, an optional list of dependencies (names of sibling
+checks that must pass first), and an optional list of toolchain names
+(<<checks.toolchains>>).
A check with no command is a composite: it runs nothing itself and derives its
outcome from its dependencies alone.
A check's definition living on a meta ref means a branch under check cannot
@@ -15,10 +16,22 @@
The dependency graph is fully static — no conditional edges, no runtime
expansion — and MUST be validated when the set is written: a dependency naming
no configured check, a duplicate or self edge, a check with neither a command
-nor dependencies, and any dependency cycle MUST each be rejected before the
-set is stored. A check that sets an image MUST be rejected until the sandbox
-can honor one, rather than the image being silently ignored; the field is
-reserved in the format so honoring it later is not a data migration.
+nor dependencies, any dependency cycle, and a toolchain name that is not a
+valid ref-path segment MUST each be rejected before the set is stored. A check
+that sets an image MUST be rejected until the sandbox can honor one, rather
+than the image being silently ignored; the field is reserved in the format so
+honoring it later is not a data migration.
+--
+
+[role="requirement", id="checks.toolchains"]
+.Check Toolchains
+--
+A check's optional list of toolchain names MUST each be a valid ref-path
+segment (<<storage.meta-ref>>'s collection-key rule), validated when the set
+is written; whether a named toolchain actually exists MUST be checked
+server-side at job time instead, since it names an entry in a different ref
+namespace (`refs/meta/toolchains/<name>`, `git-toolchain`) the check set
+itself does not enumerate.
--
[role="requirement", id="checks.post-receive"]
crates/git-ents-core/src/checks.rs
@@ -45,6 +45,10 @@
/// Names of sibling checks that must pass before this one runs. Stored as
/// `None` when empty so an independent check stays a minimal tree.
depends: Option<Vec<String>>,
+ /// Names of toolchains (`git-toolchain`, `refs/meta/toolchains/<name>`)
+ /// activated on `PATH` before the command runs. Stored as `None` when
+ /// empty, like `depends`.
+ toolchains: Option<Vec<String>>,
}
/// One configured check, assembled from its map key and [`CheckBody`] at load.
@@ -59,6 +63,8 @@
pub image: Option<String>,
/// Names of sibling checks that must pass before this one runs.
pub depends: Vec<String>,
+ /// Names of toolchains activated on `PATH` before the command runs.
+ pub toolchains: Vec<String>,
}
impl component::MapDocument for Check {
@@ -71,6 +77,7 @@
command: body.command,
image: body.image,
depends: body.depends.unwrap_or_default(),
+ toolchains: body.toolchains.unwrap_or_default(),
}
}
@@ -85,6 +92,11 @@
} else {
Some(self.depends.clone())
},
+ toolchains: if self.toolchains.is_empty() {
+ None
+ } else {
+ Some(self.toolchains.clone())
+ },
},
)
}
@@ -116,10 +128,14 @@
///
/// Rejected here, at write time, so the worker only ever walks a fixed order:
/// a `depends` entry naming no configured check, a duplicate or self edge, a
-/// check with neither a command nor dependencies, and any dependency cycle
-/// (reported with its member names). A check that sets an `image` is also
-/// rejected until the Sprite sandbox can honor one — the field exists in the
-/// format now so supporting it later is not a data migration.
+/// check with neither a command nor dependencies, any dependency cycle
+/// (reported with its member names), and a `toolchains` entry that is not a
+/// valid ref-path segment. A check that sets an `image` is also rejected
+/// until the Sprite sandbox can honor one — the field exists in the format
+/// now so supporting it later is not a data migration. Whether a named
+/// toolchain actually exists is checked server-side at job time, not here —
+/// unlike `depends`, `toolchains` cross-references a different ref
+/// namespace this function has no set of configured names to check against.
pub fn order(checks: &[Check]) -> Result<Vec<&Check>, String> {
let mut by_name: std::collections::BTreeMap<&str, &Check> = std::collections::BTreeMap::new();
for check in checks {
@@ -141,6 +157,14 @@
check.name
));
}
+ for toolchain in &check.toolchains {
+ if !git_store::ref_segment_ok(toolchain) {
+ return Err(format!(
+ "check {} names an invalid toolchain {toolchain:?}",
+ check.name
+ ));
+ }
+ }
let mut seen = std::collections::BTreeSet::new();
for dep in &check.depends {
if !by_name.contains_key(dep.as_str()) {
@@ -402,6 +426,7 @@
command: Some(command.to_owned()),
image: None,
depends: Vec::new(),
+ toolchains: Vec::new(),
}
}
@@ -411,6 +436,7 @@
command: None,
image: None,
depends: depends.iter().map(|dep| (*dep).to_owned()).collect(),
+ toolchains: Vec::new(),
}
}
@@ -421,6 +447,13 @@
}
}
+ fn toolchained(name: &str, command: &str, toolchains: &[&str]) -> Check {
+ Check {
+ toolchains: toolchains.iter().map(|t| (*t).to_owned()).collect(),
+ ..check(name, command)
+ }
+ }
+
#[test]
fn store_then_load_round_trips_the_check_set() {
let repo = unique_repo();
@@ -458,10 +491,10 @@
#[test]
fn loads_the_on_disk_checks_format() {
// A fixture written as the real `checks/<name>/command/some` subtree
- // layout (the `Option`-wrapped command, with `image`/`depends` omitted
- // entirely) must keep loading, with the missing optional fields unset —
- // guarding the checks document's shape against an incompatible change
- // to data already on a ref.
+ // layout (the `Option`-wrapped command, with `image`/`depends`/
+ // `toolchains` omitted entirely) must keep loading, with the missing
+ // optional fields unset — guarding the checks document's shape
+ // against an incompatible change to data already on a ref.
let repo = unique_repo();
write_checks_doc(
&repo,
@@ -676,4 +709,33 @@
"unexpected error: {err}"
);
}
+
+ #[test]
+ fn order_accepts_a_valid_toolchain_name() {
+ let checks = vec![toolchained("build", "make", &["gcc-12"])];
+ assert_eq!(
+ order(&checks)
+ .unwrap()
+ .iter()
+ .map(|c| c.name.as_str())
+ .collect::<Vec<_>>(),
+ vec!["build"]
+ );
+ }
+
+ #[test]
+ fn order_rejects_an_invalid_toolchain_name() {
+ let checks = vec![toolchained("build", "make", &["not/valid"])];
+ let err = order(&checks).unwrap_err();
+ assert!(err.contains("invalid toolchain"), "unexpected error: {err}");
+ }
+
+ #[test]
+ fn store_then_load_round_trips_toolchains() {
+ let repo = unique_repo();
+ let written = vec![toolchained("build", "make", &["gcc-12", "cmake"])];
+ store(&repo, &written).unwrap();
+ assert_eq!(load(&repo).unwrap(), written);
+ let _ = std::fs::remove_dir_all(&repo);
+ }
}
crates/git-ents/src/main.rs
@@ -724,6 +724,7 @@
command,
image,
depends,
+ toolchains: Vec::new(),
});
let _ordered = checks::order(&checks)?;
checks::store(&repo, &checks).map_err(|error| error.to_string())?;
crates/git-ents-server/src/web/render.rs
@@ -32,9 +32,9 @@
}
/// A check's name is the key and its command the value — `(composite)` for a
-/// check with none — with its image and dependencies appended as ` · `-joined
-/// annotations rather than the raw `Option`/`Vec` the structural walk would
-/// print.
+/// check with none — with its image, dependencies, and toolchains appended
+/// as ` · `-joined annotations rather than the raw `Option`/`Vec` the
+/// structural walk would print.
impl Render for Check {
fn render(&self) -> Markup {
let mut value = self
@@ -47,6 +47,9 @@
if !self.depends.is_empty() {
value.push_str(&format!(" · needs {}", self.depends.join(", ")));
}
+ if !self.toolchains.is_empty() {
+ value.push_str(&format!(" · toolchains {}", self.toolchains.join(", ")));
+ }
row(&self.name, &value)
}
}