git-ents.gitmain
⌘K
foforge
commit cb6f1bd
feat: reshape toolchains into a typed bin/src/license document

A toolchain was a bare imported directory tree with no structure git could check: bin/ was a convention the sprite’s PATH activation just assumed, and there was nowhere to record a license. Toolchain is now a Facet document — bin: RawTree, src: Option<RawTree>, license: String — so the shape is explicit and the license is mandatory. bin and src stay raw-passthrough trees (via the new facet-git-tree RawTree) rather than Facet-modeled directories, since an imported tree’s internal layout is arbitrary.

This changes refs/meta/toolchains/<name>'s on-disk shape — breaking for any existing imported toolchain. No fallback: pre-1.0, the fix is to re-import.

feat: require a non-empty bin directory and license on toolchain import, add optional src feat: activate a toolchain’s typed bin tree on PATH instead of an assumed bin/ layout feat: reshape the toolchain import/export CLI around bin/src/license Assisted-by: Claude:claude-sonnet-5

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

Cargo.lock @@ -1514,6 +1514,8 @@ name = "git-toolchain" version = "0.0.0" dependencies = [ + "facet", + "facet-git-tree", "git-store", "gix", "tempfile",
crates/git-toolchain/Cargo.toml @@ -6,6 +6,8 @@ license.workspace = true [dependencies] +facet = { workspace = true } +facet-git-tree = { workspace = true } git-store = { workspace = true } gix = { workspace = true } thiserror = { workspace = true }
crates/git-ents-server/src/checks.rs @@ -553,10 +553,10 @@ } /// Resolve and extract every distinct toolchain named across `runnable`, -/// returning each name's extracted directory inside the Sprite. A failed -/// resolution (the named ref does not exist) is the one place `checks::order` -/// could not have caught it, since `refs/meta/toolchains/*` is a different -/// namespace than the check set itself. +/// returning each name's extracted `bin` directory inside the Sprite. A +/// failed resolution (the named ref does not exist) is the one place +/// `checks::order` could not have caught it, since `refs/meta/toolchains/*` +/// is a different namespace than the check set itself. fn resolve_toolchains( repo: &Path, sprite: &str, @@ -571,17 +571,18 @@ let mut dirs = HashMap::new(); for name in names { - let tree = git_toolchain::resolve(repo, name) + let toolchain = git_toolchain::resolve(repo, name) .map_err(|e| format!("could not resolve toolchain {name}: {e}"))?; - sync_toolchain(repo, sprite, tree)?; - dirs.insert(name.to_owned(), format!("{TOOLCHAINS_DIR}/{tree}")); + let bin = toolchain.bin.oid(); + sync_toolchain(repo, sprite, bin)?; + dirs.insert(name.to_owned(), format!("{TOOLCHAINS_DIR}/{bin}")); } Ok(dirs) } /// Prefix `command` with a `PATH` export activating `toolchains`' extracted -/// directories, declared order first (so the first-listed toolchain's `bin` -/// wins on a name collision); a check with no toolchains is returned +/// `bin` directories, declared order first (so the first-listed toolchain's +/// `bin` wins on a name collision); a check with no toolchains is returned /// unchanged. fn activate(command: &str, toolchains: &[String], dirs: &HashMap<String, String>) -> String { if toolchains.is_empty() { @@ -590,7 +591,7 @@ let path = toolchains .iter() .filter_map(|name| dirs.get(name)) - .map(|dir| format!("{dir}/bin")) + .map(String::as_str) .collect::<Vec<_>>() .join(":"); format!("export PATH={path}:$PATH; {command}") @@ -874,7 +875,7 @@ let toolchains = vec!["gcc".to_owned(), "cmake".to_owned()]; assert_eq!( activate("make", &toolchains, &dirs), - "export PATH=/toolchains/aaa/bin:/toolchains/bbb/bin:$PATH; make" + "export PATH=/toolchains/aaa:/toolchains/bbb:$PATH; make" ); }
crates/git-ents/src/main.rs @@ -235,9 +235,17 @@ /// Name to record the toolchain under (`toolchains/<name>`). #[facet(args::positional, default)] name: Option<String>, - /// Directory to import. + /// Directory of executables to import, activated on `PATH` when a + /// check requests this toolchain. #[facet(args::positional, default)] - path: Option<String>, + bin: Option<String>, + /// Directory of source to import alongside `bin`, if any — kept for + /// provenance, never activated on `PATH`. + #[facet(args::named)] + src: Option<String>, + /// License covering `bin` (and `src`, if given). + #[facet(args::named, default)] + license: Option<String>, }, /// List the toolchains configured on a remote. List, @@ -447,32 +455,49 @@ fn run_toolchain(action: ToolchainAction, remote: &str) -> Result<(), String> { match action { - ToolchainAction::Import { name, path } => toolchain_import(name, path, remote), + ToolchainAction::Import { + name, + bin, + src, + license, + } => toolchain_import(name, bin, src, license, remote), ToolchainAction::List => toolchain_list(remote), ToolchainAction::Export { name, dest } => toolchain_export(&name, &dest, remote), ToolchainAction::Remove { name } => toolchain_remove(&name, remote), } } -/// Import `path`'s contents as toolchain `name` on `remote` and push it. -/// Prompts for any field left unset when run at an interactive terminal. +/// Import `bin`'s (and, optionally, `src`'s) contents as toolchain `name` on +/// `remote` and push it. Prompts for any field left unset when run at an +/// interactive terminal. fn toolchain_import( name: Option<String>, - path: Option<String>, + bin: Option<String>, + src: Option<String>, + license: Option<String>, remote: &str, ) -> Result<(), String> { let name = interactive::text_or(name, "Toolchain name")?; - let path = interactive::text_or(path, "Directory to import")?; + let bin = interactive::text_or(bin, "Directory of executables to import")?; + let src = interactive::optional_text_or(src, "Directory of source to import (optional)")?; + let license = interactive::text_or(license, "License")?; let refname = format!("{TOOLCHAINS_NS}/{name}"); let expected = sync(remote, &refname)?; let repo = repo()?; - git_toolchain::import(&repo, &name, Path::new(&path)).map_err(|error| error.to_string())?; + git_toolchain::import( + &repo, + &name, + Path::new(&bin), + src.as_deref().map(Path::new), + &license, + ) + .map_err(|error| error.to_string())?; push_signed(remote, &refname, expected.as_deref())?; println!("imported toolchain {name}"); Ok(()) } -/// Print every toolchain configured on `remote` as `<name> <tree>`. +/// Print every toolchain configured on `remote` as `<name> <bin> <license>`. fn toolchain_list(remote: &str) -> Result<(), String> { let repo = repo()?; sync_namespace(remote, TOOLCHAINS_NS)?; @@ -481,8 +506,12 @@ println!("no toolchains configured on {remote}"); return Ok(()); } - for (name, tree) in toolchains { - println!("{name} {}", short_id(&tree.to_string())); + for (name, toolchain) in toolchains { + println!( + "{name} {} {}", + short_id(&toolchain.bin.oid().to_string()), + toolchain.license + ); } Ok(()) } @@ -493,8 +522,12 @@ let refname = format!("{TOOLCHAINS_NS}/{name}"); sync(remote, &refname)?.ok_or_else(|| format!("no toolchain {name} on {remote}"))?; let repo = repo()?; - git_toolchain::export(&repo, name, Path::new(dest)).map_err(|error| error.to_string())?; - println!("exported toolchain {name} to {dest}"); + let toolchain = + git_toolchain::export(&repo, name, Path::new(dest)).map_err(|error| error.to_string())?; + println!( + "exported toolchain {name} to {dest} (license: {})", + toolchain.license + ); Ok(()) }
crates/git-toolchain/src/lib.rs @@ -1,13 +1,19 @@ -//! Toolchains stored as plain git trees, identity = root tree hash. +//! Toolchains stored as a typed document over two raw-passthrough git trees. //! //! A toolchain is a directory tree (a compiler, an SDK, any reproducible -//! build environment) captured as an ordinary git tree rather than shipped in -//! a container image: [`import`] walks a local directory and writes it as the -//! tip of `refs/meta/toolchains/<name>`, [`resolve`] reads that tip's tree id -//! back, and [`export`] walks a resolved tree back onto disk. There is no -//! hardlink manager or blob store here — a Sprite extracts a resolved tree -//! once into a hash-keyed directory, and its persistent filesystem is the -//! cache. +//! build environment) captured as ordinary git trees rather than shipped in +//! a container image, plus a license: [`import`] walks a local `bin` +//! directory (and, optionally, a `src` directory) and writes them as the tip +//! of `refs/meta/toolchains/<name>`, [`resolve`] reads that tip back as a +//! [`Toolchain`], and [`export`] walks a resolved toolchain's trees back onto +//! disk. There is no hardlink manager or blob store here — a Sprite extracts +//! a resolved `bin` tree once into a hash-keyed directory, and its +//! persistent filesystem is the cache. +//! +//! `bin` and `src` are each captured whole as a [`facet_git_tree::RawTree`]: +//! their internal layout is arbitrary and untyped, so `Toolchain` only +//! records the two trees' object ids and the license, rather than modeling +//! directory contents as `Facet` fields. //! //! Permissions beyond the executable bit are dropped and empty directories //! are skipped (a git tree cannot represent either), so importing the same @@ -19,6 +25,8 @@ use std::os::unix::fs::PermissionsExt as _; use std::path::{Path, PathBuf}; +use facet::Facet; +use facet_git_tree::RawTree; use git_store::Store; use gix::ObjectId; use gix::bstr::ByteSlice as _; @@ -30,6 +38,25 @@ /// tree hash, so importing identical contents twice is a no-op churn-wise. pub const TOOLCHAINS_NS: &str = "refs/meta/toolchains"; +/// A toolchain: an executable `bin` directory, an optional `src` directory, +/// and the license covering them — the document stored at the tip of +/// `refs/meta/toolchains/<name>`. +/// +/// `bin` and `src` are [`RawTree`]: each is captured as a single opaque git +/// tree by [`import`], not walked field-by-field, since a toolchain's +/// on-disk layout has no fixed shape for `Facet` to model. +#[derive(Debug, Clone, PartialEq, Facet)] +pub struct Toolchain { + /// The toolchain's executables, activated on `PATH` when a check + /// requests it. + pub bin: RawTree, + /// The toolchain's source, if imported — not activated on `PATH`, kept + /// only for provenance. + pub src: Option<RawTree>, + /// The license covering `bin` (and `src`, if present). + pub license: String, +} + /// A failure importing, resolving, listing, exporting, or removing a /// toolchain. #[derive(Debug, thiserror::Error)] @@ -40,6 +67,9 @@ /// database `git-store` uses. #[error(transparent)] Store(#[from] git_store::Error), + /// A [`Toolchain`] could not be (de)serialized from its git tree. + #[error(transparent)] + Facet(#[from] facet_git_tree::Error), /// `name` failed [`git_store::ref_segment_ok`]. #[error("{0:?} is not a valid toolchain name")] InvalidName(String), @@ -54,19 +84,55 @@ /// clobber them. #[error("{0} already exists and is not empty")] DestNotEmpty(PathBuf), + /// [`import`]'s `bin` directory produced no entries. A toolchain that + /// activates nothing on `PATH` is not a toolchain. + #[error("{0} contains nothing importable; a toolchain's bin directory must not be empty")] + EmptyBin(PathBuf), + /// [`import`]'s `license` argument was empty. + #[error("toolchain license must not be empty")] + EmptyLicense, } -/// Import `dir`'s contents into `repo` as the toolchain `name`: write its -/// directory tree bottom-up into the object database and fast-forward -/// `refs/meta/toolchains/<name>` to a commit over it. Returns the root -/// tree's object id. -pub fn import(repo: &Path, name: &str, dir: &Path) -> Result<ObjectId, Error> { +/// Import `bin_dir` (and, optionally, `src_dir`) into `repo` as the +/// toolchain `name`: write each directory tree bottom-up into the object +/// database, assemble a [`Toolchain`] document over them and `license`, and +/// fast-forward `refs/meta/toolchains/<name>` to a commit over it. Returns +/// the document's root tree object id. +pub fn import( + repo: &Path, + name: &str, + bin_dir: &Path, + src_dir: Option<&Path>, + license: &str, +) -> Result<ObjectId, Error> { if !git_store::ref_segment_ok(name) { return Err(Error::InvalidName(name.to_owned())); } + if license.is_empty() { + return Err(Error::EmptyLicense); + } let odb = odb_at(repo)?; - let tree = build_tree(&odb, dir)?; - let oid = write_object(&odb, &tree)?; + + let bin_tree = build_tree(&odb, bin_dir)?; + if bin_tree.entries.is_empty() { + return Err(Error::EmptyBin(bin_dir.to_owned())); + } + let bin = RawTree::new(write_object(&odb, &bin_tree)?); + + let src = src_dir + .map(|dir| -> Result<RawTree, Error> { + let tree = build_tree(&odb, dir)?; + Ok(RawTree::new(write_object(&odb, &tree)?)) + }) + .transpose()?; + + let toolchain = Toolchain { + bin, + src, + license: license.to_owned(), + }; + let oid = facet_git_tree::serialize_into(&toolchain, &odb)?; + let store = Store::open(repo)?; store.store_tree( &toolchain_ref(name), @@ -76,15 +142,19 @@ Ok(oid) } -/// The root tree object id `refs/meta/toolchains/<name>`'s tip commit holds. -pub fn resolve(repo: &Path, name: &str) -> Result<ObjectId, Error> { +/// The [`Toolchain`] document `refs/meta/toolchains/<name>`'s tip commit +/// holds. +pub fn resolve(repo: &Path, name: &str) -> Result<Toolchain, Error> { let store = Store::open(repo)?; - Ok(store.ref_tree(&toolchain_ref(name))?) + let root = store.ref_tree(&toolchain_ref(name))?; + let odb = odb_at(repo)?; + Ok(facet_git_tree::deserialize(&root, &odb)?) } -/// Every toolchain configured in `repo`, paired with its root tree id. -pub fn list(repo: &Path) -> Result<Vec<(String, ObjectId)>, Error> { +/// Every toolchain configured in `repo`, paired with its resolved document. +pub fn list(repo: &Path) -> Result<Vec<(String, Toolchain)>, Error> { let store = Store::open(repo)?; + let odb = odb_at(repo)?; let prefix = format!("{TOOLCHAINS_NS}/"); let mut out = Vec::new(); for refname in store.list(&prefix)? { @@ -92,20 +162,31 @@ continue; }; let tree = store.ref_tree(&refname)?; - out.push((name.to_owned(), tree)); + let toolchain = facet_git_tree::deserialize(&tree, &odb)?; + out.push((name.to_owned(), toolchain)); } Ok(out) } -/// Recreate the toolchain `name`'s tree under `dest`, restoring the -/// executable bit and symlinks. Refuses to write into a `dest` that already -/// has contents. -pub fn export(repo: &Path, name: &str, dest: &Path) -> Result<(), Error> { - let store = Store::open(repo)?; - let tree = store.ref_tree(&toolchain_ref(name))?; +/// Recreate the toolchain `name`'s `bin` (and `src`, if present) directory +/// under `dest`, restoring the executable bit and symlinks. Refuses to write +/// into a `dest` that already has contents. Returns the resolved document, +/// so the caller can report the license alongside the exported files. +pub fn export(repo: &Path, name: &str, dest: &Path) -> Result<Toolchain, Error> { + let toolchain = resolve(repo, name)?; let odb = odb_at(repo)?; ensure_empty_dest(dest)?; - write_tree_to_disk(&odb, tree, dest) + + let bin_dest = dest.join("bin"); + fs::create_dir_all(&bin_dest).map_err(|error| Error::Io(bin_dest.clone(), error))?; + write_tree_to_disk(&odb, toolchain.bin.oid(), &bin_dest)?; + + if let Some(src) = &toolchain.src { + let src_dest = dest.join("src"); + fs::create_dir_all(&src_dest).map_err(|error| Error::Io(src_dest.clone(), error))?; + write_tree_to_disk(&odb, src.oid(), &src_dest)?; + } + Ok(toolchain) } /// Delete the toolchain `name`'s ref from `repo`. @@ -276,22 +357,21 @@ #[cfg(test)] mod tests { - #![allow(clippy::unwrap_used, reason = "unit test")] + #![allow(clippy::unwrap_used, clippy::indexing_slicing, reason = "unit test")] use git_store::test_support::repo; use super::*; - /// A file, a subdirectory with its own file, an executable, and (on unix) - /// a symlink — enough to exercise every branch of `write_entry`. + /// A file, an executable, and (on unix) a symlink, plus an empty + /// subdirectory — enough to exercise every branch of `write_entry`. fn populate(dir: &Path) { fs::write(dir.join("README"), b"hello\n").unwrap(); - fs::create_dir(dir.join("bin")).unwrap(); - fs::write(dir.join("bin/tool"), b"#!/bin/sh\necho hi\n").unwrap(); - let mut perms = fs::metadata(dir.join("bin/tool")).unwrap().permissions(); + fs::write(dir.join("tool"), b"#!/bin/sh\necho hi\n").unwrap(); + let mut perms = fs::metadata(dir.join("tool")).unwrap().permissions(); perms.set_mode(0o755); - fs::set_permissions(dir.join("bin/tool"), perms).unwrap(); - std::os::unix::fs::symlink("tool", dir.join("bin/tool-link")).unwrap(); + fs::set_permissions(dir.join("tool"), perms).unwrap(); + std::os::unix::fs::symlink("tool", dir.join("tool-link")).unwrap(); fs::create_dir(dir.join("empty")).unwrap(); } @@ -303,8 +383,8 @@ populate(a.path()); populate(b.path()); - let first = import(repo_dir.path(), "gcc", a.path()).unwrap(); - let second = import(repo_dir.path(), "clang", b.path()).unwrap(); + let first = import(repo_dir.path(), "gcc", a.path(), None, "MIT").unwrap(); + let second = import(repo_dir.path(), "clang", b.path(), None, "MIT").unwrap(); assert_eq!(first, second); } @@ -314,10 +394,11 @@ let dir = tempfile::tempdir().unwrap(); populate(dir.path()); - let oid = import(repo_dir.path(), "gcc", dir.path()).unwrap(); + import(repo_dir.path(), "gcc", dir.path(), None, "MIT").unwrap(); + let toolchain = resolve(repo_dir.path(), "gcc").unwrap(); let odb = odb_at(repo_dir.path()).unwrap(); let mut buf = Vec::new(); - let tree = odb.find_tree(&oid, &mut buf).unwrap(); + let tree = odb.find_tree(&toolchain.bin.oid(), &mut buf).unwrap(); assert!(tree.entries.iter().all(|entry| entry.filename != "empty")); } @@ -327,8 +408,16 @@ let dir = tempfile::tempdir().unwrap(); populate(dir.path()); - let oid = import(repo_dir.path(), "gcc", dir.path()).unwrap(); - assert_eq!(resolve(repo_dir.path(), "gcc").unwrap(), oid); + let oid = import(repo_dir.path(), "gcc", dir.path(), None, "MIT").unwrap(); + let toolchain = resolve(repo_dir.path(), "gcc").unwrap(); + assert_eq!(toolchain.license, "MIT"); + assert!(toolchain.src.is_none()); + + let odb = odb_at(repo_dir.path()).unwrap(); + assert_eq!( + facet_git_tree::serialize_into(&toolchain, &odb).unwrap(), + oid + ); } #[test] @@ -336,20 +425,51 @@ let repo_dir = repo(); let dir = tempfile::tempdir().unwrap(); populate(dir.path()); - import(repo_dir.path(), "gcc", dir.path()).unwrap(); + import(repo_dir.path(), "gcc", dir.path(), None, "MIT").unwrap(); let dest = tempfile::tempdir().unwrap(); let dest_path = dest.path().join("out"); - export(repo_dir.path(), "gcc", &dest_path).unwrap(); + let toolchain = export(repo_dir.path(), "gcc", &dest_path).unwrap(); + assert_eq!(toolchain.license, "MIT"); - assert_eq!(fs::read(dest_path.join("README")).unwrap(), b"hello\n"); + assert_eq!(fs::read(dest_path.join("bin/README")).unwrap(), b"hello\n"); let tool_perms = fs::metadata(dest_path.join("bin/tool")) .unwrap() .permissions(); assert_eq!(tool_perms.mode() & 0o111, 0o111); let link_target = fs::read_link(dest_path.join("bin/tool-link")).unwrap(); assert_eq!(link_target, Path::new("tool")); - assert!(!dest_path.join("empty").exists()); + assert!(!dest_path.join("bin/empty").exists()); + assert!(!dest_path.join("src").exists()); + } + + #[test] + fn import_then_export_round_trips_src_too() { + let repo_dir = repo(); + let bin_dir = tempfile::tempdir().unwrap(); + let src_dir = tempfile::tempdir().unwrap(); + populate(bin_dir.path()); + fs::write(src_dir.path().join("main.c"), b"int main() {}\n").unwrap(); + import( + repo_dir.path(), + "gcc", + bin_dir.path(), + Some(src_dir.path()), + "MIT", + ) + .unwrap(); + + let toolchain = resolve(repo_dir.path(), "gcc").unwrap(); + assert!(toolchain.src.is_some()); + + let dest = tempfile::tempdir().unwrap(); + let dest_path = dest.path().join("out"); + export(repo_dir.path(), "gcc", &dest_path).unwrap(); + assert_eq!( + fs::read(dest_path.join("src/main.c")).unwrap(), + b"int main() {}\n" + ); + assert_eq!(fs::read(dest_path.join("bin/README")).unwrap(), b"hello\n"); } #[test] @@ -357,7 +477,7 @@ let repo_dir = repo(); let dir = tempfile::tempdir().unwrap(); populate(dir.path()); - import(repo_dir.path(), "gcc", dir.path()).unwrap(); + import(repo_dir.path(), "gcc", dir.path(), None, "MIT").unwrap(); let dest = tempfile::tempdir().unwrap(); fs::write(dest.path().join("already-here"), b"x").unwrap(); @@ -366,21 +486,42 @@ } #[test] - fn list_returns_every_toolchain_with_its_tree() { + fn import_rejects_an_empty_license() { + let repo_dir = repo(); + let dir = tempfile::tempdir().unwrap(); + populate(dir.path()); + let result = import(repo_dir.path(), "gcc", dir.path(), None, ""); + assert!(matches!(result, Err(Error::EmptyLicense))); + } + + #[test] + fn import_rejects_an_empty_bin_directory() { + let repo_dir = repo(); + let dir = tempfile::tempdir().unwrap(); + // Only an empty subdirectory: `build_tree` skips it, so `bin` ends up + // with nothing importable. + fs::create_dir(dir.path().join("empty")).unwrap(); + let result = import(repo_dir.path(), "gcc", dir.path(), None, "MIT"); + assert!(matches!(result, Err(Error::EmptyBin(_)))); + } + + #[test] + fn list_returns_every_toolchain_with_its_document() { let repo_dir = repo(); let a = tempfile::tempdir().unwrap(); let b = tempfile::tempdir().unwrap(); populate(a.path()); fs::write(b.path().join("distinct"), b"x").unwrap(); - let gcc = import(repo_dir.path(), "gcc", a.path()).unwrap(); - let clang = import(repo_dir.path(), "clang", b.path()).unwrap(); + import(repo_dir.path(), "gcc", a.path(), None, "MIT").unwrap(); + import(repo_dir.path(), "clang", b.path(), None, "Apache-2.0").unwrap(); let mut listed = list(repo_dir.path()).unwrap(); - listed.sort(); - let mut expected = vec![("clang".to_owned(), clang), ("gcc".to_owned(), gcc)]; - expected.sort(); - assert_eq!(listed, expected); + listed.sort_by(|(a, _), (b, _)| a.cmp(b)); + let names: Vec<&str> = listed.iter().map(|(name, _)| name.as_str()).collect(); + assert_eq!(names, vec!["clang", "gcc"]); + assert_eq!(listed[0].1.license, "Apache-2.0"); + assert_eq!(listed[1].1.license, "MIT"); } #[test] @@ -388,7 +529,7 @@ let repo_dir = repo(); let dir = tempfile::tempdir().unwrap(); populate(dir.path()); - import(repo_dir.path(), "gcc", dir.path()).unwrap(); + import(repo_dir.path(), "gcc", dir.path(), None, "MIT").unwrap(); remove(repo_dir.path(), "gcc").unwrap(); let _ = resolve(repo_dir.path(), "gcc").unwrap_err(); @@ -399,7 +540,7 @@ let repo_dir = repo(); let dir = tempfile::tempdir().unwrap(); populate(dir.path()); - let result = import(repo_dir.path(), "not/valid", dir.path()); + let result = import(repo_dir.path(), "not/valid", dir.path(), None, "MIT"); assert!(matches!(result, Err(Error::InvalidName(_)))); } }