Skip to content

Commit

Permalink
Add support for --no-build-isolation (#2258)
Browse files Browse the repository at this point in the history
## Summary

This PR adds support for pip's `--no-build-isolation`. When enabled,
build requirements won't be installed during PEP 517-style builds, but
the source environment _will_ be used when executing the build steps
themselves.

Closes #1715.
  • Loading branch information
charliermarsh authored Mar 7, 2024
1 parent d249574 commit 5ae5980
Show file tree
Hide file tree
Showing 19 changed files with 245 additions and 65 deletions.
1 change: 0 additions & 1 deletion PIP_COMPATIBILITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ the implementation, and tend to be tracked in individual issues. For example:

- [`--trusted-host`](https://github.com/astral-sh/uv/issues/1339)
- [`--user`](https://github.com/astral-sh/uv/issues/2077)
- [`--no-build-isolation`](https://github.com/astral-sh/uv/issues/1715)

If you encounter a missing option or subcommand, please search the issue tracker to see if it has
already been reported, and if not, consider opening a new issue. Feel free to upvote any existing
Expand Down
84 changes: 50 additions & 34 deletions crates/uv-build/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ use distribution_types::Resolution;
use pep508_rs::Requirement;
use uv_fs::Simplified;
use uv_interpreter::{Interpreter, PythonEnvironment};
use uv_traits::{BuildContext, BuildKind, ConfigSettings, SetupPyStrategy, SourceBuildTrait};
use uv_traits::{
BuildContext, BuildIsolation, BuildKind, ConfigSettings, SetupPyStrategy, SourceBuildTrait,
};

/// e.g. `pygraphviz/graphviz_wrap.c:3020:10: fatal error: graphviz/cgraph.h: No such file or directory`
static MISSING_HEADER_RE: Lazy<Regex> = Lazy::new(|| {
Expand Down Expand Up @@ -357,6 +359,7 @@ impl SourceBuild {
package_id: String,
setup_py: SetupPyStrategy,
config_settings: ConfigSettings,
build_isolation: BuildIsolation<'_>,
build_kind: BuildKind,
mut environment_variables: FxHashMap<OsString, OsString>,
) -> Result<Self, Error> {
Expand Down Expand Up @@ -402,27 +405,36 @@ impl SourceBuild {
let pep517_backend = Self::get_pep517_backend(setup_py, &source_tree, &default_backend)
.map_err(|err| *err)?;

let venv = uv_virtualenv::create_venv(
&temp_dir.path().join(".venv"),
interpreter.clone(),
uv_virtualenv::Prompt::None,
false,
Vec::new(),
)?;

// Setup the build environment.
let resolved_requirements = Self::get_resolved_requirements(
build_context,
source_build_context,
&default_backend,
pep517_backend.as_ref(),
)
.await?;
// Create a virtual environment, or install into the shared environment if requested.
let venv = match build_isolation {
BuildIsolation::Isolated => uv_virtualenv::create_venv(
&temp_dir.path().join(".venv"),
interpreter.clone(),
uv_virtualenv::Prompt::None,
false,
Vec::new(),
)?,
BuildIsolation::Shared(venv) => venv.clone(),
};

build_context
.install(&resolved_requirements, &venv)
.await
.map_err(|err| Error::RequirementsInstall("build-system.requires (install)", err))?;
// Setup the build environment. If build isolation is disabled, we assume the build
// environment is already setup.
if build_isolation.is_isolated() {
let resolved_requirements = Self::get_resolved_requirements(
build_context,
source_build_context,
&default_backend,
pep517_backend.as_ref(),
)
.await?;

build_context
.install(&resolved_requirements, &venv)
.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
Expand Down Expand Up @@ -454,19 +466,23 @@ impl SourceBuild {
OsString::from(venv.scripts())
};

if let Some(pep517_backend) = &pep517_backend {
create_pep517_build_environment(
&source_tree,
&venv,
pep517_backend,
build_context,
&package_id,
build_kind,
&config_settings,
&environment_variables,
&modified_path,
)
.await?;
// Create the PEP 517 build environment. If build isolation is disabled, we assume the build
// environment is already setup.
if build_isolation.is_isolated() {
if let Some(pep517_backend) = &pep517_backend {
create_pep517_build_environment(
&source_tree,
&venv,
pep517_backend,
build_context,
&package_id,
build_kind,
&config_settings,
&environment_variables,
&modified_path,
)
.await?;
}
}

Ok(Self {
Expand Down
6 changes: 5 additions & 1 deletion crates/uv-dev/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ use uv_dispatch::BuildDispatch;
use uv_installer::NoBinary;
use uv_interpreter::PythonEnvironment;
use uv_resolver::InMemoryIndex;
use uv_traits::{BuildContext, BuildKind, ConfigSettings, InFlight, NoBuild, SetupPyStrategy};
use uv_traits::{
BuildContext, BuildIsolation, BuildKind, ConfigSettings, InFlight, NoBuild, SetupPyStrategy,
};

#[derive(Parser)]
pub(crate) struct BuildArgs {
Expand Down Expand Up @@ -74,6 +76,7 @@ pub(crate) async fn build(args: BuildArgs) -> Result<PathBuf> {
&in_flight,
setup_py,
&config_settings,
BuildIsolation::Isolated,
&NoBuild::None,
&NoBinary::None,
);
Expand All @@ -87,6 +90,7 @@ pub(crate) async fn build(args: BuildArgs) -> Result<PathBuf> {
args.sdist.display().to_string(),
setup_py,
config_settings.clone(),
BuildIsolation::Isolated,
build_kind,
FxHashMap::default(),
)
Expand Down
3 changes: 2 additions & 1 deletion crates/uv-dev/src/install_many.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use uv_installer::{Downloader, NoBinary};
use uv_interpreter::PythonEnvironment;
use uv_normalize::PackageName;
use uv_resolver::{DistFinder, InMemoryIndex};
use uv_traits::{BuildContext, ConfigSettings, InFlight, NoBuild, SetupPyStrategy};
use uv_traits::{BuildContext, BuildIsolation, ConfigSettings, InFlight, NoBuild, SetupPyStrategy};

#[derive(Parser)]
pub(crate) struct InstallManyArgs {
Expand Down Expand Up @@ -81,6 +81,7 @@ pub(crate) async fn install_many(args: InstallManyArgs) -> Result<()> {
&in_flight,
setup_py,
&config_settings,
BuildIsolation::Isolated,
&no_build,
&NoBinary::None,
);
Expand Down
3 changes: 2 additions & 1 deletion crates/uv-dev/src/resolve_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use uv_dispatch::BuildDispatch;
use uv_installer::NoBinary;
use uv_interpreter::PythonEnvironment;
use uv_resolver::{InMemoryIndex, Manifest, Options, Resolver};
use uv_traits::{ConfigSettings, InFlight, NoBuild, SetupPyStrategy};
use uv_traits::{BuildIsolation, ConfigSettings, InFlight, NoBuild, SetupPyStrategy};

#[derive(ValueEnum, Default, Clone)]
pub(crate) enum ResolveCliFormat {
Expand Down Expand Up @@ -85,6 +85,7 @@ pub(crate) async fn resolve_cli(args: ResolveCliArgs) -> Result<()> {
&in_flight,
SetupPyStrategy::default(),
&config_settings,
BuildIsolation::Isolated,
&no_build,
&NoBinary::None,
);
Expand Down
3 changes: 2 additions & 1 deletion crates/uv-dev/src/resolve_many.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use uv_installer::NoBinary;
use uv_interpreter::PythonEnvironment;
use uv_normalize::PackageName;
use uv_resolver::InMemoryIndex;
use uv_traits::{BuildContext, ConfigSettings, InFlight, NoBuild, SetupPyStrategy};
use uv_traits::{BuildContext, BuildIsolation, ConfigSettings, InFlight, NoBuild, SetupPyStrategy};

#[derive(Parser)]
pub(crate) struct ResolveManyArgs {
Expand Down Expand Up @@ -109,6 +109,7 @@ pub(crate) async fn resolve_many(args: ResolveManyArgs) -> Result<()> {
&in_flight,
setup_py,
&config_settings,
BuildIsolation::Isolated,
&no_build,
&NoBinary::None,
);
Expand Down
26 changes: 21 additions & 5 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use uv_client::{FlatIndex, RegistryClient};
use uv_installer::{Downloader, Installer, NoBinary, Plan, Planner, Reinstall, SitePackages};
use uv_interpreter::{Interpreter, PythonEnvironment};
use uv_resolver::{InMemoryIndex, Manifest, Options, Resolver};
use uv_traits::{BuildContext, BuildKind, ConfigSettings, InFlight, NoBuild, SetupPyStrategy};
use uv_traits::{
BuildContext, BuildIsolation, BuildKind, ConfigSettings, InFlight, NoBuild, SetupPyStrategy,
};

/// The main implementation of [`BuildContext`], used by the CLI, see [`BuildContext`]
/// documentation.
Expand All @@ -33,6 +35,7 @@ pub struct BuildDispatch<'a> {
index: &'a InMemoryIndex,
in_flight: &'a InFlight,
setup_py: SetupPyStrategy,
build_isolation: BuildIsolation<'a>,
no_build: &'a NoBuild,
no_binary: &'a NoBinary,
config_settings: &'a ConfigSettings,
Expand All @@ -53,6 +56,7 @@ impl<'a> BuildDispatch<'a> {
in_flight: &'a InFlight,
setup_py: SetupPyStrategy,
config_settings: &'a ConfigSettings,
build_isolation: BuildIsolation<'a>,
no_build: &'a NoBuild,
no_binary: &'a NoBinary,
) -> Self {
Expand All @@ -66,6 +70,7 @@ impl<'a> BuildDispatch<'a> {
in_flight,
setup_py,
config_settings,
build_isolation,
no_build,
no_binary,
source_build_context: SourceBuildContext::default(),
Expand Down Expand Up @@ -107,6 +112,10 @@ impl<'a> BuildContext for BuildDispatch<'a> {
self.interpreter
}

fn build_isolation(&self) -> BuildIsolation {
self.build_isolation
}

fn no_build(&self) -> &NoBuild {
self.no_build
}
Expand Down Expand Up @@ -180,7 +189,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
local,
remote,
reinstalls,
extraneous,
extraneous: _,
} = Planner::with_requirements(&resolution.requirements()).build(
site_packages,
&Reinstall::None,
Expand All @@ -191,6 +200,12 @@ impl<'a> BuildContext for BuildDispatch<'a> {
tags,
)?;

// Nothing to do.
if remote.is_empty() && local.is_empty() && reinstalls.is_empty() {
debug!("No build requirements to install for build");
return Ok(());
}

// Resolve any registry-based requirements.
let remote = remote
.iter()
Expand All @@ -207,7 +222,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
vec![]
} else {
// TODO(konstin): Check that there is no endless recursion.
let downloader = Downloader::new(self.cache(), tags, self.client, self);
let downloader = Downloader::new(self.cache, tags, self.client, self);
debug!(
"Downloading and building requirement{} for build: {}",
if remote.len() == 1 { "" } else { "s" },
Expand All @@ -221,8 +236,8 @@ impl<'a> BuildContext for BuildDispatch<'a> {
};

// Remove any unnecessary packages.
if !extraneous.is_empty() || !reinstalls.is_empty() {
for dist_info in extraneous.iter().chain(reinstalls.iter()) {
if !reinstalls.is_empty() {
for dist_info in &reinstalls {
let summary = uv_installer::uninstall(dist_info)
.await
.context("Failed to uninstall build dependencies")?;
Expand Down Expand Up @@ -295,6 +310,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
package_id.to_string(),
self.setup_py,
self.config_settings.clone(),
self.build_isolation,
build_kind,
self.build_extra_env_vars.clone(),
)
Expand Down
2 changes: 2 additions & 0 deletions crates/uv-distribution/src/source/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,8 @@ impl<'a, T: BuildContext> SourceDistCachedBuilder<'a, T> {
editable_wheel_dir: &Path,
) -> Result<(Dist, String, WheelFilename, Metadata21), Error> {
debug!("Building (editable) {editable}");

// Build the wheel.
let disk_filename = self
.build_context
.setup_build(
Expand Down
9 changes: 6 additions & 3 deletions crates/uv-interpreter/src/python_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,11 @@ impl PythonEnvironment {
}

/// Create a [`PythonEnvironment`] from an existing [`Interpreter`] and root directory.
pub fn from_interpreter(interpreter: Interpreter, root: PathBuf) -> Self {
Self { root, interpreter }
pub fn from_interpreter(interpreter: Interpreter) -> Self {
Self {
root: interpreter.prefix().to_path_buf(),
interpreter,
}
}

/// Returns the location of the Python interpreter.
Expand Down Expand Up @@ -100,7 +103,7 @@ impl PythonEnvironment {
self.interpreter.scripts()
}

/// Lock the virtual environment to prevent concurrent writes.
/// Grab a file lock for the virtual environment to prevent concurrent writes across processes.
pub fn lock(&self) -> Result<LockedFile, std::io::Error> {
if self.interpreter.is_virtualenv() {
// If the environment a virtualenv, use a virtualenv-specific lock file.
Expand Down
8 changes: 7 additions & 1 deletion crates/uv-resolver/tests/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ use uv_resolver::{
DisplayResolutionGraph, InMemoryIndex, Manifest, Options, OptionsBuilder, PreReleaseMode,
ResolutionGraph, ResolutionMode, Resolver,
};
use uv_traits::{BuildContext, BuildKind, NoBinary, NoBuild, SetupPyStrategy, SourceBuildTrait};
use uv_traits::{
BuildContext, BuildIsolation, BuildKind, NoBinary, NoBuild, SetupPyStrategy, SourceBuildTrait,
};

// Exclude any packages uploaded after this date.
static EXCLUDE_NEWER: Lazy<DateTime<Utc>> = Lazy::new(|| {
Expand Down Expand Up @@ -57,6 +59,10 @@ impl BuildContext for DummyContext {
&self.interpreter
}

fn build_isolation(&self) -> BuildIsolation {
BuildIsolation::Isolated
}

fn no_build(&self) -> &NoBuild {
&NoBuild::None
}
Expand Down
19 changes: 18 additions & 1 deletion crates/uv-traits/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,19 @@ use uv_normalize::PackageName;
/// `uv-build` to depend on `uv-resolver` which having actual crate dependencies between
/// them.

// TODO(konstin): Proper error types
pub trait BuildContext: Sync {
type SourceDistBuilder: SourceBuildTrait + Send + Sync;

/// Return a reference to the cache.
fn cache(&self) -> &Cache;

/// All (potentially nested) source distribution builds use the same base python and can reuse
/// it's metadata (e.g. wheel compatibility tags).
fn interpreter(&self) -> &Interpreter;

/// Whether to enforce build isolation when building source distributions.
fn build_isolation(&self) -> BuildIsolation;

/// Whether source distribution building is disabled. This [`BuildContext::setup_build`] calls
/// will fail in this case. This method exists to avoid fetching source distributions if we know
/// we can't build them
Expand Down Expand Up @@ -137,6 +140,20 @@ pub struct InFlight {
pub downloads: OnceMap<DistributionId, Result<CachedDist, String>>,
}

/// Whether to enforce build isolation when building source distributions.
#[derive(Debug, Copy, Clone)]
pub enum BuildIsolation<'a> {
Isolated,
Shared(&'a PythonEnvironment),
}

impl<'a> BuildIsolation<'a> {
/// Returns `true` if build isolation is enforced.
pub fn is_isolated(&self) -> bool {
matches!(self, Self::Isolated)
}
}

/// The strategy to use when building source distributions that lack a `pyproject.toml`.
#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)]
pub enum SetupPyStrategy {
Expand Down
3 changes: 1 addition & 2 deletions crates/uv-virtualenv/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,5 @@ pub fn create_venv(

// Create the corresponding `PythonEnvironment`.
let interpreter = interpreter.with_virtualenv(virtualenv);
let root = interpreter.prefix().to_path_buf();
Ok(PythonEnvironment::from_interpreter(interpreter, root))
Ok(PythonEnvironment::from_interpreter(interpreter))
}
Loading

0 comments on commit 5ae5980

Please sign in to comment.