diff --git a/Makefile b/Makefile index 6a53ca068b..97eb104069 100644 --- a/Makefile +++ b/Makefile @@ -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 diff --git a/local/scripts/build-redbear.sh b/local/scripts/build-redbear.sh index ef1cdf3662..40ea11f6aa 100755 --- a/local/scripts/build-redbear.sh +++ b/local/scripts/build-redbear.sh @@ -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 diff --git a/mk/config.mk b/mk/config.mk index f665790677..430295f78e 100644 --- a/mk/config.mk +++ b/mk/config.mk @@ -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 diff --git a/mk/qemu.mk b/mk/qemu.mk index e3cf3f2901..d38b99d005 100644 --- a/mk/qemu.mk +++ b/mk/qemu.mk @@ -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) diff --git a/mk/redbear.mk b/mk/redbear.mk index 10a4300b7f..6fada91f24 100644 --- a/mk/redbear.mk +++ b/mk/redbear.mk @@ -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// # Runs after `make all` and also standalone via `make sources` SOURCES_DIR=$(BUILD)/../sources/$(TARGET) diff --git a/src/bin/repo.rs b/src/bin/repo.rs index d561b40209..c62718b087 100644 --- a/src/bin/repo.rs +++ b/src/bin/repo.rs @@ -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); diff --git a/src/bin/repo_builder.rs b/src/bin/repo_builder.rs index becbdf41ea..f10697e142 100644 --- a/src/bin/repo_builder.rs +++ b/src/bin/repo_builder.rs @@ -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); + } } } diff --git a/src/cook/cook_build.rs b/src/cook/cook_build.rs index 5f5fc99614..fe7aa50568 100644 --- a/src/cook/cook_build.rs +++ b/src/cook/cook_build.rs @@ -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, } impl DepHashes { - pub fn read(path: &Path) -> Option { - 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, 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::(&toml_content) { - let auto: BTreeSet = 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::(&toml_content) { + let auto: BTreeSet = 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] diff --git a/src/cook/fetch.rs b/src/cook/fetch.rs index 0506c89404..d15c2027cc 100644 --- a/src/cook/fetch.rs +++ b/src/cook/fetch.rs @@ -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); } diff --git a/src/cook/fs.rs b/src/cook/fs.rs index f53b623c27..585adccd47 100644 --- a/src/cook/fs.rs +++ b/src/cook/fs.rs @@ -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 = 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(())