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

fix: harden build cache against 7 correctness issues

Browse Source 
C1 (Critical): Binary store restore now iterates ALL stage directories
  instead of only the first, matching how cook_creates handles multi-stage.
C2 (Critical): All binary store restore errors are now logged via
  log_to_pty! instead of being silently discarded with let _ =.
H1 (High): dep_hashes.toml keys now use full PackageName (including
  host: prefix) via name.to_string() instead of name.without_prefix(),
  preventing host/target key collisions.
H2 (High): Patch file mtimes are now included in source_modified
  calculation, so editing a patch correctly triggers a rebuild.
H4 (High): All to_str().unwrap() calls replaced with safe alternatives
  (to_string_lossy, direct PathBuf refs) to prevent panics on non-UTF8
  paths.
H5 (High): auto_deps.toml reconstruction now logs a warning that it
  may be incomplete (does not include ELF-discovered dynamic linking deps).
M1 (Medium): dep_hashes.toml is now written atomically via write-to-tmp
  + fs::rename, preventing corrupted/partial files on crash.
M3 (Medium): Missing source dir now triggers rebuild (SystemTime::now()
  fallback) instead of being masked as no-change via UNIX_EPOCH.
This commit is contained in:
vasilito
2026-06-30 18:24:39 +03:00
parent 4673eab7a5
commit 3c9ddc1407
2 changed files with 100 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files
local/sources/installer
+1 -1
Submodule local/sources/installer updated: e45ce4d57a...8d1bad9eb4
src/cook/cook_build.rs
+99 -32
View File
@@ -6,7 +6,7 @@ use crate::cook::package::{package_source_paths, package_target};
use crate::cook::pty::PtyOut;
use crate::cook::script::*;
use crate::cook::{fetch, fs::*};
use crate::recipe::Recipe;
use crate::recipe::{Recipe, SourceRecipe};
use crate::recipe::{AutoDeps, CookRecipe};
use crate::recipe::{BuildKind, OptionalPackageRecipe};
use serde::{Deserialize, Serialize};
@@ -38,8 +38,12 @@ impl DepHashes {
pub fn write(&self, path: &Path) -> Result<(), String> {
let content = toml::to_string(self)
.map_err(|e| format!("failed to serialize dep_hashes.toml: {e}"))?;
fs::write(path, content)
.map_err(|e| format!("failed to write dep_hashes.toml: {e:?}"))?;
// Atomic write: write to temp file, then rename
let tmp_path = path.with_extension("tmp");
fs::write(&tmp_path, &content)
.map_err(|e| format!("failed to write dep_hashes.toml tmp: {e:?}"))?;
fs::rename(&tmp_path, path)
.map_err(|e| format!("failed to rename dep_hashes.toml: {e:?}"))?;
Ok(())
}
}
@@ -54,11 +58,16 @@ fn collect_current_dep_hashes(
if let Ok(toml_content) = fs::read_to_string(&toml_path) {
if let Ok(pkg) = toml::from_str::<Package>(&toml_content) {
if !pkg.blake3.is_empty() {
hashes.insert(name.without_prefix().to_string(), pkg.blake3);
// Key by full package name (including host: prefix) to avoid
// collisions between host and target deps with the same base name.
hashes.insert(name.to_string(), pkg.blake3);
}
}
}
}
// Missing .toml or parse failure: dep is excluded from current hashes.
// If it was previously tracked, dep_hashes_changed() will detect the
// length mismatch and trigger a rebuild (conservative behavior).
}
hashes
}
@@ -328,51 +337,109 @@ pub fn build(
.map(|t| PathBuf::from("repo").join(t))
.unwrap_or_else(|| PathBuf::from("repo").join(redoxer::target()));
let recipe_name = name.name();
let main_pkgar = repo_target.join(format!("{}.pkgar", recipe_name));
let main_toml = repo_target.join(format!("{}.toml", recipe_name));
if main_pkgar.is_file() && main_toml.is_file() {
if cli_verbose {
log_to_pty!(logger, "DEBUG: restoring stage artifacts from repo/ binary store");
let pkey_path = "build/id_ed25519.pub.toml";
let pkey_exists = Path::new(pkey_path).is_file();
let mut all_restored = true;
for (stage_dir, stage_pkgar) in stage_dirs.iter().zip(stage_pkgars.iter()) {
if stage_pkgar.is_file() && stage_dir.is_dir() {
continue;
}
if let Some(last_stage) = stage_dirs.last() {
if !last_stage.is_dir() {
let pkey_path = "build/id_ed25519.pub.toml";
if Path::new(pkey_path).is_file() {
let _ = pkgar::extract(pkey_path, &main_pkgar, last_stage.to_str().unwrap());
// Derive the repo pkgar name from the stage dir name.
// "stage" → recipe_name, "stage.feature" → recipe_name.feature
let stage_suffix = stage_dir
.file_name()
.and_then(|n| n.to_str())
.and_then(|s| s.strip_prefix("stage"))
.unwrap_or("");
let repo_pkgar_name = format!("{}{}.pkgar", recipe_name, stage_suffix);
let repo_toml_name = format!("{}{}.toml", recipe_name, stage_suffix);
let repo_pkgar = repo_target.join(&repo_pkgar_name);
let repo_toml = repo_target.join(&repo_toml_name);
if !repo_pkgar.is_file() || !repo_toml.is_file() {
if stage_suffix.is_empty() {
log_to_pty!(logger, "WARN: binary store missing {} — will rebuild", repo_pkgar.display());
}
all_restored = false;
continue;
}
if !stage_dir.is_dir() {
if pkey_exists {
if let Err(e) = pkgar::extract(pkey_path, &repo_pkgar, stage_dir) {
log_to_pty!(logger, "WARN: pkgar extract failed for {}: {e}", repo_pkgar.display());
all_restored = false;
continue;
}
} else {
log_to_pty!(logger, "WARN: signing key {} missing — cannot extract {}", pkey_path, repo_pkgar.display());
all_restored = false;
continue;
}
}
if let Some(last_pkgar) = stage_pkgars.last() {
if !last_pkgar.is_file() {
let _ = fs::copy(&main_pkgar, last_pkgar);
if !stage_pkgar.is_file() {
if let Err(e) = fs::copy(&repo_pkgar, stage_pkgar) {
log_to_pty!(logger, "WARN: failed to copy {}: {e}", repo_pkgar.display());
all_restored = false;
}
}
if let Some(last_toml) = stage_pkgars.last().map(|p| p.with_extension("toml")) {
if !last_toml.is_file() {
let _ = fs::copy(&main_toml, &last_toml);
let stage_toml = stage_pkgar.with_extension("toml");
if !stage_toml.is_file() {
if let Err(e) = fs::copy(&repo_toml, &stage_toml) {
log_to_pty!(logger, "WARN: failed to copy {}: {e}", repo_toml.display());
}
}
let repo_dep_hashes = repo_target.join(format!("{}.dep_hashes.toml", recipe_name));
let local_dep_hashes = get_sub_target_dir(target_dir, "dep_hashes.toml");
if repo_dep_hashes.is_file() && !local_dep_hashes.is_file() {
let _ = fs::copy(&repo_dep_hashes, &local_dep_hashes);
}
let repo_dep_hashes = repo_target.join(format!("{}.dep_hashes.toml", recipe_name));
let local_dep_hashes = get_sub_target_dir(target_dir, "dep_hashes.toml");
if repo_dep_hashes.is_file() && !local_dep_hashes.is_file() && all_restored {
if let Err(e) = fs::copy(&repo_dep_hashes, &local_dep_hashes) {
log_to_pty!(logger, "WARN: failed to copy dep_hashes.toml: {e}");
}
if !auto_deps_file.is_file() {
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 };
let _ = serialize_and_write(&auto_deps_file, &wrapper);
}
// Reconstruct auto_deps.toml from repo metadata (may be incomplete —
// does not include ELF-discovered dynamic linking deps)
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)");
}
}
}
}
}
let mut source_modified = modified_dir_ignore_git(source_dir).unwrap_or(SystemTime::UNIX_EPOCH);
let mut source_modified = modified_dir_ignore_git(source_dir).unwrap_or_else(|_| {
log_to_pty!(logger, "WARN: source dir {} inaccessible — treating as modified", source_dir.display());
SystemTime::now()
});
if let Ok(recipe_modified) = modified(&recipe_dir.join("recipe.toml")) {
source_modified = source_modified.max(recipe_modified);
}
let recipe_patches: &[String] = match &recipe.source {
Some(SourceRecipe::Git { patches, .. }) => patches,
Some(SourceRecipe::Tar { patches, .. }) => patches,
_ => &[],
};
for patch in recipe_patches {
let patch_path = recipe_dir.join(patch);
if let Ok(patch_modified) = modified(&patch_path) {
source_modified = source_modified.max(patch_modified);
}
}
let deps_modified = modified_all_btree(
dep_pkgars.iter().map(|(_dep, pkgar)| pkgar.as_path()),
@@ -778,7 +845,7 @@ fn build_deps_dir(
let tag_file = tags_dir.join(name.without_prefix());
fs::write(&tag_file, "")
.map_err(|e| format!("failed to write tag file {}: {:?}", tag_file.display(), e))?;
pkgar::extract(pkey_path, &archive_path, deps_dir_tmp.to_str().unwrap()).map_err(
pkgar::extract(pkey_path, &archive_path, &deps_dir_tmp).map_err(
|err| {
format!(
"failed to install '{}' in '{}': {:?}",