فهرست منبع

Add support for GIT_TRACE environment variable

Supported values:
- "1", "2", "true": Output to stderr with DEBUG level
- "3" through "9": Output to file descriptor
- Absolute path: Append to file
- Directory: Create per-process trace files (trace.{pid})
- "0", "false", empty, unset: Disabled

Fixes #1863
Jelmer Vernooij 3 ماه پیش
والد
کامیت
fbf86ab79a
4فایلهای تغییر یافته به همراه277 افزوده شده و 16 حذف شده
  1. 5 0
      NEWS
  2. 7 4
      dulwich/cli.py
  3. 110 6
      dulwich/log_utils.py
  4. 155 6
      tests/test_log_utils.py

+ 5 - 0
NEWS

@@ -1,5 +1,10 @@
 0.24.3	UNRELEASED
 
+ * Add support for ``GIT_TRACE`` environment variable for debugging. Supports
+   output to stderr (values "1", "2", or "true"), file descriptors (3-9),
+   file paths, and directories (creates per-process trace files).
+   (Jelmer Vernooij, #1863)
+
  * Add ``extract_signature()`` method to ``Commit`` and ``Tag`` classes that
    returns (payload, signature, signature_type) tuple. Supports both PGP and SSH
    signature detection. (Jelmer Vernooij)

+ 7 - 4
dulwich/cli.py

@@ -62,6 +62,7 @@ from .client import get_transport_and_path
 from .config import Config
 from .errors import ApplyDeltaError, GitProtocolError
 from .index import Index
+from .log_utils import _configure_logging_from_trace
 from .objects import Commit, sha_to_hex, valid_hexsha
 from .objectspec import parse_commit_range
 from .pack import Pack
@@ -4501,10 +4502,12 @@ def main(argv: Optional[Sequence[str]] = None) -> Optional[int]:
         parser.print_help()
         return 1
 
-    logging.basicConfig(
-        level=logging.INFO,
-        format="%(message)s",
-    )
+    # Try to configure from GIT_TRACE, fall back to default if it fails
+    if not _configure_logging_from_trace():
+        logging.basicConfig(
+            level=logging.INFO,
+            format="%(message)s",
+        )
 
     # First remaining arg is the command
     cmd = remaining[0]

+ 110 - 6
dulwich/log_utils.py

@@ -37,7 +37,9 @@ directly.
 """
 
 import logging
+import os
 import sys
+from typing import Optional, Union
 
 getLogger = logging.getLogger
 
@@ -54,14 +56,116 @@ _DULWICH_LOGGER = getLogger("dulwich")
 _DULWICH_LOGGER.addHandler(_NULL_HANDLER)
 
 
+def _should_trace() -> bool:
+    """Check if GIT_TRACE is enabled.
+
+    Returns True if tracing should be enabled, False otherwise.
+    """
+    trace_value = os.environ.get("GIT_TRACE", "")
+    if not trace_value or trace_value.lower() in ("0", "false"):
+        return False
+    return True
+
+
+def _get_trace_target() -> Optional[Union[str, int]]:
+    """Get the trace target from GIT_TRACE environment variable.
+
+    Returns:
+        - None if tracing is disabled
+        - 2 for stderr output (values "1", "2", "true")
+        - int (3-9) for file descriptor
+        - str for file path (absolute paths or directories)
+    """
+    trace_value = os.environ.get("GIT_TRACE", "")
+
+    if not trace_value or trace_value.lower() in ("0", "false"):
+        return None
+
+    if trace_value.lower() in ("1", "2", "true"):
+        return 2  # stderr
+
+    # Check if it's a file descriptor (integer 3-9)
+    try:
+        fd = int(trace_value)
+        if 3 <= fd <= 9:
+            return fd
+    except ValueError:
+        pass
+
+    # If it's an absolute path, return it as a string
+    if trace_value.startswith("/"):
+        return trace_value
+
+    # For any other value, treat it as disabled
+    return None
+
+
+def _configure_logging_from_trace() -> bool:
+    """Configure logging based on GIT_TRACE environment variable.
+
+    Returns True if trace configuration was successful, False otherwise.
+    """
+    trace_target = _get_trace_target()
+    if trace_target is None:
+        return False
+
+    trace_format = "%(asctime)s %(name)s %(levelname)s: %(message)s"
+
+    if trace_target == 2:
+        # stderr
+        logging.basicConfig(level=logging.DEBUG, stream=sys.stderr, format=trace_format)
+        return True
+
+    if isinstance(trace_target, int):
+        # File descriptor
+        try:
+            stream = os.fdopen(trace_target, "w", buffering=1)
+            logging.basicConfig(level=logging.DEBUG, stream=stream, format=trace_format)
+            return True
+        except OSError as e:
+            sys.stderr.write(
+                f"Warning: Failed to open GIT_TRACE fd {trace_target}: {e}\n"
+            )
+            return False
+
+    # File path
+    try:
+        if os.path.isdir(trace_target):
+            # For directories, create a file per process
+            filename = os.path.join(trace_target, f"trace.{os.getpid()}")
+        else:
+            filename = trace_target
+
+        logging.basicConfig(
+            level=logging.DEBUG, filename=filename, filemode="a", format=trace_format
+        )
+        return True
+    except OSError as e:
+        sys.stderr.write(
+            f"Warning: Failed to open GIT_TRACE file {trace_target}: {e}\n"
+        )
+        return False
+
+
 def default_logging_config() -> None:
-    """Set up the default Dulwich loggers."""
+    """Set up the default Dulwich loggers.
+
+    Respects the GIT_TRACE environment variable for trace output:
+    - If GIT_TRACE is set to "1", "2", or "true", trace to stderr
+    - If GIT_TRACE is set to an integer 3-9, trace to that file descriptor
+    - If GIT_TRACE is set to an absolute path, trace to that file
+    - If the path is a directory, trace to files in that directory (per process)
+    - Otherwise, use default stderr output
+    """
     remove_null_handler()
-    logging.basicConfig(
-        level=logging.INFO,
-        stream=sys.stderr,
-        format="%(asctime)s %(levelname)s: %(message)s",
-    )
+
+    # Try to configure from GIT_TRACE, fall back to default if it fails
+    if not _configure_logging_from_trace():
+        logging.basicConfig(
+            level=logging.INFO,
+            stream=sys.stderr,
+            format="%(asctime)s %(levelname)s: %(message)s",
+        )
 
 
 def remove_null_handler() -> None:

+ 155 - 6
tests/test_log_utils.py

@@ -22,11 +22,15 @@
 """Tests for dulwich.log_utils."""
 
 import logging
+import os
+import tempfile
 
 from dulwich.log_utils import (
     _DULWICH_LOGGER,
     _NULL_HANDLER,
+    _get_trace_target,
     _NullHandler,
+    _should_trace,
     default_logging_config,
     getLogger,
     remove_null_handler,
@@ -38,17 +42,31 @@ from . import TestCase
 class LogUtilsTests(TestCase):
     """Tests for log_utils."""
 
-    def setUp(self):
+    def setUp(self) -> None:
         super().setUp()
         # Save original handler configuration
         self.original_handlers = list(_DULWICH_LOGGER.handlers)
+        # Save original GIT_TRACE value
+        self.original_git_trace = os.environ.get("GIT_TRACE")
 
-    def tearDown(self):
+    def tearDown(self) -> None:
         # Restore original handler configuration
         _DULWICH_LOGGER.handlers = self.original_handlers
+        # Restore original GIT_TRACE value
+        if self.original_git_trace is None:
+            os.environ.pop("GIT_TRACE", None)
+        else:
+            os.environ["GIT_TRACE"] = self.original_git_trace
         super().tearDown()
 
-    def test_null_handler(self):
+    def _set_git_trace(self, value: str | None) -> None:
+        """Helper to set GIT_TRACE environment variable."""
+        if value is None:
+            os.environ.pop("GIT_TRACE", None)
+        else:
+            os.environ["GIT_TRACE"] = value
+
+    def test_null_handler(self) -> None:
         """Test the _NullHandler class."""
         handler = _NullHandler()
         # Create a test record
@@ -64,14 +82,14 @@ class LogUtilsTests(TestCase):
         # Should not raise any exceptions
         handler.emit(record)
 
-    def test_get_logger(self):
+    def test_get_logger(self) -> None:
         """Test the getLogger function."""
         # Should return a logger instance
         logger = getLogger("dulwich.test")
         self.assertIsInstance(logger, logging.Logger)
         self.assertEqual(logger.name, "dulwich.test")
 
-    def test_remove_null_handler(self):
+    def test_remove_null_handler(self) -> None:
         """Test removing the null handler."""
         # Make sure _NULL_HANDLER is in the handlers
         if _NULL_HANDLER not in _DULWICH_LOGGER.handlers:
@@ -83,7 +101,7 @@ class LogUtilsTests(TestCase):
         # Check that it was removed
         self.assertNotIn(_NULL_HANDLER, _DULWICH_LOGGER.handlers)
 
-    def test_default_logging_config(self):
+    def test_default_logging_config(self) -> None:
         """Test the default logging configuration."""
         # Apply default config
         default_logging_config()
@@ -97,3 +115,134 @@ class LogUtilsTests(TestCase):
 
         # Reset the root logger to not affect other tests
         root_logger.handlers = []
+
+    def test_should_trace_disabled(self) -> None:
+        """Test _should_trace with tracing disabled."""
+        # Test with unset environment variable
+        self._set_git_trace(None)
+        self.assertFalse(_should_trace())
+
+        # Test with empty string
+        self._set_git_trace("")
+        self.assertFalse(_should_trace())
+
+        # Test with "0"
+        self._set_git_trace("0")
+        self.assertFalse(_should_trace())
+
+        # Test with "false" (case insensitive)
+        self._set_git_trace("false")
+        self.assertFalse(_should_trace())
+
+        self._set_git_trace("FALSE")
+        self.assertFalse(_should_trace())
+
+    def test_should_trace_enabled(self) -> None:
+        """Test _should_trace with tracing enabled."""
+        self._set_git_trace("1")
+        self.assertTrue(_should_trace())
+
+        self._set_git_trace("/tmp/trace.log")
+        self.assertTrue(_should_trace())
+
+    def test_get_trace_target_disabled(self) -> None:
+        """Test _get_trace_target with tracing disabled."""
+        # Test with unset environment variable
+        self._set_git_trace(None)
+        self.assertIsNone(_get_trace_target())
+
+        # Test with empty string
+        self._set_git_trace("")
+        self.assertIsNone(_get_trace_target())
+
+        # Test with "0"
+        self._set_git_trace("0")
+        self.assertIsNone(_get_trace_target())
+
+        # Test with "false"
+        self._set_git_trace("false")
+        self.assertIsNone(_get_trace_target())
+
+    def test_get_trace_target_stderr(self) -> None:
+        """Test _get_trace_target with stderr output."""
+        # Test with "1"
+        self._set_git_trace("1")
+        self.assertEqual(_get_trace_target(), 2)
+
+        # Test with "2"
+        self._set_git_trace("2")
+        self.assertEqual(_get_trace_target(), 2)
+
+        # Test with "true" (case insensitive)
+        self._set_git_trace("true")
+        self.assertEqual(_get_trace_target(), 2)
+
+        self._set_git_trace("TRUE")
+        self.assertEqual(_get_trace_target(), 2)
+
+    def test_get_trace_target_file_descriptor(self) -> None:
+        """Test _get_trace_target with file descriptor."""
+        for fd in range(3, 10):
+            self._set_git_trace(str(fd))
+            self.assertEqual(_get_trace_target(), fd)
+
+        # Test out of range values
+        self._set_git_trace("10")
+        self.assertIsNone(_get_trace_target())
+
+        self._set_git_trace("2")
+        self.assertEqual(_get_trace_target(), 2)  # Special case: stderr
+
+    def test_get_trace_target_file(self) -> None:
+        """Test _get_trace_target with file path."""
+        with tempfile.NamedTemporaryFile(mode="w", delete=False) as f:
+            trace_file = f.name
+
+        try:
+            self._set_git_trace(trace_file)
+            self.assertEqual(_get_trace_target(), trace_file)
+        finally:
+            if os.path.exists(trace_file):
+                os.unlink(trace_file)
+
+    def test_get_trace_target_directory(self) -> None:
+        """Test _get_trace_target with directory path."""
+        with tempfile.TemporaryDirectory() as tmpdir:
+            self._set_git_trace(tmpdir)
+            self.assertEqual(_get_trace_target(), tmpdir)
+
+    def test_get_trace_target_other_values(self) -> None:
+        """Test _get_trace_target with other values."""
+        # Any non-absolute path should be treated as disabled
+        self._set_git_trace("relative/path")
+        self.assertIsNone(_get_trace_target())
+
+    def test_default_logging_config_with_trace(self) -> None:
+        """Test default_logging_config with GIT_TRACE enabled."""
+        # Save current root logger state
+        root_logger = logging.getLogger()
+        original_level = root_logger.level
+        original_handlers = list(root_logger.handlers)
+
+        # Clean up after test
+        def cleanup() -> None:
+            root_logger.handlers = original_handlers
+            root_logger.level = original_level
+
+        self.addCleanup(cleanup)
+
+        # Reset root logger to ensure clean state
+        root_logger.handlers = []
+        root_logger.level = logging.WARNING
+
+        self._set_git_trace("1")
+        default_logging_config()
+
+        # Check that the null handler was removed
+        self.assertNotIn(_NULL_HANDLER, _DULWICH_LOGGER.handlers)
+
+        # Check that the root logger has a handler
+        self.assertTrue(root_logger.handlers)
+
+        # Check that the level is DEBUG when tracing is enabled
+        self.assertEqual(root_logger.level, logging.DEBUG)