From db0dcb2ff54d94215421ef7d67149d3cae2a52b9 Mon Sep 17 00:00:00 2001 From: SFG545 Date: Sun, 15 Mar 2026 22:05:10 -0500 Subject: [PATCH] feat: enhance automatic test skipping logic for multilib builds --- src/builder/autotools.rs | 10 +++++-- src/builder/cmake.rs | 10 +++++-- src/builder/meson.rs | 10 +++++-- src/builder/perl.rs | 10 +++++-- src/commands.rs | 65 ++++++++++++++++++++++++++++++++++++---- src/deps.rs | 16 ++++++++-- src/package/spec.rs | 23 ++++++++++++++ src/planner.rs | 7 +++-- 8 files changed, 132 insertions(+), 19 deletions(-) diff --git a/src/builder/autotools.rs b/src/builder/autotools.rs index 172bd2a..4c1f780 100755 --- a/src/builder/autotools.rs +++ b/src/builder/autotools.rs @@ -177,8 +177,14 @@ pub fn build( } } - if flags.skip_tests { - crate::log_info!("Skipping tests: disabled by build.flags.skip_tests"); + if spec.should_skip_automatic_tests() { + if flags.skip_tests { + crate::log_info!("Skipping tests: disabled by build.flags.skip_tests"); + } else { + crate::log_info!( + "Skipping tests: automatic tests are disabled for multilib builds" + ); + } } else { let test_dirs = resolve_make_dirs( &build_dir, diff --git a/src/builder/cmake.rs b/src/builder/cmake.rs index 8a7e6ef..2e97194 100755 --- a/src/builder/cmake.rs +++ b/src/builder/cmake.rs @@ -127,8 +127,14 @@ pub fn build( anyhow::bail!("cmake build failed"); } - if flags.skip_tests { - crate::log_info!("Skipping tests: disabled by build.flags.skip_tests"); + if spec.should_skip_automatic_tests() { + if flags.skip_tests { + crate::log_info!("Skipping tests: disabled by build.flags.skip_tests"); + } else { + crate::log_info!( + "Skipping tests: automatic tests are disabled for multilib builds" + ); + } } else { let test_targets = phase_targets(&flags.make_test_target, &flags.make_test_targets); if !cmake_uses_default_ctest(flags) { diff --git a/src/builder/meson.rs b/src/builder/meson.rs index 042d882..970826e 100755 --- a/src/builder/meson.rs +++ b/src/builder/meson.rs @@ -86,8 +86,14 @@ pub fn build( anyhow::bail!("ninja build failed"); } - if flags.skip_tests { - crate::log_info!("Skipping tests: disabled by build.flags.skip_tests"); + if spec.should_skip_automatic_tests() { + if flags.skip_tests { + crate::log_info!("Skipping tests: disabled by build.flags.skip_tests"); + } else { + crate::log_info!( + "Skipping tests: automatic tests are disabled for multilib builds" + ); + } } else { let test_suites = meson_test_suites(flags); if test_suites.is_empty() { diff --git a/src/builder/perl.rs b/src/builder/perl.rs index 87ac752..d20a20d 100644 --- a/src/builder/perl.rs +++ b/src/builder/perl.rs @@ -116,8 +116,14 @@ pub fn build( } } - if flags.skip_tests { - crate::log_info!("Skipping tests: disabled by build.flags.skip_tests"); + if spec.should_skip_automatic_tests() { + if flags.skip_tests { + crate::log_info!("Skipping tests: disabled by build.flags.skip_tests"); + } else { + crate::log_info!( + "Skipping tests: automatic tests are disabled for multilib builds" + ); + } } else { let test_dirs = autotools::resolve_make_dirs( &actual_src, diff --git a/src/commands.rs b/src/commands.rs index 88e0bd8..b82223f 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -927,12 +927,19 @@ fn build_type_runs_automatic_tests(spec: &package::PackageSpec) -> bool { ) } +fn automatic_tests_disabled_for_outputs( + pkg_spec: &package::PackageSpec, + requested_outputs: deps::RequestedOutputs, +) -> bool { + pkg_spec.should_skip_automatic_tests() || requested_outputs.includes_lib32() +} + fn maybe_disable_tests_for_missing_deps( pkg_spec: &mut package::PackageSpec, db_path: &Path, requested_outputs: deps::RequestedOutputs, ) -> Result<()> { - if pkg_spec.build.flags.skip_tests + if automatic_tests_disabled_for_outputs(pkg_spec, requested_outputs) || !build_type_runs_automatic_tests(pkg_spec) || deps::declared_test_deps(pkg_spec, requested_outputs).is_empty() { @@ -956,7 +963,7 @@ fn maybe_prompt_to_skip_tests_for_missing_requested_deps( missing_test: &[String], reason: &str, ) -> Result { - if pkg_spec.build.flags.skip_tests + if pkg_spec.should_skip_automatic_tests() || !build_type_runs_automatic_tests(pkg_spec) || missing_test.is_empty() { @@ -993,7 +1000,7 @@ fn should_install_test_deps( requested_outputs: deps::RequestedOutputs, ) -> bool { install_test_deps - && !pkg_spec.build.flags.skip_tests + && !automatic_tests_disabled_for_outputs(pkg_spec, requested_outputs) && !deps::declared_test_deps(pkg_spec, requested_outputs).is_empty() } @@ -4200,7 +4207,9 @@ fn run_direct_install_request( anyhow::bail!("Missing test dependencies: {}", unavailable_test.join(", ")); } - if !pkg_spec.build.flags.skip_tests && !dep_spec_paths.is_empty() { + if !automatic_tests_disabled_for_outputs(&pkg_spec, requested_outputs) + && !dep_spec_paths.is_empty() + { ui::warn(format!( "Missing test dependencies: {}", missing_test.join(", ") @@ -4714,7 +4723,12 @@ pub fn run(cli: Cli) -> Result<()> { Err(err) => return Err(err), }; - if cleanup_deps && !pkg_spec.build.flags.skip_tests { + if cleanup_deps + && !automatic_tests_disabled_for_outputs( + &pkg_spec, + requested_outputs, + ) + { auto_installed_deps.record_plan( &dep_plan, &missing_test, @@ -4722,7 +4736,9 @@ pub fn run(cli: Cli) -> Result<()> { ); } - if !pkg_spec.build.flags.skip_tests && !dep_plan.steps.is_empty() { + if !automatic_tests_disabled_for_outputs(&pkg_spec, requested_outputs) + && !dep_plan.steps.is_empty() + { ui::warn(format!( "Missing test dependencies: {}", missing_test.join(", ") @@ -7385,6 +7401,43 @@ optional = [] Ok(()) } + #[test] + fn requested_test_deps_prompt_is_ignored_for_multilib_builds() -> Result<()> { + let _guard = ASSUME_YES_TEST_LOCK.lock().expect("assume-yes test lock"); + let mut spec = test_package_spec(package::BuildType::Meson, None, &[]); + spec.build.flags.build_32 = true; + spec.dependencies.test = vec!["pytest".into()]; + + ui::set_assume_yes(true); + let prompted = maybe_prompt_to_skip_tests_for_missing_requested_deps( + &mut spec, + &["pytest".into()], + "Requested test dependencies are missing", + )?; + ui::set_assume_yes(false); + + assert!(!prompted); + assert!(!spec.build.flags.skip_tests); + Ok(()) + } + + #[test] + fn should_not_install_test_deps_for_cli_lib32_only_builds() { + let mut spec = test_package_spec(package::BuildType::Meson, None, &[]); + spec.dependencies.lib32 = Some(package::DependencyGroup { + build: Vec::new(), + runtime: Vec::new(), + test: vec!["lib32-pytest".into()], + optional: Vec::new(), + }); + + assert!(!should_install_test_deps( + &spec, + true, + deps::RequestedOutputs::Lib32Only + )); + } + #[test] fn rootfs_is_system_root_detects_live_rootfs() { assert!(rootfs_is_system_root(Path::new("/"))); diff --git a/src/deps.rs b/src/deps.rs index cb03881..3a500ea 100755 --- a/src/deps.rs +++ b/src/deps.rs @@ -40,6 +40,12 @@ pub(crate) enum RequestedOutputs { Lib32Only, } +impl RequestedOutputs { + pub(crate) fn includes_lib32(self) -> bool { + matches!(self, Self::PrimaryAndLib32 | Self::Lib32Only) + } +} + /// Parse a dependency string into name, version, and operator fn parse_dep(dep: &str) -> ParsedDep<'_> { // Try operators in order of specificity (>= before >, etc.) @@ -154,6 +160,10 @@ fn build_type_runs_automatic_tests(spec: &PackageSpec) -> bool { ) } +fn automatic_tests_disabled_for_outputs(spec: &PackageSpec, outputs: RequestedOutputs) -> bool { + spec.should_skip_automatic_tests() || outputs.includes_lib32() +} + /// Check whether a dependency expression is satisfied by the installed package DB. pub fn is_dep_satisfied_in_db(dep: &str, db_path: &Path) -> Result { if !db_path.exists() { @@ -358,7 +368,8 @@ pub(crate) fn print_dep_status_for_outputs( "Test dependencies", &primary.test, &missing_test, - !spec.build.flags.skip_tests && build_type_runs_automatic_tests(spec), + !automatic_tests_disabled_for_outputs(spec, outputs) + && build_type_runs_automatic_tests(spec), ); if !primary.optional.is_empty() { ui::info(format!( @@ -395,7 +406,8 @@ pub(crate) fn print_dep_status_for_outputs( "Lib32 test dependencies", &lib32.test, &missing_test, - !spec.build.flags.skip_tests && build_type_runs_automatic_tests(spec), + !automatic_tests_disabled_for_outputs(spec, outputs) + && build_type_runs_automatic_tests(spec), ); if !lib32.optional.is_empty() { ui::info(format!( diff --git a/src/package/spec.rs b/src/package/spec.rs index 64ca230..9ccb19d 100755 --- a/src/package/spec.rs +++ b/src/package/spec.rs @@ -266,6 +266,14 @@ impl PackageSpec { self.build.flags.lib32_only } + /// Return true when builder-managed automatic tests should be skipped. + /// + /// Automatic test phases are disabled when `build.flags.skip_tests` is set and for + /// multilib builds, because the generated lib32 output is built in a separate 32-bit pass. + pub fn should_skip_automatic_tests(&self) -> bool { + self.build.flags.skip_tests || self.builds_lib32_output() + } + /// Return the effective dependency set used by the generated lib32 companion package. pub fn lib32_dependencies(&self) -> Dependencies { let mut deps = self @@ -3086,6 +3094,19 @@ type = "autotools" assert_eq!(spec.build.flags.post_install_lib32, vec!["echo lib32"]); } + #[test] + fn multilib_builds_skip_automatic_tests() { + let mut spec = mk_spec("foo", "1.0"); + assert!(!spec.should_skip_automatic_tests()); + + spec.build.flags.build_32 = true; + assert!(spec.should_skip_automatic_tests()); + + spec.build.flags.build_32 = false; + spec.build.flags.skip_tests = true; + assert!(spec.should_skip_automatic_tests()); + } + #[test] fn parse_post_configure_from_spec() { let tmp = tempfile::tempdir().unwrap(); @@ -3982,6 +4003,8 @@ pub struct BuildFlags { )] pub no_compress_man: bool, /// Skip automatic build-system test execution (e.g. Autotools `make check`/`make test`). + /// + /// Automatic tests are also skipped for multilib (`build_32` / `lib32_only`) builds. #[serde(default, alias = "skip-tests")] pub skip_tests: bool, /// Run an additional lib32 build pass and emit a `lib32-*` package. diff --git a/src/planner.rs b/src/planner.rs index 9f771a5..aa94340 100644 --- a/src/planner.rs +++ b/src/planner.rs @@ -624,6 +624,7 @@ fn source_deps_for_install( let mut deps_all = Vec::new(); let lib32_only = lib32_only || spec.builds_only_lib32_output(); let include_lib32 = lib32_only || spec.builds_lib32_output(); + let skip_automatic_tests = spec.should_skip_automatic_tests() || lib32_only; let local_provides = spec.local_dependency_provides_for_selection(!lib32_only, include_lib32); if !lib32_only && !spec.is_metapackage() { for dep in &spec.dependencies.build { @@ -658,7 +659,7 @@ fn source_deps_for_install( } } } - if include_test_deps && !spec.build.flags.skip_tests { + if include_test_deps && !skip_automatic_tests { if !lib32_only { for dep in &spec.dependencies.test { push_unique(&mut deps_all, dep.clone()); @@ -961,7 +962,7 @@ mod tests { let deps = source_deps_for_install(&spec, true, true); assert!(deps.contains(&"gcc-multilib".to_string())); assert!(deps.contains(&"lib32-zlib".to_string())); - assert!(deps.contains(&"lib32-bats".to_string())); + assert!(!deps.contains(&"lib32-bats".to_string())); assert!(!deps.contains(&"make".to_string())); assert!(!deps.contains(&"zlib".to_string())); assert!(!deps.contains(&"bats".to_string())); @@ -981,7 +982,7 @@ mod tests { let deps = source_deps_for_install(&spec, true, false); assert!(deps.contains(&"gcc-multilib".to_string())); assert!(deps.contains(&"lib32-zlib".to_string())); - assert!(deps.contains(&"lib32-bats".to_string())); + assert!(!deps.contains(&"lib32-bats".to_string())); assert!(!deps.contains(&"make".to_string())); assert!(!deps.contains(&"zlib".to_string())); assert!(!deps.contains(&"bats".to_string()));