mirrors/uv
1
0
Fork 0
mirror of https://github.com/astral-sh/uv.git synced 2025-08-04 19:08:04 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Add option pass environment variables for SDist building (#2039)

Browse source 
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

With this PR I've added the option environment variables to the wheel
building process, through the `BuildDispatch`. When integrating uv with
our project pixi (https://github.com/prefix-dev/pixi/pull/863). We ran
into this missing requirement, I've made a rough version here, could
maybe use some refinement.

### Why do we need this?

Because pixi allow the user to use a conda activated prefix for wheel
building, this comes with a number of environment variables, like `PATH`
but also `CONDA_PREFIX` amongst others. This allows the user to use
system dependencies from conda-forge to use during an sdist build.
Because we use `uv` as a library we need to pass in the options
programatically. Additionally, in general there is nothing holding a
python sdist back from actually depending on an environment variable,
see
e.g the test package: https://pypi.org/project/env-test-package/

### What about `ConfigSettings`

I think `ConfigSettings` does not suffice because e.g. CMake could
function differently when the `CONDA_PREFIX` is set. Also, we do not
know if the user supplied backend actually support these settings.

### Path handling

Because the user can now also supply a PATH in the environment map, the
logic I had was the following, I format the path so that it has the
following precedence

1. venv scripts dir.
2. user supplied path.
3. system path.

### Improvements

There is some path modification and copying happening everytime we use
the `run_python_script` function, I think we could improve this but
would like some pointers where to best put the maybe split and cached
version, we might also want to use some types to split these things up.


### Finally

I did not add any of these options to the uv executables, I first would
like to know if this is a direction we would want to go in. I'm happy to
do this or make any changes that you feel would benefit this project.

Also tagging @wolfv to keep track of this as well.  

## Test Plan

<!-- How was it tested? -->

---------

Co-authored-by: konsti <konstin@mailbox.org>
This commit is contained in:
Tim de Jager 2024-02-29 11:55:10 +01:00 • committed by GitHub
parent 2838542ade
commit 0fbfa11013
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 103 additions and 18 deletions
Download patch file Download diff file Expand all files Collapse all files

2
Cargo.lock generated
View file

@ -4228,6 +4228,7 @@ dependencies = [
"pypi-types",
"pyproject-toml",
"regex",
"rustc-hash",
"serde",
"serde_json",
"tempfile",
@ -4374,6 +4375,7 @@ dependencies = [
"platform-host",
"platform-tags",
"pypi-types",
"rustc-hash",
"tempfile",
"tokio",
"tracing",

1
crates/uv-build/Cargo.toml
View file

@ -38,6 +38,7 @@ thiserror = { workspace = true }
tokio = { workspace = true, features = ["sync", "process"] }
toml = { workspace = true }
tracing = { workspace = true }
rustc-hash = { workspace = true }
[dev-dependencies]
insta = { version = "1.35.1" }

93
crates/uv-build/src/lib.rs
View file

@ -18,6 +18,7 @@ use itertools::Itertools;
use once_cell::sync::Lazy;
use pyproject_toml::Project;
use regex::Regex;
use rustc_hash::FxHashMap;
use serde::de::{value, SeqAccess, Visitor};
use serde::{de, Deserialize, Deserializer, Serialize};
use tempfile::{tempdir_in, TempDir};
@ -331,6 +332,10 @@ pub struct SourceBuild {
package_id: String,
/// Whether we do a regular PEP 517 build or an PEP 660 editable build
build_kind: BuildKind,
/// Modified PATH that contains the `venv_bin`, `user_path` and `system_path` variables in that order
modified_path: OsString,
/// Environment variables to be passed in during metadata or wheel building
environment_variables: FxHashMap<OsString, OsString>,
}
impl SourceBuild {
@ -349,6 +354,7 @@ impl SourceBuild {
setup_py: SetupPyStrategy,
config_settings: ConfigSettings,
build_kind: BuildKind,
mut environment_variables: FxHashMap<OsString, OsString>,
) -> Result<Self, Error> {
let temp_dir = tempdir_in(build_context.cache().root())?;
@ -413,6 +419,36 @@ impl SourceBuild {
.await
.map_err(|err| Error::RequirementsInstall("build-system.requires (install)", err))?;
// Figure out what the modified path should be
// Remove the PATH variable from the environment variables if it's there
let user_path = environment_variables.remove(&OsString::from("PATH"));
// See if there is an OS PATH variable
let os_path = env::var_os("PATH");
// Prepend the user supplied PATH to the existing OS PATH
let modified_path = if let Some(user_path) = user_path {
match os_path {
// Prepend the user supplied PATH to the existing PATH
Some(env_path) => {
let user_path = PathBuf::from(user_path);
let new_path = env::split_paths(&user_path).chain(env::split_paths(&env_path));
Some(env::join_paths(new_path).map_err(Error::BuildScriptPath)?)
}
// Use the user supplied PATH
None => Some(user_path),
}
} else {
os_path
};
// Prepend the venv bin directory to the modified path
let modified_path = if let Some(path) = modified_path {
let venv_path = iter::once(venv.scripts().to_path_buf()).chain(env::split_paths(&path));
env::join_paths(venv_path).map_err(Error::BuildScriptPath)?
} else {
OsString::from(venv.scripts())
};
if let Some(pep517_backend) = &pep517_backend {
create_pep517_build_environment(
&source_tree,
@ -422,6 +458,8 @@ impl SourceBuild {
&package_id,
build_kind,
&config_settings,
&environment_variables,
&modified_path,
)
.await?;
}
@ -435,6 +473,8 @@ impl SourceBuild {
config_settings,
metadata_directory: None,
package_id,
environment_variables,
modified_path,
})
}
@ -575,9 +615,15 @@ impl SourceBuild {
script="prepare_metadata_for_build_wheel",
python_version = %self.venv.interpreter().python_version()
);
let output = run_python_script(&self.venv, &script, &self.source_tree)
.instrument(span)
.await?;
let output = run_python_script(
&self.venv,
&script,
&self.source_tree,
&self.environment_variables,
&self.modified_path,
)
.instrument(span)
.await?;
if !output.status.success() {
return Err(Error::from_command_output(
"Build backend failed to determine metadata through `prepare_metadata_for_build_wheel`".to_string(),
@ -707,9 +753,15 @@ impl SourceBuild {
script=format!("build_{}", self.build_kind),
python_version = %self.venv.interpreter().python_version()
);
let output = run_python_script(&self.venv, &script, &self.source_tree)
.instrument(span)
.await?;
let output = run_python_script(
&self.venv,
&script,
&self.source_tree,
&self.environment_variables,
&self.modified_path,
)
.instrument(span)
.await?;
if !output.status.success() {
return Err(Error::from_command_output(
format!(
@ -755,6 +807,7 @@ fn escape_path_for_python(path: &Path) -> String {
}
/// Not a method because we call it before the builder is completely initialized
#[allow(clippy::too_many_arguments)]
async fn create_pep517_build_environment(
source_tree: &Path,
venv: &PythonEnvironment,
@ -763,6 +816,8 @@ async fn create_pep517_build_environment(
package_id: &str,
build_kind: BuildKind,
config_settings: &ConfigSettings,
environment_variables: &FxHashMap<OsString, OsString>,
modified_path: &OsString,
) -> Result<(), Error> {
debug!(
"Calling `{}.get_requires_for_build_{}()`",
@ -786,9 +841,15 @@ async fn create_pep517_build_environment(
script=format!("get_requires_for_build_{}", build_kind),
python_version = %venv.interpreter().python_version()
);
let output = run_python_script(venv, &script, source_tree)
.instrument(span)
.await?;
let output = run_python_script(
venv,
&script,
source_tree,
environment_variables,
modified_path,
)
.instrument(span)
.await?;
if !output.status.success() {
return Err(Error::from_command_output(
format!("Build backend failed to determine extra requires with `build_{build_kind}()`"),
@ -849,20 +910,18 @@ async fn run_python_script(
venv: &PythonEnvironment,
script: &str,
source_tree: &Path,
environment_variables: &FxHashMap<OsString, OsString>,
modified_path: &OsString,
) -> Result<Output, Error> {
// Prepend the venv bin dir to PATH
let new_path = if let Some(old_path) = env::var_os("PATH") {
let new_path = iter::once(venv.scripts().to_path_buf()).chain(env::split_paths(&old_path));
env::join_paths(new_path).map_err(Error::BuildScriptPath)?
} else {
OsString::from("")
};
Command::new(venv.python_executable())
.args(["-c", script])
.current_dir(source_tree.simplified())
// Pass in remaining environment variables
.envs(environment_variables)
// Set the modified PATH
.env("PATH", modified_path)
// Activate the venv
.env("VIRTUAL_ENV", venv.root())
.env("PATH", new_path)
.output()
.await
.map_err(|err| Error::CommandFailed(venv.python_executable().to_path_buf(), err))

2
crates/uv-dev/src/build.rs
View file

@ -7,6 +7,7 @@ use fs_err as fs;
use distribution_types::IndexLocations;
use platform_host::Platform;
use rustc_hash::FxHashMap;
use uv_build::{SourceBuild, SourceBuildContext};
use uv_cache::{Cache, CacheArgs};
use uv_client::{FlatIndex, RegistryClientBuilder};
@ -87,6 +88,7 @@ pub(crate) async fn build(args: BuildArgs) -> Result<PathBuf> {
setup_py,
config_settings.clone(),
build_kind,
FxHashMap::default(),
)
.await?;
Ok(wheel_dir.join(builder.build(&wheel_dir).await?))

1
crates/uv-dispatch/Cargo.toml
View file

@ -28,6 +28,7 @@ uv-interpreter = { path = "../uv-interpreter" }
uv-resolver = { path = "../uv-resolver" }
uv-traits = { path = "../uv-traits" }
pypi-types = { path = "../pypi-types" }
rustc-hash = { workspace = true }
anyhow = { workspace = true }
fs-err = { workspace = true }

22
crates/uv-dispatch/src/lib.rs
View file

@ -2,12 +2,14 @@
//! [installer][`uv_installer`] and [build][`uv_build`] through [`BuildDispatch`]
//! implementing [`BuildContext`].
use std::future::Future;
use std::ffi::OsStr;
use std::path::Path;
use std::{ffi::OsString, future::Future};
use anyhow::{bail, Context, Result};
use futures::FutureExt;
use itertools::Itertools;
use rustc_hash::FxHashMap;
use tracing::{debug, instrument};
use distribution_types::{IndexLocations, Name, Resolution, SourceDist};
@ -36,6 +38,7 @@ pub struct BuildDispatch<'a> {
config_settings: &'a ConfigSettings,
source_build_context: SourceBuildContext,
options: Options,
build_extra_env_vars: FxHashMap<OsString, OsString>,
}
impl<'a> BuildDispatch<'a> {
@ -67,6 +70,7 @@ impl<'a> BuildDispatch<'a> {
no_binary,
source_build_context: SourceBuildContext::default(),
options: Options::default(),
build_extra_env_vars: FxHashMap::default(),
}
}
@ -75,6 +79,21 @@ impl<'a> BuildDispatch<'a> {
self.options = options;
self
}
/// Set the environment variables to be used when building a source distribution.
#[must_use]
pub fn with_build_extra_env_vars<I, K, V>(mut self, sdist_build_env_variables: I) -> Self
where
I: IntoIterator<Item = (K, V)>,
K: AsRef<OsStr>,
V: AsRef<OsStr>,
{
self.build_extra_env_vars = sdist_build_env_variables
.into_iter()
.map(|(key, value)| (key.as_ref().to_owned(), value.as_ref().to_owned()))
.collect();
self
}
}
impl<'a> BuildContext for BuildDispatch<'a> {
@ -277,6 +296,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.setup_py,
self.config_settings.clone(),
build_kind,
self.build_extra_env_vars.clone(),
)
.boxed()
.await?;