From 7a6337e1cb2899ed333296bd60afa5fa007052aa Mon Sep 17 00:00:00 2001 From: Valentin Haudiquet Date: Tue, 22 Sep 2026 14:31:25 +0200 Subject: [PATCH] deb: build the tree the caller pointed at before the name search MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build_binary_package stages the parent of the requested cwd, then re-derived the package directory inside the staging area from package/version name patterns plus the calling process's working directory. That only works by accident for interactive users sitting in the package directory: an embedded caller whose tree lives at /tree matches no pattern, and the process cwd means nothing to a library consumer — the bc build above failed here even though the tree was staged correctly. The pointed-at tree is authoritative anyway: its changelog defined the package, version and series for this build. Resolve its staged copy outright when it carries a debian/ tree, keep the pattern search (with the quirks overrides) as a fallback, and hand the resolved directory to local::build instead of searching again. --- src/deb/local.rs | 7 +- src/deb/mod.rs | 172 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+), 4 deletions(-) diff --git a/src/deb/local.rs b/src/deb/local.rs index 0adbb0b..be678a4 100644 --- a/src/deb/local.rs +++ b/src/deb/local.rs @@ -34,6 +34,7 @@ pub async fn build( series: &str, pocket: Option<&str>, build_root: &str, + package_dir: &Path, cross: bool, ppa: &[String], inject_packages: &[String], @@ -231,10 +232,8 @@ pub async fn build( return Err("Could not install essential packages for the build".into()); } - // Find the actual package directory - // Find the actual package directory - let package_dir = - crate::deb::find_package_directory(Path::new(build_root), package, version, series, &ctx)?; + // The package directory was resolved by the caller (the staged copy of + // the tree the user pointed at, or the name-pattern search fallback) let package_dir_str = package_dir .to_str() .ok_or("Invalid package directory path")?; diff --git a/src/deb/mod.rs b/src/deb/mod.rs index 3fc4166..d4e8915 100644 --- a/src/deb/mod.rs +++ b/src/deb/mod.rs @@ -269,6 +269,19 @@ async fn build_binary_package_impl( .ok_or("Cannot find parent directory name")?; let build_root = format!("{}/{}", build_root, parent_dir_name.to_str().unwrap()); + // 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. + let package_dir = resolve_package_directory( + Path::new(&build_root), + cwd, + &package, + &version, + series, + &build_ctx, + )?; + // Run the build using target build mode. It returns the exact set of // artifacts produced by this build (binary packages registered in // debian/files plus the generated .buildinfo/.changes), as paths @@ -282,6 +295,7 @@ async fn build_binary_package_impl( series, pocket.as_deref(), &build_root, + &package_dir, cross, ppa, inject, @@ -329,6 +343,38 @@ async fn build_binary_package_impl( result } +/// 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. +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); + } + } + find_package_directory(build_root, package, version, series, ctx) +} + /// Find the current package directory by trying both patterns: /// - package/package /// - package/package-origversion @@ -548,6 +594,56 @@ 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. + #[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 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"), + "bc", + "1.07.1-1ubuntu1", + "questing", + &ctx, + ) + .expect("the staged copy of the pointed-at tree must resolve"); + + assert_eq!(resolved, PathBuf::from("/tmp/pkh-build-1/j-42/tree")); + } + + /// When the pointed-at tree is not in the staging area under its own + /// name, resolution falls back to the name-pattern search. + #[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 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"), + "bc", + "1.07.1-1ubuntu1", + "questing", + &ctx, + ) + .expect("the pulled-tree layout must resolve via the name search"); + + assert_eq!( + resolved, + PathBuf::from("/tmp/pkh-build-1/j-42/bc/bc-1.07.1") + ); + } + async fn test_build_end_to_end( package: &str, series: &str, @@ -884,4 +980,80 @@ mod tests { "error should name the unsatisfied dependency: {err}" ); } + + /// An embedded-caller layout — the tree checked out at /tree, a + /// name matching none of the search patterns — must build: the staged + /// copy of the tree the caller pointed at is resolved directly instead + /// of being re-derived from package/version names (which used to fail + /// with 'Could not find package directory'). + #[tokio::test] + #[test_log::test] + async fn test_deb_builds_a_tree_named_directory_end_to_end() { + let temp_dir = tempfile::tempdir().unwrap(); + let pkg_dir = temp_dir.path().join("j-42/tree"); + std::fs::create_dir_all(pkg_dir.join("debian/source")).unwrap(); + + std::fs::write( + pkg_dir.join("debian/changelog"), + "pkh-treetest (1.0) noble; urgency=medium\n\n \ + * Synthetic package built from a directory named 'tree'.\n\n \ + -- pkh tests Tue, 15 Sep 2026 08:00:00 +0000\n", + ) + .unwrap(); + + std::fs::write( + pkg_dir.join("debian/control"), + "Source: pkh-treetest\n\ + Section: devel\n\ + Priority: optional\n\ + Maintainer: pkh tests \n\ + Standards-Version: 4.7.4\n\ + Build-Depends: debhelper-compat (= 13)\n\ + Architecture: any\n\ + \n\ + Package: pkh-treetest\n\ + Architecture: any\n\ + Depends: ${misc:Depends}, ${shlibs:Depends}\n\ + Description: Package-directory resolution regression package\n \ + Its tree lives in a directory whose name matches none of the\n \ + package-directory search patterns.\n", + ) + .unwrap(); + + std::fs::write( + pkg_dir.join("debian/rules"), + "#!/usr/bin/make -f\n%:\n\tdh $@\n", + ) + .unwrap(); + use std::os::unix::fs::PermissionsExt; + let rules = pkg_dir.join("debian/rules"); + let mut perms = std::fs::metadata(&rules).unwrap().permissions(); + perms.set_mode(0o755); + std::fs::set_permissions(&rules, perms).unwrap(); + + std::fs::write(pkg_dir.join("debian/source/format"), "3.0 (native)\n").unwrap(); + + let ctx = Arc::new(Context::new(crate::context::ContextConfig::Local).unwrap()); + + crate::deb::build_binary_package(DebBuildOptions { + series: Some("noble".to_string()), + cwd: Some(pkg_dir), + ctx: Some(ctx), + ..Default::default() + }) + .await + .expect("a tree named 'tree' must build"); + + let deb_files: Vec = std::fs::read_dir(temp_dir.path().join("j-42")) + .unwrap() + .filter_map(|e| e.ok()) + .map(|e| e.file_name().to_string_lossy().to_string()) + .collect(); + assert!( + deb_files + .iter() + .any(|f| f.starts_with("pkh-treetest_1.0_") && f.ends_with(".deb")), + ".deb not produced for the 'tree'-named directory, got: {deb_files:?}" + ); + } }