Skip to content

Add missing git feature dep to preserve_executable_bit test#12850

Merged
charliermarsh merged 1 commit intoastral-sh:mainfrom
mgorny:git-feat
May 18, 2025
Merged

Add missing git feature dep to preserve_executable_bit test#12850
charliermarsh merged 1 commit intoastral-sh:mainfrom
mgorny:git-feat

Conversation

@mgorny
Copy link
Contributor

@mgorny mgorny commented Apr 12, 2025

Summary

Without the git feature, it fails with:

error: Failed to initialize Git repository at `/home/mgorny/.local/share/uv/tests/.tmp01wGGK/temp/preserve_executable_bit`
stdout:
stderr: error: `git` operations are not allowed — are you missing a cfg for the `git` feature?

Test Plan

cargo test --features python --profile=fast-build --no-default-features

Without the `git` feature, it fails with:

```
error: Failed to initialize Git repository at `/home/mgorny/.local/share/uv/tests/.tmp01wGGK/temp/preserve_executable_bit`
stdout:
stderr: error: `git` operations are not allowed — are you missing a cfg for the `git` feature?
```
@charliermarsh
Copy link
Member

@konstin -- Do you know why this test uses Git? I think uv init is meant to be robust to a missing git.

@charliermarsh charliermarsh added the bug Something isn't working label Apr 14, 2025
@konstin
Copy link
Member

konstin commented Apr 14, 2025

We first check if we are in a git repository in uv init. If that fails, we fall back to assuming VCS defaults (init a git repo). We don't check the error condition, so both fatal: not a git repository (or any of the parent directories): .git and our git blocking shim cause this path. We then try to init a git repo, but in that case we only ignore a missing git installation, but fail at the git blocking shim.

Can we rely on errors such as not a git repository being stable? If so, we could differentiate between the two cases.

@konstin konstin added the internal A refactor or improvement that is not user-facing label Apr 14, 2025
@charliermarsh
Copy link
Member

But doesn't "init a Git repo" return VersionControlError::GitNotInstalled, which we explicitly handle?

@konstin
Copy link
Member

konstin commented Apr 14, 2025

Our shims put a git in PATH so we're falling in a gap where git is installed (which git find the git-not-allowed shim), but invoking git errors with the error seen above:

First we take the None-branch, since detecting an existing git repo errors (through the shim):

But that was a check for an existing git repo, so we ignore it:

(None, None) => VersionControlSystem::default(),

When we finally initialize the repo, we can run the git shim, but it errors on execution:

if !output.status.success() {

If we want to handle this better, we should find a way to better tell apart "there is no git repo" (this is fine) from "git errored" (don't use git at all) in the first check.

@konstin
Copy link
Member

konstin commented Apr 15, 2025

I tried to make this more resilient: #12895

@charliermarsh charliermarsh merged commit 061751f into astral-sh:main May 18, 2025
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working internal A refactor or improvement that is not user-facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants