diff --git a/src/deb/local.rs b/src/deb/local.rs index c5a5ec8..3a7441d 100644 --- a/src/deb/local.rs +++ b/src/deb/local.rs @@ -251,10 +251,11 @@ fn rules_command<'a>( } /// The fakeroot state file shared by the rules steps of a rootless -/// build, in the staged build root: outside the package tree, so it -/// pollutes neither the sources nor the collected artifacts. -fn fakeroot_state_file(build_root: &str) -> PathBuf { - Path::new(build_root).join(".pkh-fakeroot.state") +/// build, in the upload directory next to the staged tree: outside the +/// package tree, so it pollutes neither the sources nor the collected +/// artifacts. +fn fakeroot_state_file(upload_dir: &str) -> PathBuf { + Path::new(upload_dir).join(".pkh-fakeroot.state") } #[allow(clippy::too_many_arguments)] @@ -265,7 +266,6 @@ pub async fn build( series: &str, pocket: Option<&str>, mirror: Option<&str>, - build_root: &str, package_dir: &Path, cross: bool, ppa: &[String], @@ -556,6 +556,17 @@ pub async fn build( .to_str() .ok_or("Invalid package directory path")?; + // The directory the build produces into: the package directory's + // parent (dh_builddeb's `..`), wherever the staging put the tree. + // The artifact collection, the upload metadata and the scratch of + // the dependency diagnosis all work there — outside the staged + // tree, so nothing of it lands in the tree or its overlay upper. + let upload_dir = Path::new(package_dir_str) + .parent() + .ok_or("cannot find the staged tree's parent directory")? + .to_path_buf(); + let upload_dir = upload_dir.to_str().ok_or("Invalid upload directory")?; + // Reproducibility: export SOURCE_DATE_EPOCH from the changelog entry, // like dpkg-buildpackage does. match ctx.read_file(&package_dir.join("debian/changelog")) { @@ -617,7 +628,7 @@ pub async fn build( arch, series, package_dir_str, - build_root, + upload_dir, cross, &env, ctx.clone(), @@ -700,7 +711,7 @@ pub async fn build( // succeeds — faked — and carries into the .deb, instead of failing // per file with EPERM spam. With the context's own user the build // step runs bare; the binary step keeps its unconditional wrap. - let fakeroot_state = fakeroot_state_file(build_root); + let fakeroot_state = fakeroot_state_file(upload_dir); if rule_ids.is_some() { // A state left by an interrupted previous attempt must not // seed this session. @@ -780,13 +791,13 @@ pub async fn build( // generators) plus the .buildinfo/.changes generated below. This avoids // globbing the build root, which would also surface stale files copied // alongside the package tree. - let mut artifacts = collect_binary_artifacts(&ctx, package_dir_str, build_root)?; + let mut artifacts = collect_binary_artifacts(&ctx, package_dir_str, upload_dir)?; // Generate the upload metadata (.buildinfo + .changes) natively, the // equivalent of dpkg-genbuildinfo -b + dpkg-genchanges -b, consuming // debian/files produced by the build. Failures are logged but do not // discard the produced binaries. - match generate_upload_metadata(package_dir_str, build_root, arch, cross, &env, &ctx) { + match generate_upload_metadata(package_dir_str, upload_dir, arch, cross, &env, &ctx) { Ok((buildinfo, changes)) => { log::info!( "generated upload metadata: {} and {}", @@ -1055,7 +1066,7 @@ fn install_build_dependencies( arch: &str, series: &str, package_dir: &str, - build_root: &str, + upload_dir: &str, cross: bool, env: &HashMap, ctx: Arc, @@ -1151,7 +1162,7 @@ fn install_build_dependencies( if !violated.is_empty() { view.suspend(); if let Err(e) = - dose3_explain_dependencies(package, version, arch, build_root, cross, ctx.clone()) + dose3_explain_dependencies(package, version, arch, upload_dir, cross, ctx.clone()) { log::debug!("dose-builddebcheck diagnosis failed: {e}"); } @@ -1184,7 +1195,7 @@ fn install_build_dependencies( if specs.is_empty() { view.suspend(); if let Err(e) = - dose3_explain_dependencies(package, version, arch, build_root, cross, ctx.clone()) + dose3_explain_dependencies(package, version, arch, upload_dir, cross, ctx.clone()) { log::debug!("dose-builddebcheck diagnosis failed: {e}"); } @@ -1223,7 +1234,7 @@ fn install_build_dependencies( package, version, arch, - build_root, + upload_dir, cross, ctx.clone(), ) @@ -1241,21 +1252,22 @@ fn install_build_dependencies( /// Collect the binary artifacts (.deb/.udeb) registered by the build in /// `debian/files`, returning their paths inside the build context -/// (`/`). `debian/files` is the canonical record of +/// (`/`, dh_builddeb's `..`: the package +/// directory's parent). `debian/files` is the canonical record of /// which binary packages `debian/rules binary` produced, so we retrieve /// exactly those instead of globbing the build root (which would also pick /// up stale files copied alongside the package tree). fn collect_binary_artifacts( ctx: &Arc, package_dir: &str, - build_root: &str, + upload_dir: &str, ) -> Result, Box> { let files_content = ctx .read_file(&Path::new(package_dir).join("debian/files")) .unwrap_or_default(); let files_list = crate::debian::FilesList::parse(&files_content) .map_err(|e| format!("invalid debian/files in {package_dir}: {e}"))?; - let upload_dir = Path::new(build_root); + let upload_dir = Path::new(upload_dir); let mut artifacts = Vec::new(); for entry in files_list.iter() { if entry.is_binary_artifact() { @@ -1269,7 +1281,7 @@ fn collect_binary_artifacts( /// inside the build context. fn generate_upload_metadata( package_dir: &str, - build_root: &str, + upload_dir: &str, arch: &str, cross: bool, env: &HashMap, @@ -1328,7 +1340,7 @@ fn generate_upload_metadata( let (buildinfo, changes) = crate::build::binary::generate_binary_metadata( ctx, Path::new(package_dir), - Path::new(build_root), + Path::new(upload_dir), &opts, )?; Ok((buildinfo, changes)) @@ -1607,7 +1619,7 @@ fn dose3_explain_dependencies( package: &str, version: &str, arch: &str, - build_root: &str, + upload_dir: &str, cross: bool, ctx: Arc, ) -> Result<(), Box> { @@ -1632,11 +1644,11 @@ fn dose3_explain_dependencies( // Transform the dsc file into a 'Source' stanza (replacing 'Source' with 'Package') // TODO: Remove potential GPG headers/signature - let dsc_path = find_dsc_file(build_root, package, version, &ctx)?; + let dsc_path = find_dsc_file(upload_dir, package, version, &ctx)?; let mut dsc_content = ctx.read_file(&dsc_path)?; dsc_content = dsc_content.replace("Source", "Package"); ctx.write_file( - Path::new(&format!("{build_root}/dsc-processed")), + Path::new(&format!("{upload_dir}/dsc-processed")), &dsc_content, )?; @@ -1655,7 +1667,7 @@ fn dose3_explain_dependencies( .arg(format!("--deb-foreign-archs={}", arch)); } - cmd.args(bg_args).arg(format!("{build_root}/dsc-processed")); + cmd.args(bg_args).arg(format!("{upload_dir}/dsc-processed")); cmd.status()?; Ok(()) } diff --git a/src/deb/mod.rs b/src/deb/mod.rs index e565d13..0a9869b 100644 --- a/src/deb/mod.rs +++ b/src/deb/mod.rs @@ -605,13 +605,14 @@ async fn build_binary_package_impl( // the upperdir) and re-mount it, so the previous attempt's build // artifacts (object files) are visible to make; a cold staging // (no snapshot, or no overlay support) stages a fresh copy. - let parent_dir = host_tree - .parent() - .ok_or("Cannot find parent directory")? - .to_path_buf(); + // + // The staged tree is the pointed-at tree itself — its content is + // the merge point of the build root — never its parent: siblings + // stay on the host, so a tree sitting in a busy directory does + // not drag it into the chroot. let mut build_resume = false; if let Some(s) = &session { - build_resume = session::sync_upper_for_resume(s, &parent_dir)?; + build_resume = session::sync_upper_for_resume(s, &host_tree)?; if build_resume { log::info!( "Reusing the build artifacts of the previous attempt (incremental build)" @@ -627,7 +628,7 @@ async fn build_binary_package_impl( // host-deleted files into the build. Still environment- // resumable, just not artifact-resumable. let staging = match build_ctx.ensure_available_with_overlay( - &parent_dir, + &host_tree, &build_root, &s.upper_dir(), &s.work_dir(), @@ -640,7 +641,7 @@ async fn build_binary_package_impl( e ); s.wipe_staged_tree()?; - let path = build_ctx.ensure_available(&parent_dir, &build_root)?; + let path = build_ctx.ensure_available(&host_tree, &build_root)?; build_resume = false; crate::context::OverlayStaging { path, @@ -655,10 +656,10 @@ async fn build_binary_package_impl( } staging.path } else { - build_ctx.ensure_available(&parent_dir, &build_root)? + build_ctx.ensure_available(&host_tree, &build_root)? }; - // The staged parent (build root + workspace name) is where the - // build steps and the artifact collection work. + // The staged tree (build root + tree name) is where the build + // steps and the artifact collection work. let build_root = staged_path.to_string_lossy().into_owned(); if let Some(s) = &session { @@ -667,11 +668,11 @@ async fn build_binary_package_impl( // Resolve the package directory inside the staging area. The tree // the caller pointed at is authoritative (its changelog defined the - // package/version/series above), so its staged copy wins; the - // name-pattern search only runs as a fallback. + // package/version/series above), so its staged content wins; the + // name-pattern search only runs when the tree holds a layout + // instead (a nested checkout, a version-suffixed unpack). let package_dir = resolve_package_directory( Path::new(&build_root), - cwd, &package, &version, series, @@ -691,7 +692,6 @@ async fn build_binary_package_impl( series, pocket.as_deref(), mirror.as_deref(), - &build_root, &package_dir, cross, ppa, @@ -713,9 +713,14 @@ async fn build_binary_package_impl( }; // Retrieve the produced artifacts (binary packages plus the upload - // metadata) to the parent directory. + // metadata) to the parent directory: the .debs land next to the + // tree, on the host, whatever the staging scope is. enter_phase(view, Phase::RetrievingArtifacts, &mut profiler); let total_debs = remote_files.len(); + let parent_dir = host_tree + .parent() + .ok_or("Cannot find parent directory")? + .to_path_buf(); let mut artifacts = Vec::with_capacity(total_debs); for (idx, remote_file) in remote_files.iter().enumerate() { @@ -903,32 +908,26 @@ fn resolve_record_in( /// Resolve the package directory for a build inside the staged build root. /// -/// The tree the caller pointed at is authoritative: `cwd`'s changelog -/// already defined the package, version and series for this build, so its -/// staged copy is used outright when it carries a `debian/` tree. The -/// name-pattern search ([`find_package_directory`], including the quirks -/// overrides) only runs when that copy cannot be resolved — a default `.` -/// cwd has no basename, and the pointed-at tree may live outside the staged -/// parent. Embedded callers are the motivation: their working directory -/// names (`tree`, `checkout`, ...) match none of the search patterns. +/// The staged tree is the merge point of the build root: its content is +/// the content of the tree the caller pointed at — whose changelog already +/// defined the package, version and series — so the package directory is +/// the root itself when the tree carries a `debian/` directory. Otherwise +/// the tree holds a layout to search (a nested checkout, a +/// version-suffixed unpack): [`find_package_directory`], including the +/// quirks overrides. pub(crate) fn resolve_package_directory( build_root: &Path, - cwd: &Path, package: &str, version: &str, series: &str, ctx: &context::Context, ) -> Result> { - if let Some(tree_name) = cwd.file_name() { - let staged_tree = build_root.join(tree_name); - if ctx.is_dir(&staged_tree)? && ctx.exists(&staged_tree.join("debian"))? { - log::debug!( - "Using the staged copy of {} at {}", - cwd.display(), - staged_tree.display() - ); - return Ok(staged_tree); - } + if ctx.is_dir(build_root)? && ctx.exists(&build_root.join("debian"))? { + log::debug!( + "Using the staged copy of the tree at {}", + build_root.display() + ); + return Ok(build_root.to_path_buf()); } find_package_directory(build_root, package, version, series, ctx) } @@ -1209,10 +1208,11 @@ mod tests { #[test] fn find_package_directory_lists_staged_directories_through_the_context() { let chroot = tempfile::tempdir().unwrap(); - // Staged parent holding a single tree whose name matches none of - // the search patterns (the embedded-caller layout: /tree) - let staged_parent = chroot.path().join("tmp/pkh-build-1/j-42"); - std::fs::create_dir_all(staged_parent.join("tree/debian")).unwrap(); + // Staged tree holding a single nested checkout whose name matches + // none of the search patterns (the embedded-caller layout: + // /tree) + let staged_tree = chroot.path().join("tmp/pkh-build-1/j-42"); + std::fs::create_dir_all(staged_tree.join("tree/debian")).unwrap(); let ctx = unshare_test_context(chroot.path()); let err = find_package_directory( @@ -1231,43 +1231,43 @@ mod tests { ); } - /// An explicit cwd must resolve to its staged copy even when its name - /// matches none of the search patterns: the pointed-at tree is what the - /// parsed changelog came from. + /// The staged tree is the merge point of the build root: when the + /// pointed-at tree is itself the package, its content sits at the + /// root and resolves outright — even when the tree name matches none + /// of the search patterns (the embedded-caller layout: /tree). #[test] fn resolve_package_directory_prefers_the_pointed_tree() { let chroot = tempfile::tempdir().unwrap(); - let staged_parent = chroot.path().join("tmp/pkh-build-1/j-42"); - std::fs::create_dir_all(staged_parent.join("tree/debian/source")).unwrap(); + let staged_tree = chroot.path().join("tmp/pkh-build-1/tree"); + std::fs::create_dir_all(staged_tree.join("debian/source")).unwrap(); let ctx = unshare_test_context(chroot.path()); let resolved = resolve_package_directory( - Path::new("/tmp/pkh-build-1/j-42"), - Path::new("/work/jobs/j-42/tree"), + Path::new("/tmp/pkh-build-1/tree"), "bc", "1.07.1-1ubuntu1", "questing", &ctx, ) - .expect("the staged copy of the pointed-at tree must resolve"); + .expect("the staged tree must resolve to the build root"); - assert_eq!(resolved, PathBuf::from("/tmp/pkh-build-1/j-42/tree")); + assert_eq!(resolved, PathBuf::from("/tmp/pkh-build-1/tree")); } - /// When the pointed-at tree is not in the staging area under its own - /// name, resolution falls back to the name-pattern search. + /// When the staged tree holds a layout instead of the package itself + /// (no debian/ at its root), resolution falls back to the + /// name-pattern search over the staged content. #[test] fn resolve_package_directory_falls_back_to_the_name_search() { let chroot = tempfile::tempdir().unwrap(); - let staged_parent = chroot.path().join("tmp/pkh-build-1/j-42"); - // Staged copy of a pulled tree: /- - std::fs::create_dir_all(staged_parent.join("bc/bc-1.07.1/debian")).unwrap(); + let staged_tree = chroot.path().join("tmp/pkh-build-1/checkout"); + // A pulled-tree layout inside the pointed-at tree: + // /- + std::fs::create_dir_all(staged_tree.join("bc/bc-1.07.1/debian")).unwrap(); let ctx = unshare_test_context(chroot.path()); let resolved = resolve_package_directory( - Path::new("/tmp/pkh-build-1/j-42"), - // A tree never staged under that name - Path::new("/work/other/checkout"), + Path::new("/tmp/pkh-build-1/checkout"), "bc", "1.07.1-1ubuntu1", "questing", @@ -1277,7 +1277,7 @@ mod tests { assert_eq!( resolved, - PathBuf::from("/tmp/pkh-build-1/j-42/bc/bc-1.07.1") + PathBuf::from("/tmp/pkh-build-1/checkout/bc/bc-1.07.1") ); } diff --git a/src/deb/session.rs b/src/deb/session.rs index e14ee3e..fe6e558 100644 --- a/src/deb/session.rs +++ b/src/deb/session.rs @@ -1933,8 +1933,8 @@ pub(crate) fn load_snapshot(path: &Path) -> Option> } /// Record the host tree snapshot for a fresh staging. -pub(crate) fn take_snapshot(host_parent: &Path, session: &Session) -> io::Result<()> { - let meta = walk_tree_meta(host_parent)?; +pub(crate) fn take_snapshot(host_tree: &Path, session: &Session) -> io::Result<()> { + let meta = walk_tree_meta(host_tree)?; write_snapshot(&meta, &session.snapshot_file()) } @@ -2114,17 +2114,17 @@ pub(crate) fn apply_upper_sync(upper_root: &Path, ops: &[UpperOp]) -> io::Result /// possible (a usable snapshot existed and the upper was synced): /// without a snapshot the upper is wiped and the build stages cold, /// but safely. -pub(crate) fn sync_upper_for_resume(session: &Session, host_parent: &Path) -> io::Result { +pub(crate) fn sync_upper_for_resume(session: &Session, host_tree: &Path) -> io::Result { let Some(snapshot) = load_snapshot(&session.snapshot_file()) else { log::info!( "No host-tree snapshot in the record: build artifacts are \ discarded, the environment is still reused" ); session.wipe_build_artifacts(); - take_snapshot(host_parent, session)?; + take_snapshot(host_tree, session)?; return Ok(false); }; - let current = walk_tree_meta(host_parent)?; + let current = walk_tree_meta(host_tree)?; let upper_entries = walk_upper(&session.upper_dir()).unwrap_or_default(); let ops = plan_upper_sync(&snapshot, ¤t, &upper_entries); let removed = apply_upper_sync(&session.upper_dir(), &ops)?;