Ask git where its daemon is and use that#1697

EliahKagan

This changes the test helpers on Windows to use git --exec-path (with whatever git GitPython is using) to find the directory that contains git-daemon.exe, instead of finding git-daemon.exe in a PATH search.

Because it is only on Windows that the tests run git-daemon directly rather than using git daemon, this only affects Windows. Note that this does not affect Cygwin, which like other Unix-like systems uses git daemon for the tests.

This change has three benefits:

The documentation is shortened and simplified, because the part on how to get git-daemon working on Windows is removed.
The effort required to set up GitPython for testing is reduced because that no longer has to be done (but see below).
Users will not be tempted to put Git's libexec\git-core directory in their PATH. This directory contains a number of .dll files, which are there for the executables in the directory that use them. But Windows includes all PATH directories when searching for libraries as well as programs, which can create very strange and confusing problems where completely unrelated programs end up using them instead of the versions of the libraries they should use.

Note, however, what it does not cover:

The tests that use git-daemon are still skipped by default on Windows. These are exactly the tests that are skipped when HIDE_WINDOWS_FREEZE_ERRORS is set to True, which is the case by default on Windows. I temporarily changed its default value to False to test this PR. That change is deliberately not included in this PR and probably should not be made without further accompanying changes. (See this comment, but also the information below about how the tests usually fail, which is not by freezing.)
The mechanism to skip them remains to be fixed. The HIDE_WINDOWS_KNOWN_ERRORS and HIDE_WINDOWS_FREEZE_ERRORS variables are intended to be affected by the same-named environment variables, but this does not work properly. They are True by default, but the only way to use environment variables to set them to a false value is to define the environment variable with an empty value (since everything else, as a string, is truthy). In addition to being very unlikely to have been the intended behavior, this is also hard to do on Windows when using typical shells, because in both cmd.exe and PowerShell setting an environment variable as empty actually removes the variable altogether. Rather than attempting to fix that here, I tested by temporarily changing the default value instead of using an environment variable.
The tests that use git-daemon still usually fail on Windows. The failure mode is the same as when they are run with git-daemon.exe in a directory in PATH, however. That is, when git-daemon is not found it looks like this, while when it is found by a PATH search it looks like this, and the new way, finding it via git --exec-path, looks similar. Note how all errors of this form occur in the log with-no-git-daemon.txt:
```
git.exc.GitCommandNotFound: Cmd('git-daemon') not found due to: FileNotFoundError('[WinError 2] The system cannot find the file specified')
```
The files can be compared with diff or with this convenient diff tool. I would also be happy to produce diffs using a particular technique, on request. Those diffs were also produced before rebasing this to include the changes from #1693, since I had neglected by merge that on my Windows system before working on this, but I believe none of the changes there are able to affect this.

That is to say that this PR is very narrow in its scope of effects--it automates, and at least as reliably achieves, the desired effect of a setup step that had to be done manually before, but it doesn't address any of the other problems related to it.

One aspect of the way I did this change deserves special scrutiny; I've made a review comment about it.

This changes the test helpers on Windows to use "git --exec-path" (with whatever "git" GitPython is using) to find the directory that contains "git-daemon.exe", instead of finding it in a PATH search.

Byron

Thanks so much for your help! Despite a small improvement, it's much appreciated and I see how it will help to get the tests on windows to work.

Since gitoxide also has tests that rely on git daemon to run which work on Windows, I'd believe that getting these to work should generally possible.

Regarding the implementation of Windows- and test-specific environment variables, I hope their implementation can be adjusted to be usable on Windows as well.

Thanks for everything.

EliahKagan

Regarding the implementation of Windows- and test-specific environment variables, I hope their implementation can be adjusted to be usable on Windows as well.

I've described this in #1698 and proposed a fix as part of #1700.

(In the longer term, I hope these can be eliminated or at least moved, along with all associated test-specific logic, out of the git module, either of which would solve #790.)

This removes the Windows-specific information in the warning message in git_daemon_launched. After the associated functionality was updated in gitpython-developers#1684 and the warning message was abridged accordingly, the functionality was updated again in gitpython-developers#1697, causing the message to be outdated and no longer helpeful (since having git-daemon.exe in a PATH directory is no longer necessary or useful), without any corresponding change to the message.

Ask git where its daemon is and use that …

04f3200

This changes the test helpers on Windows to use "git --exec-path" (with whatever "git" GitPython is using) to find the directory that contains "git-daemon.exe", instead of finding it in a PATH search.

EliahKagan force-pushed the find-daemon branch from 8f5f65d to 04f3200 Compare October 7, 2023 16:28

EliahKagan commented Oct 7, 2023

View reviewed changes

EliahKagan marked this pull request as ready for review October 7, 2023 16:38

Byron added this to the v3.1.38 - Bugfixes milestone Oct 8, 2023

Byron approved these changes Oct 8, 2023

View reviewed changes

Byron merged commit 8107cbf into gitpython-developers:main Oct 8, 2023

EliahKagan deleted the find-daemon branch October 8, 2023 07:03

EliahKagan mentioned this pull request Oct 9, 2023

HIDE_WINDOWS_*_ERRORS environment variables are interpreted strangely #1698

Closed

EliahKagan mentioned this pull request Oct 12, 2023

Update CI, in line with gitdb gitpython-developers/smmap#53

Merged

EliahKagan mentioned this pull request Jan 19, 2024

Override bash executable, defaulting to Git for Windows git bash over WSL bash #1791

Closed

EliahKagan mentioned this pull request Feb 23, 2024

Fix version_info cache invalidation, typing, parsing, and serialization #1838

Merged

EliahKagan mentioned this pull request Mar 30, 2024

Issue and test deprecation warnings #1886

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ask git where its daemon is and use that#1697

Ask git where its daemon is and use that#1697
Byron merged 1 commit into
gitpython-developers:mainfrom
EliahKagan:find-daemon

EliahKagan commented Oct 7, 2023 •

edited

Loading

Uh oh!

Byron left a comment

Uh oh!

EliahKagan commented Oct 9, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

EliahKagan commented Oct 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Byron left a comment

Choose a reason for hiding this comment

Uh oh!

EliahKagan commented Oct 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

EliahKagan commented Oct 7, 2023 •

edited

Loading

EliahKagan commented Oct 9, 2023 •

edited

Loading