fix(reflink): move reflink checks up to node-maintainer (#195)

Fixes: #193

Cargo.toml

@@ -79,7 +79,6 @@ maplit = "1.0.2"
 miette = "5.5.0"
 node-semver = "2.1.0"
 nom = "7.1.3"
-once_cell = "1.17.1"
 percent-encoding = "2.1.0"
 pretty_assertions = "1.3.0"
 proc-macro2 = "1.0.18"

crates/nassun/Cargo.toml

@@ -37,8 +37,6 @@ backon = { workspace = true }
 cacache = { workspace = true }
 flate2 = { workspace = true }
 io_tee = { workspace = true }
-once_cell = { workspace = true }
-reflink = { workspace = true }
 tar = { workspace = true }
 tempfile = { workspace = true }
 which = { workspace = true }

crates/nassun/src/client.rs

@@ -25,7 +25,6 @@ pub struct NassunOpts {
     base_dir: Option<PathBuf>,
     default_tag: Option<String>,
     registries: HashMap<Option<String>, Url>,
-    prefer_copy: bool,
 }
 
 impl NassunOpts {
@@ -38,17 +37,6 @@ impl NassunOpts {
         self
     }
 
-    /// When extracting tarballs, prefer to copy files to their destination as
-    /// separate, standalone files instead of hard linking them. Full copies
-    /// will still happen when hard linking fails. Furthermore, on filesystems
-    /// that support Copy-on-Write (zfs, btrfs, APFS (macOS), etc), this
-    /// option will use that feature for all copies.
-    #[cfg(not(target_arch = "wasm32"))]
-    pub fn prefer_copy(mut self, prefer_copy: bool) -> Self {
-        self.prefer_copy = prefer_copy;
-        self
-    }
-
     pub fn registry(mut self, registry: Url) -> Self {
         self.registries.insert(None, registry);
         self
@@ -96,8 +84,6 @@ impl NassunOpts {
                     .unwrap_or_else(|| std::env::current_dir().expect("failed to get cwd.")),
                 default_tag: self.default_tag.unwrap_or_else(|| "latest".into()),
             },
-            #[cfg(not(target_arch = "wasm32"))]
-            prefer_copy: self.prefer_copy,
             #[cfg(target_arch = "wasm32")]
             prefer_copy: false,
             npm_fetcher: Arc::new(NpmFetcher::new(
@@ -117,7 +103,6 @@ impl NassunOpts {
 #[derive(Clone)]
 pub struct Nassun {
     cache: Arc<Option<PathBuf>>,
-    prefer_copy: bool,
     resolver: PackageResolver,
     npm_fetcher: Arc<dyn PackageFetcher>,
     #[cfg(not(target_arch = "wasm32"))]
@@ -216,7 +201,7 @@ impl Nassun {
         let fetcher = self.pick_fetcher(&spec);
         let name = fetcher.name(&spec, &self.resolver.base_dir).await?;
         self.resolver
-            .resolve(name, spec, fetcher, self.cache.clone(), self.prefer_copy)
+            .resolve(name, spec, fetcher, self.cache.clone())
             .await
     }
 
@@ -231,14 +216,8 @@ impl Nassun {
         resolved: PackageResolution,
     ) -> Package {
         let fetcher = self.pick_fetcher(&from);
-        self.resolver.resolve_from(
-            name,
-            from,
-            resolved,
-            fetcher,
-            self.cache.clone(),
-            self.prefer_copy,
-        )
+        self.resolver
+            .resolve_from(name, from, resolved, fetcher, self.cache.clone())
     }
 
     /// Creates a "resolved" package from a plain [`oro_common::Manifest`].
@@ -250,7 +229,6 @@ impl Nassun {
             from: PackageSpec::Dir {
                 path: PathBuf::from("."),
             },
-            prefer_copy: false,
             name: manifest.name.clone().unwrap_or_else(|| "dummy".to_string()),
             resolved: PackageResolution::Dir {
                 name: manifest.name.clone().unwrap_or_else(|| "dummy".to_string()),

crates/nassun/src/package.rs

@@ -22,8 +22,6 @@ pub struct Package {
     pub(crate) fetcher: Arc<dyn PackageFetcher>,
     pub(crate) base_dir: PathBuf,
     pub(crate) cache: Arc<Option<PathBuf>>,
-    #[cfg_attr(not(target_arch = "wasm32"), allow(dead_code))]
-    pub(crate) prefer_copy: bool,
 }
 
 impl Package {
@@ -123,22 +121,31 @@ impl Package {
     /// tarball stream will have its integrity validated based on package
     /// metadata. See [`Package::tarball`] for more information.
     #[cfg(not(target_arch = "wasm32"))]
-    pub async fn extract_to_dir(&self, dir: impl AsRef<Path>) -> Result<Integrity> {
-        async fn inner(me: &Package, dir: &Path) -> Result<Integrity> {
-            me.extract_to_dir_inner(dir, me.resolved.integrity()).await
+    pub async fn extract_to_dir(
+        &self,
+        dir: impl AsRef<Path>,
+        prefer_copy: bool,
+    ) -> Result<Integrity> {
+        async fn inner(me: &Package, dir: &Path, prefer_copy: bool) -> Result<Integrity> {
+            me.extract_to_dir_inner(dir, me.resolved.integrity(), prefer_copy)
+                .await
         }
-        inner(self, dir.as_ref()).await
+        inner(self, dir.as_ref(), prefer_copy).await
     }
 
     /// Extract tarball to a directory, optionally caching its contents. The
     /// tarball stream will NOT have its integrity validated. See
     /// [`Package::tarball_unchecked`] for more information.
     #[cfg(not(target_arch = "wasm32"))]
-    pub async fn extract_to_dir_unchecked(&self, dir: impl AsRef<Path>) -> Result<Integrity> {
-        async fn inner(me: &Package, dir: &Path) -> Result<Integrity> {
-            me.extract_to_dir_inner(dir, None).await
+    pub async fn extract_to_dir_unchecked(
+        &self,
+        dir: impl AsRef<Path>,
+        prefer_copy: bool,
+    ) -> Result<Integrity> {
+        async fn inner(me: &Package, dir: &Path, prefer_copy: bool) -> Result<Integrity> {
+            me.extract_to_dir_inner(dir, None, prefer_copy).await
         }
-        inner(self, dir.as_ref()).await
+        inner(self, dir.as_ref(), prefer_copy).await
     }
 
     /// Extract tarball to a directory, optionally caching its contents. The
@@ -149,18 +156,25 @@ impl Package {
         &self,
         dir: impl AsRef<Path>,
         sri: Integrity,
+        prefer_copy: bool,
     ) -> Result<Integrity> {
-        async fn inner(me: &Package, dir: &Path, sri: Integrity) -> Result<Integrity> {
-            me.extract_to_dir_inner(dir, Some(&sri)).await
+        async fn inner(
+            me: &Package,
+            dir: &Path,
+            sri: Integrity,
+            prefer_copy: bool,
+        ) -> Result<Integrity> {
+            me.extract_to_dir_inner(dir, Some(&sri), prefer_copy).await
         }
-        inner(self, dir.as_ref(), sri).await
+        inner(self, dir.as_ref(), sri, prefer_copy).await
     }
 
     #[cfg(not(target_arch = "wasm32"))]
     async fn extract_to_dir_inner(
         &self,
         dir: &Path,
         integrity: Option<&Integrity>,
+        prefer_copy: bool,
     ) -> Result<Integrity> {
         if let Some(sri) = integrity {
             if let Some(cache) = self.cache.as_deref() {
@@ -172,7 +186,7 @@ impl Package {
                     // (bad data, etc), then go ahead and do a network
                     // extract.
                     match self
-                        .extract_from_cache(dir, cache, entry, self.prefer_copy)
+                        .extract_from_cache(dir, cache, entry, prefer_copy)
                         .await
                     {
                         Ok(_) => return Ok(sri),
@@ -181,30 +195,26 @@ impl Package {
                             return self
                                 .tarball_checked(sri)
                                 .await?
-                                .extract_from_tarball_data(
-                                    dir,
-                                    self.cache.as_deref(),
-                                    self.prefer_copy,
-                                )
+                                .extract_from_tarball_data(dir, self.cache.as_deref(), prefer_copy)
                                 .await;
                         }
                     }
                 } else {
                     return self
                         .tarball_checked(sri.clone())
                         .await?
-                        .extract_from_tarball_data(dir, self.cache.as_deref(), self.prefer_copy)
+                        .extract_from_tarball_data(dir, self.cache.as_deref(), prefer_copy)
                         .await;
                 }
             }
             self.tarball_checked(sri.clone())
                 .await?
-                .extract_from_tarball_data(dir, self.cache.as_deref(), self.prefer_copy)
+                .extract_from_tarball_data(dir, self.cache.as_deref(), prefer_copy)
                 .await
         } else {
             self.tarball_unchecked()
                 .await?
-                .extract_from_tarball_data(dir, self.cache.as_deref(), self.prefer_copy)
+                .extract_from_tarball_data(dir, self.cache.as_deref(), prefer_copy)
                 .await
         }
     }

crates/nassun/src/resolver.rs

@@ -109,15 +109,13 @@ impl PackageResolver {
         resolved: PackageResolution,
         fetcher: Arc<dyn PackageFetcher>,
         cache: Arc<Option<PathBuf>>,
-        prefer_copy: bool,
     ) -> Package {
         Package {
             name,
             from,
             resolved,
             fetcher,
             cache,
-            prefer_copy,
             base_dir: self.base_dir.clone(),
         }
     }
@@ -128,7 +126,6 @@ impl PackageResolver {
         wanted: PackageSpec,
         fetcher: Arc<dyn PackageFetcher>,
         cache: Arc<Option<PathBuf>>,
-        prefer_copy: bool,
     ) -> Result<Package, NassunError> {
         let packument = fetcher.corgi_packument(&wanted, &self.base_dir).await?;
         let resolved = self.get_resolution(&name, &wanted, &packument)?;
@@ -139,7 +136,6 @@ impl PackageResolver {
             fetcher,
             base_dir: self.base_dir.clone(),
             cache,
-            prefer_copy,
         })
     }
 

crates/nassun/src/tarball.rs

@@ -25,10 +25,6 @@ use crate::TarballStream;
 
 const MAX_IN_MEMORY_TARBALL_SIZE: usize = 1024 * 1024 * 5;
 
-#[cfg(not(target_arch = "wasm32"))]
-pub(crate) static SUPPORTS_REFLINK: once_cell::sync::Lazy<bool> =
-    once_cell::sync::Lazy::new(supports_reflink);
-
 pub struct Tarball {
     checker: Option<IntegrityChecker>,
     reader: TarballStream,
@@ -359,46 +355,13 @@ pub(crate) fn tarball_key(integrity: &Integrity) -> String {
     format!("nassun::package::{integrity}")
 }
 
-#[cfg(not(target_arch = "wasm32"))]
-fn supports_reflink() -> bool {
-    let tempdir = match tempfile::tempdir() {
-        Ok(t) => t,
-        Err(e) => {
-            tracing::warn!("error creating temp dir while checking for reflink support: {e}.");
-            return false;
-        }
-    };
-    match std::fs::write(tempdir.path().join("a"), "a") {
-        Ok(_) => {}
-        Err(e) => {
-            tracing::warn!("error writing to tempfile while checking for reflink support: {e}.");
-            return false;
-        }
-    };
-    let supports_reflink = reflink::reflink(tempdir.path().join("a"), tempdir.path().join("b"))
-        .map(|_| true)
-        .map_err(|e| {
-            tracing::info!(
-                "reflink support check failed. Files will be hard linked or copied. ({e})"
-            );
-            e
-        })
-        .unwrap_or(false);
-
-    if supports_reflink {
-        tracing::info!("Filesystem supports reflinks, extracted data will use copy-on-write reflinks instead of hard links or full copies (unless cache in in a separate disk).")
-    }
-
-    supports_reflink
-}
-
 pub(crate) fn extract_from_cache(
     cache: &Path,
     sri: &Integrity,
     to: &Path,
     prefer_copy: bool,
 ) -> Result<()> {
-    if *SUPPORTS_REFLINK || prefer_copy {
+    if prefer_copy {
         cacache::copy_hash_unchecked_sync(cache, sri, to)
             .map_err(|e| NassunError::ExtractCacheError(e, Some(PathBuf::from(to))))?;
     } else {

crates/node-maintainer/Cargo.toml

@@ -30,7 +30,9 @@ unicase = "2.6.0"
 url = { workspace = true }
 
 [target.'cfg(not(target_arch = "wasm32"))'.dependencies]
+reflink = { workspace = true }
 indicatif = { workspace = true }
+tempfile = { workspace = true }
 
 [target.'cfg(target_arch = "wasm32")'.dependencies]
 console_error_panic_hook = { workspace = true }