feat: add metapackage build support and fail-fast custom scripts
- add BuildType::Meta handling in builder and interactive spec output - allow metapackage specs without sources and expose is_metapackage() - avoid build deps in install planning for metapackages - run custom non-function build.sh scripts with `sh -e` to stop on first failure - add regression test for custom script fail-fast behavior - apply small cleanup refactors (let-chains/iterator simplifications)
This commit is contained in:
+31
-1
@@ -113,7 +113,9 @@ pub fn build(
|
||||
build_function_mode_command(spec, destdir, &abs_build_script)?
|
||||
} else {
|
||||
let mut cmd = fakeroot::wrap_install_command("sh", destdir);
|
||||
cmd.arg(&abs_build_script);
|
||||
// Run custom scripts with `-e` so command failures stop the build immediately
|
||||
// instead of being masked by later shell commands.
|
||||
cmd.arg("-e").arg(&abs_build_script);
|
||||
cmd
|
||||
};
|
||||
cmd.current_dir(&build_dir);
|
||||
@@ -422,4 +424,32 @@ depot_install() {
|
||||
assert!(err.to_string().contains("Custom build script failed"));
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_build_non_function_mode_stops_on_first_command_failure() -> Result<()> {
|
||||
let tmp_src = tempdir()?;
|
||||
let tmp_dest = tempdir()?;
|
||||
|
||||
let build_sh = tmp_src.path().join("build.sh");
|
||||
std::fs::write(
|
||||
&build_sh,
|
||||
r#"#!/bin/sh
|
||||
false
|
||||
exit 0
|
||||
"#,
|
||||
)?;
|
||||
#[cfg(unix)]
|
||||
{
|
||||
use std::os::unix::fs::PermissionsExt;
|
||||
let mut perms = std::fs::metadata(&build_sh)?.permissions();
|
||||
perms.set_mode(0o755);
|
||||
std::fs::set_permissions(&build_sh, perms)?;
|
||||
}
|
||||
|
||||
let spec = mk_spec("custom-fail-fast", "1.0");
|
||||
let err = build(&spec, tmp_src.path(), tmp_dest.path(), None, true)
|
||||
.expect_err("non-function custom scripts should fail when a command fails");
|
||||
assert!(err.to_string().contains("Custom build script failed"));
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -226,6 +226,12 @@ pub fn build(
|
||||
BuildType::Python => python::build(spec, src_dir, destdir, cross, export_compiler_flags),
|
||||
BuildType::Rust => rust::build(spec, src_dir, destdir, cross, export_compiler_flags),
|
||||
BuildType::Bin => bin::build(spec, src_dir, destdir, cross, export_compiler_flags),
|
||||
BuildType::Meta => {
|
||||
// Metapackages are metadata-only; create an empty staging root and let
|
||||
// packaging/installation metadata carry dependencies.
|
||||
std::fs::create_dir_all(destdir)?;
|
||||
Ok(())
|
||||
}
|
||||
BuildType::Makefile => {
|
||||
makefile::build(spec, src_dir, destdir, cross, export_compiler_flags)
|
||||
}
|
||||
|
||||
+4
-12
@@ -29,21 +29,13 @@ fn resolve_rootfs_base(rootfs: &Path) -> PathBuf {
|
||||
}
|
||||
|
||||
/// Global repo behavior settings loaded from `repos.toml`.
|
||||
#[derive(Clone, Debug, Serialize, Deserialize)]
|
||||
#[derive(Clone, Debug, Default, Serialize, Deserialize)]
|
||||
pub struct RepoSettings {
|
||||
/// Prefer binary repos over source repos when both can satisfy a request.
|
||||
#[serde(default)]
|
||||
pub prefer_binary: bool,
|
||||
}
|
||||
|
||||
impl Default for RepoSettings {
|
||||
fn default() -> Self {
|
||||
Self {
|
||||
prefer_binary: false,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Source repository configuration entry loaded from `repos.toml`.
|
||||
#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct SourceRepo {
|
||||
@@ -425,14 +417,14 @@ impl Config {
|
||||
}
|
||||
|
||||
// Allow overriding repo clone dir via [repo] clone_dir in depot.toml
|
||||
if let Some(repo_table) = self.build_overrides.get("repo").and_then(|v| v.as_table()) {
|
||||
if let Some(clone_val) = repo_table.get("clone_dir").and_then(|v| v.as_str()) {
|
||||
if let Some(repo_table) = self.build_overrides.get("repo").and_then(|v| v.as_table())
|
||||
&& let Some(clone_val) = repo_table.get("clone_dir").and_then(|v| v.as_str())
|
||||
{
|
||||
let p = PathBuf::from(clone_val);
|
||||
// If relative path, make it relative to rootfs
|
||||
let repo_dir = if p.is_absolute() { p } else { rootfs.join(p) };
|
||||
self.repo_clone_dir = repo_dir;
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
+3
-3
@@ -208,13 +208,13 @@ fn find_tool(prefix: &str, suffixes: &[&str], required: bool) -> Result<String>
|
||||
for suffix in suffixes {
|
||||
let tool_name = format!("{}-{}", prefix, suffix);
|
||||
// Use `which` to resolve to an absolute path (so callers get a usable path)
|
||||
if let Ok(output) = Command::new("which").arg(&tool_name).output() {
|
||||
if output.status.success() {
|
||||
if let Ok(output) = Command::new("which").arg(&tool_name).output()
|
||||
&& output.status.success()
|
||||
{
|
||||
let path = String::from_utf8_lossy(&output.stdout).trim().to_string();
|
||||
return Ok(path);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if required {
|
||||
anyhow::bail!(
|
||||
|
||||
+3
-3
@@ -83,8 +83,9 @@ pub fn register_package(db_path: &Path, spec: &PackageSpec, destdir: &Path) -> R
|
||||
params![file],
|
||||
|row| row.get(0),
|
||||
);
|
||||
if let Ok(owner) = owner_res {
|
||||
if owner != spec.package.name {
|
||||
if let Ok(owner) = owner_res
|
||||
&& owner != spec.package.name
|
||||
{
|
||||
if is_auto_removable_path(file) {
|
||||
auto_conflicts.push((file.clone(), owner));
|
||||
} else {
|
||||
@@ -92,7 +93,6 @@ pub fn register_package(db_path: &Path, spec: &PackageSpec, destdir: &Path) -> R
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if !fatal_conflicts.is_empty() {
|
||||
let mut msg = String::from("File ownership conflict detected:\n");
|
||||
|
||||
+7
-7
@@ -1019,13 +1019,13 @@ pub fn sync_mirrors(
|
||||
.with_context(|| format!("Failed to fetch updates for {}", url))?;
|
||||
|
||||
// Try to fast-forward HEAD to origin/HEAD by resetting to FETCH_HEAD if present
|
||||
if let Ok(fetch_head) = repo.find_reference("FETCH_HEAD") {
|
||||
if let Some(oid) = fetch_head.target() {
|
||||
if let Ok(fetch_head) = repo.find_reference("FETCH_HEAD")
|
||||
&& let Some(oid) = fetch_head.target()
|
||||
{
|
||||
let obj = repo.find_object(oid, None)?;
|
||||
repo.reset(&obj, ResetType::Hard, None)?;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Make the tree readable and writable by everyone
|
||||
for entry in walkdir::WalkDir::new(&target) {
|
||||
@@ -1055,7 +1055,7 @@ pub fn mirrors_status(
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
for (name, _url) in mirrors {
|
||||
for name in mirrors.keys() {
|
||||
let target = base.join(name);
|
||||
crate::log_info!("--- {} ---", name);
|
||||
if !target.exists() {
|
||||
@@ -1080,12 +1080,12 @@ pub fn mirrors_status(
|
||||
|
||||
// Commit time (seconds since epoch) if available
|
||||
let mut commit_time = String::new();
|
||||
if let Some(oid) = oid {
|
||||
if let Ok(commit) = repo.find_commit(oid) {
|
||||
if let Some(oid) = oid
|
||||
&& let Ok(commit) = repo.find_commit(oid)
|
||||
{
|
||||
let t = commit.time().seconds();
|
||||
commit_time = format!("{}", t);
|
||||
}
|
||||
}
|
||||
|
||||
// Dirty status
|
||||
let statuses = match repo.statuses(None) {
|
||||
|
||||
+12
-9
@@ -46,8 +46,9 @@ impl PackageIndex {
|
||||
if let Ok(files) = fs::read_dir(&dir) {
|
||||
for file in files.flatten() {
|
||||
let path = file.path();
|
||||
if path.extension().map(|e| e == "toml").unwrap_or(false) {
|
||||
if let Ok(spec) = PackageSpec::from_file(&path) {
|
||||
if path.extension().map(|e| e == "toml").unwrap_or(false)
|
||||
&& let Ok(spec) = PackageSpec::from_file(&path)
|
||||
{
|
||||
index
|
||||
.by_name
|
||||
.insert(spec.package.name.clone(), path.clone());
|
||||
@@ -63,17 +64,21 @@ impl PackageIndex {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Also scan system mirrors under provided repo_dir (or default)
|
||||
let sys_dir = repo_dir.unwrap_or_else(|| PathBuf::from("/usr/src/depot"));
|
||||
|
||||
if sys_dir.exists() {
|
||||
for entry in walkdir::WalkDir::new(&sys_dir).min_depth(1).max_depth(5) {
|
||||
if let Ok(entry) = entry {
|
||||
for entry in walkdir::WalkDir::new(&sys_dir)
|
||||
.min_depth(1)
|
||||
.max_depth(5)
|
||||
.into_iter()
|
||||
.flatten()
|
||||
{
|
||||
let path = entry.path().to_path_buf();
|
||||
if path.extension().map(|e| e == "toml").unwrap_or(false) {
|
||||
if let Ok(spec) = PackageSpec::from_file(&path) {
|
||||
if path.extension().map(|e| e == "toml").unwrap_or(false)
|
||||
&& let Ok(spec) = PackageSpec::from_file(&path)
|
||||
{
|
||||
index
|
||||
.by_name
|
||||
.insert(spec.package.name.clone(), path.clone());
|
||||
@@ -87,8 +92,6 @@ impl PackageIndex {
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
crate::log_info!(
|
||||
"Indexed {} packages ({} provides)",
|
||||
|
||||
@@ -103,7 +103,7 @@ pub fn installed_scripts_dir(rootfs: &Path, pkg_name: &str) -> PathBuf {
|
||||
/// Returns `true` if scripts were found and staged.
|
||||
pub fn stage_scripts_from_spec_dir(spec: &PackageSpec, destdir: &Path) -> Result<bool> {
|
||||
let source_dir = spec.spec_dir.join(STAGED_SCRIPTS_DIR);
|
||||
let has_scripts_dir = if source_dir.exists() { true } else { false };
|
||||
let has_scripts_dir = source_dir.exists();
|
||||
|
||||
if has_scripts_dir && !source_dir.is_dir() {
|
||||
bail!(
|
||||
|
||||
@@ -21,6 +21,7 @@ impl fmt::Display for BuildType {
|
||||
BuildType::Rust => write!(f, "Rust"),
|
||||
BuildType::Makefile => write!(f, "Makefile"),
|
||||
BuildType::Bin => write!(f, "Binary installer"),
|
||||
BuildType::Meta => write!(f, "Metapackage"),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -624,6 +625,7 @@ pub fn spec_to_minimal_toml(spec: &PackageSpec) -> anyhow::Result<String> {
|
||||
BuildType::Rust => "rust",
|
||||
BuildType::Makefile => "makefile",
|
||||
BuildType::Bin => "bin",
|
||||
BuildType::Meta => "meta",
|
||||
};
|
||||
build_tbl.insert("type".into(), Value::String(build_type.to_string()));
|
||||
|
||||
|
||||
+42
-3
@@ -70,9 +70,11 @@ impl PackageSpec {
|
||||
.unwrap_or_else(|| PathBuf::from("."));
|
||||
spec.apply_spec_appends(&appends)?;
|
||||
|
||||
// Require at least one source (remote or manual)
|
||||
if spec.source.is_empty() && spec.manual_sources.is_empty() {
|
||||
anyhow::bail!("Package must have at least one source or manual_sources entry");
|
||||
// Require at least one source (remote or manual) unless this is a metapackage.
|
||||
if spec.source.is_empty() && spec.manual_sources.is_empty() && !spec.is_metapackage() {
|
||||
anyhow::bail!(
|
||||
"Package must have at least one source or manual_sources entry (except build.type = \"meta\")"
|
||||
);
|
||||
}
|
||||
spec.validate_manual_sources()?;
|
||||
|
||||
@@ -167,6 +169,11 @@ impl PackageSpec {
|
||||
&self.source
|
||||
}
|
||||
|
||||
/// Returns true when this spec is a metadata-only package that exists to pull dependencies.
|
||||
pub fn is_metapackage(&self) -> bool {
|
||||
matches!(self.build.build_type, BuildType::Meta)
|
||||
}
|
||||
|
||||
/// Return all package outputs this spec will produce (primary + any extras)
|
||||
pub fn outputs(&self) -> Vec<PackageInfo> {
|
||||
let mut v = Vec::new();
|
||||
@@ -1202,6 +1209,37 @@ type = "custom"
|
||||
assert!(PackageSpec::from_file(&path).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_allows_metapackage_without_sources() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let path = tmp.path().join("pkg.toml");
|
||||
|
||||
std::fs::write(
|
||||
&path,
|
||||
r#"
|
||||
[package]
|
||||
name = "foo-meta"
|
||||
version = "1.0"
|
||||
description = "metapackage"
|
||||
homepage = "https://example.com"
|
||||
license = "MIT"
|
||||
|
||||
[build]
|
||||
type = "meta"
|
||||
|
||||
[dependencies]
|
||||
runtime = ["foo", "bar"]
|
||||
"#,
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let spec = PackageSpec::from_file(&path).unwrap();
|
||||
assert!(spec.source.is_empty());
|
||||
assert!(spec.manual_sources.is_empty());
|
||||
assert!(spec.is_metapackage());
|
||||
assert_eq!(spec.dependencies.runtime, vec!["foo", "bar"]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_manual_source_with_url() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
@@ -2297,6 +2335,7 @@ pub enum BuildType {
|
||||
Rust,
|
||||
Makefile,
|
||||
Bin,
|
||||
Meta,
|
||||
}
|
||||
|
||||
/// Build flags and toolchain configuration
|
||||
|
||||
@@ -601,9 +601,11 @@ impl<'a> Resolver<'a> {
|
||||
|
||||
fn source_deps_for_install(spec: &PackageSpec) -> Vec<String> {
|
||||
let mut deps_all = Vec::new();
|
||||
if !spec.is_metapackage() {
|
||||
for dep in &spec.dependencies.build {
|
||||
push_unique(&mut deps_all, dep.clone());
|
||||
}
|
||||
}
|
||||
for dep in &spec.dependencies.runtime {
|
||||
push_unique(&mut deps_all, dep.clone());
|
||||
}
|
||||
|
||||
+12
-12
@@ -112,9 +112,9 @@ fn extract_tar_gz(path: &Path, dest: &Path) -> Result<()> {
|
||||
// preserve the top-level folder (move the directory itself under dest)
|
||||
fs::create_dir_all(dest)?;
|
||||
let dest_top = dest.join(top_name);
|
||||
if fs::rename(&top[0].path(), &dest_top).is_err() {
|
||||
if fs::rename(top[0].path(), &dest_top).is_err() {
|
||||
copy_dir_recursive_local(&top[0].path(), &dest_top)?;
|
||||
fs::remove_dir_all(&top[0].path())?;
|
||||
fs::remove_dir_all(top[0].path())?;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
@@ -157,9 +157,9 @@ fn extract_tar_xz(path: &Path, dest: &Path) -> Result<()> {
|
||||
// preserve the top-level folder (move the directory itself under dest)
|
||||
fs::create_dir_all(dest)?;
|
||||
let dest_top = dest.join(top_name);
|
||||
if fs::rename(&top[0].path(), &dest_top).is_err() {
|
||||
if fs::rename(top[0].path(), &dest_top).is_err() {
|
||||
copy_dir_recursive_local(&top[0].path(), &dest_top)?;
|
||||
fs::remove_dir_all(&top[0].path())?;
|
||||
fs::remove_dir_all(top[0].path())?;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
@@ -202,9 +202,9 @@ fn extract_tar_bz2(path: &Path, dest: &Path) -> Result<()> {
|
||||
// preserve the top-level folder (move the directory itself under dest)
|
||||
fs::create_dir_all(dest)?;
|
||||
let dest_top = dest.join(top_name);
|
||||
if fs::rename(&top[0].path(), &dest_top).is_err() {
|
||||
if fs::rename(top[0].path(), &dest_top).is_err() {
|
||||
copy_dir_recursive_local(&top[0].path(), &dest_top)?;
|
||||
fs::remove_dir_all(&top[0].path())?;
|
||||
fs::remove_dir_all(top[0].path())?;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
@@ -246,9 +246,9 @@ fn extract_tar(path: &Path, dest: &Path) -> Result<()> {
|
||||
// preserve the top-level folder (move the directory itself under dest)
|
||||
fs::create_dir_all(dest)?;
|
||||
let dest_top = dest.join(top_name);
|
||||
if fs::rename(&top[0].path(), &dest_top).is_err() {
|
||||
if fs::rename(top[0].path(), &dest_top).is_err() {
|
||||
copy_dir_recursive_local(&top[0].path(), &dest_top)?;
|
||||
fs::remove_dir_all(&top[0].path())?;
|
||||
fs::remove_dir_all(top[0].path())?;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
@@ -290,9 +290,9 @@ fn extract_zip(path: &Path, dest: &Path) -> Result<()> {
|
||||
// preserve the top-level folder (move the directory itself under dest)
|
||||
fs::create_dir_all(dest)?;
|
||||
let dest_top = dest.join(top_name);
|
||||
if fs::rename(&top[0].path(), &dest_top).is_err() {
|
||||
if fs::rename(top[0].path(), &dest_top).is_err() {
|
||||
copy_dir_recursive_local(&top[0].path(), &dest_top)?;
|
||||
fs::remove_dir_all(&top[0].path())?;
|
||||
fs::remove_dir_all(top[0].path())?;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
@@ -335,9 +335,9 @@ fn extract_tar_zst(path: &Path, dest: &Path) -> Result<()> {
|
||||
// preserve the top-level folder (move the directory itself under dest)
|
||||
fs::create_dir_all(dest)?;
|
||||
let dest_top = dest.join(top_name);
|
||||
if fs::rename(&top[0].path(), &dest_top).is_err() {
|
||||
if fs::rename(top[0].path(), &dest_top).is_err() {
|
||||
copy_dir_recursive_local(&top[0].path(), &dest_top)?;
|
||||
fs::remove_dir_all(&top[0].path())?;
|
||||
fs::remove_dir_all(top[0].path())?;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
|
||||
+6
-6
@@ -39,11 +39,11 @@ pub fn copy_manual_sources(spec: &PackageSpec, cache_dir: &Path, build_dir: &Pat
|
||||
|
||||
for manual in &spec.manual_sources {
|
||||
let mut local_entries: Vec<String> = Vec::new();
|
||||
if let Some(file) = manual.file.as_ref() {
|
||||
if !file.trim().is_empty() {
|
||||
if let Some(file) = manual.file.as_ref()
|
||||
&& !file.trim().is_empty()
|
||||
{
|
||||
local_entries.push(file.clone());
|
||||
}
|
||||
}
|
||||
local_entries.extend(
|
||||
manual
|
||||
.files
|
||||
@@ -53,11 +53,11 @@ pub fn copy_manual_sources(spec: &PackageSpec, cache_dir: &Path, build_dir: &Pat
|
||||
);
|
||||
|
||||
let mut url_entries: Vec<String> = Vec::new();
|
||||
if let Some(url) = manual.url.as_ref() {
|
||||
if !url.trim().is_empty() {
|
||||
if let Some(url) = manual.url.as_ref()
|
||||
&& !url.trim().is_empty()
|
||||
{
|
||||
url_entries.push(url.clone());
|
||||
}
|
||||
}
|
||||
url_entries.extend(manual.urls.iter().filter(|s| !s.trim().is_empty()).cloned());
|
||||
|
||||
if !local_entries.is_empty() {
|
||||
|
||||
Reference in New Issue
Block a user