fix: don't keep reinstalling local pypi archives

src/install_pypi/plan/test/harness.rs

@@ -356,12 +356,7 @@ impl PyPIPackageDataBuilder {
         }
     }
 
-    fn directory<S: AsRef<str>>(
-        name: S,
-        version: S,
-        path: PathBuf,
-        editable: bool,
-    ) -> PypiPackageData {
+    fn path<S: AsRef<str>>(name: S, version: S, path: PathBuf, editable: bool) -> PypiPackageData {
         PypiPackageData {
             name: pep508_rs::PackageName::new(name.as_ref().to_owned()).unwrap(),
             version: pep440_rs::Version::from_str(version.as_ref()).unwrap(),
@@ -456,7 +451,15 @@ impl RequiredPackages {
     ) -> Self {
         let package_name = uv_normalize::PackageName::from_owned(name.as_ref().to_owned())
             .expect("should be correct");
-        let data = PyPIPackageDataBuilder::directory(name, version, path, editable);
+        let data = PyPIPackageDataBuilder::path(name, version, path, editable);
+        self.required.insert(package_name, data);
+        self
+    }
+
+    pub fn add_local_wheel<S: AsRef<str>>(mut self, name: S, version: S, path: PathBuf) -> Self {
+        let package_name = uv_normalize::PackageName::from_owned(name.as_ref().to_owned())
+            .expect("should be correct");
+        let data = PyPIPackageDataBuilder::path(name, version, path, false);
         self.required.insert(package_name, data);
         self
     }
@@ -489,6 +492,10 @@ pub fn install_planner() -> InstallPlanner {
     InstallPlanner::new(uv_cache::Cache::temp().unwrap(), PathBuf::new())
 }
 
+pub fn install_planner_with_lock_dir(lock_dir: PathBuf) -> InstallPlanner {
+    InstallPlanner::new(uv_cache::Cache::temp().unwrap(), lock_dir)
+}
+
 /// Create a fake pyproject.toml file in a temp dir
 /// return the temp dir
 pub fn fake_pyproject_toml(
@@ -514,3 +521,10 @@ pub fn fake_pyproject_toml(
     }
     (temp_dir, pyproject_toml)
 }
+
+pub fn fake_wheel(name: &str) -> (TempDir, std::fs::File, PathBuf) {
+    let temp_dir = tempfile::tempdir().unwrap();
+    let wheel_path = temp_dir.path().join(format!("{}.whl", name));
+    let wheel = std::fs::File::create(wheel_path.clone()).unwrap();
+    (temp_dir, wheel, wheel_path)
+}

src/install_pypi/plan/test/mod.rs

@@ -2,6 +2,8 @@ use self::harness::{InstalledDistOptions, MockedSitePackages, NoCache, RequiredP
 use crate::install_pypi::plan::test::harness::AllCached;
 use crate::install_pypi::NeedReinstall;
 use assert_matches::assert_matches;
+use harness::fake_wheel;
+use std::path::PathBuf;
 use std::str::FromStr;
 use url::Url;
 
@@ -614,3 +616,38 @@ fn test_uv_refresh() {
     assert!(installs.local.is_empty());
     assert_eq!(installs.remote.len(), 1);
 }
+
+/// Test when we have locked the dependency as a path that we do not re-install when all
+/// data is the same.
+/// this was a bug that occurred that when having a dependency like
+/// ```
+/// [pypi-dependencies]
+/// foobar = { path = "./foobar-0.1.0-py3-none-any.whl" }
+/// ```
+/// we would keep reinstalling foobar
+#[test]
+fn test_archive_is_path() {
+    let (tmp, _file, wheel_path) = fake_wheel("foobar");
+    // This needs to be absolute otherwise we cannot parse it into a file url
+    let site_packages = MockedSitePackages::new().add_archive(
+        "foobar",
+        "0.1.0",
+        Url::from_file_path(&wheel_path).unwrap(),
+        InstalledDistOptions::default(),
+    );
+
+    // Requires following package
+    let required = RequiredPackages::new().add_local_wheel(
+        "foobar",
+        "0.1.0",
+        PathBuf::from("./some-dir/../foobar.whl"),
+    );
+    let plan = harness::install_planner_with_lock_dir(tmp.path().to_path_buf());
+    let installs = plan
+        .plan_install(&site_packages, AllCached, &required.to_borrowed())
+        .expect("should install");
+    // Should not install package
+    assert!(installs.reinstalls.is_empty());
+    assert!(installs.local.is_empty());
+    assert!(installs.remote.is_empty());
+}

src/install_pypi/plan/validation.rs

@@ -18,13 +18,13 @@ pub enum NeedsReinstallError {
     #[error(transparent)]
     UvConversion(#[from] ConversionError),
     #[error(transparent)]
-    Io(#[from] std::io::Error),
-    #[error(transparent)]
     Conversion(#[from] url::ParseError),
     #[error(transparent)]
     ParsedUrl(#[from] ParsedUrlError),
-    #[error("Error converting to parsed git url {0}")]
+    #[error("error converting to parsed git url {0}")]
     PixiGitUrl(String),
+    #[error("while checking freshness {0}")]
+    FreshnessError(std::io::Error),
 }
 
 /// Check if a package needs to be reinstalled
@@ -111,7 +111,9 @@ pub(crate) fn need_reinstall(
                             if url == locked_url {
                                 // Okay so these are the same, but we need to check if the cache is newer
                                 // than the source directory
-                                if !check_url_freshness(&url, installed)? {
+                                if !check_url_freshness(&url, installed)
+                                    .map_err(NeedsReinstallError::FreshnessError)?
+                                {
                                     return Ok(ValidateCurrentInstall::Reinstall(
                                         NeedReinstall::SourceDirectoryNewerThanCache,
                                     ));
@@ -148,14 +150,31 @@ pub(crate) fn need_reinstall(
                     // Subdirectory is either in the url or not supported
                     subdirectory: _,
                 } => {
+                    let lock_file_dir = typed_path::Utf8TypedPathBuf::from(
+                        lock_file_dir.to_string_lossy().as_ref(),
+                    );
                     let locked_url = match &locked.location {
                         // Remove `direct+` scheme if it is there so we can compare the required to
                         // the installed url
-                        UrlOrPath::Url(url) => strip_direct_scheme(url),
-                        UrlOrPath::Path(_path) => {
-                            return Ok(ValidateCurrentInstall::Reinstall(
-                                NeedReinstall::GitArchiveIsPath,
-                            ))
+                        UrlOrPath::Url(url) => strip_direct_scheme(url).into_owned(),
+                        UrlOrPath::Path(path) => {
+                            let path = if path.is_absolute() {
+                                path.clone()
+                            } else {
+                                // Relative paths will be relative to the lock file directory
+                                lock_file_dir.join(path).normalize()
+                            };
+                            let url = Url::from_file_path(PathBuf::from(path.as_str()));
+                            match url {
+                                Ok(url) => url,
+                                Err(_) => {
+                                    return Ok(ValidateCurrentInstall::Reinstall(
+                                        NeedReinstall::UnableToParseFileUrl {
+                                            url: path.to_string(),
+                                        },
+                                    ))
+                                }
+                            }
                         }
                     };
 
@@ -171,9 +190,11 @@ pub(crate) fn need_reinstall(
                         ));
                     };
 
-                    if locked_url.as_ref() == &installed_url {
+                    if locked_url == installed_url {
                         // Check cache freshness
-                        if !check_url_freshness(&locked_url, installed)? {
+                        if !check_url_freshness(&locked_url, installed)
+                            .map_err(NeedsReinstallError::FreshnessError)?
+                        {
                             return Ok(ValidateCurrentInstall::Reinstall(
                                 NeedReinstall::ArchiveDistNewerThanCache,
                             ));