Skip to content

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

Merged
merged 6 commits into from
May 21, 2025

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented May 21, 2025

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

  1. cargo test in a git clone.
  2. cargo clean, moved .git away, cargo test again.

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
@konstin konstin requested a review from Gankra May 21, 2025 09:05
Copy link
Contributor

@Gankra Gankra left a 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!

Comment on lines 18 to 23
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");
}
}
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks!

@Gankra Gankra merged commit dbcfe9f into astral-sh:main May 21, 2025
86 checks passed
@Gankra Gankra added the internal A refactor or improvement that is not user-facing label May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal A refactor or improvement that is not user-facing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

version::self_version_json and version::version_get_fallback_unmanaged_json test failures (outside git checkout?)
2 participants