From a9b74c6a6073db09dabaf8202169c90373302924 Mon Sep 17 00:00:00 2001 From: Frank Date: Mon, 2 Nov 2026 11:02:49 +0100 Subject: [PATCH 1/2] Split out ScriptEnv from BuildEnv --- src/builder/build_env.rs | 132 +++----------------- src/builder/mod.rs | 1 - src/installer/installer.rs | 29 ++++- src/installer/mod.rs | 4 + src/installer/script_env.rs | 193 ++++++++++++++++++++++++++++++ src/installer/scripts.rs | 29 ++--- src/integrity/verifier/package.rs | 24 +++- 7 files changed, 271 insertions(+), 141 deletions(-) create mode 100644 src/installer/script_env.rs diff --git a/src/builder/build_env.rs b/src/builder/build_env.rs index 5e344dd1..2191118a 100644 --- a/src/builder/build_env.rs +++ b/src/builder/build_env.rs @@ -1,17 +1,8 @@ // SPDX-License-Identifier: GPL-3.0-only -use std::{ - collections::HashMap, - path::{Path, PathBuf}, -}; - -use thiserror::Error; +use std::collections::HashMap; use crate::{ - cli::display::logging::warning, - platforms::{ - Target, - tool_detection::{self, error::ToolDetectionError}, - }, + installer::{ScriptEnv, ScriptEnvError}, register::installed_package_version::InstalledPackageVersion, repositories::types::Requirement, utils::env::Environment, @@ -37,23 +28,7 @@ const PATH_SEPARATOR: &str = ":"; #[cfg(target_os = "windows")] const PATH_SEPARATOR: &str = ";"; -/// The errors that occur during creating the build environment. -#[derive(Error, Debug)] -pub enum BuildEnvError { - #[error("Cannot add {} to build env {variable}: cannot convert PathBuf to string", path.display())] - PathBufConversion { - path: PathBuf, - variable: String, - }, - - #[error("Cannot find tool {0} on the system")] - ToolNotFound(String), - - #[error("Error while detecting tool on the system")] - ToolDetectionError(#[from] ToolDetectionError), -} - -pub type Result = core::result::Result; +pub type Result = core::result::Result; /// Holds all the data necessary to build a normalized build environment. pub struct BuildEnv<'a> { @@ -63,16 +38,16 @@ pub struct BuildEnv<'a> { } impl<'a> TryInto for BuildEnv<'a> { - type Error = BuildEnvError; + type Error = ScriptEnvError; /// Converts the `BuildEnv` struct into a normalized `Environment` struct. fn try_into(self) -> Result { - let mut env = Environment::new(); + let mut env: Environment = + ScriptEnv::new_with_extra_paths(self.create_extra_paths()?, self.dependencies, self.build_requirements).try_into()?; // TODO: maybe also sandbox TMPDIR variable // TODO: maybe use active version path instead of real version path for all dependencies env.insert_vars(HashMap::from([ - ("PATH", self.create_path()?), ("PKG_CONFIG_PATH", self.create_pkg_config_path()?), ("PKG_CONFIG_LIBDIR", "".into()), ("CMAKE_PREFIX_PATH", self.create_cmake_prefix_path()?), @@ -87,12 +62,9 @@ impl<'a> TryInto for BuildEnv<'a> { // Add M4 variable if m4 is a build dependency if let Some(m4) = self.build_dependencies.iter().find(|x| *x.package_id.name == "m4") { - env.insert_var("M4", path_to_string(&m4.install_path.join("bin").join("m4"), "M4")?); + env.insert_var("M4", ScriptEnv::path_to_string(&m4.install_path.join("bin").join("m4"), "M4")?); } - // Add requirement specific vars to the build env - env.expand(Self::create_requirement_environment(self.build_requirements)?); - // Add macos specific environment variables #[cfg(target_os = "macos")] { @@ -120,16 +92,16 @@ impl<'a> BuildEnv<'a> { } } - /// Creates the `PATH` for the `Environment`. The path will include the bin directories - /// of all (build) dependencies and standard Unix system bin paths (if on Unix). - fn create_path(&self) -> Result { + /// Creates the extra `PATH` entries for the `Environment`. The path will include + /// the bin directories of all build dependencies. It relies on a base `PATH` + /// provided by `ScriptEnv`. + fn create_extra_paths(&self) -> Result> { let mut parts = Vec::new(); //TODO: add compiler wrappers to path - // Add all dependencies to PATH - let dependencies = self.dependencies.iter().chain(self.build_dependencies.iter()); - for dependency in dependencies { + // Add all build dependencies to PATH + for dependency in &self.build_dependencies { let bin_path = dependency.install_path.join("bin"); // Skip adding if the bin dir does not exist @@ -137,32 +109,10 @@ impl<'a> BuildEnv<'a> { continue; } - parts.push(path_to_string(&bin_path, "PATH")?); - } - - // Add standard Unix system bin paths to PATH - #[cfg(any(target_os = "macos", target_os = "linux"))] - { - parts.append(&mut vec!["/usr/bin", "/bin", "/usr/sbin", "/sbin"].into_iter().map(String::from).collect()); - } - - // Add standard Windows system bin paths to PATH - #[cfg(target_os = "windows")] - { - parts.append( - &mut vec![ - "C:\\Windows", - "C:\\Windows\\system32", - "C:\\Windows\\System32\\Wbem", - "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\", - ] - .into_iter() - .map(String::from) - .collect(), - ); + parts.push(ScriptEnv::path_to_string(&bin_path, "PATH")?); } - Ok(parts.join(PATH_SEPARATOR)) + Ok(parts) } /// Creates the `PKG_CONFIG_PATH` to pkgconfig inside of the lib and share directories of the dependencies. @@ -177,12 +127,12 @@ impl<'a> BuildEnv<'a> { // Add lib dir to PKG_CONFIG_PATH if it exists and is a directory if lib_path.is_dir() { - parts.push(path_to_string(&lib_path, "PKG_CONFIG_PATH")?); + parts.push(ScriptEnv::path_to_string(&lib_path, "PKG_CONFIG_PATH")?); } // Add share dir to PKG_CONFIG_PATH if it exists and is a directory if share_path.is_dir() { - parts.push(path_to_string(&share_path, "PKG_CONFIG_PATH")?); + parts.push(ScriptEnv::path_to_string(&share_path, "PKG_CONFIG_PATH")?); } } @@ -201,7 +151,7 @@ impl<'a> BuildEnv<'a> { // Add dependencies to CMAKE_PREFIX_PATH for dependency in self.dependencies { - parts.push(path_to_string(&dependency.install_path, "CMAKE_PREFIX_PATH")?); + parts.push(ScriptEnv::path_to_string(&dependency.install_path, "CMAKE_PREFIX_PATH")?); } Ok(parts.join(PATH_SEPARATOR)) @@ -217,54 +167,10 @@ impl<'a> BuildEnv<'a> { // Adding the share dir if it exists if share_path.exists() { - parts.push(path_to_string(&share_path, "ACLOCAL_PATH")?); + parts.push(ScriptEnv::path_to_string(&share_path, "ACLOCAL_PATH")?); } } Ok(parts.join(PATH_SEPARATOR)) } - - /// Creates an environment from the given requirements. - pub fn create_requirement_environment(requirements: &Vec) -> Result { - let mut environment = Environment::new(); - - for requirement in requirements { - match requirement { - Requirement::Msvc { .. } => { - // Detect MSVC toolchain - let Some(msvc) = tool_detection::detect_msvc()? else { - return Err(BuildEnvError::ToolNotFound("msvc".into())); - }; - - // Get arch, skip requirement if arch is `None`. - let Some(arch) = msvc.get_vcvarsall_arch(&Target::current()) else { - warning!("Tried to load MSVC for an unsupported target, skipping adding to env"); - continue; - }; - - // Add requirement specific environment vars to result - environment.insert_vars(HashMap::from([ - ("PACKIT_VS_PATH", path_to_string(msvc.get_vs_path(), "PACKIT_VS_PATH")?), - ("PACKIT_VCVARSALL", path_to_string(&msvc.get_vcvarsall_path(), "PACKIT_VCVARSALL")?), - ("PACKIT_VCVARSALL_ARCH", arch.into()), - ("PACKIT_MSVC_VERSION", msvc.get_version().to_string()), - ])); - }, - } - } - - Ok(environment) - } -} - -/// Converts a `PathBuf` to a string. -/// Returns a `BuildEnvError::PathBufConversion` if the path cannot be converted. -fn path_to_string(path: &Path, env_var: &str) -> Result { - match path.to_str() { - Some(string) => Ok(string.into()), - None => Err(BuildEnvError::PathBufConversion { - path: path.to_path_buf(), - variable: env_var.into(), - }), - } } diff --git a/src/builder/mod.rs b/src/builder/mod.rs index 3319ab4e..0dd0ef4f 100644 --- a/src/builder/mod.rs +++ b/src/builder/mod.rs @@ -7,7 +7,6 @@ mod patcher; pub use self::builder::Builder; pub use self::build_env::BuildEnv; -pub use self::build_env::BuildEnvError; pub use self::patcher::BinaryPatcher; pub use self::patcher::BinaryPatcherError; diff --git a/src/installer/installer.rs b/src/installer/installer.rs index 0211f9fc..fcc31e07 100644 --- a/src/installer/installer.rs +++ b/src/installer/installer.rs @@ -15,7 +15,7 @@ use crate::{ }, config::Config, installer::{ - InstallLabel, + InstallLabel, ScriptEnv, ScriptEnvError, error::{InstallerError, Result}, install_tree::{InstallMeta, InstallTree, InstallTreeBuilder, InstallType}, options::InstallerOptions, @@ -247,7 +247,7 @@ impl<'a> Installer<'a> { self.register.add_package( &install_meta.package_metadata, &install_meta.version_metadata, - dependencies, + dependencies.clone(), source_repository, install_meta.repository_name.clone(), &install_directory, @@ -280,7 +280,24 @@ impl<'a> Installer<'a> { // Only run the test if the skip test option is false if !self.options.skip_test { - self.execute_test(&package_id, install_meta, &install_directory, &script_args, target_meta)?; + let mut installed_dependencies = Vec::new(); + for dependency in &dependencies { + let Some(installed_package_version) = self.register.get_package_version(&dependency) else { + return Err(InstallerError::UnreachableError { + msg: "Package version cannot be found even though it was inserted right before".to_string(), + }); + }; + installed_dependencies.push(installed_package_version); + } + + self.execute_test( + &package_id, + install_meta, + &install_directory, + &script_args, + target_meta, + installed_dependencies, + )?; } if let Some(notice) = &version_meta.postinstall_notice { @@ -519,6 +536,7 @@ impl<'a> Installer<'a> { install_directory: &Path, script_args: &HashMap<&str, &str>, target: &PackageTarget, + dependencies: Vec<&InstalledPackageVersion>, ) -> Result<()> { // Download and run test script if it exists let script_path = install_meta.version_metadata.get_test_script_path(&install_meta.target_bounds)?; @@ -572,9 +590,10 @@ impl<'a> Installer<'a> { }; // Run script - match scripts::run_test_script(&script_data, &read_files, &target.test_requirements) { + let script_env = ScriptEnv::new(&dependencies, &target.test_requirements); + match scripts::run_test_script(&script_data, &read_files, script_env) { Ok(_) => (), - Err(ScriptError::RequirementNotSatisfied(requirement)) => { + Err(ScriptError::ScriptEnvError(ScriptEnvError::RequirementNotSatisfied(requirement))) => { warning!( "Skipping test, because requirement '{requirement}' is not satisfied.\n{}", requirement.get_not_satisfied_message() diff --git a/src/installer/mod.rs b/src/installer/mod.rs index bcf609e0..83754021 100644 --- a/src/installer/mod.rs +++ b/src/installer/mod.rs @@ -3,6 +3,7 @@ pub mod error; pub mod install_tree; mod installer; mod options; +mod script_env; pub mod scripts; mod symlinker; pub mod types; @@ -17,3 +18,6 @@ pub use self::options::InstallerOptions; pub use self::symlinker::SYMLINK_DIRECTORIES; pub use self::symlinker::Symlinker; + +pub use self::script_env::ScriptEnv; +pub use self::script_env::ScriptEnvError; diff --git a/src/installer/script_env.rs b/src/installer/script_env.rs new file mode 100644 index 00000000..5c4804ab --- /dev/null +++ b/src/installer/script_env.rs @@ -0,0 +1,193 @@ +// SPDX-License-Identifier: GPL-3.0-only +use std::{ + collections::HashMap, + path::{Path, PathBuf}, +}; + +use thiserror::Error; + +use crate::{ + cli::display::logging::warning, + platforms::{ + Target, + tool_detection::{self, error::ToolDetectionError}, + }, + register::installed_package_version::InstalledPackageVersion, + repositories::types::Requirement, + utils::env::Environment, +}; + +#[cfg(target_family = "unix")] +const PATH_SEPARATOR: &str = ":"; + +#[cfg(target_os = "windows")] +const PATH_SEPARATOR: &str = ";"; + +/// The errors that occur during the creation of the script environment. +#[derive(Error, Debug)] +pub enum ScriptEnvError { + #[error("Cannot add {} to script env {variable}: cannot convert PathBuf to string", path.display())] + PathBufConversion { + path: PathBuf, + variable: String, + }, + + #[error("Requirement {0} is not satisfied")] + RequirementNotSatisfied(Requirement), + + #[error("Cannot find tool {0} on the system")] + ToolNotFound(String), + + #[error("Error while detecting tool on the system")] + ToolDetectionError(#[from] ToolDetectionError), +} + +pub type Result = core::result::Result; + +/// Holds all the data necessary to build a normalized script environment. +pub struct ScriptEnv<'a> { + extra_paths: Vec, + dependencies: &'a Vec<&'a InstalledPackageVersion>, + requirements: &'a Vec, +} + +impl<'a> TryInto for ScriptEnv<'a> { + type Error = ScriptEnvError; + + /// Converts the `ScriptEnv` struct into a normalized `Environment` struct. + fn try_into(self) -> Result { + let mut env = Environment::new(); + + env.insert_var("PATH", self.create_path()?); + + // Add requirement specific vars to the script env + env.expand(self.create_requirement_environment()?); + + Ok(env) + } +} + +impl<'a> ScriptEnv<'a> { + /// Creates a new `ScriptEnv`. + pub fn new(dependencies: &'a Vec<&'a InstalledPackageVersion>, requirements: &'a Vec) -> Self { + Self::new_with_extra_paths(Vec::new(), dependencies, requirements) + } + + /// Creates a new `ScriptEnv` with extra PATH entries. + pub fn new_with_extra_paths( + extra_paths: Vec, + dependencies: &'a Vec<&'a InstalledPackageVersion>, + requirements: &'a Vec, + ) -> Self { + Self { + extra_paths, + dependencies, + requirements, + } + } + + /// Creates the `PATH` for the `Environment`. + /// The path will include the bin directories of all dependencies and standard system bin paths. + fn create_path(&self) -> Result { + let mut parts = Vec::new(); + + // Add all dependencies to PATH + for dependency in self.dependencies { + let bin_path = dependency.install_path.join("bin"); + + // Skip adding if the bin dir does not exist + if !bin_path.exists() { + continue; + } + + parts.push(Self::path_to_string(&bin_path, "PATH")?); + } + + // Add all extra paths to the path + for extra_path in &self.extra_paths { + parts.push(extra_path.clone()); + } + + // Add standard Unix system bin paths to PATH + #[cfg(any(target_os = "macos", target_os = "linux"))] + { + parts.append(&mut vec!["/usr/bin", "/bin", "/usr/sbin", "/sbin"].into_iter().map(String::from).collect()); + } + + // Add standard Windows system bin paths to PATH + #[cfg(target_os = "windows")] + { + parts.append( + &mut vec![ + "C:\\Windows", + "C:\\Windows\\system32", + "C:\\Windows\\System32\\Wbem", + "C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\", + ] + .into_iter() + .map(String::from) + .collect(), + ); + } + + Ok(parts.join(PATH_SEPARATOR)) + } + + /// Checks if the requirements are all satisfied. + /// Returns a `ScriptEnvError::RequirementNotSatisfied` is not all requirements are satisfied. + pub fn check_requirements_satisfied(&self) -> Result<()> { + for requirement in self.requirements { + if !requirement.is_satisfied()? { + return Err(ScriptEnvError::RequirementNotSatisfied(requirement.clone())); + } + } + + Ok(()) + } + + /// Creates an environment from the given requirements. + fn create_requirement_environment(&self) -> Result { + let mut environment = Environment::new(); + + for requirement in self.requirements { + match requirement { + Requirement::Msvc { .. } => { + // Detect MSVC toolchain + let Some(msvc) = tool_detection::detect_msvc()? else { + return Err(ScriptEnvError::ToolNotFound("msvc".into())); + }; + + // Get arch, skip requirement if arch is `None`. + let Some(arch) = msvc.get_vcvarsall_arch(&Target::current()) else { + warning!("Tried to load MSVC for an unsupported target, skipping adding to env"); + continue; + }; + + let vcvarsall_path = &msvc.get_vcvarsall_path(); + + // Add requirement specific environment vars to result + environment.insert_vars(HashMap::from([ + ("PACKIT_VS_PATH", Self::path_to_string(msvc.get_vs_path(), "PACKIT_VS_PATH")?), + ("PACKIT_VCVARSALL", Self::path_to_string(vcvarsall_path, "PACKIT_VCVARSALL")?), + ("PACKIT_VCVARSALL_ARCH", arch.into()), + ("PACKIT_MSVC_VERSION", msvc.get_version().to_string()), + ])); + }, + } + } + + Ok(environment) + } + + /// Converts a `PathBuf` to a string. + /// Returns a `ScriptEnvError::PathBufConversion` if the path cannot be converted. + pub fn path_to_string(path: &Path, env_var: &str) -> Result { + match path.to_str() { + Some(string) => Ok(string.into()), + None => Err(ScriptEnvError::PathBufConversion { + path: path.to_path_buf(), + variable: env_var.into(), + }), + } + } +} diff --git a/src/installer/scripts.rs b/src/installer/scripts.rs index a3746caf..7c02bdd3 100644 --- a/src/installer/scripts.rs +++ b/src/installer/scripts.rs @@ -14,11 +14,14 @@ use tempfile::{NamedTempFile, TempDir}; use thiserror::Error; use crate::{ - builder::{BuildEnv, BuildEnvError}, + builder::BuildEnv, config::Config, - installer::types::{PackageId, PackageName}, + installer::{ + ScriptEnv, ScriptEnvError, + types::{PackageId, PackageName}, + }, platforms::{Os, TargetArchitecture, tool_detection::error::ToolDetectionError}, - repositories::{error::RepositoryError, manager::RepositoryManager, types::Requirement}, + repositories::{error::RepositoryError, manager::RepositoryManager}, utils::{ env::Environment, ioerror::{self, IOResultExt}, @@ -34,9 +37,6 @@ pub enum ScriptError { #[error("Cannot parse PathBuf to string")] InvalidPathString, - #[error("Requirement {0} is not satisfied")] - RequirementNotSatisfied(Requirement), - #[error("Cannot find script '{0}'")] ScriptNotFound(String), @@ -61,8 +61,8 @@ pub enum ScriptError { #[error("Cannot fetch script from repository")] FetchScriptError(#[from] RepositoryError), - #[error("Cannot create build environment")] - BuildEnvError(#[from] BuildEnvError), + #[error("Cannot create script environment")] + ScriptEnvError(#[from] ScriptEnvError), #[error("Error while detecting tool on the system")] ToolDetectionError(#[from] ToolDetectionError), @@ -154,13 +154,9 @@ pub fn run_post_script(script_data: &ScriptData) -> Result<()> { /// Runs the given test script, in a newly created temp directory. /// It also writes the specified external test files to the temp directory. /// Note that the script should be a `.sh` script on Linux and macOS and a `.bat` on Windows. -pub fn run_test_script(script_data: &ScriptData, external_files: &Vec<(&String, Bytes)>, requirements: &Vec) -> Result<()> { +pub fn run_test_script(script_data: &ScriptData, external_files: &Vec<(&String, Bytes)>, script_env: ScriptEnv) -> Result<()> { // Check if all test requirements are satisfied - for requirement in requirements { - if !requirement.is_satisfied()? { - return Err(ScriptError::RequirementNotSatisfied(requirement.clone())); - } - } + script_env.check_requirements_satisfied()?; let temp_dir = TempDir::new().map_err(ScriptError::TempCreationError)?; @@ -169,10 +165,7 @@ pub fn run_test_script(script_data: &ScriptData, external_files: &Vec<(&String, fs::write(temp_dir.path().join(file_name), file_content).map_err(ScriptError::SaveError)?; } - // Create environment containing the requirements - let env = BuildEnv::create_requirement_environment(requirements)?; - - run_script(script_data, &temp_dir, env, true) + run_script(script_data, &temp_dir, script_env.try_into()?, true) } /// Runs the given uninstall script, in the package install directory. diff --git a/src/integrity/verifier/package.rs b/src/integrity/verifier/package.rs index 40234a01..fd03ce06 100644 --- a/src/integrity/verifier/package.rs +++ b/src/integrity/verifier/package.rs @@ -8,11 +8,15 @@ use crate::{ }, config::{Config, Repository}, installer::{ - self, + self, ScriptEnv, ScriptEnvError, scripts::{self, ScriptData, ScriptError}, types::{Dependency, PackageId, PackageName}, }, - integrity::{Issue, error::Result, utils::get_storage_packages}, + integrity::{ + Issue, + error::{Result, VerifierError}, + utils::get_storage_packages, + }, packager, platforms::Target, register::{ @@ -633,6 +637,17 @@ fn check_package_test(package_id: &PackageId, register: &PackageRegister, config false, ); + // Get installed dependencies + let mut installed_dependencies = Vec::new(); + for dependency in &package_version.dependencies { + let Some(installed_package_version) = register.get_package_version(&dependency) else { + return Err(VerifierError::UnreachableError { + msg: "Package version cannot be found even though it was inserted right before".to_string(), + }); + }; + installed_dependencies.push(installed_package_version); + } + // Get the external test files let mut read_files = Vec::new(); for file in &local_metadata.external_test_files { @@ -641,9 +656,10 @@ fn check_package_test(package_id: &PackageId, register: &PackageRegister, config } // Run the test script and only return true if the test itself failed - match scripts::run_test_script(&script_data, &read_files, &local_metadata.test_requirements) { + let script_env = ScriptEnv::new(&installed_dependencies, &local_metadata.test_requirements); + match scripts::run_test_script(&script_data, &read_files, script_env) { Ok(_) => Ok(false), - Err(ScriptError::RequirementNotSatisfied(requirement)) => { + Err(ScriptError::ScriptEnvError(ScriptEnvError::RequirementNotSatisfied(requirement))) => { warning!( "Skipping {} test, because requirement '{requirement}' is not satisfied.\n{}", package_id.style(), From 6e2510ec3d9634fcec140557b0f04be56634b608 Mon Sep 17 00:00:00 2001 From: Frank Date: Mon, 2 Nov 2026 15:56:43 +0100 Subject: [PATCH 2/2] Fix clippy warnings --- src/installer/installer.rs | 2 +- src/integrity/verifier/package.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/installer/installer.rs b/src/installer/installer.rs index fcc31e07..5c5eed49 100644 --- a/src/installer/installer.rs +++ b/src/installer/installer.rs @@ -282,7 +282,7 @@ impl<'a> Installer<'a> { if !self.options.skip_test { let mut installed_dependencies = Vec::new(); for dependency in &dependencies { - let Some(installed_package_version) = self.register.get_package_version(&dependency) else { + let Some(installed_package_version) = self.register.get_package_version(dependency) else { return Err(InstallerError::UnreachableError { msg: "Package version cannot be found even though it was inserted right before".to_string(), }); diff --git a/src/integrity/verifier/package.rs b/src/integrity/verifier/package.rs index fd03ce06..520a5ba9 100644 --- a/src/integrity/verifier/package.rs +++ b/src/integrity/verifier/package.rs @@ -640,7 +640,7 @@ fn check_package_test(package_id: &PackageId, register: &PackageRegister, config // Get installed dependencies let mut installed_dependencies = Vec::new(); for dependency in &package_version.dependencies { - let Some(installed_package_version) = register.get_package_version(&dependency) else { + let Some(installed_package_version) = register.get_package_version(dependency) else { return Err(VerifierError::UnreachableError { msg: "Package version cannot be found even though it was inserted right before".to_string(), });