Don't suppress messages when logging is not configured by EliahKagan · Pull Request #1813 · gitpython-developers/GitPython · GitHub
Skip to content

Don't suppress messages when logging is not configured #1813

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.

Sign up for GitHub

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

Merged
Byron merged 4 commits into from
Jan 26, 2024
Merged

Don't suppress messages when logging is not configured #1813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
987dbf4
Use consistent style for passing logger name
EliahKagan Jan 26, 2024
7bbccb4
Rename logger globals from log to _logger
EliahKagan Jan 26, 2024
78a82b3
Don't access logger in tests by nonpublic attribute
EliahKagan Jan 26, 2024
bc42ee5
Don't add `NullHandler`s
EliahKagan Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 10 additions & 11 deletions git/cmd.py
View file
Open in desktop
9 changes: 3 additions & 6 deletions git/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@

__all__ = ("GitConfigParser", "SectionConstraint")


log = logging.getLogger("git.config")
log.addHandler(logging.NullHandler())

_logger = logging.getLogger(__name__)

CONFIG_LEVELS: ConfigLevels_Tup = ("system", "user", "global", "repository")
"""The configuration level of a configuration file."""
Expand Down Expand Up @@ -412,7 +409,7 @@ def release(self) -> None:
try:
self.write()
except IOError:
log.error("Exception during destruction of GitConfigParser", exc_info=True)
_logger.error("Exception during destruction of GitConfigParser", exc_info=True)
except ReferenceError:
# This happens in Python 3... and usually means that some state cannot be
# written as the sections dict cannot be iterated. This usually happens when
Expand Down Expand Up @@ -712,7 +709,7 @@ def write(self) -> None:
# END assert multiple files

if self._has_includes():
log.debug(
_logger.debug(
"Skipping write-back of configuration file as include files were merged in."
+ "Set merge_includes=False to prevent this."
)
Expand Down
9 changes: 4 additions & 5 deletions git/objects/commit.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@

# ------------------------------------------------------------------------

log = logging.getLogger("git.objects.commit")
log.addHandler(logging.NullHandler())
_logger = logging.getLogger(__name__)

__all__ = ("Commit",)

Expand Down Expand Up @@ -767,7 +766,7 @@ def _deserialize(self, stream: BytesIO) -> "Commit":
self.author_tz_offset,
) = parse_actor_and_date(author_line.decode(self.encoding, "replace"))
except UnicodeDecodeError:
log.error(
_logger.error(
"Failed to decode author line '%s' using encoding %s",
author_line,
self.encoding,
Expand All @@ -781,7 +780,7 @@ def _deserialize(self, stream: BytesIO) -> "Commit":
self.committer_tz_offset,
) = parse_actor_and_date(committer_line.decode(self.encoding, "replace"))
except UnicodeDecodeError:
log.error(
_logger.error(
"Failed to decode committer line '%s' using encoding %s",
committer_line,
self.encoding,
Expand All @@ -795,7 +794,7 @@ def _deserialize(self, stream: BytesIO) -> "Commit":
try:
self.message = self.message.decode(self.encoding, "replace")
except UnicodeDecodeError:
log.error(
_logger.error(
"Failed to decode message '%s' using encoding %s",
self.message,
self.encoding,
Expand Down
18 changes: 8 additions & 10 deletions git/objects/submodule/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@


# typing ----------------------------------------------------------------------

from typing import Callable, Dict, Mapping, Sequence, TYPE_CHECKING, cast
from typing import Any, Iterator, Union

Expand All @@ -50,14 +51,11 @@
from git.repo import Repo
from git.refs import Head


# -----------------------------------------------------------------------------

__all__ = ["Submodule", "UpdateProgress"]


log = logging.getLogger("git.objects.submodule.base")
log.addHandler(logging.NullHandler())
_logger = logging.getLogger(__name__)


class UpdateProgress(RemoteProgress):
Expand Down Expand Up @@ -731,7 +729,7 @@ def update(
)
mrepo.head.reference.set_tracking_branch(remote_branch)
except (IndexError, InvalidGitRepositoryError):
log.warning("Failed to checkout tracking branch %s", self.branch_path)
_logger.warning("Failed to checkout tracking branch %s", self.branch_path)
# END handle tracking branch

# NOTE: Have to write the repo config file as well, otherwise the
Expand Down Expand Up @@ -761,14 +759,14 @@ def update(
binsha = rcommit.binsha
hexsha = rcommit.hexsha
else:
log.error(
_logger.error(
"%s a tracking branch was not set for local branch '%s'",
msg_base,
mrepo.head.reference,
)
# END handle remote ref
else:
log.error("%s there was no local tracking branch", msg_base)
_logger.error("%s there was no local tracking branch", msg_base)
# END handle detached head
# END handle to_latest_revision option

Expand All @@ -786,15 +784,15 @@ def update(
if force:
msg = "Will force checkout or reset on local branch that is possibly in the future of"
msg += " the commit it will be checked out to, effectively 'forgetting' new commits"
log.debug(msg)
_logger.debug(msg)
else:
msg = "Skipping %s on branch '%s' of submodule repo '%s' as it contains un-pushed commits"
msg %= (
is_detached and "checkout" or "reset",
mrepo.head,
mrepo,
)
log.info(msg)
_logger.info(msg)
may_reset = False
# END handle force
# END handle if we are in the future
Expand Down Expand Up @@ -834,7 +832,7 @@ def update(
except Exception as err:
if not keep_going:
raise
log.error(str(err))
_logger.error(str(err))
# END handle keep_going

# HANDLE RECURSION
Expand Down
7 changes: 3 additions & 4 deletions git/objects/submodule/root.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@

__all__ = ["RootModule", "RootUpdateProgress"]

log = logging.getLogger("git.objects.submodule.root")
log.addHandler(logging.NullHandler())
_logger = logging.getLogger(__name__)


class RootUpdateProgress(UpdateProgress):
Expand Down Expand Up @@ -321,7 +320,7 @@ def update(
# this way, it will be checked out in the next step.
# This will change the submodule relative to us, so
# the user will be able to commit the change easily.
log.warning(
_logger.warning(
"Current sha %s was not contained in the tracking\
branch at the new remote, setting it the the remote's tracking branch",
sm.hexsha,
Expand Down Expand Up @@ -393,7 +392,7 @@ def update(
except Exception as err:
if not keep_going:
raise
log.error(str(err))
_logger.error(str(err))
# END handle keep_going

# FINALLY UPDATE ALL ACTUAL SUBMODULES
Expand Down
19 changes: 8 additions & 11 deletions git/remote.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,7 @@

# -------------------------------------------------------------


log = logging.getLogger("git.remote")
log.addHandler(logging.NullHandler())

_logger = logging.getLogger(__name__)

__all__ = ("RemoteProgress", "PushInfo", "FetchInfo", "Remote")

Expand Down Expand Up @@ -857,7 +854,7 @@ def _get_fetch_info_from_stderr(
stderr_text = progress.error_lines and "\n".join(progress.error_lines) or ""
proc.wait(stderr=stderr_text)
if stderr_text:
log.warning("Error lines received while fetching: %s", stderr_text)
_logger.warning("Error lines received while fetching: %s", stderr_text)

for line in progress.other_lines:
line = force_text(line)
Expand All @@ -878,9 +875,9 @@ def _get_fetch_info_from_stderr(
msg += "length of progress lines %i should be equal to lines in FETCH_HEAD file %i\n"
msg += "Will ignore extra progress lines or fetch head lines."
msg %= (l_fil, l_fhi)
log.debug(msg)
log.debug(b"info lines: " + str(fetch_info_lines).encode("UTF-8"))
log.debug(b"head info: " + str(fetch_head_info).encode("UTF-8"))
_logger.debug(msg)
_logger.debug(b"info lines: " + str(fetch_info_lines).encode("UTF-8"))
_logger.debug(b"head info: " + str(fetch_head_info).encode("UTF-8"))
if l_fil < l_fhi:
fetch_head_info = fetch_head_info[:l_fil]
else:
Expand All @@ -892,8 +889,8 @@ def _get_fetch_info_from_stderr(
try:
output.append(FetchInfo._from_line(self.repo, err_line, fetch_line))
except ValueError as exc:
log.debug("Caught error while parsing line: %s", exc)
log.warning("Git informed while fetching: %s", err_line.strip())
_logger.debug("Caught error while parsing line: %s", exc)
_logger.warning("Git informed while fetching: %s", err_line.strip())
return output

def _get_push_info(
Expand Down Expand Up @@ -935,7 +932,7 @@ def stdout_handler(line: str) -> None:
if not output:
raise
elif stderr_text:
log.warning("Error lines received while fetching: %s", stderr_text)
_logger.warning("Error lines received while fetching: %s", stderr_text)
output.error = e

return output
Expand Down
8 changes: 4 additions & 4 deletions git/repo/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@

# -----------------------------------------------------------

log = logging.getLogger(__name__)
_logger = logging.getLogger(__name__)

__all__ = ("Repo",)

Expand Down Expand Up @@ -772,7 +772,7 @@ def is_valid_object(self, sha: str, object_type: Union[str, None] = None) -> boo
if object_info.type == object_type.encode():
return True
else:
log.debug(
_logger.debug(
"Commit hash points to an object of type '%s'. Requested were objects of type '%s'",
object_info.type.decode(),
object_type,
Expand All @@ -781,7 +781,7 @@ def is_valid_object(self, sha: str, object_type: Union[str, None] = None) -> boo
else:
return True
except BadObject:
log.debug("Commit hash is invalid.")
_logger.debug("Commit hash is invalid.")
return False

def _get_daemon_export(self) -> bool:
Expand Down Expand Up @@ -1298,7 +1298,7 @@ def _clone(
cmdline = getattr(proc, "args", "")
cmdline = remove_password_if_present(cmdline)

log.debug("Cmd(%s)'s unused stdout: %s", cmdline, stdout)
_logger.debug("Cmd(%s)'s unused stdout: %s", cmdline, stdout)

Check failure

Code scanning / CodeQL

Clear-text logging of sensitive information

This expression logs [sensitive data (password)](1) as clear text. This expression logs [sensitive data (password)](1) as clear text. This expression logs [sensitive data (password)](1) as clear text. This expression logs [sensitive data (password)](1) as clear text.
finalize_process(proc, stderr=stderr)

# Our git command could have a different working dir than our actual
Expand Down
8 changes: 4 additions & 4 deletions git/util.py
View file
Open in desktop
Loading