Skip to content
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

"not a git repository" when running pre-commit hook on older git client #203

Closed
rogalski opened this issue Sep 23, 2021 · 4 comments
Closed
Assignees
Labels
bug Something isn't working maybe invalid? Can't reproduce, or seems already fixed, or need more information question Further information is requested
Milestone

Comments

@rogalski
Copy link
Contributor

rogalski commented Sep 23, 2021

This bug is based on my experiments with darker on closed-source repo.
I am not at liberty to disclose any code. We'll have to work on recreating failure criteria on our own.

Steps to reproduce:

  1. Use darker as pre-commit hook with older Git (e.g. Git 2.7.x on Ubuntu 16.04 LTS)
  2. Modify python file stored in subdirectory of repository, e.g. /repository/subdir/file.py
  3. Run darker in pre-commit hook, e.g. using pre-commit.

What happens:

  1. Common root is resolved to /repository/subdir, not /repository
  2. Unfortunately, older Git client runs hook with GIT_DIR=.git environment variable
  3. This environment variable disables traversal in search for actual .git directory
  4. Git expects (due to cwd and env vars) that repo is in /repository/subdirectory/.git, not /repository/.git
  5. Darker crashes with not a repository error.

Possible fixes:

  1. Leave as it is (treat old Git client as unsupported)
  2. Drop GIT_DIR from our git subprocess invocations
  3. Run traversal manually to always use repository top-level dir as cwd
@akaihola akaihola self-assigned this Sep 24, 2021
@akaihola akaihola added the bug Something isn't working label Sep 24, 2021
@akaihola akaihola added this to the 1.3.1 milestone Sep 24, 2021
@akaihola
Copy link
Owner

akaihola commented Sep 24, 2021

Thanks @rogalski, that's very informative! This may be a duplicate of issue #112.

Could you try upgrading darker to master in your .pre-commit.yaml

- repo: https://github.com/akaihola/darker
  rev: master

to see whether #185 fixed this (included in the soon-to-be-released 1.3.1 version) as an unintended side effect?

(The original intention of #185 was to just add a fixed locale to Git invocations, but it turned out all other environment variables were removed, too.)

@akaihola akaihola added the question Further information is requested label Sep 24, 2021
@akaihola akaihola added the maybe invalid? Can't reproduce, or seems already fixed, or need more information label Oct 5, 2021
@akaihola
Copy link
Owner

akaihola commented Oct 5, 2021

Ping @rogalski, could you try if #203 is solved already in master?

@akaihola akaihola modified the milestones: 1.3.1, 1.3.2 Oct 5, 2021
@akaihola
Copy link
Owner

@rogalski, does version 1.3.2 solve this problem?

@akaihola akaihola modified the milestones: 1.3.2, 1.4.0 Oct 28, 2021
@akaihola
Copy link
Owner

I tried to reproduce the original /repository/subdir/file.py example in an Ubuntu 16.04 Docker image by installing Python 3.6 from the deadsnakes repository and then pip installing pre-commit.

Unfortunately I couldn't get the not a repository error with any version of Darker.

I'm closing this issue – @rogalski please submit it again with step-by-step instructions for reproducing the problem if the latest Darker version still vails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working maybe invalid? Can't reproduce, or seems already fixed, or need more information question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants