Remove some filesystem calls from the installer (#834)

Noticed these when working on something unrelated. Generally:

- Prefer `entry.file_type()` over `entry.path().is_file()` or similar,
as the former is almost always free on Unix.
- Call `entry.path()` once, since it allocates internally (returns a
`PathBuf`).

crates/install-wheel-rs/src/linker.rs

@@ -136,8 +136,9 @@ fn find_dist_info(path: impl AsRef<Path>) -> Result<String, Error> {
     // Iterate over `path` to find the `.dist-info` directory. It should be at the top-level.
     let Some(dist_info) = fs::read_dir(path.as_ref())?.find_map(|entry| {
         let entry = entry.ok()?;
-        let path = entry.path();
-        if path.is_dir() {
+        let file_type = entry.file_type().ok()?;
+        if file_type.is_dir() {
+            let path = entry.path();
             if path.extension().map_or(false, |ext| ext == "dist-info") {
                 Some(path)
             } else {
@@ -317,7 +318,9 @@ fn copy_wheel_files(
     // Walk over the directory.
     for entry in walkdir::WalkDir::new(&wheel) {
         let entry = entry?;
-        let relative = entry.path().strip_prefix(&wheel).unwrap();
+        let path = entry.path();
+
+        let relative = path.strip_prefix(&wheel).unwrap();
         let out_path = site_packages.as_ref().join(relative);
 
         if entry.file_type().is_dir() {
@@ -326,7 +329,7 @@ fn copy_wheel_files(
         }
 
         // Copy the file.
-        fs::copy(entry.path(), &out_path)?;
+        fs::copy(path, &out_path)?;
 
         #[cfg(unix)]
         {
@@ -370,7 +373,9 @@ fn hardlink_wheel_files(
     // Walk over the directory.
     for entry in walkdir::WalkDir::new(&wheel) {
         let entry = entry?;
-        let relative = entry.path().strip_prefix(&wheel).unwrap();
+        let path = entry.path();
+
+        let relative = path.strip_prefix(&wheel).unwrap();
         let out_path = site_packages.as_ref().join(relative);
 
         if entry.file_type().is_dir() {
@@ -379,8 +384,8 @@ fn hardlink_wheel_files(
         }
 
         // The `RECORD` file is modified during installation, so we copy it instead of hard-linking.
-        if entry.path().ends_with("RECORD") {
-            fs::copy(entry.path(), &out_path)?;
+        if path.ends_with("RECORD") {
+            fs::copy(path, &out_path)?;
             count += 1;
             continue;
         }
@@ -390,33 +395,33 @@ fn hardlink_wheel_files(
             Attempt::Initial => {
                 // Once https://github.com/rust-lang/rust/issues/86442 is stable, use that.
                 attempt = Attempt::Subsequent;
-                if let Err(err) = fs::hard_link(entry.path(), &out_path) {
+                if let Err(err) = fs::hard_link(path, &out_path) {
                     // If the file already exists, remove it and try again.
                     if err.kind() == std::io::ErrorKind::AlreadyExists {
                         fs::remove_file(&out_path)?;
-                        if fs::hard_link(entry.path(), &out_path).is_err() {
-                            fs::copy(entry.path(), &out_path)?;
+                        if fs::hard_link(path, &out_path).is_err() {
+                            fs::copy(path, &out_path)?;
                             attempt = Attempt::UseCopyFallback;
                         }
                     } else {
-                        fs::copy(entry.path(), &out_path)?;
+                        fs::copy(path, &out_path)?;
                         attempt = Attempt::UseCopyFallback;
                     }
                 }
             }
             Attempt::Subsequent => {
-                if let Err(err) = fs::hard_link(entry.path(), &out_path) {
+                if let Err(err) = fs::hard_link(path, &out_path) {
                     // If the file already exists, remove it and try again.
                     if err.kind() == std::io::ErrorKind::AlreadyExists {
                         fs::remove_file(&out_path)?;
-                        fs::hard_link(entry.path(), &out_path)?;
+                        fs::hard_link(path, &out_path)?;
                     } else {
                         return Err(err.into());
                     }
                 }
             }
             Attempt::UseCopyFallback => {
-                fs::copy(entry.path(), &out_path)?;
+                fs::copy(path, &out_path)?;
             }
         }
 

crates/install-wheel-rs/src/wheel.rs

@@ -655,16 +655,18 @@ fn install_script(
     file: &DirEntry,
     location: &InstallLocation<impl AsRef<Path>>,
 ) -> Result<(), Error> {
-    let path = file.path();
-    if !path.is_file() {
+    if !file.file_type()?.is_file() {
         return Err(Error::InvalidWheel(format!(
             "Wheel contains entry in scripts directory that is not a file: {}",
-            path.display()
+            file.path().display()
         )));
     }
 
     let target_path = bin_rel().join(file.file_name());
+
+    let path = file.path();
     let mut script = File::open(&path)?;
+
     // https://sphinx-locales.github.io/peps/pep-0427/#recommended-installer-features
     // > In wheel, scripts are packaged in {distribution}-{version}.data/scripts/.
     // > If the first line of a file in scripts/ starts with exactly b'#!python',
@@ -735,15 +737,17 @@ pub(crate) fn install_data(
     gui_scripts: &[Script],
     record: &mut [RecordEntry],
 ) -> Result<(), Error> {
-    for data_entry in fs::read_dir(data_dir)? {
-        let data_entry = data_entry?;
-        match data_entry.file_name().as_os_str().to_str() {
+    for entry in fs::read_dir(data_dir)? {
+        let entry = entry?;
+        let path = entry.path();
+
+        match path.file_name().and_then(|name| name.to_str()) {
             Some("data") => {
                 // Move the content of the folder to the root of the venv
-                move_folder_recorded(&data_entry.path(), venv_root, site_packages, record)?;
+                move_folder_recorded(&path, venv_root, site_packages, record)?;
             }
             Some("scripts") => {
-                for file in fs::read_dir(data_entry.path())? {
+                for file in fs::read_dir(path)? {
                     let file = file?;
 
                     // Couldn't find any docs for this, took it directly from
@@ -775,17 +779,17 @@ pub(crate) fn install_data(
                         location.python_version().1
                     ))
                     .join(dist_name);
-                move_folder_recorded(&data_entry.path(), &target_path, site_packages, record)?;
+                move_folder_recorded(&path, &target_path, site_packages, record)?;
             }
             Some("purelib" | "platlib") => {
                 // purelib and platlib locations are not relevant when using venvs
                 // https://stackoverflow.com/a/27882460/3549270
-                move_folder_recorded(&data_entry.path(), site_packages, site_packages, record)?;
+                move_folder_recorded(&path, site_packages, site_packages, record)?;
             }
             _ => {
                 return Err(Error::InvalidWheel(format!(
                     "Unknown wheel data type: {:?}",
-                    data_entry.file_name()
+                    entry.file_name()
                 )));
             }
         }

crates/puffin-installer/src/site_packages.rs

@@ -38,10 +38,10 @@ impl<'a> SitePackages<'a> {
         for entry in fs::read_dir(venv.site_packages())? {
             let entry = entry?;
             if entry.file_type()?.is_dir() {
-                let Some(dist_info) =
-                    InstalledDist::try_from_path(&entry.path()).with_context(|| {
-                        format!("Failed to read metadata: from {}", entry.path().display())
-                    })?
+                let path = entry.path();
+
+                let Some(dist_info) = InstalledDist::try_from_path(&path)
+                    .with_context(|| format!("Failed to read metadata: from {}", path.display()))?
                 else {
                     continue;
                 };
@@ -55,7 +55,7 @@ impl<'a> SitePackages<'a> {
                         "Found duplicate package in environment: {} ({} vs. {})",
                         existing.name(),
                         existing.path().display(),
-                        entry.path().display()
+                        path.display()
                     );
                 }
 
@@ -67,7 +67,7 @@ impl<'a> SitePackages<'a> {
                             "Found duplicate editable in environment: {} ({} vs. {})",
                             existing.name(),
                             existing.path().display(),
-                            entry.path().display()
+                            path.display()
                         );
                     }
                 }