From daf785e5b1793efe35d8a2d2d3e7eadcca5c05eb Mon Sep 17 00:00:00 2001 From: Wolf Vollprecht Date: Tue, 20 Feb 2024 12:18:01 +0100 Subject: [PATCH] fix: always add python version as `$PY_VER` (#645) --- src/env_vars.rs | 87 ++++++++++++----------------- src/metadata.rs | 47 +++++++++++++++- src/post_process/python.rs | 55 ++++++------------ test-data/recipes/flask/recipe.yaml | 10 +++- 4 files changed, 108 insertions(+), 91 deletions(-) diff --git a/src/env_vars.rs b/src/env_vars.rs index ac529fba0..fa3a47cfc 100644 --- a/src/env_vars.rs +++ b/src/env_vars.rs @@ -1,5 +1,4 @@ //! Functions to collect environment variables that are used during the build process. -use std::collections::BTreeMap; use std::path::{Path, PathBuf}; use std::{collections::HashMap, env}; @@ -34,26 +33,33 @@ fn get_sitepackages_dir(prefix: &Path, platform: &Platform, py_ver: &str) -> Pat /// - SP_DIR: Python site-packages directory /// - NPY_VER: Numpy version (major.minor), e.g. 1.19 /// - NPY_DISTUTILS_APPEND_FLAGS: 1 (https://github.com/conda/conda-build/pull/3015) -pub fn python_vars( - prefix: &Path, - platform: &Platform, - variant: &BTreeMap, -) -> HashMap { +pub fn python_vars(output: &Output) -> HashMap { let mut result = HashMap::::new(); - if platform.is_windows() { - let python = prefix.join("python.exe"); + if output.host_platform().is_windows() { + let python = output.prefix().join("python.exe"); result.insert("PYTHON".to_string(), python.to_string_lossy().to_string()); } else { - let python = prefix.join("bin/python"); + let python = output.prefix().join("bin/python"); result.insert("PYTHON".to_string(), python.to_string_lossy().to_string()); } - if let Some(py_ver) = variant.get("python") { + // find python in the host dependencies + let mut python_version = output.variant().get("python").map(|s| s.to_string()); + if python_version.is_none() { + if let Some((record, requested)) = output.find_resolved_package("python") { + if requested { + python_version = Some(record.package_record.version.to_string()); + } + } + } + + if let Some(py_ver) = python_version { let py_ver = py_ver.split('.').collect::>(); let py_ver_str = format!("{}.{}", py_ver[0], py_ver[1]); - let stdlib_dir = get_stdlib_dir(prefix, platform, &py_ver_str); - let site_packages_dir = get_sitepackages_dir(prefix, platform, &py_ver_str); + let stdlib_dir = get_stdlib_dir(output.prefix(), output.host_platform(), &py_ver_str); + let site_packages_dir = + get_sitepackages_dir(output.prefix(), output.host_platform(), &py_ver_str); result.insert( "PY3K".to_string(), if py_ver[0] == "3" { @@ -73,7 +79,7 @@ pub fn python_vars( ); } - if let Some(npy_version) = variant.get("numpy") { + if let Some(npy_version) = output.variant().get("numpy") { let np_ver = npy_version.split('.').collect::>(); let np_ver = format!("{}.{}", np_ver[0], np_ver[1]); @@ -91,23 +97,19 @@ pub fn python_vars( /// - R: Path to R executable /// - R_USER: Path to R user directory /// -pub fn r_vars( - prefix: &Path, - platform: &Platform, - variant: &BTreeMap, -) -> HashMap { +pub fn r_vars(output: &Output) -> HashMap { let mut result = HashMap::::new(); - if let Some(r_ver) = variant.get("r-base") { + if let Some(r_ver) = output.variant().get("r-base") { result.insert("R_VER".to_string(), r_ver.clone()); - let r_bin = if platform.is_windows() { - prefix.join("Scripts/R.exe") + let r_bin = if output.host_platform().is_windows() { + output.prefix().join("Scripts/R.exe") } else { - prefix.join("bin/R") + output.prefix().join("bin/R") }; - let r_user = prefix.join("Libs/R"); + let r_user = output.prefix().join("Libs/R"); result.insert("R".to_string(), r_bin.to_string_lossy().to_string()); result.insert("R_USER".to_string(), r_user.to_string_lossy().to_string()); @@ -116,15 +118,11 @@ pub fn r_vars( result } -pub fn language_vars( - prefix: &Path, - platform: &Platform, - variant: &BTreeMap, -) -> HashMap { +pub fn language_vars(output: &Output) -> HashMap { let mut result = HashMap::::new(); - result.extend(python_vars(prefix, platform, variant)); - result.extend(r_vars(prefix, platform, variant)); + result.extend(python_vars(output)); + result.extend(r_vars(output)); result } @@ -214,13 +212,7 @@ pub fn vars(output: &Output, build_state: &str) -> HashMap { insert!(vars, "CONDA_BUILD", "1"); insert!(vars, "PYTHONNOUSERSITE", "1"); - if let Some((_, host_arch)) = output - .build_configuration - .host_platform - .to_string() - .rsplit_once('-') - { - // TODO clear if we want/need this variable this seems to be pretty bad (in terms of cross compilation, etc.) + if let Some((_, host_arch)) = output.host_platform().to_string().rsplit_once('-') { insert!(vars, "ARCH", host_arch); } @@ -282,7 +274,7 @@ pub fn vars(output: &Output, build_state: &str) -> HashMap { output.recipe.build().number().to_string() ); - // TODO this is inaccurate + let hash = output.build_configuration.hash.clone(); insert!( vars, "PKG_BUILD_STRING", @@ -290,10 +282,11 @@ pub fn vars(output: &Output, build_state: &str) -> HashMap { .recipe .build() .string() - .unwrap_or_default() - .to_owned() + .map(|s| s.to_string()) + .unwrap_or_else(|| hash.to_string()) ); - insert!(vars, "PKG_HASH", output.build_configuration.hash.clone()); + insert!(vars, "PKG_HASH", hash); + if output.build_configuration.cross_compilation() { insert!(vars, "CONDA_BUILD_CROSS_COMPILATION", "1"); } else { @@ -316,12 +309,7 @@ pub fn vars(output: &Output, build_state: &str) -> HashMap { ); insert!(vars, "CONDA_BUILD_STATE", build_state); - vars.extend(language_vars( - &directories.host_prefix, - // Note: host_platform cannot be noarch - &output.build_configuration.host_platform, - &output.build_configuration.variant, - )); + vars.extend(language_vars(output)); // for reproducibility purposes, set the SOURCE_DATE_EPOCH to the configured timestamp // this value will be taken from the previous package for rebuild purposes @@ -331,10 +319,5 @@ pub fn vars(output: &Output, build_state: &str) -> HashMap { timestamp_epoch_secs.to_string(), ); - // let vars: Vec<(String, String)> = vec![ - // // build configuration - // // (s!("CONDA_BUILD_SYSROOT"), s!("")), - // ]; - vars } diff --git a/src/metadata.rs b/src/metadata.rs index e0df46155..67010d388 100644 --- a/src/metadata.rs +++ b/src/metadata.rs @@ -14,7 +14,7 @@ use fs_err as fs; use indicatif::HumanBytes; use rattler_conda_types::{ package::{ArchiveType, PathType, PathsEntry, PathsJson}, - PackageName, Platform, + PackageName, Platform, RepoDataRecord, }; use rattler_index::index; use rattler_package_streaming::write::CompressionLevel; @@ -366,6 +366,51 @@ impl Output { summary.build_end = Some(chrono::Utc::now()); } + /// Shorthand to retrieve the variant configuration for this output + pub fn variant(&self) -> &BTreeMap { + &self.build_configuration.variant + } + + /// Shorthand to retrieve the host prefix for this output + pub fn prefix(&self) -> &Path { + &self.build_configuration.directories.host_prefix + } + + /// Shorthand to retrieve the build prefix for this output + pub fn build_prefix(&self) -> &Path { + &self.build_configuration.directories.build_prefix + } + + /// Shorthand to retrieve the target platform for this output + pub fn target_platform(&self) -> &Platform { + &self.build_configuration.target_platform + } + + /// Shorthand to retrieve the target platform for this output + pub fn host_platform(&self) -> &Platform { + &self.build_configuration.host_platform + } + + /// Search for the resolved package with the given name in the host prefix + /// Returns a tuple of the package and a boolean indicating whether the package is directly requested + pub fn find_resolved_package(&self, name: &str) -> Option<(&RepoDataRecord, bool)> { + let host = self.finalized_dependencies.as_ref()?.host.as_ref()?; + let record = host + .resolved + .iter() + .find(|p| p.package_record.name.as_normalized() == name); + + let is_requested = host.specs.iter().any(|s| { + s.spec() + .name + .as_ref() + .map(|n| n.as_normalized() == name) + .unwrap_or(false) + }); + + record.map(|r| (r, is_requested)) + } + /// Print a nice summary of the build pub fn log_build_summary(&self) -> Result<(), std::io::Error> { let summary = self.build_summary.lock().unwrap(); diff --git a/src/post_process/python.rs b/src/post_process/python.rs index bdf7dbd73..3589b7c56 100644 --- a/src/post_process/python.rs +++ b/src/post_process/python.rs @@ -303,49 +303,30 @@ pub(crate) fn create_entry_points( return Ok(Vec::new()); } - let target_platform = &output.build_configuration.target_platform; let mut new_files = Vec::new(); - let host_deps = output - .finalized_dependencies - .as_ref() - .ok_or_else(|| PackagingError::DependenciesNotFinalized)? - .host - .clone() - .ok_or_else(|| { - PackagingError::CannotCreateEntryPoint("Could not find host dependencies".to_string()) - })?; - - let python_record = host_deps - .resolved - .iter() - .find(|dep| dep.package_record.name.as_normalized() == "python"); - let python_version = if let Some(python_record) = python_record { - python_record.package_record.version.version().clone() - } else { - return Err(PackagingError::CannotCreateEntryPoint( + let (python_record, _) = output.find_resolved_package("python").ok_or_else(|| { + PackagingError::CannotCreateEntryPoint( "Could not find python in host dependencies".to_string(), - )); - }; + ) + })?; + + let python_version = python_record.package_record.version.clone(); for ep in &output.recipe.build().python().entry_points { let script = python_entry_point_template( - &output - .build_configuration - .directories - .host_prefix - .to_string_lossy(), + &output.prefix().to_string_lossy(), ep, - &PythonInfo::from_version(&python_version, output.build_configuration.target_platform) - .map_err(|e| { - PackagingError::CannotCreateEntryPoint(format!( - "Could not create python info: {}", - e - )) - })?, + // using target_platform is OK because this should never be noarch + &PythonInfo::from_version(&python_version, *output.target_platform()).map_err(|e| { + PackagingError::CannotCreateEntryPoint(format!( + "Could not create python info: {}", + e + )) + })?, ); - if target_platform.is_windows() { + if output.target_platform().is_windows() { fs::create_dir_all(tmp_dir_path.join("Scripts"))?; let script_path = tmp_dir_path.join(format!("Scripts/{}-script.py", ep.command)); @@ -355,7 +336,7 @@ pub(crate) fn create_entry_points( // write exe launcher as well let exe_path = tmp_dir_path.join(format!("Scripts/{}.exe", ep.command)); let mut exe = fs::File::create(&exe_path)?; - exe.write_all(get_windows_launcher(target_platform))?; + exe.write_all(get_windows_launcher(output.target_platform()))?; new_files.extend(vec![script_path, exe_path]); } else { @@ -371,12 +352,12 @@ pub(crate) fn create_entry_points( std::os::unix::fs::PermissionsExt::from_mode(0o775), )?; - if output.build_configuration.target_platform.is_osx() + if output.target_platform().is_osx() && output.recipe.build().python().use_python_app_entrypoint { fix_shebang( &script_path, - &output.build_configuration.directories.host_prefix, + output.prefix(), output.recipe.build().python().use_python_app_entrypoint, )?; } diff --git a/test-data/recipes/flask/recipe.yaml b/test-data/recipes/flask/recipe.yaml index 6017bbd54..951d069ad 100644 --- a/test-data/recipes/flask/recipe.yaml +++ b/test-data/recipes/flask/recipe.yaml @@ -15,7 +15,15 @@ source: build: number: 0 - script: python -m pip install . -vv --no-deps --no-build-isolation + script: + - python -m pip install . -vv --no-deps --no-build-isolation + # make sure that PY_VER is set even for noarch packages + - if: unix + then: + - test ! -z "$PY_VER" + else: + - if not defined PY_VER exit 1 + python: entry_points: - flask = flask.cli:main