deb: survive crashed sessions and config-blind resumes
Two failures from resumed kernel sessions: A resumed build re-runs debian/rules build, but the kernels keep their wrapper-step stamps in debian/stamps and the flavour config rule there exports .config from the annotations with no config prerequisite: the resumed build silently kept the previous attempt's configuration and a config edit never reached the .deb. Dropping the stamp cache unconditionally would impose the kernel packaging's shape on every package, so it is data-driven instead: packages declare the caches under a resume_clear quirk, and resumed builds remove those tree-relative paths before the build. The inner kbuild keeps its own incremental state, so only the cheap wrapper passes re-run and config-affected objects recompile. A SIGKILLed build (OOM) leaves its overlay mounts and /proc bind mount behind, and overlayfs creates root-owned work state inside the workdir: the next plain build (recording is always on, it replaces the session of its identity) failed to clear the leftovers with a permission error. Session clearing now unmounts everything under each entry at depth (re-reading /proc/mounts, tolerating mount stacks from consecutive crashes) and escalates through sudo; when the tree still cannot be cleared, the build reports it and continues without a session instead of layering over a half-cleared one.
This commit is contained in:
@@ -134,6 +134,14 @@ A plain `pkh deb` never reuses a session — everything is rechecked from
|
||||
scratch — and it replaces the session of its target. `pkh deb --resume`
|
||||
refuses to adopt a session built for a different series/architecture.
|
||||
|
||||
Packages whose `debian/rules` keeps an input-untracked stamp cache (the
|
||||
kernels' `debian/stamps`) declare it in the quirks data (`resume_clear`):
|
||||
resumed builds drop the cache so the wrapper steps — including the
|
||||
kernel's flavour config export — re-run over the incremental inner
|
||||
builds, and a config edit is never silently ignored. If a build died
|
||||
hard (OOM kill), the next build unmounts and clears the leftover session
|
||||
state first (escalating through sudo when needed).
|
||||
|
||||
### Running the DEP-8 tests
|
||||
|
||||
`pkh test` runs the package's as-installed tests (`debian/tests/control`,
|
||||
|
||||
@@ -23,8 +23,20 @@ quirks:
|
||||
# architecture only, so a cross build installs no build-architecture
|
||||
# variants and the kernel's host-side tools cannot link. Inject the
|
||||
# build-architecture variants until the control is fixed upstream.
|
||||
# The kernel's debian/rules keeps its wrapper-step stamps in
|
||||
# debian/stamps, and the flavour config rule there exports .config from
|
||||
# the annotations with no config prerequisite — a resumed build would
|
||||
# keep the previous attempt's configuration and silently ignore config
|
||||
# edits. List the stamp cache under `resume_clear` so resumed builds
|
||||
# drop it and re-run the wrapper steps; the inner kbuild keeps its own
|
||||
# incremental state, so only the cheap wrapper passes re-run. This is
|
||||
# the packaging's long-standing shape rather than a fixable regression,
|
||||
# so the entry applies to every series.
|
||||
linux:
|
||||
deb:
|
||||
- series: []
|
||||
resume_clear:
|
||||
- debian/stamps
|
||||
- series: [resolute]
|
||||
dependencies:
|
||||
replace:
|
||||
@@ -37,6 +49,9 @@ quirks:
|
||||
- libssl-dev:native
|
||||
linux-riscv:
|
||||
deb:
|
||||
- series: []
|
||||
resume_clear:
|
||||
- debian/stamps
|
||||
- series: [resolute]
|
||||
dependencies:
|
||||
replace:
|
||||
|
||||
@@ -427,3 +427,29 @@ ephemeral}.rs`, `src/context/{api,unshare}.rs`, `src/prune.rs` and
|
||||
build → same; overlay mount unsupported/failed on resume → fresh
|
||||
copy staging (environment resume only, artifacts discarded); no
|
||||
host-tree snapshot → upper wiped and re-snapshotted.
|
||||
|
||||
# Addendum: resume correctness (post-merge findings)
|
||||
|
||||
Two real-world failures against a kept kernel session, both fixed:
|
||||
|
||||
- **The kernel's config chain is stamp-blind** — `stamp-prepare-%`
|
||||
(`debian/rules.d/2-binary-arch.mk`) exports `.config` from the
|
||||
annotations with no config prerequisite, so a resumed build silently
|
||||
kept the previous attempt's configuration; the config edit never
|
||||
reached the .deb even though `debian/rules build` ran. Dropping the
|
||||
stamp cache unconditionally would be a package-specific decision, so
|
||||
it is data-driven: the kernels declare `resume_clear: [debian/stamps]`
|
||||
in `data/quirks.yml`, and resumed builds remove those tree-relative
|
||||
paths before `debian/rules build`. The inner kbuild keeps its own
|
||||
state, so the re-run wrappers stay cheap and only config-affected
|
||||
objects recompile.
|
||||
- **A SIGKILLed build (OOM) poisons the next one.** The crashed attempt
|
||||
leaves its overlay mounts and /proc bind mount behind, and overlayfs
|
||||
creates root-owned `work` state inside the workdir; the next PLAIN
|
||||
build (recording is always on: it replaces the session of its
|
||||
identity) failed to clear the leftovers with a permission error.
|
||||
`clear_dir()` now unmounts everything under each entry at depth
|
||||
(re-reading /proc/mounts, tolerating mount stacks from consecutive
|
||||
crashes) and escalates through sudo; if the tree still cannot be
|
||||
cleared, the plain build reports it and continues WITHOUT a session
|
||||
instead of failing or layering over a half-cleared tree.
|
||||
|
||||
+76
-6
@@ -147,15 +147,48 @@ pub(crate) fn privileged_remove(path: &Path) -> std::io::Result<()> {
|
||||
}
|
||||
}
|
||||
|
||||
/// Remove the contents of `dir` but keep the directory itself.
|
||||
fn wipe_dir_contents(dir: &Path) -> std::io::Result<()> {
|
||||
/// Clear a directory's contents, keeping the directory itself.
|
||||
///
|
||||
/// Built for re-cleaning a crashed build's leftovers: a SIGKILLed build
|
||||
/// (OOM, power loss) leaves its overlay mounts and /proc bind mount behind,
|
||||
/// and the kernel creates root-owned `work` state inside an overlay
|
||||
/// workdir that a plain `rm -rf` cannot remove. So every child is first
|
||||
/// unmounted at depth (re-reading /proc/mounts until nothing is left under
|
||||
/// it), then removed with a privileged `rm -rf` fallback.
|
||||
///
|
||||
/// Returns an error when some content could not be removed: callers should
|
||||
/// refuse to layer a fresh build over a half-cleared tree.
|
||||
pub(crate) fn clear_dir(dir: &Path) -> std::io::Result<()> {
|
||||
fs::create_dir_all(dir)?;
|
||||
const MAX_UNMOUNT_PASSES: usize = 10;
|
||||
for entry in fs::read_dir(dir)?.flatten() {
|
||||
let path = entry.path();
|
||||
if path.is_dir() {
|
||||
fs::remove_dir_all(&path)?;
|
||||
// Unmount everything under the child, deepest first; the loop
|
||||
// handles stacks (a re-mount over an earlier, never-unmounted one —
|
||||
// exactly what consecutive crashed builds leave behind).
|
||||
for _ in 0..MAX_UNMOUNT_PASSES {
|
||||
let mounts = host_mounts_under(&path);
|
||||
if mounts.is_empty() {
|
||||
break;
|
||||
}
|
||||
let is_root = crate::utils::root::is_root().unwrap_or(false);
|
||||
for mount in mounts.into_iter().rev() {
|
||||
if !unmount_path(&mount, is_root) {
|
||||
log::warn!("Failed to unmount {} (retrying)", mount.display());
|
||||
}
|
||||
}
|
||||
}
|
||||
let result = if path.is_dir() {
|
||||
fs::remove_dir_all(&path)
|
||||
} else {
|
||||
fs::remove_file(&path)?;
|
||||
fs::remove_file(&path)
|
||||
}
|
||||
.or_else(|_| privileged_remove(&path));
|
||||
if let Err(e) = result {
|
||||
return Err(std::io::Error::other(format!(
|
||||
"cannot remove {}: {e}",
|
||||
path.display()
|
||||
)));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
@@ -487,7 +520,7 @@ impl EphemeralContextGuard {
|
||||
}
|
||||
// Not reusing (fresh session or failed probe): never extract
|
||||
// over stale content.
|
||||
wipe_dir_contents(chroot_path)?;
|
||||
clear_dir(chroot_path)?;
|
||||
}
|
||||
|
||||
// Clone ctx for use in create_device_nodes after download_chroot_tarball consumes it
|
||||
@@ -1051,4 +1084,41 @@ mod chroot_cleanup_tests {
|
||||
|
||||
assert!(!chroot.exists());
|
||||
}
|
||||
|
||||
/// clear_dir removes content a plain `rm -rf` cannot: a root-owned
|
||||
/// 0700 directory (the shape of an overlay workdir left by a crashed
|
||||
/// build). Skipped when neither root nor working non-interactive sudo
|
||||
/// is available, since escalation is then legitimately impossible.
|
||||
#[test]
|
||||
fn clear_dir_removes_root_owned_subdirectories() {
|
||||
let is_root = unsafe { libc::geteuid() } == 0;
|
||||
if !is_root
|
||||
&& !privileged_command("true", false)
|
||||
.status()
|
||||
.is_ok_and(|s| s.success())
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let cleared = dir.path().join("chroot");
|
||||
std::fs::create_dir_all(cleared.join("stale")).unwrap();
|
||||
std::fs::write(cleared.join("stale").join("f"), "x").unwrap();
|
||||
// Root-owned, non-traversable: the crashed-workdir shape.
|
||||
let _ = privileged_command("chmod", is_root)
|
||||
.arg("-R")
|
||||
.arg("0700")
|
||||
.arg(cleared.join("stale"))
|
||||
.status();
|
||||
let _ = privileged_command("chown", is_root)
|
||||
.arg("-R")
|
||||
.arg("root:root")
|
||||
.arg(cleared.join("stale"))
|
||||
.status();
|
||||
|
||||
clear_dir(&cleared).expect("clear_dir removes root-owned leftovers");
|
||||
|
||||
let read = std::fs::read_dir(&cleared).unwrap();
|
||||
assert_eq!(read.count(), 0, "the directory must be empty");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -340,6 +340,31 @@ pub async fn build(
|
||||
);
|
||||
}
|
||||
|
||||
// A resumed build continues in the previous attempt's tree. Packages
|
||||
// whose debian/rules keeps an input-untracked stamp cache (quirk:
|
||||
// `resume_clear`, e.g. the kernels' `debian/stamps`, whose flavour
|
||||
// config rule exports .config with no config prerequisite) would build
|
||||
// from the previous attempt's configuration: drop the listed caches so
|
||||
// the wrapper steps re-run over the incremental inner builds.
|
||||
if resume {
|
||||
for rel in crate::quirks::get_resume_clear_paths(package, series) {
|
||||
let path = Path::new(package_dir_str).join(&rel);
|
||||
if !ctx.exists(&path).unwrap_or(false) {
|
||||
continue;
|
||||
}
|
||||
log::info!("Dropping the resumed build's cached '{rel}' (quirk)");
|
||||
let status = ctx
|
||||
.command("rm")
|
||||
.arg("-rf")
|
||||
.arg(&path)
|
||||
.status()
|
||||
.map_err(|e| format!("cannot drop '{rel}': {e}"))?;
|
||||
if !status.success() {
|
||||
return Err(format!("cannot drop '{rel}'").into());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Run the build step
|
||||
log::debug!("Building (debian/rules build) package...");
|
||||
enter_phase(view, Phase::Building);
|
||||
|
||||
@@ -524,6 +524,10 @@ fn resolve_session(
|
||||
log::info!("{busy}; building without a session");
|
||||
Ok(None)
|
||||
}
|
||||
Err(session::SessionOpenError::Unusable(e)) => {
|
||||
log::error!("{e}; building without a session");
|
||||
Ok(None)
|
||||
}
|
||||
Err(session::SessionOpenError::Other(e)) => Err(e.into()),
|
||||
},
|
||||
SessionResume::Latest => {
|
||||
|
||||
+16
-36
@@ -350,20 +350,7 @@ impl Session {
|
||||
/// attempt's tree would merge stale build artifacts and resurrect
|
||||
/// host-deleted files into the build.
|
||||
pub fn wipe_staged_tree(&self) -> io::Result<()> {
|
||||
let staged = self.chroot_path().join("tmp/pkh-build");
|
||||
fs::create_dir_all(&staged)?;
|
||||
for entry in fs::read_dir(&staged)?.flatten() {
|
||||
let path = entry.path();
|
||||
let result = if path.is_dir() {
|
||||
fs::remove_dir_all(&path)
|
||||
} else {
|
||||
fs::remove_file(&path)
|
||||
}
|
||||
.or_else(|_| crate::deb::ephemeral::privileged_remove(&path));
|
||||
result
|
||||
.map_err(|e| io::Error::other(format!("cannot clear {}: {e}", staged.display())))?;
|
||||
}
|
||||
Ok(())
|
||||
crate::deb::ephemeral::clear_dir(&self.chroot_path().join("tmp/pkh-build"))
|
||||
}
|
||||
|
||||
/// Remove the whole session tree (unmounting anything mounted under
|
||||
@@ -557,6 +544,9 @@ fn open_from_dir(root: PathBuf) -> Option<Session> {
|
||||
pub enum SessionOpenError {
|
||||
/// The session is locked by a concurrent build.
|
||||
Busy(SessionBusy),
|
||||
/// The previous session could not be cleared (crashed-build leftovers
|
||||
/// that even the privileged removal could not lift).
|
||||
Unusable(String),
|
||||
/// Anything else (I/O, manifest errors).
|
||||
Other(String),
|
||||
}
|
||||
@@ -565,6 +555,7 @@ impl std::fmt::Display for SessionOpenError {
|
||||
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
|
||||
match self {
|
||||
SessionOpenError::Busy(busy) => write!(f, "{busy}"),
|
||||
SessionOpenError::Unusable(message) => write!(f, "{message}"),
|
||||
SessionOpenError::Other(message) => write!(f, "{message}"),
|
||||
}
|
||||
}
|
||||
@@ -618,28 +609,17 @@ pub fn fresh_session(
|
||||
|
||||
session.acquire().map_err(SessionOpenError::Busy)?;
|
||||
|
||||
// Wipe the previous attempt (the lock file lives outside the root): a
|
||||
// previous chroot holds root-owned device nodes, so escalate on
|
||||
// failure, like the prune removal does.
|
||||
for entry in fs::read_dir(&root)
|
||||
.map_err(|e| SessionOpenError::Other(format!("cannot clear {}: {e}", root.display())))?
|
||||
.flatten()
|
||||
{
|
||||
let path = entry.path();
|
||||
let result = if path.is_dir() {
|
||||
fs::remove_dir_all(&path)
|
||||
} else {
|
||||
fs::remove_file(&path)
|
||||
}
|
||||
.or_else(|_| crate::deb::ephemeral::privileged_remove(&path));
|
||||
if let Err(e) = result {
|
||||
log::warn!(
|
||||
"Failed to clear the previous session file '{}': {}",
|
||||
path.display(),
|
||||
e
|
||||
);
|
||||
}
|
||||
}
|
||||
// Wipe the previous attempt (the lock file lives outside the root). A
|
||||
// crashed build leaves its mounts behind and the kernel creates
|
||||
// root-owned state inside overlay workdirs, so the clearing unmounts
|
||||
// at depth and escalates via sudo when needed; see clear_dir.
|
||||
crate::deb::ephemeral::clear_dir(&root).map_err(|e| {
|
||||
SessionOpenError::Unusable(format!(
|
||||
"cannot clear the previous build session {}: {e}. \
|
||||
Free it with `pkh prune` (or remove it manually) and rebuild",
|
||||
root.display()
|
||||
))
|
||||
})?;
|
||||
|
||||
session.reset_fresh(host_tree, identity, tree_version);
|
||||
Ok(session)
|
||||
|
||||
@@ -54,6 +54,20 @@ pub struct OperationQuirks {
|
||||
/// like linux packages that use directories like "linux-main" or other custom names
|
||||
#[serde(default)]
|
||||
pub package_directory: Vec<String>,
|
||||
|
||||
/// Paths (relative to the package tree) removed before
|
||||
/// `debian/rules build` when the build resumes a previous attempt.
|
||||
///
|
||||
/// For packages whose debian/rules keeps a stamp cache that does not
|
||||
/// track its inputs (the Ubuntu/Debian kernels' `debian/stamps`: the
|
||||
/// flavour config rule exports `.config` from the annotations with no
|
||||
/// config prerequisite), a resumed build would keep building from the
|
||||
/// previous attempt's configuration. Listing the stamp directories
|
||||
/// here drops the cache so the wrapper steps re-run, while the inner
|
||||
/// build system (kbuild) keeps its own incremental state. Only list
|
||||
/// caches whose re-run is cheap or whose inner build is incremental.
|
||||
#[serde(default)]
|
||||
pub resume_clear: Vec<String>,
|
||||
}
|
||||
|
||||
/// Quirks for a specific package
|
||||
@@ -166,6 +180,33 @@ pub fn get_package_directories(package: &str, series: &str) -> Vec<String> {
|
||||
directories
|
||||
}
|
||||
|
||||
/// Get the paths to clear before a resumed `debian/rules build`
|
||||
///
|
||||
/// Every matching deb entry contributes its paths, in file order. See
|
||||
/// [`OperationQuirks::resume_clear`] for what belongs here.
|
||||
///
|
||||
/// # Arguments
|
||||
/// * `package` - The package name
|
||||
/// * `series` - The distribution series (e.g. "resolute")
|
||||
///
|
||||
/// # Returns
|
||||
/// * `Vec<String>` - Tree-relative paths to remove, empty when the package
|
||||
/// opts out
|
||||
pub fn get_resume_clear_paths(package: &str, series: &str) -> Vec<String> {
|
||||
let Some(quirks) = get_package_quirks(&QUIRKS_DATA, package) else {
|
||||
return Vec::new();
|
||||
};
|
||||
let mut paths = Vec::new();
|
||||
for deb in quirks
|
||||
.deb
|
||||
.iter()
|
||||
.filter(|q| entry_applies_to_series(q, series))
|
||||
{
|
||||
paths.extend(deb.resume_clear.iter().cloned());
|
||||
}
|
||||
paths
|
||||
}
|
||||
|
||||
/// Apply the dependency quirks of `package` in `series` to parsed
|
||||
/// build-dependency clauses
|
||||
///
|
||||
@@ -374,4 +415,23 @@ mod tests {
|
||||
};
|
||||
assert!(apply_rules(&mut clauses, &deps, &opts()).is_err());
|
||||
}
|
||||
|
||||
/// The kernels opt in to clearing their stamp cache on resume, for
|
||||
/// every series; packages without the entry opt out by default.
|
||||
#[test]
|
||||
fn resume_clear_paths_are_data_driven() {
|
||||
for package in ["linux", "linux-riscv"] {
|
||||
assert_eq!(
|
||||
get_resume_clear_paths(package, "resolute"),
|
||||
vec!["debian/stamps".to_string()]
|
||||
);
|
||||
assert_eq!(
|
||||
get_resume_clear_paths(package, "some-future-series"),
|
||||
vec!["debian/stamps".to_string()],
|
||||
"the stamp-cache entry applies to every series"
|
||||
);
|
||||
}
|
||||
assert!(get_resume_clear_paths("hello", "resolute").is_empty());
|
||||
assert!(get_resume_clear_paths("no-such-package", "resolute").is_empty());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user