Skip to content

Commit

Permalink
Align with cargo (#69)
Browse files Browse the repository at this point in the history
As noted in the comment, this changes the url kind from `u64` -> `usize`
to match cargo, which unfortunately uses the default `derive(Hash)`
which for enums uses `::core::intrinsics::discriminant_value` which for
non-repr enums is...pointer size. This means the hash computed is
different between 32 and 64-bit targets when they could easily be the
same. :(

Resolves:
EmbarkStudios/cargo-deny#540 (comment)
  • Loading branch information
Jake-Shadle authored Aug 1, 2024
1 parent b964b2c commit bf367b8
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

<!-- next-header -->
## [Unreleased] - ReleaseDate
### Fixed
- [PR#69](https://github.com/EmbarkStudios/tame-index/pull/69) resolved an issue where 32-bit targets would have a different ident hash from what cargo would have due to cargo being target dependent in the hash calculation.

## [0.13.0] - 2024-07-25
### Changed
- [PR#67](https://github.com/EmbarkStudios/tame-index/pull/67) updated `gix` -> 0.64.
Expand Down
29 changes: 24 additions & 5 deletions src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,16 @@ pub fn canonicalize_url(url: &str) -> Result<String, Error> {
pub fn url_to_local_dir(url: &str) -> Result<UrlDir, Error> {
use std::hash::{Hash, Hasher, SipHasher};

const GIT_REPO: u64 = 0;
const GIT_REGISTRY: u64 = 2;
const SPARSE_REGISTRY: u64 = 3;
// This is extremely irritating, but we need to use usize for the kind, which
// impacts the hash calculation, making it different based on pointer size.
//
// The reason for this is that cargo just uses #[derive(Hash)] for the SourceKind
// https://github.com/rust-lang/cargo/blob/88b4b3bcd3bbb66873734d97ae412a6bcf9b75ee/crates/cargo-util-schemas/src/core/source_kind.rs#L4-L5,
// which then uses https://doc.rust-lang.org/core/intrinsics/fn.discriminant_value.html
// to get the discriminant and add to the hash...and that is pointer width :(
const GIT_REPO: usize = 0;
const GIT_REGISTRY: usize = 2;
const SPARSE_REGISTRY: usize = 3;

// Ensure we have a registry or bare url
let (url, scheme_ind, kind) = {
Expand Down Expand Up @@ -272,7 +279,7 @@ mod test {
use crate::PathBuf;

#[test]
#[cfg(target_endian = "little")]
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
fn canonicalizes_git_urls() {
let super::UrlDir { dir_name, canonical } = url_to_local_dir("git+https://github.com/EmbarkStudios/cpal.git?rev=d59b4de#d59b4decf72a96932a1482cc27fe4c0b50c40d32").unwrap();

Expand Down Expand Up @@ -322,7 +329,7 @@ mod test {
}

#[test]
#[cfg(target_endian = "little")]
#[cfg(all(target_pointer_width = "64", target_endian = "little"))]
fn matches_cargo() {
assert_eq!(
get_index_details(crate::CRATES_IO_INDEX, Some(PathBuf::new())).unwrap(),
Expand Down Expand Up @@ -369,6 +376,18 @@ mod test {
);
}

#[test]
#[cfg(all(target_pointer_width = "32", target_endian = "little"))]
fn matches_cargo_32bit() {
assert_eq!(
get_index_details(crate::CRATES_IO_HTTP_INDEX, Some(PathBuf::new())).unwrap(),
(
"registry/index/index.crates.io-1cd66030c949c28d".into(),
crate::CRATES_IO_HTTP_INDEX.to_owned(),
)
);
}

#[test]
fn gets_cargo_version() {
const MINIMUM: semver::Version = semver::Version::new(1, 70, 0);
Expand Down
8 changes: 8 additions & 0 deletions tests/cache.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
#![cfg(target_pointer_width = "64")]

//! Note we only run these tests if _built_ for 64-bit, as the only test targets
//! this project cares about are 64 bit, and the issue is that, to match cargo
//! we need to calculate the hash of the index the same, the rub is that when
//! running a 32-bit target (eg. i686) on a 64-bit host where the cargo on the
//! host is built for the 64-bit target as well, the local directories won't match
mod utils;

use tame_index::{index::cache::ValidCacheEntry, utils::get_index_details, IndexCache};
Expand Down

0 comments on commit bf367b8

Please sign in to comment.