-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix version json tests to work outside git checkout #13566
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.
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
Conversation
Fix the two version json tests to account for the possibility that uv was built outside a git checkout (e.g. from an unpacked git archive) and therefore does not have the commit info available. This approach uses separate snapshots for the two cases, as suggested in discussion of pull request astral-sh#13251. Fixes astral-sh#13212
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh geez, apologies, finishing this work slipped through the cracks!
crates/uv/build.rs
Outdated
fn has_git(workspace_root: &Path) { | ||
let git_dir = workspace_root.join(".git"); | ||
if git_dir.exists() { | ||
println!("cargo:rustc-env=UV_TEST_HAS_COMMIT_HASH=1"); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing this in a build.rs seems like an extremely heavy hammer that might mess up build caching -- can we just do this .git check in the tests themselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose so, but I have no clue how to get the right path :-). I should probably mention I don't really know Cargo/Rust, and I'm just bashing things randomly until they work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok no prob, I'll take a quick crack at it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Summary
Fix the two version json tests to account for the possibility that uv was built outside a git checkout (e.g. from an unpacked git archive) and therefore does not have the commit info available. This approach uses separate snapshots for the two cases, as suggested in discussion of pull request #13251.
Fixes #13212
Test Plan
cargo test
in a git clone.cargo clean
, moved.git
away,cargo test
again.