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

Avoid Git's dubious ownership error #74

Merged
merged 4 commits into from
Aug 9, 2024
Merged

Conversation

akaihola
Copy link
Owner

@akaihola akaihola commented Jul 31, 2024

When running Git, don't remove any environment variables. Instead, just override LC_ALL=C.UTF-8. Also add a test which verifies that this is sufficient to ensure Git's output is in English.

Fixes akaihola/darker#524
Fixes akaihola/darker#590


The original flawed plan was:

add some environment variables which are needed to avoid Git's dubious ownership detection:

  • $HOME on Windows/MinGW
  • $EUID and $SUDO_UID on Unix

See:

@akaihola akaihola added the bug Something isn't working label Jul 31, 2024
@akaihola akaihola added this to the Darkgraylib 2.0.1 milestone Jul 31, 2024
@akaihola akaihola self-assigned this Jul 31, 2024
@akaihola akaihola changed the title Git dubious ownership Avoid Git's dubious ownership error Jul 31, 2024
@DeinAlptraum
Copy link
Collaborator

I've tested this via

pip install darkgraylib@git+https://github.com/akaihola/darkgraylib@git-dubious-ownership

like you said, but I am still getting the error. If I understood correctly, your change works specifically for the "ownership" case, i.e. when the "dubious" repo is owned by the user. That is not the case for me afaik, I am passing the check because I have an exemption for the directory in my ~/.gitconfig, but apparently the environment changes make git unable to find the config file. I.e. the relevant check in my case is this: https://github.com/git/git/blob/39bf06adf96da25b87c9aa7d35a32ef3683eb4a4/setup.c#L1281
rather than the one 10 lines above it

Side note: the installation command I quoted above produces errors due to version incompatibility with current darker, which I've fixed manually to test this

ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
darker 2.1.1 requires darkgraylib~=1.2.0, but you have darkgraylib 2.0.0 which is incompatible.
graylint 1.1.1 requires darkgraylib~=1.2.0, but you have darkgraylib 2.0.0 which is incompatible.

and calling darker after that crashes with the following error:

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "~/.local/share/virtualenvs/python-1_CbI4TK/lib/python3.12/site-packages/darker/__main__.py", line 641, in <module>
    RETVAL = main_with_error_handling()
             ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "~./local/share/virtualenvs/python-1_CbI4TK/lib/python3.12/site-packages/darker/__main__.py", line 633, in main_with_error_handling
    return main()
           ^^^^^^
  File "~/.local/share/virtualenvs/python-1_CbI4TK/lib/python3.12/site-packages/darker/__main__.py", line 495, in main
    show_config_if_debug(config, config_nondefault, args.log_level)
TypeError: show_config_if_debug() missing 1 required positional argument: 'section_name'

which I've fixed manually by passing None as a fourth argument

@akaihola
Copy link
Owner Author

akaihola commented Aug 1, 2024

Thanks @DeinAlptraum, this clarified the issue! I refreshed my memory from Darker's commit history, and the only reason for invoking Git with a clean environment was to ensure its terminal output is always in English. So I'll attack this from the opposite direction and only modify locale related variables while keeping the rest of the environment intact.

The Darker/Darkgraylib version incompatibility was expected, I'll make sure to take care of that one too.

@akaihola akaihola requested a review from DeinAlptraum August 1, 2024 06:28
@akaihola
Copy link
Owner Author

akaihola commented Aug 1, 2024

@DeinAlptraum, the latest fix in this branch now keeps the evironment and only modifies LC_ALL. You can try it out with:

pip install darker@git+https://github.com/akaihola/darker@master
pip install darkgraylib@git+https://github.com/akaihola/darkgraylib@git-dubious-ownership

Copy link
Collaborator

@DeinAlptraum DeinAlptraum left a comment

Choose a reason for hiding this comment

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

Small comments, but this looks good to me and fixes the issue on my end!

src/darkgraylib/git.py Outdated Show resolved Hide resolved
src/darkgraylib/git.py Outdated Show resolved Hide resolved
src/darkgraylib/tests/test_git.py Show resolved Hide resolved
`LC_ALL` takes precedence over other language affecting locale
environment variables. Set it to `C.UTF-8` before calling Git, and
otherwise pass through the environment unmodified.
@akaihola akaihola force-pushed the git-dubious-ownership branch from c28c65a to e95b765 Compare August 9, 2024 06:19
@akaihola akaihola merged commit 25b2013 into main Aug 9, 2024
35 checks passed
@akaihola akaihola deleted the git-dubious-ownership branch August 9, 2024 06:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

dubious ownership error in GitLab CI
2 participants