From 6b2f54c9bdd152fb4ba4d60fc28bd4eaad988d45 Mon Sep 17 00:00:00 2001 From: vasilito Date: Fri, 10 Jul 2026 21:57:46 +0300 Subject: [PATCH] docs: add canonical build system improvement proposal Documents 8 concrete improvements to build-redbear.sh and the cookbook tool, derived from the redbear-mini boot crash debug session: 1. Trap-based stash-and-restore for ALL fork sources (eliminates the 'where did my edits go?' pain) 2. Source content-hashing to replace mtime-only cache invalidation 3. Cookbook binary freshness check (currently existence-only) 4. Strict-by-default uncommitted-edit gate 5. Stash-all-forks consistency 6. Failure-cleanup trap with diagnostics 7. Pre-cook offline/upstream consistency 8. Cookbook protection against out-of-band invocations Each improvement has a file:line location, implementation sketch, and impact/complexity rating. Implementation order suggested. Problem motivation: the user has repeatedly hit silent stash-loss, stale cookbook binaries, and mtime-based cache staleness while debugging kernel/relibc/base forks. The build pipeline should make these mistakes impossible, not require users to remember workarounds. --- .../docs/BUILD-SYSTEM-IMPROVEMENT-PROPOSAL.md | 356 ++++++++++++++++++ 1 file changed, 356 insertions(+) create mode 100644 local/docs/BUILD-SYSTEM-IMPROVEMENT-PROPOSAL.md diff --git a/local/docs/BUILD-SYSTEM-IMPROVEMENT-PROPOSAL.md b/local/docs/BUILD-SYSTEM-IMPROVEMENT-PROPOSAL.md new file mode 100644 index 0000000000..26d6534f01 --- /dev/null +++ b/local/docs/BUILD-SYSTEM-IMPROVEMENT-PROPOSAL.md @@ -0,0 +1,356 @@ +# Canonical Build System Improvement Proposal + +**Status:** Proposal — not yet implemented +**Context:** Identified while debugging the `redbear-mini` boot crash chain (`require_zero_offset`, `ContextHandle::Start`, `sys_statfs cbindgen.toml`, `SYS_MKNS`/`SYS_SETNS`, `mmap_min_addr` capping). The crash fixes were correct, but getting them through the build pipeline was painful and error-prone. This document proposes specific improvements to make that workflow reliable. + +--- + +## Problems Encountered (Root-Caused) + +1. **Edits in `local/sources//` are silently stashed** by `build-redbear.sh` (line 160-172), then never restored. Build runs against the clean HEAD. User loses WIP from the working tree. +2. **`--force-rebuild` bypasses the BLAKE3 content-hash cache** (cook_build.rs:462) and all 14 validation gates in `build-redbear.sh`. +3. **Manual `repo cook` outside `build-redbear.sh`** skips stale detection, prefix rebuild, pre-cook sequencing, source-fingerprint tracking, and CI=1 (TUI protection). +4. **Source invalidation is mtime-only** (cook_build.rs:425-442). A `git checkout` or `cp -a` that preserves timestamps produces a stale cache. No content hash is computed for any source file. +5. **Cookbook binary staleness** (build-redbear.sh:182-185): only checks existence, not whether `src/` has changed since last build. +6. **No failure cleanup** in `build-redbear.sh`: no `trap` handler, no signal handler, partially-built artifacts left in `repo/` after failure. +7. **No strict-by-default gate** for uncommitted edits: `verify-durable-source-edits.py` only blocks under `--strict`, which is not the default. +8. **`recipes/core/relibc/source` is stashed but the other 5 fork sources are not** — silent inconsistency. + +--- + +## Proposed Improvements (Ranked by Impact) + +### Improvement 1: Make `stash_nested_repo_if_dirty` restore on exit + +**Where:** `local/scripts/build-redbear.sh:160-172` +**Impact:** HIGH — eliminates the "where did my edits go?" confusion +**Complexity:** LOW + +The current `stash_nested_repo_if_dirty` pushes a stash with `--all` and never restores it. Replace with a `trap`-based push-and-restore pattern: + +```bash +REDBEAR_STASHED_REPOS=() + +stash_nested_repo_if_dirty() { + local target_dir="$1" + local label="$2" + if [ -d "$target_dir/.git" ]; then + if ! git -C "$target_dir" diff --quiet || \ + ! git -C "$target_dir" diff --cached --quiet || \ + [ -n "$(git -C "$target_dir" ls-files --others --exclude-standard)" ]; then + echo ">>> Stashing dirty $label checkout..." + rm -f "$target_dir/.git/index.lock" + if git -C "$target_dir" stash push --all \ + -m "build-redbear-auto-stash-$(date +%s)" >/dev/null 2>&1; then + REDBEAR_STASHED_REPOS+=("$label:$target_dir") + fi + fi + fi +} + +restore_all_stashes() { + local rc=$? + local entry + for entry in "${REDBEAR_STASHED_REPOS[@]}"; do + local label="${entry%%:*}" + local dir="${entry#*:}" + if [ -d "$dir/.git" ]; then + echo ">>> Restoring $label working tree..." + rm -f "$dir/.git/index.lock" + git -C "$dir" stash pop >/dev/null 2>&1 || \ + echo "WARN: failed to restore $label stash — check 'git -C $dir stash list'" + fi + done + exit $rc +} + +trap restore_all_stashes EXIT +``` + +Apply this to ALL fork sources, not just relibc: + +```bash +for fork in relibc kernel base bootloader installer redoxfs; do + local_fork_dir="$PROJECT_ROOT/local/sources/$fork" + if [ -d "$local_fork_dir/.git" ]; then + stash_nested_repo_if_dirty "$local_fork_dir" "$fork" + fi +done +``` + +--- + +### Improvement 2: Source-content hashing for cache invalidation + +**Where:** `src/cook/cook_build.rs:425-442` and `src/cook/fs.rs:160-168` +**Impact:** HIGH — eliminates the "stale cache after git checkout" bug class +**Complexity:** MEDIUM + +Replace the mtime-based `modified_dir_ignore_git()` with a content-hash approach that covers all meaningful inputs: + +```rust +// In src/cook/fs.rs or a new src/cook/source_hash.rs +fn compute_source_content_hash(source_dir: &Path, recipe_toml: &Path, patches: &[Path]) -> String { + let mut hasher = blake3::Hasher::new(); + + // Hash source directory contents (file-by-file, sorted by path) + if let Ok(entries) = walk_dir_files_sorted(source_dir) { + for (rel_path, abs_path) in entries { + // Skip .git, target/, *.swp, etc. + if rel_path.starts_with(".git/") || rel_path.contains("/target/") { + continue; + } + hasher.update(rel_path.as_bytes()); + hasher.update(b"\0"); + if let Ok(content) = std::fs::read(&abs_path) { + hasher.update(&content); + } + hasher.update(b"\0"); + } + } + + // Hash recipe.toml + if let Ok(content) = std::fs::read(recipe_toml) { + hasher.update(b"recipe.toml\0"); + hasher.update(&content); + } + + // Hash each patch file + for patch in patches { + if let Ok(content) = std::fs::read(patch) { + hasher.update(patch.file_name().unwrap().as_encoded_bytes()); + hasher.update(b"\0"); + hasher.update(&content); + } + } + + hasher.finalize().to_hex().to_string() +} +``` + +In `build()` (cook_build.rs:425-460): + +```rust +// Replace mtime-based source_modified with content hash +let source_hash = compute_source_content_hash(source_dir, &recipe_dir.join("recipe.toml"), &patches); +let stored_hash_file = get_sub_target_dir(target_dir, "source_hash.txt"); + +let source_changed = match std::fs::read_to_string(&stored_hash_file) { + Ok(stored) if stored.trim() == source_hash => false, + _ => true, +}; + +// ... after successful build: +std::fs::write(&stored_hash_file, &source_hash)?; +``` + +This makes the cache invalidation robust against `git checkout`, `cp -a`, and any other operation that changes content without updating mtime. It also eliminates the cbindgen.toml-specific fragility: the generated headers' source is the `*.rs` files + `cbindgen.toml`, all of which are hashed. + +--- + +### Improvement 3: Cookbook binary freshness check + +**Where:** `local/scripts/build-redbear.sh:182-185` +**Impact:** MEDIUM — prevents stale binary causing silent failures +**Complexity:** LOW + +```bash +COOKBOOK_SRC_FINGERPRINT="$PROJECT_ROOT/target/release/.cookbook-src-fingerprint" +COOKBOOK_BIN="$PROJECT_ROOT/target/release/repo" +COOKBOOK_SRC_HASH=$(find "$PROJECT_ROOT/src" -name "*.rs" -print0 | sort -z | xargs -0 sha256sum | sha256sum | cut -d' ' -f1) + +NEEDS_REBUILD=0 +if [ ! -f "$COOKBOOK_BIN" ]; then + NEEDS_REBUILD=1 +elif [ ! -f "$COOKBOOK_SRC_FINGERPRINT" ] || [ "$(cat "$COOKBOOK_SRC_FINGERPRINT")" != "$COOKBOOK_SRC_HASH" ]; then + NEEDS_REBUILD=1 +fi + +if [ "$NEEDS_REBUILD" = "1" ]; then + echo ">>> Rebuilding cookbook binary (source changed or missing)..." + cargo build --release + echo -n "$COOKBOOK_SRC_HASH" > "$COOKBOOK_SRC_FINGERPRINT" +fi +``` + +--- + +### Improvement 4: Strict-by-default uncommitted-edit gate + +**Where:** `local/scripts/build-redbear.sh` and `local/scripts/verify-durable-source-edits.py:48` +**Impact:** MEDIUM — prevents "what was that warning?" confusion +**Complexity:** LOW + +Add an explicit gate before any source-touching operation: + +```bash +# After .config parsing, before any fetch/cook +echo ">>> Checking for uncommitted source edits..." +DIRTY_FORKS=() +for fork in local/sources/relibc local/sources/kernel local/sources/base \ + local/sources/bootloader local/sources/installer local/sources/redoxfs; do + fork_dir="$PROJECT_ROOT/$fork" + if [ -d "$fork_dir/.git" ] && \ + (! git -C "$fork_dir" diff --quiet HEAD 2>/dev/null || \ + ! git -C "$fork_dir" diff --cached --quiet HEAD 2>/dev/null || \ + [ -n "$(git -C "$fork_dir" ls-files --others --exclude-standard 2>/dev/null)" ]); then + DIRTY_FORKS+=("$fork") + fi +done + +if [ ${#DIRTY_FORKS[@]} -gt 0 ]; then + echo "WARNING: uncommitted edits detected in:" + for f in "${DIRTY_FORKS[@]}"; do + echo " - $f" + done + echo "" + if [ "${REDBEAR_ALLOW_DIRTY:-0}" = "1" ]; then + echo ">>> REDBEAR_ALLOW_DIRTY=1 set; proceeding with WIP edits." + else + echo "ERROR: refuse to build with uncommitted edits." + echo " Either commit your changes or set REDBEAR_ALLOW_DIRTY=1 to override." + exit 1 + fi +fi +``` + +For `verify-durable-source-edits.py`, flip the default to strict and make `--no-strict` the override: + +```python +# Line 48: change default from non-strict to strict +if not args.no_strict and dirty: + print("ERROR: uncommitted edits detected in upstream-owned source trees.") + print(" Use --no-strict to allow, or commit your changes.") + sys.exit(1) +``` + +--- + +### Improvement 5: Stash-all-forks consistency + +**Where:** `local/scripts/build-redbear.sh:160-172` +**Impact:** MEDIUM — eliminates the "only relibc is special" asymmetry +**Complexity:** LOW + +Combined with Improvement 1, replace the single-relibc stash with a loop over all fork sources. The key insight: `local/sources//` is the source of truth, but the cookbook fetches into `recipes/core//source` (which is a symlink to `local/sources//`). Stashing must happen at the `local/sources//` level, not at the recipe-level symlink. + +--- + +### Improvement 6: Failure-cleanup trap + +**Where:** `local/scripts/build-redbear.sh` (top-level, after `set -euo pipefail`) +**Impact:** MEDIUM — makes failed builds debuggable +**Complexity:** LOW + +```bash +REDBEAR_BUILD_STATE_DIR=$(mktemp -d) + +cleanup_on_failure() { + local rc=$? + if [ $rc -ne 0 ]; then + echo "" + echo "========================================" + echo " BUILD FAILED (exit code: $rc)" + echo "========================================" + echo "Diagnostic info preserved at: $REDBEAR_BUILD_STATE_DIR" + echo "" + echo "Last 30 lines of each recipe log:" + for log in /tmp/build-*.log; do + [ -f "$log" ] || continue + echo "--- $log ---" + tail -30 "$log" + echo "" + done 2>/dev/null + echo "Recipe build state:" + for d in "$PROJECT_ROOT"/recipes/*/target/*/; do + [ -d "$d" ] || continue + echo " $d: $(du -sh "$d" 2>/dev/null | cut -f1)" + done + fi + rm -rf "$REDBEAR_BUILD_STATE_DIR" + # Then restore stashes (from Improvement 1) + restore_all_stashes +} +trap cleanup_on_failure EXIT +``` + +--- + +### Improvement 7: Pre-cook offline/upstream consistency + +**Where:** `local/scripts/build-redbear.sh:339-357` +**Impact:** LOW-MEDIUM — prevents pre-cook from fetching packages the main build can't use +**Complexity:** LOW + +```bash +# Replace line 349 +if [ "${REDBEAR_ALLOW_UPSTREAM:-0}" = "1" ]; then + COOKBOOK_OFFLINE=false "$PROJECT_ROOT/target/release/repo" cook "$pkg" >"$log" 2>&1 +elif [ -n "${REDBEAR_RELEASE:-}" ]; then + COOKBOOK_OFFLINE=true "$PROJECT_ROOT/target/release/repo" cook "$pkg" >"$log" 2>&1 +else + COOKBOOK_OFFLINE=true "$PROJECT_ROOT/target/release/repo" cook "$pkg" >"$log" 2>&1 +fi +``` + +--- + +### Improvement 8: Cookbook protection against out-of-band invocations + +**Where:** `src/bin/repo.rs` (CLI entry point) +**Impact:** LOW — prevents accidental misuse, gentle guardrail +**Complexity:** LOW + +Add a non-blocking warning when `repo cook` is invoked outside `build-redbear.sh`: + +```rust +// In src/bin/repo.rs main_inner() +let canonical_marker = std::env::var_os("REDBEAR_CANONICAL_BUILD"); +if canonical_marker.is_none() { + eprintln!("WARNING: repo cook invoked outside build-redbear.sh."); + eprintln!(" Cache invalidation, prefix rebuild, and source-fingerprint"); + eprintln!(" tracking will not run. Set REDBEAR_CANONICAL_BUILD=1 to suppress"); + eprintln!(" this warning if you know what you're doing."); +} +``` + +In `build-redbear.sh`: +```bash +export REDBEAR_CANONICAL_BUILD=1 +``` + +--- + +## Implementation Order + +Suggested order (each step is independently useful and can be merged separately): + +1. **Improvement 1 + 5** (trap-based stash restore, all forks) — eliminates the biggest UX pain point +2. **Improvement 6** (failure-cleanup trap with diagnostics) — makes failures debuggable +3. **Improvement 3** (cookbook binary freshness) — 5-line change, high signal +4. **Improvement 4** (strict-by-default dirty gate) — prevents repeat-accumulation of uncommitted edits +5. **Improvement 7** (pre-cook offline consistency) — small correctness fix +6. **Improvement 8** (cookbook out-of-band warning) — gentle nudge +7. **Improvement 2** (source content hashing) — biggest engineering effort, defer until after the above land + +--- + +## Testing Strategy + +Each improvement needs: +1. Unit test in `src/` if it touches cookbook code (cookbook has a test directory) +2. Manual smoke test: `./local/scripts/build-redbear.sh redbear-mini` end-to-end +3. Regression test: dirty `local/sources/relibc/` + `./local/scripts/build-redbear.sh redbear-mini` + verify stash is restored on exit +4. Documentation update: this file's "Problems Encountered" should be empty after all improvements land + +--- + +## Related Documents + +- `local/AGENTS.md` — project-wide build and commit policies +- `local/docs/LOCAL-FORK-SUPREMACY-POLICY.md` — why local forks must be complete and committed +- `local/docs/BUILD-CACHE-PLAN.md` — content-hash cache design (Phase 1-3 status, sysroot deferral) +- `local/docs/BUILD-SYSTEM-INVARIANTS.md` — what the build system guarantees +- `local/docs/BUILD-SYSTEM-HARDENING-PLAN.md` — broader hardening roadmap +- `mk/repo.mk` — make-integration of cookbook +- `local/scripts/build-redbear.sh` — canonical entry point \ No newline at end of file