Skip to content

Commit

Permalink
fix: fix satisfiability for short sha hashes (prefix-dev#1530)
Browse files Browse the repository at this point in the history
Short sha hashes failed satisfiability checks, this has been fixed and
the test has been updated to include said check
  • Loading branch information
tdejager authored Jun 24, 2024
1 parent f2f4bd7 commit e6ee9c1
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 3 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ repos:
name: clippy
language: system
types: [file, rust]
entry: cargo clippy --all -- -D warnings # Use -D warnings option to ensure the job fails when encountering warnings
entry: cargo clippy --all -- -D warnings -Dclippy::dbg_macro # Use -D warnings option to ensure the job fails when encountering warnings
pass_filenames: false

- id: test
Expand Down
36 changes: 34 additions & 2 deletions src/lock_file/satisfiability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,20 @@ pub fn pypi_satifisfies_editable(
if *spec_git_url.url.reference() == GitReference::DefaultBranch {
return base_is_same;
}

// If the spec has a short commit than we can do a partial match
// E.g `git.com/user/repo@adbdd` is the same as `git.com/user/repo@adbdd123`
// Currently this resolves to BranchOrTag
if let GitReference::BranchOrTag(branch_or_tag) = spec_git_url.url.reference() {
if seems_like_commit_sha(branch_or_tag) {
// We expect the lock file to have a long commit hash
// in this case
if let GitReference::FullCommit(sha) = locked_data_url.url.reference() {
return base_is_same && sha.starts_with(branch_or_tag);
}
}
}

// If the spec does specify a revision than the revision must match
base_is_same && spec_git_url.url.reference() == locked_data_url.url.reference()
} else {
Expand All @@ -333,6 +347,11 @@ pub fn pypi_satifisfies_editable(
}
}

/// Checks if the string seems like a git commit sha
fn seems_like_commit_sha(s: &str) -> bool {
s.len() >= 4 && s.chars().all(|c| c.is_ascii_hexdigit())
}

/// Check satatisfiability of a pypi requirement against a locked pypi package
/// This also does an additional check for git urls when using direct url references
pub fn pypi_satifisfies_requirement(locked_data: &PypiPackageData, spec: &Requirement) -> bool {
Expand Down Expand Up @@ -370,6 +389,19 @@ pub fn pypi_satifisfies_requirement(locked_data: &PypiPackageData, spec: &Requir
if *spec_git_url.url.reference() == GitReference::DefaultBranch {
return base_is_same;
}
// If the spec has a short commit than we can do a partial match
// E.g `git.com/user/repo@adbdd` is the same as `git.com/user/repo@adbdd123`
// Currently this resolves to BranchOrTag
if let GitReference::BranchOrTag(branch_or_tag) = spec_git_url.url.reference() {
if seems_like_commit_sha(branch_or_tag) {
// We expect the lock file to have a long commit hash
// in this case
if let GitReference::FullCommit(sha) = locked_data_url.url.reference() {
return base_is_same && sha.starts_with(branch_or_tag);
}
}
}

// If the spec does specify a revision than the revision must match
base_is_same && spec_git_url.url.reference() == locked_data_url.url.reference()
} else {
Expand Down Expand Up @@ -980,15 +1012,15 @@ mod tests {
let locked_data = PypiPackageData {
name: "mypkg".parse().unwrap(),
version: Version::from_str("0.1.0").unwrap(),
url_or_path: "git+https://github.com/mypkg@abcd"
url_or_path: "git+https://github.com/mypkg@29932f3915935d773dc8d52c292cadd81c81071d"
.parse()
.expect("failed to parse url"),
hash: None,
requires_dist: vec![],
requires_python: None,
editable: false,
};
let spec = Requirement::from_str("mypkg @ git+https://github.com/mypkg@abcd").unwrap();
let spec = Requirement::from_str("mypkg @ git+https://github.com/mypkg@2993").unwrap();
// This should satisfy:
assert!(pypi_satifisfies_requirement(&locked_data, &spec));
let non_matching_spec =
Expand Down

0 comments on commit e6ee9c1

Please sign in to comment.