vasilito/RedBear-OS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat: build-system resilience (fetch retry, strict cache parse, auto_deps preserve, Tier 1 messages)

Browse Source 
Tier 3 code improvements (from BUILD-SYSTEM-ASSESSMENT-2026-07-18 Part 6.2):

- Q2: Add run_command_with_retry() helper with exponential backoff (1s, 2s, 4s).
  Wrap download_wget and git clone with 3 attempts. Git clone closure cleans
  partial tmp dir between retries (git refuses non-empty clone targets).

- Q3: Preserve auto_deps.toml alongside dep_hashes.toml in binary store publish
  (repo_builder.rs). Restore path prefers the preserved copy, falling back to
  declared-depends-only reconstruction only when the preserved copy is absent
  (e.g. published by an older cookbook). Preserves full ELF dynamic dep graph.

- Q4: DepHashes::read now returns Result<Option<Self>, String> discriminating
  'missing' (legitimate first build, fall back to mtime) from 'corrupt' (loud
  WARN + force rebuild). Closes a silent-swallow gap where a corrupt
  dep_hashes.toml produced an incorrect mtime-fallback rebuild.

- Q6: Standardize error reporting in repo.rs nonstop path to {:#} format,
  matching the pattern established in the earlier Tier 1 message commit.

Tier 1 message fixes (remaining items from assessment Part 5.3/5.4):

- M26: build-redbear.sh:569 '30-60 minutes' -> config-dependent range
- A6/A8: Makefile container messages add remediation hint
- B3: mk/config.mk sccache hint adds install guidance
- H2: mk/qemu.mk Unsupported ARCH lists supported values
- mk/redbear.mk comment 'fully-patched' -> 'versioned'

Verification: cargo check 0 errors, cargo test --lib 35/35 passed,
cargo clippy 0 new warnings vs baseline, make -n live + validate clean.
This commit is contained in:
vasilito
2026-07-18 16:54:24 +09:00
parent 0d5043f3fc
commit de44b9324e
10 changed files with 187 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Makefile
+2 -2
View File
@@ -159,7 +159,7 @@ ifeq ($(PODMAN_BUILD),1)
ifneq ("$(wildcard $(CONTAINER_TAG))","")
$(PODMAN_RUN) make $@
else
$(info will not run cookbook clean as container is not built)
$(info will not run cookbook clean as container is not built — run 'make container' first to build the Podman image, or set PODMAN_BUILD=0 in .config for a native build)
$(MAKE) clean PODMAN_BUILD=0 NOT_ON_PODMAN=1 SKIP_CHECK_TOOLS=1
endif # CONTAINER_TAG
else
@@ -186,7 +186,7 @@ ifeq ($(PODMAN_BUILD),1)
ifneq ("$(wildcard $(CONTAINER_TAG))","")
$(PODMAN_RUN) make $@
else
$(info will not run cookbook unfetch as container is not built)
$(info will not run cookbook unfetch as container is not built — run 'make container' first to build the Podman image, or set PODMAN_BUILD=0 in .config for a native build)
$(MAKE) distclean PODMAN_BUILD=0 NOT_ON_PODMAN=1 SKIP_CHECK_TOOLS=1
endif # CONTAINER_TAG
else
local/scripts/build-redbear.sh
+1 -1
View File
@@ -566,7 +566,7 @@ if [ "$CONFIG" = "redbear-full" ]; then
fi
echo ">>> Building Red Bear OS with config: $CONFIG"
echo ">>> This may take 30-60 minutes on first build..."
echo ">>> Build time varies by config (redbear-mini ~30-60 min, redbear-full ~60-120 min on first build)..."
# Stale-build prevention: if a low-level source repo has commits newer
# than its pkgar, delete that package's pkgar and target dir AND clean
mk/config.mk
+1 -1
View File
@@ -87,7 +87,7 @@ endif
ifeq ($(SCCACHE_BUILD),1)
ifeq (,$(shell command -v sccache))
$(info sccache not found in PATH)
$(info sccache not found in PATH — install with 'cargo install sccache' or your distro package manager; falling back to no caching)
SCCACHE_BUILD=0
endif
endif
mk/qemu.mk
+1 -1
View File
@@ -137,7 +137,7 @@ else ifeq ($(ARCH),riscv64gc)
QEMUFLAGS+=-device qemu-xhci -device usb-kbd -device usb-tablet
endif
else
$(error Unsupported ARCH for QEMU "$(ARCH)"))
$(error Unsupported ARCH for QEMU "$(ARCH)". Supported ARCH values: x86_64, i586, aarch64, riscv64gc)
endif
QEMUFLAGS+=-smp $(QEMU_SMP) -m $(QEMU_MEM)
mk/redbear.mk
+1 -1
View File
@@ -30,7 +30,7 @@ redbear: $(REDBEAR_TAG)
redbear_clean:
rm -f "$(REDBEAR_TAG)" "$(REDBEAR_FP)"
# Source archival — exports fully-patched, versioned source archives
# Source archival — exports versioned source archives
# for all recipes with source/ directories to sources/<target>/
# Runs after `make all` and also standalone via `make sources`
SOURCES_DIR=$(BUILD)/../sources/$(TARGET)
src/bin/repo.rs
+2 -2
View File
@@ -291,10 +291,10 @@ fn main_inner() -> anyhow::Result<()> {
Err(e) => {
if config.cook.nonstop {
if verbose {
eprintln!("{:?}", e);
eprintln!("repo: {e:#}");
}
if let Err(e) = handle_nonstop_fail(recipe) {
eprintln!("{:?}", e)
eprintln!("repo: {e:#}")
};
}
print_failed(&command, &recipe.name);
src/bin/repo_builder.rs
+7
View File
@@ -134,6 +134,13 @@ fn publish_packages(config: &CliConfig) -> anyhow::Result<()> {
if dep_hashes_src.is_file() {
let _ = fs::copy(&dep_hashes_src, &dep_hashes_dst);
}
// Preserves the full auto-dep graph (incl. ELF dynamic deps); otherwise restores fall back to declared-depends-only reconstruction.
let auto_deps_src = target_dir.join("auto_deps.toml");
let auto_deps_dst = repo_path.join(format!("{}.auto_deps.toml", recipe_name));
if auto_deps_src.is_file() {
let _ = fs::copy(&auto_deps_src, &auto_deps_dst);
}
}
}
src/cook/cook_build.rs
+84 -45
View File
@@ -23,16 +23,32 @@ use std::{
use crate::{is_redox, log_to_pty, wrap_io_err};
#[derive(Serialize, Deserialize, Default)]
#[derive(Serialize, Deserialize, Default, Debug)]
pub struct DepHashes {
#[serde(flatten)]
pub hashes: BTreeMap<String, String>,
}
impl DepHashes {
pub fn read(path: &Path) -> Option<Self> {
let content = fs::read_to_string(path).ok()?;
toml::from_str(&content).ok()
/// Read a previously stored dep_hashes.toml.
///
/// Returns:
/// - `Ok(Some(_))` — file exists and parsed cleanly
/// - `Ok(None)` — file does not exist (legitimate first-build state)
/// - `Err(_)` — file exists but is unreadable or unparseable (corrupt)
///
/// Callers MUST treat `Err` as a loud signal (force rebuild) and NOT
/// silently fall back to mtime comparison — a corrupt dep_hashes.toml
/// means we cannot trust content-based caching at all.
pub fn read(path: &Path) -> std::result::Result<Option<Self>, String> {
let content = match fs::read_to_string(path) {
Ok(c) => c,
Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None),
Err(e) => return Err(format!("failed to read {}: {e}", path.display())),
};
toml::from_str(&content)
.map(Some)
.map_err(|e| format!("failed to parse {}: {e}", path.display()))
}
pub fn write(&self, path: &Path) -> Result<(), String> {
@@ -542,19 +558,31 @@ pub fn build(
}
}
// Reconstruct auto_deps.toml from repo metadata (may be incomplete —
// does not include ELF-discovered dynamic linking deps)
// Prefer the preserved auto_deps.toml from the binary store — it
// retains the full auto-dep graph (incl. ELF dynamic deps). Only
// fall back to declared-depends-only reconstruction when the
// preserved copy is absent (e.g. published by an older cookbook).
let repo_auto_deps = repo_target.join(format!("{}.auto_deps.toml", recipe_name));
if !auto_deps_file.is_file() && all_restored {
let main_toml = repo_target.join(format!("{}.toml", recipe_name));
if let Ok(toml_content) = fs::read_to_string(&main_toml) {
if let Ok(pkg_meta) = toml::from_str::<Package>(&toml_content) {
let auto: BTreeSet<PackageName> = pkg_meta.depends.into_iter().collect();
let wrapper = AutoDeps { packages: auto };
if let Err(e) = serialize_and_write(&auto_deps_file, &wrapper) {
log_to_pty!(logger, "WARN: failed to write auto_deps.toml: {e}");
}
if cli_verbose {
log_to_pty!(logger, "DEBUG: auto_deps.toml reconstructed from repo depends (may miss ELF dynamic deps)");
if repo_auto_deps.is_file() {
if let Err(e) = fs::copy(&repo_auto_deps, &auto_deps_file) {
log_to_pty!(logger, "WARN: failed to copy auto_deps.toml: {e}");
}
if cli_verbose {
log_to_pty!(logger, "DEBUG: auto_deps.toml restored from binary store");
}
} else {
let main_toml = repo_target.join(format!("{}.toml", recipe_name));
if let Ok(toml_content) = fs::read_to_string(&main_toml) {
if let Ok(pkg_meta) = toml::from_str::<Package>(&toml_content) {
let auto: BTreeSet<PackageName> = pkg_meta.depends.into_iter().collect();
let wrapper = AutoDeps { packages: auto };
if let Err(e) = serialize_and_write(&auto_deps_file, &wrapper) {
log_to_pty!(logger, "WARN: failed to write auto_deps.toml: {e}");
}
if cli_verbose {
log_to_pty!(logger, "WARN: auto_deps.toml reconstructed from declared depends only (no preserved copy in binary store — ELF dynamic deps may be missing)");
}
}
}
}
@@ -654,25 +682,34 @@ pub fn build(
log_to_pty!(logger, "DEBUG: force rebuild requested");
}
true
} else if let Some(stored_hashes) = DepHashes::read(&dep_hashes_file) {
let current = collect_current_dep_hashes(&dep_pkgars);
let current_host = collect_current_dep_hashes(&dep_host_pkgars);
let mut all_current = current;
all_current.extend(current_host);
let changed = dep_hashes_changed(&stored_hashes.hashes, &all_current);
if cli_verbose {
if changed {
log_to_pty!(logger, "DEBUG: dependency blake3 hashes changed");
} else {
log_to_pty!(logger, "DEBUG: dependency blake3 hashes unchanged (content-based cache)");
} else {
match DepHashes::read(&dep_hashes_file) {
Ok(Some(stored_hashes)) => {
let current = collect_current_dep_hashes(&dep_pkgars);
let current_host = collect_current_dep_hashes(&dep_host_pkgars);
let mut all_current = current;
all_current.extend(current_host);
let changed = dep_hashes_changed(&stored_hashes.hashes, &all_current);
if cli_verbose {
if changed {
log_to_pty!(logger, "DEBUG: dependency blake3 hashes changed");
} else {
log_to_pty!(logger, "DEBUG: dependency blake3 hashes unchanged (content-based cache)");
}
}
changed
}
Ok(None) => {
if cli_verbose {
log_to_pty!(logger, "DEBUG: no dep_hashes.toml, falling back to mtime comparison");
}
stage_modified < deps_modified || stage_modified < deps_host_modified
}
Err(e) => {
log_to_pty!(logger, "WARN: {}: forcing rebuild (content cache cannot be trusted)", e);
true
}
}
changed
} else {
if cli_verbose {
log_to_pty!(logger, "DEBUG: no dep_hashes.toml, falling back to mtime comparison");
}
stage_modified < deps_modified || stage_modified < deps_host_modified
};
if source_changed || deps_changed || !auto_deps_file.is_file()
@@ -1248,7 +1285,9 @@ mod tests {
original.write(&path).expect("write should succeed");
assert!(path.exists(), "dep_hashes.toml should exist after write");
let loaded = DepHashes::read(&path).expect("read should return Some after write");
let loaded = DepHashes::read(&path)
.expect("read should not error on a file we just wrote")
.expect("read should return Some after write");
assert_eq!(
loaded.hashes, original.hashes,
"round-trip must preserve all key-value pairs"
@@ -1256,26 +1295,26 @@ mod tests {
}
#[test]
fn dep_hashes_read_returns_none_for_missing_file() {
fn dep_hashes_read_returns_ok_none_for_missing_file() {
let dir = temp_dir("dep_hashes_missing");
let path = dir.join("nonexistent.toml");
assert!(
DepHashes::read(&path).is_none(),
"read on a missing path must return None, not panic"
);
match DepHashes::read(&path) {
Ok(None) => (),
other => panic!("read on a missing path must return Ok(None), got {other:?}"),
}
}
#[test]
fn dep_hashes_read_returns_none_for_corrupt_toml() {
fn dep_hashes_read_returns_err_for_corrupt_toml() {
let dir = temp_dir("dep_hashes_corrupt");
let path = dir.join("dep_hashes.toml");
fs::write(&path, b"this is not valid toml [[[").expect("write corrupt file");
assert!(
DepHashes::read(&path).is_none(),
"read on corrupt TOML must return None — this documents the current \
silent-swallow behavior flagged in the build-system assessment"
);
match DepHashes::read(&path) {
Err(_) => (),
Ok(Some(_)) => panic!("read on corrupt TOML must return Err, got Ok(Some)"),
Ok(None) => panic!("read on corrupt TOML must return Err, got Ok(None)"),
}
}
#[test]
src/cook/fetch.rs
+28 -15
View File
@@ -527,21 +527,34 @@ fn reclone_git_source(
}
create_dir_clean(&source_dir_tmp)?;
let mut command = Command::new("git");
command
.arg("clone")
.arg("--recursive")
.arg(translate_mirror(git));
if let Some(branch) = branch {
command.arg("--branch").arg(branch);
}
if shallow_clone {
command
.arg("--filter=tree:0")
.arg("--also-filter-submodules");
}
command.arg(&source_dir_tmp);
if let Err(e) = run_command(command, logger) {
let translated_git = translate_mirror(git);
let clone_result = run_command_with_retry(
|attempt| {
if attempt > 1 {
let _ = std::fs::remove_dir_all(&source_dir_tmp);
let _ = create_dir_clean(&source_dir_tmp);
}
let mut command = Command::new("git");
command
.arg("clone")
.arg("--recursive")
.arg(&translated_git);
if let Some(branch) = branch {
command.arg("--branch").arg(branch);
}
if shallow_clone {
command
.arg("--filter=tree:0")
.arg("--also-filter-submodules");
}
command.arg(&source_dir_tmp);
command
},
logger,
3,
&format!("git clone {}", translated_git),
);
if let Err(e) = clone_result {
if !is_redox() {
return Err(e);
}
src/cook/fs.rs
+60 -5
View File
@@ -13,7 +13,7 @@ use crate::{
Error, Result, bail_other_err,
config::translate_mirror,
cook::pty::{PtyOut, spawn_to_pipe},
wrap_io_err, wrap_other_err,
log_to_pty, wrap_io_err, wrap_other_err,
};
//TODO: pub(crate) for all of these functions
@@ -207,6 +207,50 @@ pub fn run_command(mut command: process::Command, stdout_pipe: &PtyOut) -> Resul
Ok(())
}
/// Run a command, retrying on failure up to `max_attempts` times with
/// exponential backoff (1s, 2s, 4s, ...). `build_cmd` is invoked once per
/// attempt with the 1-indexed attempt number, so callers can perform
/// per-attempt setup (e.g. clearing a partial clone directory) before
/// constructing a fresh `Command`. `run_command` consumes the `Command`,
/// so a fresh one must be built each attempt.
pub fn run_command_with_retry(
build_cmd: impl Fn(u32) -> Command,
logger: &PtyOut,
max_attempts: u32,
what: &str,
) -> Result<()> {
assert!(max_attempts >= 1, "max_attempts must be at least 1");
let mut last_err: Option<Error> = None;
for attempt in 1..=max_attempts {
if attempt > 1 {
let backoff_secs = 1u64 << (attempt - 2);
log_to_pty!(
logger,
"DEBUG: {} retry attempt {}/{} (backoff {}s)",
what,
attempt,
max_attempts,
backoff_secs
);
std::thread::sleep(std::time::Duration::from_secs(backoff_secs));
}
match run_command(build_cmd(attempt), logger) {
Ok(()) => return Ok(()),
Err(e) => {
log_to_pty!(
logger,
"WARN: {} attempt {}/{} failed",
what,
attempt,
max_attempts
);
last_err = Some(e);
}
}
}
Err(last_err.expect("retry loop runs at least once when max_attempts >= 1"))
}
pub fn run_command_stdin(
mut command: process::Command,
stdin_data: &[u8],
@@ -258,10 +302,21 @@ pub fn offline_check_exists(path: &PathBuf) -> Result<()> {
pub fn download_wget(url: &str, dest: &PathBuf, logger: &PtyOut) -> Result<()> {
if !dest.is_file() {
let dest_tmp = PathBuf::from(format!("{}.tmp", dest.display()));
let mut command = Command::new("wget");
command.arg(translate_mirror(url));
command.arg("--continue").arg("-O").arg(&dest_tmp);
run_command(command, logger)?;
let translated = translate_mirror(url);
run_command_with_retry(
|_| {
let mut command = Command::new("wget");
command
.arg(&translated)
.arg("--continue")
.arg("-O")
.arg(&dest_tmp);
command
},
logger,
3,
&format!("download {}", translated),
)?;
rename(&dest_tmp, &dest)?;
}
Ok(())