fix(vendor)!: direct extraction for registry sources

`PathSource::list_files` has some heurstic rules for listing files.
Those rules are mainly designed for `cargo package`.

Previously, cargo-vendor relies on those rules to understand what
files to vendor. However, it shouldn't use those rules because:

* Package extracted from a `.crate` tarball isn't Git-controlled,
  some rules may apply differently.
* The extracted package already went through `PathSource::list_files`
  during packaging. It should be clean enough.
* Should keep crate sources from registry sources in a pristine state,
  which is exactly what vendoring is meant for.

Instead, we switch to direct extraction into the vendor directory
to ensure source code is the same as in the `.crate` tarball.

There are some caveats:

* The overwrite protection in `unpack_package` assumes the unpack
  directory is always `<pkg>-<version`>.
  We don't want to remove this,
  but for cargo-vendor supports vendoring without version suffix.
  For that case, we need a temporary staging area,
  and move the unpacked source then.
* The heurstic in `PathSource::list_files` did something "good" in
  general cases, like excluding hidden directories. That means
  common directorys like `.github` or `.config` won't be vendored.
  After this, those get included. This is another round of churns.
  We might want to get other `cargo-vendor` changes along with this
  in one single release.

Author: weihanglo

src/cargo/ops/vendor.rs

@@ -4,13 +4,18 @@ use crate::core::{GitReference, Package, Workspace};
 use crate::ops;
 use crate::sources::path::PathSource;
 use crate::sources::PathEntry;
+use crate::sources::RegistrySource;
 use crate::sources::SourceConfigMap;
 use crate::sources::CRATES_IO_REGISTRY;
 use crate::util::cache_lock::CacheLockMode;
 use crate::util::{try_canonicalize, CargoResult, GlobalContext};
+
 use anyhow::{bail, Context as _};
 use cargo_util::{paths, Sha256};
+use cargo_util_schemas::core::SourceKind;
 use serde::Serialize;
+use walkdir::WalkDir;
+
 use std::collections::HashSet;
 use std::collections::{BTreeMap, BTreeSet, HashMap};
 use std::ffi::OsStr;
@@ -86,9 +91,16 @@ struct SourceReplacementCache<'gctx> {
 }
 
 impl SourceReplacementCache<'_> {
-    fn new(gctx: &GlobalContext) -> CargoResult<SourceReplacementCache<'_>> {
+    fn new(
+        gctx: &GlobalContext,
+        respect_source_config: bool,
+    ) -> CargoResult<SourceReplacementCache<'_>> {
         Ok(SourceReplacementCache {
-            map: SourceConfigMap::new(gctx)?,
+            map: if respect_source_config {
+                SourceConfigMap::new(gctx)
+            } else {
+                SourceConfigMap::empty(gctx)
+            }?,
             cache: Default::default(),
         })
     }
@@ -130,7 +142,8 @@ fn sync(
         }
     }
 
-    let mut source_replacement_cache = SourceReplacementCache::new(gctx)?;
+    let mut source_replacement_cache =
+        SourceReplacementCache::new(gctx, opts.respect_source_config)?;
 
     // First up attempt to work around rust-lang/cargo#5956. Apparently build
     // artifacts sprout up in Cargo's global cache for whatever reason, although
@@ -263,11 +276,69 @@ fn sync(
         )?;
 
         let _ = fs::remove_dir_all(&dst);
-        let pathsource = PathSource::new(src, id.source_id(), gctx);
-        let paths = pathsource.list_files(pkg)?;
+
         let mut file_cksums = BTreeMap::new();
-        cp_sources(pkg, src, &paths, &dst, &mut file_cksums, &mut tmp_buf, gctx)
-            .with_context(|| format!("failed to copy over vendored sources for: {}", id))?;
+
+        // Need this mapping anyway because we will directly consult registry sources,
+        // otherwise builtin source replacement (sparse registry) won't be respected.
+        let sid = source_replacement_cache.get(id.source_id())?;
+
+        if sid.is_registry() {
+            // To keep the unpacked source from registry in a pristine state,
+            // we'll do a direct extraction into the vendor directory.
+            let registry = match sid.kind() {
+                SourceKind::Registry | SourceKind::SparseRegistry => {
+                    RegistrySource::remote(sid, &Default::default(), gctx)?
+                }
+                SourceKind::LocalRegistry => {
+                    let path = sid.url().to_file_path().expect("local path");
+                    RegistrySource::local(sid, &path, &Default::default(), gctx)
+                }
+                _ => unreachable!("not registry source: {sid}"),
+            };
+
+            let mut compute_file_cksums = |root| {
+                let walkdir = WalkDir::new(root)
+                    .into_iter()
+                    // It is safe to skip errors,
+                    // since we'll hit them during copying/reading later anyway.
+                    .filter_map(|e| e.ok())
+                    // There should be no symlink in tarballs on crates.io,
+                    // but might be wrong for local registries.
+                    // Hence here be conservative and include symlinks.
+                    .filter(|e| e.file_type().is_file() || e.file_type().is_symlink());
+                for e in walkdir {
+                    let path = e.path();
+                    let relative = path.strip_prefix(&dst).unwrap();
+                    let cksum = Sha256::new()
+                        .update_path(path)
+                        .map(Sha256::finish_hex)
+                        .with_context(|| format!("failed to checksum `{}`", path.display()))?;
+                    file_cksums.insert(relative.to_str().unwrap().replace("\\", "/"), cksum);
+                }
+                Ok::<_, anyhow::Error>(())
+            };
+            if dir_has_version_suffix {
+                registry.unpack_package_in(id, &vendor_dir, &vendor_this)?;
+                compute_file_cksums(&dst)?;
+            } else {
+                // Due to the extra sanity check in registry unpack
+                // (ensure it contain only one top-level directory with name `pkg-version`),
+                // we can only unpack a directory with version suffix,
+                // and move it to the no suffix directory.
+                let staging_dir = tempfile::Builder::new()
+                    .prefix(".vendor-staging")
+                    .tempdir_in(vendor_dir)?;
+                let unpacked_src =
+                    registry.unpack_package_in(id, staging_dir.path(), &vendor_this)?;
+                fs::rename(&unpacked_src, &dst)?;
+                compute_file_cksums(&dst)?;
+            }
+        } else {
+            let paths = PathSource::new(src, sid, gctx).list_files(pkg)?;
+            cp_sources(pkg, src, &paths, &dst, &mut file_cksums, &mut tmp_buf, gctx)
+                .with_context(|| format!("failed to copy vendored sources for {id}"))?;
+        }
 
         // Finally, emit the metadata about this package
         let json = serde_json::json!({

src/cargo/sources/registry/mod.rs

@@ -248,6 +248,8 @@ pub struct RegistrySource<'gctx> {
     source_id: SourceId,
     /// The path where crate files are extracted (`$CARGO_HOME/registry/src/$REG-HASH`).
     src_path: Filesystem,
+    /// Path to the cache of `.crate` files (`$CARGO_HOME/registry/cache/$REG-HASH`).
+    cache_path: Filesystem,
     /// Local reference to [`GlobalContext`] for convenience.
     gctx: &'gctx GlobalContext,
     /// Abstraction for interfacing to the different registry kinds.
@@ -532,6 +534,7 @@ impl<'gctx> RegistrySource<'gctx> {
         RegistrySource {
             name: name.into(),
             src_path: gctx.registry_source_path().join(name),
+            cache_path: gctx.registry_cache_path().join(name),
             gctx,
             source_id,
             index: index::RegistryIndex::new(source_id, ops.index_path(), gctx),
@@ -631,7 +634,7 @@ impl<'gctx> RegistrySource<'gctx> {
         }
         dst.create_dir()?;
 
-        let bytes_written = unpack(self.gctx, tarball, unpack_dir)?;
+        let bytes_written = unpack(self.gctx, tarball, unpack_dir, &|_| true)?;
 
         // Now that we've finished unpacking, create and write to the lock file to indicate that
         // unpacking was successful.
@@ -656,6 +659,29 @@ impl<'gctx> RegistrySource<'gctx> {
         Ok(unpack_dir.to_path_buf())
     }
 
+    /// Unpacks the `.crate` tarball of the package in a given directory.
+    ///
+    /// Returns the path to the crate tarball directory,
+    /// whch is always `<unpack_dir>/<pkg>-<version>`.
+    ///
+    /// This holds an assumption that the associated tarball already exists.
+    pub fn unpack_package_in(
+        &self,
+        pkg: &PackageId,
+        unpack_dir: &Path,
+        include: &dyn Fn(&Path) -> bool,
+    ) -> CargoResult<PathBuf> {
+        let path = self.cache_path.join(pkg.tarball_name());
+        let path = self
+            .gctx
+            .assert_package_cache_locked(CacheLockMode::DownloadExclusive, &path);
+        let dst = unpack_dir.join(format!("{}-{}", pkg.name(), pkg.version()));
+        let tarball =
+            File::open(path).with_context(|| format!("failed to open {}", path.display()))?;
+        unpack(self.gctx, &tarball, &dst, include)?;
+        Ok(dst)
+    }
+
     /// Turns the downloaded `.crate` tarball file into a [`Package`].
     ///
     /// This unconditionally sets checksum for the returned package, so it
@@ -996,7 +1022,12 @@ fn set_mask<R: Read>(tar: &mut Archive<R>) {
 }
 
 /// Unpack a tarball with zip bomb and overwrite protections.
-fn unpack(gctx: &GlobalContext, tarball: &File, unpack_dir: &Path) -> CargoResult<u64> {
+fn unpack(
+    gctx: &GlobalContext,
+    tarball: &File,
+    unpack_dir: &Path,
+    include: &dyn Fn(&Path) -> bool,
+) -> CargoResult<u64> {
     let mut tar = {
         let size_limit = max_unpack_size(gctx, tarball.metadata()?.len());
         let gz = GzDecoder::new(tarball);
@@ -1015,20 +1046,23 @@ fn unpack(gctx: &GlobalContext, tarball: &File, unpack_dir: &Path) -> CargoResul
             .context("failed to read entry path")?
             .into_owned();
 
-        // We're going to unpack this tarball into the global source
-        // directory, but we want to make sure that it doesn't accidentally
-        // (or maliciously) overwrite source code from other crates. Cargo
-        // itself should never generate a tarball that hits this error, and
-        // crates.io should also block uploads with these sorts of tarballs,
-        // but be extra sure by adding a check here as well.
-        if !entry_path.starts_with(prefix) {
+        if let Ok(path) = entry_path.strip_prefix(prefix) {
+            if !include(path) {
+                continue;
+            }
+        } else {
+            // We're going to unpack this tarball into the global source
+            // directory, but we want to make sure that it doesn't accidentally
+            // (or maliciously) overwrite source code from other crates. Cargo
+            // itself should never generate a tarball that hits this error, and
+            // crates.io should also block uploads with these sorts of tarballs,
+            // but be extra sure by adding a check here as well.
             anyhow::bail!(
                 "invalid tarball downloaded, contains \
-                     a file at {:?} which isn't under {:?}",
-                entry_path,
-                prefix
+                     a file at {entry_path:?} which isn't under {prefix:?}",
             )
         }
+
         // Prevent unpacking the lockfile from the crate itself.
         if entry_path
             .file_name()

tests/testsuite/vendor.rs

@@ -213,12 +213,13 @@ fn package_exclude() {
 
     p.cargo("vendor --respect-source-config").run();
     let csum = p.read_file("vendor/bar/.cargo-checksum.json");
+    // Everything is included because `cargo-vendor`
+    // do direct extractions from tarballs
+    // (Some are excluded like `.git` or `.cargo-ok` though.)
     assert!(csum.contains(".include"));
-    assert!(!csum.contains(".exclude"));
-    assert!(!csum.contains(".dotdir/exclude"));
-    // Gitignore doesn't re-include a file in an excluded parent directory,
-    // even if negating it explicitly.
-    assert!(!csum.contains(".dotdir/include"));
+    assert!(csum.contains(".exclude"));
+    assert!(csum.contains(".dotdir/exclude"));
+    assert!(csum.contains(".dotdir/include"));
 }
 
 #[cargo_test]