Skip to content
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

Support locks path deps with path deps #6438

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/bench/benches/uv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ mod resolver {
&hashes,
&build_context,
installed_packages,
DistributionDatabase::new(client, &build_context, concurrency.downloads),
DistributionDatabase::new(client, &build_context, None, concurrency.downloads),
)?;

Ok(resolver.resolve().await?)
Expand Down
16 changes: 14 additions & 2 deletions crates/uv-dispatch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,13 @@ impl<'a> BuildContext for BuildDispatch<'a> {
&HashStrategy::None,
self,
EmptyInstalledPackages,
DistributionDatabase::new(self.client, self, self.concurrency.downloads),
DistributionDatabase::new(
self.client,
self,
// Not needed, there's no `uv.lock`.
None,
self.concurrency.downloads,
),
)?;
let graph = resolver.resolve().await.with_context(|| {
format!(
Expand Down Expand Up @@ -238,7 +244,13 @@ impl<'a> BuildContext for BuildDispatch<'a> {
tags,
&HashStrategy::None,
self.build_options,
DistributionDatabase::new(self.client, self, self.concurrency.downloads),
DistributionDatabase::new(
self.client,
self,
// Not needed, there's no `uv.lock`.
None,
self.concurrency.downloads,
),
);

debug!(
Expand Down
18 changes: 15 additions & 3 deletions crates/uv-distribution/src/distribution_database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,12 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
pub fn new(
client: &'a RegistryClient,
build_context: &'a Context,
main_workspace_root: Option<&Path>,
concurrent_downloads: usize,
) -> Self {
Self {
build_context,
builder: SourceDistributionBuilder::new(build_context),
builder: SourceDistributionBuilder::new(build_context, main_workspace_root),
locks: Rc::new(Locks::default()),
client: ManagedClient::new(client, concurrent_downloads),
reporter: None,
Expand All @@ -81,6 +82,11 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
}
}

/// The directory containing `uv.lock`, used to compute relative paths in lockfiles.
pub fn main_workspace_root(&self) -> Option<&Path> {
self.builder.main_workspace_root()
}

/// Handle a specific `reqwest` error, and convert it to [`io::Error`].
fn handle_response_errors(&self, err: reqwest::Error) -> io::Error {
if err.is_timeout() {
Expand Down Expand Up @@ -495,8 +501,14 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
}

/// Return the [`RequiresDist`] from a `pyproject.toml`, if it can be statically extracted.
pub async fn requires_dist(&self, project_root: &Path) -> Result<RequiresDist, Error> {
self.builder.requires_dist(project_root).await
pub async fn requires_dist(
&self,
project_root: &Path,
main_workspace_root: Option<&Path>,
) -> Result<RequiresDist, Error> {
self.builder
.requires_dist(project_root, main_workspace_root)
.await
}

/// Stream a wheel from a URL, unzipping it into the cache as it's downloaded.
Expand Down
37 changes: 30 additions & 7 deletions crates/uv-distribution/src/metadata/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use pep440_rs::VersionSpecifiers;
use pep508_rs::{VerbatimUrl, VersionOrUrl};
use pypi_types::{ParsedUrlError, Requirement, RequirementSource, VerbatimParsedUrl};
use thiserror::Error;
use tracing::trace;
use url::Url;
use uv_fs::{relative_to, Simplified};
use uv_git::GitReference;
Expand Down Expand Up @@ -35,6 +36,7 @@ impl LoweredRequirement {
project_dir: &Path,
project_sources: &BTreeMap<PackageName, Source>,
workspace: &Workspace,
main_workspace_root: Option<&Path>,
) -> Result<Self, LoweringError> {
let (source, origin) = if let Some(source) = project_sources.get(&requirement.name) {
(Some(source), Origin::Project)
Expand Down Expand Up @@ -106,6 +108,7 @@ impl LoweredRequirement {
origin,
project_dir,
workspace.install_path(),
main_workspace_root,
editable.unwrap_or(false),
)?
}
Expand Down Expand Up @@ -204,7 +207,15 @@ impl LoweredRequirement {
if matches!(requirement.version_or_url, Some(VersionOrUrl::Url(_))) {
return Err(LoweringError::ConflictingUrls);
}
path_source(path, Origin::Project, dir, dir, editable.unwrap_or(false))?
path_source(
path,
Origin::Project,
dir,
dir,
// Currently unused, but there's no need to discard this information.
Some(dir),
editable.unwrap_or(false),
)?
}
Source::Registry { index } => registry_source(&requirement, index)?,
Source::Workspace { .. } => {
Expand Down Expand Up @@ -352,9 +363,16 @@ fn path_source(
origin: Origin,
project_dir: &Path,
workspace_root: &Path,
main_workspace_root: Option<&Path>,
editable: bool,
) -> Result<RequirementSource, LoweringError> {
let path = path.as_ref();
trace!(
"Adding `{}` with project dir `{}` and workspace dir `{}`",
path.display(),
project_dir.display(),
workspace_root.display(),
);
let base = match origin {
Origin::Project => project_dir,
Origin::Workspace => workspace_root,
Expand All @@ -365,13 +383,18 @@ fn path_source(
.absolutize_from(base)
.map_err(|err| LoweringError::Absolutize(path.to_path_buf(), err))?
.to_path_buf();
let relative_to_workspace = if path.is_relative() {
// Relative paths in a project are relative to the project root, but the lockfile is
// relative to the workspace root.
relative_to(&absolute_path, workspace_root).map_err(LoweringError::RelativeTo)?
let relative_to_workspace = if let Some(main_workspace_root) = main_workspace_root {
if path.is_relative() {
// Relative paths in a project are relative to the project root, but path in `uv.lock`
// are relative to the root of the main workspace, which contains the lockfile.
relative_to(&absolute_path, main_workspace_root).map_err(LoweringError::RelativeTo)?
} else {
// If the user gave us an absolute path, we respect that.
path.to_path_buf()
}
} else {
// If the user gave us an absolute path, we respect that.
path.to_path_buf()
// If we don't have `uv.lock`, we don't need relative paths for locking.
absolute_path.clone()
};
let is_dir = if let Ok(metadata) = absolute_path.metadata() {
metadata.is_dir()
Expand Down
2 changes: 2 additions & 0 deletions crates/uv-distribution/src/metadata/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ impl Metadata {
metadata: Metadata23,
install_path: &Path,
lock_path: &Path,
main_workspace_root: Option<&Path>,
sources: SourceStrategy,
) -> Result<Self, MetadataError> {
// Lower the requirements.
Expand All @@ -76,6 +77,7 @@ impl Metadata {
},
install_path,
lock_path,
main_workspace_root,
sources,
)
.await?;
Expand Down
7 changes: 6 additions & 1 deletion crates/uv-distribution/src/metadata/requires_dist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ impl RequiresDist {
metadata: pypi_types::RequiresDist,
install_path: &Path,
lock_path: &Path,
main_workspace_root: Option<&Path>,
sources: SourceStrategy,
) -> Result<Self, MetadataError> {
match sources {
Expand All @@ -54,7 +55,7 @@ impl RequiresDist {
return Ok(Self::from_metadata23(metadata));
};

Self::from_project_workspace(metadata, &project_workspace)
Self::from_project_workspace(metadata, &project_workspace, main_workspace_root)
}
SourceStrategy::Disabled => Ok(Self::from_metadata23(metadata)),
}
Expand All @@ -63,6 +64,7 @@ impl RequiresDist {
fn from_project_workspace(
metadata: pypi_types::RequiresDist,
project_workspace: &ProjectWorkspace,
main_workspace_root: Option<&Path>,
) -> Result<Self, MetadataError> {
// Collect any `tool.uv.sources` and `tool.uv.dev_dependencies` from `pyproject.toml`.
let empty = BTreeMap::default();
Expand Down Expand Up @@ -94,6 +96,7 @@ impl RequiresDist {
project_workspace.project_root(),
sources,
project_workspace.workspace(),
main_workspace_root,
)
.map(LoweredRequirement::into_inner)
.map_err(|err| MetadataError::LoweringError(requirement_name.clone(), err))
Expand All @@ -117,6 +120,7 @@ impl RequiresDist {
project_workspace.project_root(),
sources,
project_workspace.workspace(),
main_workspace_root,
)
.map(LoweredRequirement::into_inner)
.map_err(|err| MetadataError::LoweringError(requirement_name.clone(), err))
Expand Down Expand Up @@ -177,6 +181,7 @@ mod test {
Ok(RequiresDist::from_project_workspace(
requires_dist,
&project_workspace,
None,
)?)
}

Expand Down
53 changes: 45 additions & 8 deletions crates/uv-distribution/src/source/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ mod revision;
/// Fetch and build a source distribution from a remote source, or from a local cache.
pub(crate) struct SourceDistributionBuilder<'a, T: BuildContext> {
build_context: &'a T,
/// The directory containing `uv.lock`, used to compute relative paths in lockfiles.
main_workspace_root: Option<PathBuf>,
reporter: Option<Arc<dyn Reporter>>,
}

Expand All @@ -60,9 +62,10 @@ pub(crate) const METADATA: &str = "metadata.msgpack";

impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
/// Initialize a [`SourceDistributionBuilder`] from a [`BuildContext`].
pub(crate) fn new(build_context: &'a T) -> Self {
pub(crate) fn new(build_context: &'a T, main_workspace_root: Option<&Path>) -> Self {
Self {
build_context,
main_workspace_root: main_workspace_root.map(Path::to_path_buf),
reporter: None,
}
}
Expand Down Expand Up @@ -420,12 +423,18 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
}

/// Return the [`RequiresDist`] from a `pyproject.toml`, if it can be statically extracted.
pub(crate) async fn requires_dist(&self, project_root: &Path) -> Result<RequiresDist, Error> {

pub(crate) async fn requires_dist(
&self,
project_root: &Path,
main_workspace_root: Option<&Path>,
) -> Result<RequiresDist, Error> {
let requires_dist = read_requires_dist(project_root).await?;
let requires_dist = RequiresDist::from_project_maybe_workspace(
requires_dist,
project_root,
project_root,
main_workspace_root,
self.build_context.sources(),
)
.await?;
Expand Down Expand Up @@ -1010,6 +1019,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
metadata,
resource.install_path.as_ref(),
resource.lock_path.as_ref(),
self.main_workspace_root(),
self.build_context.sources(),
)
.await?,
Expand All @@ -1025,6 +1035,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
metadata,
resource.install_path.as_ref(),
resource.lock_path.as_ref(),
self.main_workspace_root(),
self.build_context.sources(),
)
.await?,
Expand All @@ -1050,6 +1061,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
metadata,
resource.install_path.as_ref(),
resource.lock_path.as_ref(),
self.main_workspace_root(),
self.build_context.sources(),
)
.await?,
Expand Down Expand Up @@ -1082,6 +1094,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
metadata,
resource.install_path.as_ref(),
resource.lock_path.as_ref(),
self.main_workspace_root(),
self.build_context.sources(),
)
.await?,
Expand Down Expand Up @@ -1252,8 +1265,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
Self::read_static_metadata(source, fetch.path(), resource.subdirectory).await?
{
return Ok(ArchiveMetadata::from(
Metadata::from_workspace(metadata, &path, &path, self.build_context.sources())
.await?,
Metadata::from_workspace(
metadata,
&path,
&path,
self.main_workspace_root(),
self.build_context.sources(),
)
.await?,
));
}

Expand All @@ -1276,8 +1295,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {

debug!("Using cached metadata for: {source}");
return Ok(ArchiveMetadata::from(
Metadata::from_workspace(metadata, &path, &path, self.build_context.sources())
.await?,
Metadata::from_workspace(
metadata,
&path,
&path,
self.main_workspace_root(),
self.build_context.sources(),
)
.await?,
));
}
}
Expand All @@ -1297,8 +1322,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
.map_err(Error::CacheWrite)?;

return Ok(ArchiveMetadata::from(
Metadata::from_workspace(metadata, &path, &path, self.build_context.sources())
.await?,
Metadata::from_workspace(
metadata,
&path,
&path,
self.main_workspace_root(),
self.build_context.sources(),
)
.await?,
));
}

Expand Down Expand Up @@ -1328,6 +1359,7 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
metadata,
fetch.path(),
fetch.path(),
self.main_workspace_root(),
self.build_context.sources(),
)
.await?,
Expand Down Expand Up @@ -1621,6 +1653,11 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
)
.build()
}

/// The directory containing `uv.lock`, used to compute relative paths in lockfiles.
pub(crate) fn main_workspace_root(&self) -> Option<&Path> {
self.main_workspace_root.as_deref()
}
}

/// Validate that the source distribution matches the built metadata.
Expand Down
6 changes: 5 additions & 1 deletion crates/uv-requirements/src/source_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,11 @@ impl<'a, Context: BuildContext> SourceTreeResolver<'a, Context> {
})?;

// If the path is a `pyproject.toml`, attempt to extract the requirements statically.
if let Ok(metadata) = self.database.requires_dist(source_tree).await {
if let Ok(metadata) = self
.database
.requires_dist(source_tree, self.database.main_workspace_root())
.await
{
return Ok(metadata);
}

Expand Down
Loading
Loading