Use correct lock path for workspace dependencies (#4421)

Previously, distributions created through `Source::Workspace` would have
the absolute path as lock path. This didn't cause any problems, since in
`Urls` we would later overwrite those urls with the correct one created
from being workspace members by path.

Changing the order surfaced this. This change emits the correct lock
path. I've manually checked the difference with `dbg!`, this is not
observable on main, but on the diverging urls branch it fixes lockfile
creation.
This commit is contained in:
konsti 2024-06-20 15:28:47 +02:00 committed by GitHub
parent fc7c318dd0
commit b865341517
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 48 additions and 71 deletions

View file

@ -208,12 +208,17 @@ pub(crate) fn lower_requirement(
.get(&requirement.name)
.ok_or(LoweringError::UndeclaredWorkspacePackage)?
.clone();
directory_source(
path.root(),
workspace.root(),
workspace.root(),
editable.unwrap_or(true),
)?
// The lockfile is relative to the workspace root.
let relative_to_workspace =
relative_to(path.root(), workspace.root()).map_err(LoweringError::RelativeTo)?;
let url = VerbatimUrl::parse_absolute_path(path.root())?
.with_given(relative_to_workspace.to_string_lossy());
RequirementSource::Directory {
install_path: path.root().clone(),
lock_path: relative_to_workspace,
url,
editable: editable.unwrap_or(true),
}
}
Source::CatchAll { .. } => {
// Emit a dedicated error message, which is an improvement over Serde's default error.
@ -274,33 +279,3 @@ fn path_source(
})
}
}
/// Convert a path string to a directory source.
fn directory_source(
path: impl AsRef<Path>,
project_dir: &Path,
workspace_root: &Path,
editable: bool,
) -> Result<RequirementSource, LoweringError> {
let path = path.as_ref();
let url = VerbatimUrl::parse_path(path, project_dir)?.with_given(path.to_string_lossy());
let absolute_path = path
.to_path_buf()
.absolutize_from(project_dir)
.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)?
} else {
// If the user gave us an absolute path, we respect that.
path.to_path_buf()
};
Ok(RequirementSource::Directory {
install_path: absolute_path,
lock_path: relative_to_workspace,
url,
editable,
})
}

View file

@ -332,8 +332,9 @@ pub fn relative_to(path: impl AsRef<Path>, base: impl AsRef<Path>) -> Result<Pat
.as_ref()
.ancestors()
.find_map(|ancestor| {
path.as_ref()
.strip_prefix(ancestor)
// Simplifying removes the UNC path prefix on windows.
dunce::simplified(path.as_ref())
.strip_prefix(dunce::simplified(ancestor))
.ok()
.map(|stripped| (stripped, ancestor))
})
@ -342,8 +343,8 @@ pub fn relative_to(path: impl AsRef<Path>, base: impl AsRef<Path>) -> Result<Pat
io::ErrorKind::Other,
format!(
"Trivial strip failed: {} vs. {}",
path.simplified_display(),
base.simplified_display()
path.as_ref().simplified_display(),
base.as_ref().simplified_display()
),
)
})?;

View file

@ -100,39 +100,40 @@ impl Manifest {
) -> impl Iterator<Item = &Requirement> + 'a {
match mode {
// Include all direct and transitive requirements, with constraints and overrides applied.
DependencyMode::Transitive => Either::Left( self
.lookaheads
.iter()
.flat_map(move |lookahead| {
self.overrides
.apply(lookahead.requirements())
.filter(move |requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
})
.chain(
self.overrides
.apply(&self.requirements)
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.constraints
.requirements()
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.overrides
.requirements()
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
))
,
DependencyMode::Transitive => Either::Left(
self.lookaheads
.iter()
.flat_map(move |lookahead| {
self.overrides
.apply(lookahead.requirements())
.filter(move |requirement| {
requirement.evaluate_markers(markers, lookahead.extras())
})
})
.chain(
self.overrides
.apply(&self.requirements)
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.constraints
.requirements()
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
)
.chain(
self.overrides
.requirements()
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
),
),
// Include direct requirements, with constraints and overrides applied.
DependencyMode::Direct => Either::Right(
self.overrides.apply(&self.requirements)
.chain(self.constraints.requirements())
.chain(self.overrides.requirements())
.filter(move |requirement| requirement.evaluate_markers(markers, &[]))),
self.overrides
.apply(&self.requirements)
.chain(self.constraints.requirements())
.chain(self.overrides.requirements())
.filter(move |requirement| requirement.evaluate_markers(markers, &[])),
),
}
}