address review comments

rust-lang · Oct 8, 2024 · 5455998 · 5455998
1 parent f535b67
commit 5455998
Show file tree

Hide file tree

Showing 4 changed files with 206 additions and 119 deletions.
diff --git a/crates/cargo-test-support/src/lib.rs b/crates/cargo-test-support/src/lib.rs
@@ -1582,3 +1582,82 @@ where
     let thread = std::thread::spawn(|| f());
     thread_wait_timeout(n, thread)
 }
+
+// Helper for testing dep-info files in the fingerprint dir.
+#[track_caller]
+pub fn assert_deps(project: &Project, fingerprint: &str, test_cb: impl Fn(&Path, &[(u8, &str)])) {
+    let mut files = project
+        .glob(fingerprint)
+        .map(|f| f.expect("unwrap glob result"))
+        // Filter out `.json` entries.
+        .filter(|f| f.extension().is_none());
+    let info_path = files
+        .next()
+        .unwrap_or_else(|| panic!("expected 1 dep-info file at {}, found 0", fingerprint));
+    assert!(files.next().is_none(), "expected only 1 dep-info file");
+    let dep_info = fs::read(&info_path).unwrap();
+    let dep_info = &mut &dep_info[..];
+    let deps = (0..read_usize(dep_info))
+        .map(|_| {
+            let ty = read_u8(dep_info);
+            let path = std::str::from_utf8(read_bytes(dep_info)).unwrap();
+            let checksum_present = read_bool(dep_info);
+            if checksum_present {
+                // Read out the checksum info without using it
+                let _file_len = read_u64(dep_info);
+                let _checksum = read_bytes(dep_info);
+            }
+            (ty, path)
+        })
+        .collect::<Vec<_>>();
+    test_cb(&info_path, &deps);
+
+    fn read_usize(bytes: &mut &[u8]) -> usize {
+        let ret = &bytes[..4];
+        *bytes = &bytes[4..];
+
+        u32::from_le_bytes(ret.try_into().unwrap()) as usize
+    }
+
+    fn read_u8(bytes: &mut &[u8]) -> u8 {
+        let ret = bytes[0];
+        *bytes = &bytes[1..];
+        ret
+    }
+
+    fn read_bool(bytes: &mut &[u8]) -> bool {
+        read_u8(bytes) != 0
+    }
+
+    fn read_u64(bytes: &mut &[u8]) -> u64 {
+        let ret = &bytes[..8];
+        *bytes = &bytes[8..];
+
+        u64::from_le_bytes(ret.try_into().unwrap())
+    }
+
+    fn read_bytes<'a>(bytes: &mut &'a [u8]) -> &'a [u8] {
+        let n = read_usize(bytes);
+        let ret = &bytes[..n];
+        *bytes = &bytes[n..];
+        ret
+    }
+}
+
+pub fn assert_deps_contains(project: &Project, fingerprint: &str, expected: &[(u8, &str)]) {
+    assert_deps(project, fingerprint, |info_path, entries| {
+        for (e_kind, e_path) in expected {
+            let pattern = glob::Pattern::new(e_path).unwrap();
+            let count = entries
+                .iter()
+                .filter(|(kind, path)| kind == e_kind && pattern.matches(path))
+                .count();
+            if count != 1 {
+                panic!(
+                    "Expected 1 match of {} {} in {:?}, got {}:\n{:#?}",
+                    e_kind, e_path, info_path, count, entries
+                );
+            }
+        }
+    })
+}
diff --git a/src/cargo/core/compiler/fingerprint/mod.rs b/src/cargo/core/compiler/fingerprint/mod.rs
@@ -2093,7 +2093,7 @@ where
 /// Tells the associated path in [`EncodedDepInfo::files`] is relative to package root,
 /// target root, or absolute.
 #[derive(Debug, Eq, PartialEq, Hash, Copy, Clone)]
-pub enum DepInfoPathType {
+enum DepInfoPathType {
     /// src/, e.g. src/lib.rs
     PackageRootRelative,
     /// target/debug/deps/lib...
@@ -2451,13 +2451,13 @@ impl ChecksumAlgo {
 }
 
 impl FromStr for ChecksumAlgo {
-    type Err = InvalidChecksumAlgo;
+    type Err = InvalidChecksum;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         match s {
             "sha256" => Ok(Self::Sha256),
             "blake3" => Ok(Self::Blake3),
-            _ => Err(InvalidChecksumAlgo {}),
+            _ => Err(InvalidChecksum::InvalidChecksumAlgo),
         }
     }
 }
@@ -2471,17 +2471,6 @@ impl Display for ChecksumAlgo {
     }
 }
 
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-pub struct InvalidChecksumAlgo {}
-
-impl Display for InvalidChecksumAlgo {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "expected `sha256`, or `blake3`")
-    }
-}
-
-impl std::error::Error for InvalidChecksumAlgo {}
-
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub struct Checksum {
     algo: ChecksumAlgo,
@@ -2596,18 +2585,12 @@ impl Display for Checksum {
     }
 }
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq, thiserror::Error)]
+#[derive(Debug, thiserror::Error)]
 pub enum InvalidChecksum {
-    #[error("algorithm portion incorrect, {0}")]
-    InvalidChecksumAlgo(InvalidChecksumAlgo),
+    #[error("algorithm portion incorrect, expected `sha256`, or `blake3`")]
+    InvalidChecksumAlgo,
     #[error("expected {} hexadecimal digits in checksum portion", .0.hash_len() * 2)]
     InvalidChecksum(ChecksumAlgo),
     #[error("expected a string with format \"algorithm=hex_checksum\"")]
     InvalidFormat,
 }
-
-impl From<InvalidChecksumAlgo> for InvalidChecksum {
-    fn from(value: InvalidChecksumAlgo) -> Self {
-        InvalidChecksum::InvalidChecksumAlgo(value)
-    }
-}
diff --git a/tests/testsuite/dep_info.rs b/tests/testsuite/dep_info.rs
@@ -1,99 +1,17 @@
 //! Tests for dep-info files. This includes the dep-info file Cargo creates in
 //! the output directory, and the ones stored in the fingerprint.
 
-use std::fs;
 use std::path::Path;
-use std::str;
 
 use cargo_test_support::compare::assert_e2e;
 use cargo_test_support::paths;
 use cargo_test_support::prelude::*;
 use cargo_test_support::registry::Package;
 use cargo_test_support::str;
-use cargo_test_support::{
-    basic_bin_manifest, basic_manifest, main_file, project, rustc_host, Project,
-};
+use cargo_test_support::{assert_deps, assert_deps_contains};
+use cargo_test_support::{basic_bin_manifest, basic_manifest, main_file, project, rustc_host};
 use filetime::FileTime;
 
-// Helper for testing dep-info files in the fingerprint dir.
-#[track_caller]
-fn assert_deps(project: &Project, fingerprint: &str, test_cb: impl Fn(&Path, &[(u8, &str)])) {
-    let mut files = project
-        .glob(fingerprint)
-        .map(|f| f.expect("unwrap glob result"))
-        // Filter out `.json` entries.
-        .filter(|f| f.extension().is_none());
-    let info_path = files
-        .next()
-        .unwrap_or_else(|| panic!("expected 1 dep-info file at {}, found 0", fingerprint));
-    assert!(files.next().is_none(), "expected only 1 dep-info file");
-    let dep_info = fs::read(&info_path).unwrap();
-    let dep_info = &mut &dep_info[..];
-    let deps = (0..read_usize(dep_info))
-        .map(|_| {
-            let ty = read_u8(dep_info);
-            let path = str::from_utf8(read_bytes(dep_info)).unwrap();
-            let checksum_present = read_bool(dep_info);
-            if checksum_present {
-                // Read out the checksum info without using it
-                let _file_len = read_u64(dep_info);
-                let _checksum = read_bytes(dep_info);
-            }
-            (ty, path)
-        })
-        .collect::<Vec<_>>();
-    test_cb(&info_path, &deps);
-
-    fn read_usize(bytes: &mut &[u8]) -> usize {
-        let ret = &bytes[..4];
-        *bytes = &bytes[4..];
-
-        u32::from_le_bytes(ret.try_into().unwrap()) as usize
-    }
-
-    fn read_u8(bytes: &mut &[u8]) -> u8 {
-        let ret = bytes[0];
-        *bytes = &bytes[1..];
-        ret
-    }
-
-    fn read_bool(bytes: &mut &[u8]) -> bool {
-        read_u8(bytes) != 0
-    }
-
-    fn read_u64(bytes: &mut &[u8]) -> u64 {
-        let ret = &bytes[..8];
-        *bytes = &bytes[8..];
-
-        u64::from_le_bytes(ret.try_into().unwrap())
-    }
-
-    fn read_bytes<'a>(bytes: &mut &'a [u8]) -> &'a [u8] {
-        let n = read_usize(bytes);
-        let ret = &bytes[..n];
-        *bytes = &bytes[n..];
-        ret
-    }
-}
-
-fn assert_deps_contains(project: &Project, fingerprint: &str, expected: &[(u8, &str)]) {
-    assert_deps(project, fingerprint, |info_path, entries| {
-        for (e_kind, e_path) in expected {
-            let pattern = glob::Pattern::new(e_path).unwrap();
-            let count = entries
-                .iter()
-                .filter(|(kind, path)| kind == e_kind && pattern.matches(path))
-                .count();
-            if count != 1 {
-                panic!(
-                    "Expected 1 match of {} {} in {:?}, got {}:\n{:#?}",
-                    e_kind, e_path, info_path, count, entries
-                );
-            }
-        }
-    })
-}
-
 #[cargo_test]
 fn build_dep_info() {
     let p = project()