Skip to content

Fix version json tests to work outside git checkout#13566

Merged
Gankra merged 6 commits intoastral-sh:mainfrom
mgorny:commit-info-nogit
May 21, 2025
Merged

Fix version json tests to work outside git checkout#13566
Gankra merged 6 commits intoastral-sh:mainfrom
mgorny:commit-info-nogit

Conversation

@mgorny
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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