diff --git a/src/builder/custom.rs b/src/builder/custom.rs index ee65e78..0ecad1c 100755 --- a/src/builder/custom.rs +++ b/src/builder/custom.rs @@ -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(()) + } } diff --git a/src/builder/mod.rs b/src/builder/mod.rs index 1eaba33..ae16a75 100755 --- a/src/builder/mod.rs +++ b/src/builder/mod.rs @@ -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) } diff --git a/src/config.rs b/src/config.rs index d964fdf..d5c280b 100755 --- a/src/config.rs +++ b/src/config.rs @@ -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,13 +417,13 @@ 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()) { - 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; - } + 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(()) diff --git a/src/cross.rs b/src/cross.rs index 867449f..2d464ef 100755 --- a/src/cross.rs +++ b/src/cross.rs @@ -208,11 +208,11 @@ fn find_tool(prefix: &str, suffixes: &[&str], required: bool) -> Result 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() { - let path = String::from_utf8_lossy(&output.stdout).trim().to_string(); - return Ok(path); - } + 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); } } diff --git a/src/db/mod.rs b/src/db/mod.rs index a9159f9..a9a2591 100755 --- a/src/db/mod.rs +++ b/src/db/mod.rs @@ -83,13 +83,13 @@ 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 is_auto_removable_path(file) { - auto_conflicts.push((file.clone(), owner)); - } else { - fatal_conflicts.push((file.clone(), owner)); - } + if let Ok(owner) = owner_res + && owner != spec.package.name + { + if is_auto_removable_path(file) { + auto_conflicts.push((file.clone(), owner)); + } else { + fatal_conflicts.push((file.clone(), owner)); } } } diff --git a/src/db/repo.rs b/src/db/repo.rs index fd9f4cd..18f2934 100644 --- a/src/db/repo.rs +++ b/src/db/repo.rs @@ -1019,11 +1019,11 @@ 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() { - let obj = repo.find_object(oid, None)?; - repo.reset(&obj, ResetType::Hard, None)?; - } + 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)?; } } @@ -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,11 +1080,11 @@ 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) { - let t = commit.time().seconds(); - commit_time = format!("{}", t); - } + if let Some(oid) = oid + && let Ok(commit) = repo.find_commit(oid) + { + let t = commit.time().seconds(); + commit_time = format!("{}", t); } // Dirty status diff --git a/src/index.rs b/src/index.rs index 21ff1f7..37b4939 100755 --- a/src/index.rs +++ b/src/index.rs @@ -46,34 +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) { - index - .by_name - .insert(spec.package.name.clone(), path.clone()); - for provided in &spec.alternatives.provides { - index - .by_provides - .entry(provided.clone()) - .or_default() - .push(path.clone()); - } - } - } - } - } - } - } - - // 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 { - 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()); @@ -90,6 +65,34 @@ 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) + .into_iter() + .flatten() + { + let path = entry.path().to_path_buf(); + 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()); + for provided in &spec.alternatives.provides { + index + .by_provides + .entry(provided.clone()) + .or_default() + .push(path.clone()); + } + } + } + } + crate::log_info!( "Indexed {} packages ({} provides)", index.by_name.len(), diff --git a/src/install/scripts.rs b/src/install/scripts.rs index 6f00fc5..47697ab 100644 --- a/src/install/scripts.rs +++ b/src/install/scripts.rs @@ -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 { 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!( diff --git a/src/package/interactive.rs b/src/package/interactive.rs index 9ba67fa..cc6ec7a 100644 --- a/src/package/interactive.rs +++ b/src/package/interactive.rs @@ -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 { BuildType::Rust => "rust", BuildType::Makefile => "makefile", BuildType::Bin => "bin", + BuildType::Meta => "meta", }; build_tbl.insert("type".into(), Value::String(build_type.to_string())); diff --git a/src/package/spec.rs b/src/package/spec.rs index a2449b2..1b7a74a 100755 --- a/src/package/spec.rs +++ b/src/package/spec.rs @@ -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 { 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 diff --git a/src/planner.rs b/src/planner.rs index 7710519..83981e2 100644 --- a/src/planner.rs +++ b/src/planner.rs @@ -601,8 +601,10 @@ impl<'a> Resolver<'a> { fn source_deps_for_install(spec: &PackageSpec) -> Vec { let mut deps_all = Vec::new(); - for dep in &spec.dependencies.build { - push_unique(&mut deps_all, dep.clone()); + 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()); diff --git a/src/source/extractor.rs b/src/source/extractor.rs index 3c8ee35..a6ef4a8 100755 --- a/src/source/extractor.rs +++ b/src/source/extractor.rs @@ -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 { diff --git a/src/source/mod.rs b/src/source/mod.rs index 22dc9a9..d63e846 100755 --- a/src/source/mod.rs +++ b/src/source/mod.rs @@ -39,10 +39,10 @@ pub fn copy_manual_sources(spec: &PackageSpec, cache_dir: &Path, build_dir: &Pat for manual in &spec.manual_sources { let mut local_entries: Vec = Vec::new(); - if let Some(file) = manual.file.as_ref() { - if !file.trim().is_empty() { - local_entries.push(file.clone()); - } + if let Some(file) = manual.file.as_ref() + && !file.trim().is_empty() + { + local_entries.push(file.clone()); } local_entries.extend( manual @@ -53,10 +53,10 @@ pub fn copy_manual_sources(spec: &PackageSpec, cache_dir: &Path, build_dir: &Pat ); let mut url_entries: Vec = Vec::new(); - if let Some(url) = manual.url.as_ref() { - if !url.trim().is_empty() { - url_entries.push(url.clone()); - } + 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());