Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: track version in fingerprint dep-info files #14751

Merged
merged 3 commits into from
Oct 31, 2024
Merged

fix: track version in fingerprint dep-info files #14751

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1fcf5ee
test(depinfo): basic serialization round-trip test
weihanglo Oct 30, 2024
6f2b5d9
test(depinfo): current cargo unable to parse v0 or v1
weihanglo Oct 30, 2024
634450e
fix: track version in fingerprint dep-info files
weihanglo Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 55 additions & 1 deletion src/cargo/core/compiler/fingerprint/dep_info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub enum DepInfoPathType {
/// | len of key | key bytes | value exists? | len of value | value bytes |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also have a sanity-check value on "len of key" We likely should never have a dep info key that is larger than 10k for instance

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(and ture for any length we deal with)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure. What if they do track that many environment variables?

An allocation error seems reasonable in that case. Cargo may crash in other places even it doesn't here.

Copy link
Member Author

@weihanglo weihanglo Oct 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative is https://doc.rust-lang.org/nightly/std/vec/struct.Vec.html#method.try_with_capacity.
(not yet stable though)

though somehow the core dumpe happened with the first push, not when calling with_capacity.

/// +------------+-----------+---------------+--------------+-------------+
/// ```
#[derive(Default)]
#[derive(Default, Debug, PartialEq, Eq)]
pub struct EncodedDepInfo {
pub files: Vec<(DepInfoPathType, PathBuf, Option<(u64, String)>)>,
pub env: Vec<(String, Option<String>)>,
Expand Down Expand Up @@ -614,3 +614,57 @@ pub enum InvalidChecksum {
#[error("expected a string with format \"algorithm=hex_checksum\"")]
InvalidFormat,
}

#[cfg(test)]
mod encoded_dep_info {
use super::*;

#[track_caller]
fn gen_test(checksum: bool) {
let checksum = checksum.then_some((768, "c01efc669f09508b55eced32d3c88702578a7c3e".into()));
let lib_rs = (
DepInfoPathType::TargetRootRelative,
PathBuf::from("src/lib.rs"),
checksum.clone(),
);

let depinfo = EncodedDepInfo {
files: vec![lib_rs.clone()],
env: Vec::new(),
};
let data = depinfo.serialize().unwrap();
assert_eq!(EncodedDepInfo::parse(&data).unwrap(), depinfo);

let mod_rs = (
DepInfoPathType::TargetRootRelative,
PathBuf::from("src/mod.rs"),
checksum.clone(),
);
let depinfo = EncodedDepInfo {
files: vec![lib_rs.clone(), mod_rs.clone()],
env: Vec::new(),
};
let data = depinfo.serialize().unwrap();
assert_eq!(EncodedDepInfo::parse(&data).unwrap(), depinfo);

let depinfo = EncodedDepInfo {
files: vec![lib_rs, mod_rs],
env: vec![
("Gimli".into(), Some("Legolas".into())),
("Beren".into(), Some("Lúthien".into())),
],
};
let data = depinfo.serialize().unwrap();
assert_eq!(EncodedDepInfo::parse(&data).unwrap(), depinfo);
}

#[test]
fn round_trip() {
gen_test(false);
}

#[test]
fn round_trip_with_checksums() {
gen_test(true);
}
}