Switch to latin1 encoding for binary output for debug symbol checks in compiled objects (#309)

Co-authored-by: James Lamb <[email protected]>

Author: agriyakhetarpal

Makefile

@@ -1,3 +1,5 @@
+NUMPY_WIN_DEBUG_WHL=tests/data/numpy-1.26.3-cp310-cp310-win_amd64.whl
+
 .PHONY: build
 build:
 	rm -r ./dist || true
@@ -83,14 +85,19 @@ test-data-conda-dot-conda-packages:
 		'./tests/data/*-0.tar.bz2' \
 		'.conda'
 
+$(NUMPY_WIN_DEBUG_WHL):
+	curl \
+		https://files.pythonhosted.org/packages/be/b0/611101990ddac767e54e2d27d1f4576ae1662cca64e2d55ef0e62558ec26/numpy-1.26.3-cp310-cp310-win_amd64.whl \
+		-o $(NUMPY_WIN_DEBUG_WHL)
+
 .PHONY: test
-test:
+test: $(NUMPY_WIN_DEBUG_WHL)
 	pytest \
 		--cov pydistcheck \
 		./tests
 
 .PHONY: test-local
-test-local:
+test-local: $(NUMPY_WIN_DEBUG_WHL)
 	PYTHONPATH=src \
 	pytest \
 		--cov=src/pydistcheck \

src/pydistcheck/_shared_lib_utils.py

@@ -19,7 +19,9 @@
 def _run_command(args: List[str]) -> str:
     try:
         stdout = subprocess.run(args, capture_output=True, check=True).stdout
-        return stdout.decode("utf-8")
+        # Use latin1 encoding, which can handle any byte value without data loss.
+        # See https://github.com/jameslamb/pydistcheck/issues/206 for rationale.
+        return stdout.decode("latin1")
     except subprocess.CalledProcessError:
         return _COMMAND_FAILED
     except FileNotFoundError:

tests/test_cli.py

@@ -42,6 +42,11 @@
 ]
 TEST_DATA_DIR = os.path.join(os.path.dirname(os.path.abspath(__file__)), "data")
 
+# see https://github.com/jameslamb/pydistcheck/issues/206
+NUMPY_WIN_DEBUG_WHL = "numpy-1.26.3-cp310-cp310-win_amd64.whl"
+if os.path.isfile(os.path.join(TEST_DATA_DIR, NUMPY_WIN_DEBUG_WHL)):
+    PACKAGES_WITH_DEBUG_SYMBOLS.append(NUMPY_WIN_DEBUG_WHL)
+
 
 def _assert_log_matches_pattern(
     result: Result, pattern: str, num_times: int = 1
@@ -1074,6 +1079,10 @@ def test_debug_symbols_check_works(distro_file):
         else:
             # dsymutil works on both macOS and Linux
             debug_cmd = r"'dsymutil \-s " + lib_file
+    elif NUMPY_WIN_DEBUG_WHL in distro_file:
+        # windows wheels
+        lib_file = r"\"numpy\.libs/libopenblas64__v0\.3\.23-293-gc2f4bdbb-gcc_10_3_0-2bde3a66a51006b2b53eb373ff767a3f\.dll\"'"
+        debug_cmd = r"'objdump \-\-all\-headers " + lib_file
     else:
         # linux wheels
         debug_cmd = r"'objdump \-\-all\-headers \"lib/lib_baseballmetrics\.so\"'\."

tests/test_shared_lib_utils.py

@@ -1,6 +1,43 @@
+import subprocess
 from unittest.mock import Mock, patch
 
-from pydistcheck._shared_lib_utils import _MACHO_STRIP_SYMBOL, _get_symbols
+from pydistcheck._shared_lib_utils import (
+    _MACHO_STRIP_SYMBOL,
+    _get_symbols,
+    _run_command,
+)
+
+
+def test_run_command_handles_binary_output():
+    """Test that _run_command can handle binary output containing invalid UTF-8 bytes."""
+    mock_output = bytes(
+        [0x68, 0x65, 0x6C, 0x6C, 0x6F, 0xD7, 0x77, 0x6F, 0x72, 0x6C, 0x64]
+    )  # i.e., "hello" + invalid UTF-8 + "world"
+
+    with patch("subprocess.run") as mock_run:
+        mock_run.return_value = Mock(stdout=mock_output)
+
+        # Should not raise UnicodeDecodeError,
+        result = _run_command(["some", "command"])
+        # latin1 encoding should preserve all bytes.
+        assert len(result) == len(mock_output)
+        # Check valid parts of the string.
+        assert "hello" in result
+        assert "world" in result
+
+
+def test_run_command_handles_command_failure():
+    with patch("subprocess.run") as mock_run:
+        mock_run.side_effect = subprocess.CalledProcessError(1, ["cmd"])
+        result = _run_command(["failing", "command"])
+        assert result == "__command_failed__"
+
+
+def test_run_command_handles_missing_command():
+    with patch("subprocess.run") as mock_run:
+        mock_run.side_effect = FileNotFoundError()
+        result = _run_command(["nonexistent"])
+        assert result == "__tool_not_available__"
 
 
 def test_get_symbols_filters_radr_symbol():