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.

Sign up for GitHub

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

Jump to bottom

Avoid using workspace lock_path as relative root #6157

Merged
merged 1 commit into from
Aug 16, 2024
+80 −47
Merged

Avoid using workspace lock_path as relative root #6157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
90 changes: 59 additions & 31 deletions crates/uv-resolver/src/lock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -657,14 +657,14 @@ impl Lock {
.iter()
.cloned()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
.collect::<Result<_, _>>()?;
let actual: BTreeSet<_> = self
.manifest
.constraints
.iter()
.cloned()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
.collect::<Result<_, _>>()?;
if expected != actual {
debug!(
"Mismatched constraints:\n expected: {:?}\n found: {:?}",
Expand All @@ -680,14 +680,14 @@ impl Lock {
.iter()
.cloned()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
.collect::<Result<_, _>>()?;
let actual: BTreeSet<_> = self
.manifest
.overrides
.iter()
.cloned()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
.collect::<Result<_, _>>()?;
if expected != actual {
debug!(
"Mismatched overrides:\n expected: {:?}\n found: {:?}",
Expand Down Expand Up @@ -764,14 +764,14 @@ impl Lock {
.requires_dist
.into_iter()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
.collect::<Result<_, _>>()?;
let actual: BTreeSet<_> = package
.metadata
.requires_dist
.iter()
.cloned()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect();
.collect::<Result<_, _>>()?;

if expected != actual {
debug!(
Expand All @@ -794,30 +794,30 @@ impl Lock {
.dev_dependencies
.into_iter()
.map(|(group, requirements)| {
(
Ok::<_, LockError>((
group,
requirements
.into_iter()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect(),
)
.collect::<Result<_, _>>()?,
))
})
.collect();
.collect::<Result<_, _>>()?;
let actual: BTreeMap<GroupName, BTreeSet<Requirement>> = package
.metadata
.requires_dev
.iter()
.map(|(group, requirements)| {
(
Ok::<_, LockError>((
group.clone(),
requirements
.iter()
.cloned()
.map(|requirement| normalize_requirement(requirement, workspace))
.collect(),
)
.collect::<Result<_, _>>()?,
))
})
.collect();
.collect::<Result<_, _>>()?;

if expected != actual {
debug!(
Expand Down Expand Up @@ -2735,7 +2735,10 @@ fn normalize_url(mut url: Url) -> UrlString {
/// 2. Ensures that the lock and install paths are appropriately framed with respect to the
/// current [`Workspace`].
/// 3. Removes the `origin` field, which is only used in `requirements.txt`.
fn normalize_requirement(requirement: Requirement, workspace: &Workspace) -> Requirement {
fn normalize_requirement(
requirement: Requirement,
workspace: &Workspace,
) -> Result<Requirement, LockError> {
match requirement.source {
RequirementSource::Git {
mut repository,
Expand All @@ -2754,7 +2757,7 @@ fn normalize_requirement(requirement: Requirement, workspace: &Workspace) -> Req
let _ = url.set_username("");
let url = VerbatimUrl::from_url(url);

Requirement {
Ok(Requirement {
name: requirement.name,
extras: requirement.extras,
marker: requirement.marker,
Expand All @@ -2766,18 +2769,26 @@ fn normalize_requirement(requirement: Requirement, workspace: &Workspace) -> Req
url,
},
origin: None,
}
})
}
RequirementSource::Path {
install_path,
install_path: _,
lock_path,
ext,
url: _,
} => {
let install_path = uv_fs::normalize_path(&workspace.install_path().join(install_path));
let lock_path = relative_to(workspace.lock_path(), &lock_path).unwrap();
let url = VerbatimUrl::from_path(&install_path).unwrap();
Requirement {
// When a path requirement comes from the lockfile, `install_path` and `lock_path` are
// both relative to the lockfile.
//
// When a path requirement is deserialized from package metadata, `install_path` is
// absolute, and `lock_path` is relative to the lockfile.
let install_path = uv_fs::normalize_path(&workspace.install_path().join(&lock_path));
let lock_path = relative_to(workspace.install_path(), &lock_path)
.map_err(LockErrorKind::RequirementRelativePath)?;
let url = VerbatimUrl::from_path(&install_path)
.map_err(LockErrorKind::RequirementVerbatimUrl)?;

Ok(Requirement {
name: requirement.name,
extras: requirement.extras,
marker: requirement.marker,
Expand All @@ -2788,18 +2799,21 @@ fn normalize_requirement(requirement: Requirement, workspace: &Workspace) -> Req
url,
},
origin: None,
}
})
}
RequirementSource::Directory {
install_path,
install_path: _,
lock_path,
editable,
url: _,
} => {
let install_path = uv_fs::normalize_path(&workspace.install_path().join(install_path));
let lock_path = relative_to(workspace.lock_path(), &lock_path).unwrap();
let url = VerbatimUrl::from_path(&install_path).unwrap();
Requirement {
let install_path = uv_fs::normalize_path(&workspace.install_path().join(&lock_path));
let lock_path = relative_to(workspace.install_path(), &lock_path)
.map_err(LockErrorKind::RequirementRelativePath)?;
let url = VerbatimUrl::from_path(&install_path)
.map_err(LockErrorKind::RequirementVerbatimUrl)?;

Ok(Requirement {
name: requirement.name,
extras: requirement.extras,
marker: requirement.marker,
Expand All @@ -2810,15 +2824,15 @@ fn normalize_requirement(requirement: Requirement, workspace: &Workspace) -> Req
url,
},
origin: None,
}
})
}
_ => Requirement {
_ => Ok(Requirement {
name: requirement.name,
extras: requirement.extras,
marker: requirement.marker,
source: requirement.source,
origin: None,
},
}),
}
}

Expand Down Expand Up @@ -3012,6 +3026,20 @@ enum LockErrorKind {
/// The name of the dependency that is missing a `source` field.
name: PackageName,
},
/// An error that occurs when parsing an existing requirement.
#[error("could not compute relative path between workspace and requirement")]
RequirementRelativePath(
/// The inner error we forward.
#[source]
std::io::Error,
),
/// An error that occurs when parsing an existing requirement.
#[error("could not convert between URL and path")]
RequirementVerbatimUrl(
/// The inner error we forward.
#[source]
VerbatimUrlError,
),
}

/// An error that occurs when a source string could not be parsed.
Expand Down
37 changes: 21 additions & 16 deletions crates/uv/src/commands/project/lock.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,23 +404,28 @@ async fn do_lock(

// If any of the resolution-determining settings changed, invalidate the lock.
let existing_lock = if let Some(existing_lock) = existing_lock {
Some(
ValidatedLock::validate(
existing_lock,
workspace,
&members,
&constraints,
&overrides,
interpreter,
&requires_python,
index_locations,
upgrade,
&options,
&database,
printer,
)
.await?,
match ValidatedLock::validate(
existing_lock,
workspace,
&members,
&constraints,
&overrides,
interpreter,
&requires_python,
index_locations,
upgrade,
&options,
&database,
printer,
)
.await
{
Ok(result) => Some(result),
Err(err) => {
warn_user!("Failed to validate existing lockfile: {err}");
None
}
}
} else {
None
};
Expand Down
Loading