Fix wrongly filtered out packages when using --versioned-dirs (#107)

coreos · Oct 21, 2024 · fd1a1fe · fd1a1fe
1 parent bf3bf5c
commit fd1a1fe
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 39 deletions.
diff --git a/src/main.rs b/src/main.rs
@@ -909,13 +909,20 @@ fn run() -> Result<()> {
     for (_name, mut pkgs) in pkgs_by_name {
         // Reverse sort - greater version is lower index
         pkgs.sort_by(|a, b| b.version.cmp(&a.version));
-        // SAFETY: The package set must be non-empty
-        let (first, rest) = pkgs.split_first().unwrap();
+        // If we use versioned-dirs, we insert all packages with a versioned filename
+        // If not, we split off the first package and insert it without a version-suffix
+        let versioned_pkgs = if !args.versioned_dirs {
+            // SAFETY: The package set must be non-empty
+            let (first, rest) = pkgs.split_first().unwrap();
+            if packages.contains_key(&first.id) {
+                package_filenames.insert(Cow::Borrowed(first.name.as_str()), *first);
+            }
+            rest
+        } else {
+            &pkgs
+        };
 
-        if packages.contains_key(&first.id) {
-            package_filenames.insert(Cow::Borrowed(first.name.as_str()), *first);
-        }
-        for &pkg in rest {
+        for pkg in versioned_pkgs {
             if packages.contains_key(&pkg.id) {
                 package_filenames.insert(Cow::Owned(package_versioned_filename(pkg)), pkg);
             }

diff --git a/tests/vendor_filterer/common.rs b/tests/vendor_filterer/common.rs
@@ -172,3 +172,22 @@ pub(crate) fn verify_no_macos(dir: &Utf8Path) {
     macos_lib.pop();
     assert_eq!(macos_lib.read_dir_utf8().unwrap().count(), 1);
 }
+
+pub(crate) fn verify_crate_is_no_stub(output_folder: &Utf8Path, name: &str) {
+    let crate_dir = output_folder.join(name);
+    assert!(
+        crate_dir.exists(),
+        "Package does not show up in the vendor dir"
+    );
+    let crate_lib = crate_dir.join("src/lib.rs");
+    assert!(
+        crate_lib.exists(),
+        "Package has no src/lib.rs-file in the vendor dir"
+    );
+    // Check that this was not filtered out
+    assert_ne!(
+        crate_lib.metadata().unwrap().len(),
+        0,
+        "Package was filtered out, when it shouldn't have been!"
+    );
+}
diff --git a/tests/vendor_filterer/sync.rs b/tests/vendor_filterer/sync.rs
@@ -1,4 +1,4 @@
-use crate::vendor_filterer::common::{verify_no_macos, verify_no_windows};
+use crate::vendor_filterer::common::{verify_crate_is_no_stub, verify_no_macos, verify_no_windows};
 
 use super::common::{tempdir, vendor, write_file_create_parents, VendorOptions};
 
@@ -264,12 +264,7 @@ fn filter_without_manifest_path() {
     })
     .unwrap();
     assert!(output.status.success());
-    let bitflags = output_folder.join("bitflags");
-    assert!(bitflags.exists());
-    let bitflags_lib = bitflags.join("src/lib.rs");
-    assert!(bitflags_lib.exists());
-    // Check that this was not filtered out
-    assert_ne!(bitflags_lib.metadata().unwrap().len(), 0);
+    verify_crate_is_no_stub(&output_folder, "bitflags");
 }
 
 #[test]
@@ -314,16 +309,6 @@ fn filter_without_manifest_but_sync() {
     })
     .unwrap();
     assert!(output.status.success());
-    let bitflags = output_folder.join("bitflags");
-    assert!(bitflags.exists());
-    let bitflags_lib = bitflags.join("src/lib.rs");
-    assert!(bitflags_lib.exists());
-    // Check that this was not filtered out
-    assert_ne!(bitflags_lib.metadata().unwrap().len(), 0);
-    let hex = output_folder.join("hex");
-    assert!(hex.exists());
-    let hex_lib = hex.join("src/lib.rs");
-    assert!(hex_lib.exists());
-    // Check that this was not filtered out
-    assert_ne!(hex_lib.metadata().unwrap().len(), 0);
+    verify_crate_is_no_stub(&output_folder, "bitflags");
+    verify_crate_is_no_stub(&output_folder, "hex");
 }
diff --git a/tests/vendor_filterer/toml.rs b/tests/vendor_filterer/toml.rs
@@ -1,4 +1,6 @@
-use super::common::{tempdir, vendor, write_file_create_parents, VendorOptions};
+use super::common::{
+    tempdir, vendor, verify_crate_is_no_stub, write_file_create_parents, VendorOptions,
+};
 
 #[test]
 fn manifest_path() {
@@ -25,8 +27,7 @@ fn manifest_path() {
     })
     .unwrap();
     assert!(output.status.success());
-    let bitflags = output_folder.join("bitflags");
-    assert!(bitflags.exists());
+    verify_crate_is_no_stub(&output_folder, "bitflags");
 }
 
 #[test]

diff --git a/tests/vendor_filterer/versioned_dirs.rs b/tests/vendor_filterer/versioned_dirs.rs
@@ -1,4 +1,54 @@
-use super::common::{tempdir, vendor, write_file_create_parents, VendorOptions};
+use super::common::{
+    tempdir, vendor, verify_crate_is_no_stub, write_file_create_parents, VendorOptions,
+};
+
+#[test]
+fn multiple_versions_without_flag() {
+    let (_td, test_folder) = tempdir().unwrap();
+    let dep_a = test_folder.join("A");
+    let dep_b = test_folder.join("B");
+    let manifest_a = write_file_create_parents(
+        &dep_a,
+        "Cargo.toml",
+        r#"
+        [package]
+        name = "foo"
+        version = "0.1.0"
+
+        [dependencies]
+        bitflags = "1.3.2"
+        hex = "0.4.3"
+        bar = { path="../B/" }
+    "#,
+    )
+    .unwrap();
+    write_file_create_parents(&dep_a, "src/lib.rs", "").unwrap();
+    let _manifest_b = write_file_create_parents(
+        &dep_b,
+        "Cargo.toml",
+        r#"
+        [package]
+        name = "bar"
+        version = "0.1.0"
+
+        [dependencies]
+        hex = "0.3.2"
+    "#,
+    )
+    .unwrap();
+    write_file_create_parents(&dep_b, "src/lib.rs", "").unwrap();
+    let output_folder = test_folder.join("vendor");
+    let output = vendor(VendorOptions {
+        output: Some(&output_folder),
+        manifest_path: Some(&manifest_a),
+        ..Default::default()
+    })
+    .unwrap();
+    assert!(output.status.success());
+    verify_crate_is_no_stub(&output_folder, "bitflags");
+    verify_crate_is_no_stub(&output_folder, "hex");
+    verify_crate_is_no_stub(&output_folder, "hex-0.3.2");
+}
 
 #[test]
 fn only_one_version() {
@@ -27,10 +77,8 @@ fn only_one_version() {
     })
     .unwrap();
     assert!(output.status.success());
-    let bitflags = output_folder.join("bitflags-1.3.2");
-    assert!(bitflags.exists());
-    let hex = output_folder.join("hex-0.4.3");
-    assert!(hex.exists());
+    verify_crate_is_no_stub(&output_folder, "bitflags-1.3.2");
+    verify_crate_is_no_stub(&output_folder, "hex-0.4.3");
 }
 
 #[test]
@@ -77,10 +125,7 @@ fn multiple_versions() {
     })
     .unwrap();
     assert!(output.status.success());
-    let bitflags = output_folder.join("bitflags-1.3.2");
-    assert!(bitflags.exists());
-    let hex = output_folder.join("hex-0.4.3");
-    assert!(hex.exists());
-    let hex2 = output_folder.join("hex-0.3.2");
-    assert!(hex2.exists());
+    verify_crate_is_no_stub(&output_folder, "bitflags-1.3.2");
+    verify_crate_is_no_stub(&output_folder, "hex-0.4.3");
+    verify_crate_is_no_stub(&output_folder, "hex-0.3.2");
 }