-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Build backend: Add direct builds to the resolver and installer #9621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
7fa893d
f07da1d
a5705e3
87823cf
55e6bf4
3bfb0b3
cb7766b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,8 +9,8 @@ use anyhow::{anyhow, Context, Result}; | |
use futures::FutureExt; | ||
use itertools::Itertools; | ||
use rustc_hash::FxHashMap; | ||
use tracing::{debug, instrument}; | ||
|
||
use tracing::{debug, instrument, trace}; | ||
use uv_build_backend::check_direct_build; | ||
use uv_build_frontend::{SourceBuild, SourceBuildContext}; | ||
use uv_cache::Cache; | ||
use uv_client::RegistryClient; | ||
|
@@ -397,6 +397,67 @@ impl<'a> BuildContext for BuildDispatch<'a> { | |
.await?; | ||
Ok(builder) | ||
} | ||
|
||
async fn direct_build<'data>( | ||
&'data self, | ||
source: &'data Path, | ||
subdirectory: Option<&'data Path>, | ||
output_dir: &'data Path, | ||
build_kind: BuildKind, | ||
version_id: Option<String>, | ||
) -> Result<Option<String>> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is being returned here? And should it be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh okay, it's documented below on the trait. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This initially happened because PEP 517 says that the build backend returns the filename as last line, and we passed that around as a string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More specifically, the location on disk may not be normalized the same as |
||
// Direct builds are a preview feature with the uv build backend. | ||
if self.preview.is_disabled() { | ||
trace!("Preview is disabled, not checking for direct build"); | ||
return Ok(None); | ||
} | ||
|
||
let source_tree = if let Some(subdir) = subdirectory { | ||
source.join(subdir) | ||
} else { | ||
source.to_path_buf() | ||
}; | ||
|
||
// Only perform the direct build if the backend is uv in a compatible version. | ||
let identifier = version_id.unwrap_or_else(|| source_tree.display().to_string()); | ||
if !check_direct_build(&source_tree, &identifier) { | ||
trace!("Requirements for direct build not matched: {identifier}"); | ||
return Ok(None); | ||
} | ||
|
||
debug!("Performing direct build for {identifier}"); | ||
|
||
let output_dir = output_dir.to_path_buf(); | ||
let filename = tokio::task::spawn_blocking(move || -> Result<String> { | ||
let filename = match build_kind { | ||
BuildKind::Wheel => uv_build_backend::build_wheel( | ||
&source_tree, | ||
&output_dir, | ||
None, | ||
uv_version::version(), | ||
)? | ||
.to_string(), | ||
BuildKind::Sdist => uv_build_backend::build_source_dist( | ||
&source_tree, | ||
&output_dir, | ||
uv_version::version(), | ||
)? | ||
.to_string(), | ||
BuildKind::Editable => uv_build_backend::build_editable( | ||
&source_tree, | ||
&output_dir, | ||
None, | ||
uv_version::version(), | ||
)? | ||
.to_string(), | ||
}; | ||
Ok(filename) | ||
}) | ||
.await?? | ||
.to_string(); | ||
|
||
Ok(Some(filename)) | ||
} | ||
} | ||
|
||
/// Shared state used during resolution and installation. | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,6 +121,21 @@ pub trait BuildContext { | |
build_kind: BuildKind, | ||
build_output: BuildOutput, | ||
) -> impl Future<Output = Result<Self::SourceDistBuilder>> + 'a; | ||
|
||
/// Build by calling directly into the uv build backend without PEP 517, if possible. | ||
/// | ||
/// Checks if the source tree uses uv as build backend. If not, it returns `Ok(None)`, otherwise | ||
/// it builds and returns the name of the built file. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What does it return in the case of an editable? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At this stage, the editable is a wheel that is indistinguishable from a regular wheel, except it contains a |
||
/// | ||
/// `version_id` is for error reporting only. | ||
fn direct_build<'a>( | ||
&'a self, | ||
source: &'a Path, | ||
subdirectory: Option<&'a Path>, | ||
output_dir: &'a Path, | ||
build_kind: BuildKind, | ||
version_id: Option<String>, | ||
) -> impl Future<Output = Result<Option<String>>> + 'a; | ||
} | ||
|
||
/// A wrapper for `uv_build::SourceBuild` to avoid cyclical crate dependencies. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a
Option<&str>
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. It would be kinda annoying, but doable. I don't feel strongly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more converting back and forth now, but it's also move consistent with the rest of the codebase.