Have initial refresh use a logger to warn by EliahKagan · Pull Request #1815 · gitpython-developers/GitPython

added 4 commits

February 2, 2024 05:17
This adds tests like the existing ones but for when git.refresh is
called with no arguments and the path is provided as the value of
the GIT_PYTHON_GIT_EXECUTABLE environment variable.

The preexisting tests, which this retains unchanged except with
slightly more specific names to avoid confusion with the new tests,
are of git.refresh(path).

One benefit of these tests is to make a subtle but important aspect
of the established behavior clear: relative paths are immediately
resolved when passed as a path argument, but they are kept relative
when given as the value of the environment variable.
This adds tests of the initial refresh that is attempted
automatically on import. All the refresh tests prior to this point
test subsequent refreshes. Those tests are kept, and new ones are
added that simulate the condition of not having yet done the
initial refresh by setting Git.GIT_PYTHON_GIT_EXECUTABLE to None.

Some current behavior these tests assert may change for gitpython-developers#1808.
This is instead of the current behavior writing the message to
stdout.

This commit does not change the behavior of the code under test,
but it changes tests to assert the following:

- "Bad git executable" messages are logged, at level CRITICAL.
- "log" (and "l") is recognized as another synonym of "warn".
- "silent" is recognized as a synonym of "quiet" (as "silence" is).

Although it is ambiguous whether this should be logged at level
ERROR or CRITICAL, because git.refresh is still expected to be
usable and can be called manually, not having a working git is a
condition in which GitPython, and any program that really relies on
its functionality, should be expected not work. That is the general
rationale for using CRIICAL here. There are also two specific
reasons:

- Existing messages GitPython logs as ERROR typically represent
  errors in individual operations on repositories, which could fail
  without indicating that GitPython's overall functionality is in
  an unusable state. Using the higher CRITICAL level for this
  situation makes sense for contrast.

- Prior to gitpython-developers#1813, logging messsges emitted from GitPython modules,
  no matter the level, were suppressed when logging was not
  configured, but because this message was printed instead of
  being logged, it was still shown. Now that it is to be logged,
  there may be a benefit to have an easy way for someone to bring
  back a close approximation of the old behavior. Having this
  message be at a higher logging level makes that easier to do.
  (This is a less important reason, as that should rarely be done.)

test_initial_refresh_from_bad_git_path_env_warn is the main changed
test. All tests should pass again once code is changed for gitpython-developers#1808.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Feb 6, 2024
This changes test_initial_refresh_from_bad_git_path_env_warn to
assert a message with no added "WARNING:" prefix. As discussed
in gitpython-developers#1815, the extra prefix is confusing with logging configured,
showing "CRITICAL:git.cmd:WARNING: Bad git executable." instead of
"CRITICAL:git.cmd: Bad git executable."

lettuce-bot bot referenced this pull request in lettuce-financial/github-bot-signed-commit

Feb 15, 2024

renovate bot referenced this pull request in allenporter/flux-local

Feb 16, 2024

This was referenced

Feb 19, 2024

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Feb 24, 2024
The fragment of the refresh failure message (which is often written
to a terminal) about the effect of setting GIT_PYTHON_REFRESH to
control refesh failure reporting had previously been formatted with
a maximum line length of 79 columns--in the message itself, not the
code for the message--so that it would fit in a traditional 80
column wide terminal. This remains one of the popular widths to set
for terminal windows in a GUI, so it seems worthwhile to preserve.

In 3a6e3ef (gitpython-developers#1815), I had inadvertently made the line one character
too long for that; at 80 columns, it would cause the newline to be
written at the start of the next line, creating an unsightly extra
line break.

This is pretty minor, but what seems to me like an equally good
alternative wording avoids it, so this commit shortens the wording.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Feb 26, 2024
The fragment of the refresh failure message (which is often written
to a terminal) about the effect of setting GIT_PYTHON_REFRESH to
control refesh failure reporting had previously been formatted with
a maximum line length of 79 columns--in the message itself, not the
code for the message--so that it would fit in a traditional 80
column wide terminal. This remains one of the popular widths to set
for terminal windows in a GUI, so it seems worthwhile to preserve.

In 3a6e3ef (gitpython-developers#1815), I had inadvertently made the line one character
too long for that; at 80 columns, it would cause the newline to be
written at the start of the next line, creating an unsightly extra
line break.

This is pretty minor, but what seems to me like an equally good
alternative wording avoids it, so this commit shortens the wording.