From b86534151714cd08c781c355b00d4ad858c6d3f1 Mon Sep 17 00:00:00 2001
From: konsti <konstin@mailbox.org>
Date: Thu, 20 Jun 2024 15:28:47 +0200
Subject: [PATCH] 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.
---
 .../uv-distribution/src/metadata/lowering.rs  | 47 ++++----------
 crates/uv-fs/src/path.rs                      |  9 +--
 crates/uv-resolver/src/manifest.rs            | 63 ++++++++++---------
 3 files changed, 48 insertions(+), 71 deletions(-)

diff --git a/crates/uv-distribution/src/metadata/lowering.rs b/crates/uv-distribution/src/metadata/lowering.rs
index 8c35f16bb..bd8f69971 100644
--- a/crates/uv-distribution/src/metadata/lowering.rs
+++ b/crates/uv-distribution/src/metadata/lowering.rs
@@ -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,
-    })
-}
diff --git a/crates/uv-fs/src/path.rs b/crates/uv-fs/src/path.rs
index 0c361915b..2a48259f6 100644
--- a/crates/uv-fs/src/path.rs
+++ b/crates/uv-fs/src/path.rs
@@ -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()
                 ),
             )
         })?;
diff --git a/crates/uv-resolver/src/manifest.rs b/crates/uv-resolver/src/manifest.rs
index 1952e0273..22ae0df8b 100644
--- a/crates/uv-resolver/src/manifest.rs
+++ b/crates/uv-resolver/src/manifest.rs
@@ -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, &[])),
+            ),
         }
     }