diff --git a/CHANGELOG.md b/CHANGELOG.md index 00496cbe5..a9a0bca0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), `VARBINARY`/`BINARY` `NULL` bindings to fail when a non-NULL parameter was bound first. The driver now pre-resolves unknown NULL parameter types before any `SQLBindParameter` calls, avoiding ODBC ordinal confusion. +- **GH-726:** The extension loader now derives the Windows architecture from + `sysconfig.get_platform()` (the interpreter build) instead of + `platform.machine()` (the host CPU). An x64 interpreter on a Windows ARM64 + machine previously looked for `ddbc_bindings.cpXY-arm64.pyd`, missed, and + took the fallback path on every import; it now resolves the `amd64` binary + the `win_amd64` wheel ships. The fallback notice is now emitted as a + `RuntimeWarning` instead of being printed to stdout. ## [1.0.0-alpha] - 2025-02-24 diff --git a/mssql_python/ddbc_bindings.py b/mssql_python/ddbc_bindings.py index f8fef87d1..90eaf81a8 100644 --- a/mssql_python/ddbc_bindings.py +++ b/mssql_python/ddbc_bindings.py @@ -9,6 +9,8 @@ import importlib.util import sys import platform +import sysconfig +import warnings def normalize_architecture(platform_name_param, architecture_param): @@ -72,21 +74,114 @@ def normalize_architecture(platform_name_param, architecture_param): ) +def get_interpreter_architecture(platform_name_param): + """ + Get the raw architecture string of the running interpreter. + + On Windows, ``platform.machine()`` reports the host CPU rather than the + architecture the interpreter was built for. The two differ when an x64 + interpreter runs emulated on a Windows ARM64 machine (the python.org + "64-bit" installer is x64), so the loader would look for an arm64 binary + that the installed win_amd64 wheel never shipped. ``sysconfig.get_platform()`` + is derived from the interpreter build and matches the wheel tag, so it is + the source of truth on Windows. + + Args: + platform_name_param (str): Platform name ('windows', 'darwin', 'linux') + + Returns: + str: Lower case architecture string accepted by normalize_architecture() + """ + if platform_name_param == "windows": + # 'win-amd64' -> 'amd64', 'win-arm64' -> 'arm64', 'win32' -> 'win32' + return sysconfig.get_platform().lower().removeprefix("win-") + return platform.machine().lower() + + +def get_module_architecture(platform_name_param): + """ + Get the architecture token used in the compiled ddbc_bindings filename. + + Args: + platform_name_param (str): Platform name ('windows', 'darwin', 'linux') + + Returns: + str: 'universal2' on macOS, otherwise the normalized architecture with + the Windows x64 build renamed to 'amd64' and the Windows x86 build + renamed to 'win32', matching the file names the build produces + """ + # Special handling for macOS universal2 binaries + if platform_name_param == "darwin": + return "universal2" + + architecture_name = normalize_architecture( + platform_name_param, get_interpreter_architecture(platform_name_param) + ) + + # Handle Windows-specific naming for binary files. The build names the + # x64 artifact 'amd64' and the x86 artifact 'win32' (see pybind/build.bat + # and pybind/CMakeLists.txt), so the loader has to look for those tokens. + if platform_name_param == "windows" and architecture_name == "x64": + architecture_name = "amd64" + elif platform_name_param == "windows" and architecture_name == "x86": + architecture_name = "win32" + return architecture_name + + +def find_module_path(module_dir_param, python_version_param, architecture_param, extension_param): + """ + Find the compiled ddbc_bindings module file for the running interpreter. + + Args: + module_dir_param (str): Directory that contains the compiled module + python_version_param (str): Python version tag such as 'cp312' + architecture_param (str): Architecture token from get_module_architecture() + extension_param (str): File extension, '.pyd' on Windows and '.so' elsewhere + + Returns: + str: Path of the exactly matching module file. If it does not exist, the + first ddbc_bindings file built for this interpreter's version tag is + returned instead and a RuntimeWarning is emitted. + + Raises: + ImportError: If no ddbc_bindings file for this interpreter's version + tag exists with the right extension + """ + expected_module = f"ddbc_bindings.{python_version_param}-{architecture_param}{extension_param}" + module_path_found = os.path.join(module_dir_param, expected_module) + + if os.path.exists(module_path_found): + return module_path_found + + # Fallback: only binaries built for this interpreter's version tag are + # loadable, since the cpXY tags are version specific rather than abi3. A + # different architecture may still work (for example a universal2 build), + # but a different Python version fails later inside importlib with a + # confusing DLL or symbol error, so it is excluded here. Sorted so the + # pick is deterministic when several candidates remain. + module_files = sorted( + f + for f in os.listdir(module_dir_param) + if f.startswith(f"ddbc_bindings.{python_version_param}-") and f.endswith(extension_param) + ) + if not module_files: + raise ImportError( + f"No ddbc_bindings module found for {python_version_param}-{architecture_param} " + f"with extension {extension_param}" + ) + warnings.warn( + f"Using fallback module file {module_files[0]} instead of {expected_module}", + RuntimeWarning, + stacklevel=2, + ) + return os.path.join(module_dir_param, module_files[0]) + + # Get current Python version and architecture python_version = f"cp{sys.version_info.major}{sys.version_info.minor}" platform_name = platform.system().lower() -raw_architecture = platform.machine().lower() - -# Special handling for macOS universal2 binaries -if platform_name == "darwin": - architecture = "universal2" -else: - architecture = normalize_architecture(platform_name, raw_architecture) - - # Handle Windows-specific naming for binary files - if platform_name == "windows" and architecture == "x64": - architecture = "amd64" +architecture = get_module_architecture(platform_name) # Validate supported platforms if platform_name not in ["windows", "darwin", "linux"]: @@ -104,23 +199,7 @@ def normalize_architecture(platform_name_param, architecture_param): # Find the specifically matching module file module_dir = os.path.dirname(__file__) -expected_module = f"ddbc_bindings.{python_version}-{architecture}{extension}" -module_path = os.path.join(module_dir, expected_module) - -if not os.path.exists(module_path): - # Fallback to searching for any matching module if the specific one isn't found - module_files = [ - f - for f in os.listdir(module_dir) - if f.startswith("ddbc_bindings.") and f.endswith(extension) - ] - if not module_files: - raise ImportError( - f"No ddbc_bindings module found for {python_version}-{architecture} " - f"with extension {extension}" - ) - module_path = os.path.join(module_dir, module_files[0]) - print(f"Warning: Using fallback module file {module_files[0]} instead of " f"{expected_module}") +module_path = find_module_path(module_dir, python_version, architecture, extension) # Use the original module name 'ddbc_bindings' that the C extension was compiled with diff --git a/tests/test_000_dependencies.py b/tests/test_000_dependencies.py index 02784181a..633dafa75 100644 --- a/tests/test_000_dependencies.py +++ b/tests/test_000_dependencies.py @@ -12,7 +12,12 @@ import sys from pathlib import Path -from mssql_python.ddbc_bindings import normalize_architecture +from mssql_python.ddbc_bindings import ( + find_module_path, + get_interpreter_architecture, + get_module_architecture, + normalize_architecture, +) class DependencyTester: @@ -20,7 +25,7 @@ class DependencyTester: def __init__(self): self.platform_name = platform.system().lower() - self.raw_architecture = platform.machine().lower() + self.raw_architecture = get_interpreter_architecture(self.platform_name) self.module_dir = self._get_module_directory() self.libs_base_dir = self._get_libs_base_dir() self.normalized_arch = self._normalize_architecture() @@ -802,48 +807,152 @@ def test_ddbc_bindings_import_error_scenarios(): normalize_architecture(platform_name, arch) -def test_ddbc_bindings_warning_fallback_scenario(): - """Test the warning message scenario for fallback module (Lines 114-116).""" +def test_ddbc_bindings_exact_module_match_is_silent(tmp_path, capsys): + """find_module_path returns the exact match without any warning or stdout output.""" - # We can't easily simulate the exact fallback scenario during testing - # since it would require manipulating the file system during import - # But we can test that the warning logic would work conceptually + expected_module = "ddbc_bindings.cp310-amd64.pyd" + (tmp_path / expected_module).write_bytes(b"") + (tmp_path / "ddbc_bindings.cp39-amd64.pyd").write_bytes(b"") - import io - import contextlib + import warnings - # Simulate the warning print statement - expected_module = "ddbc_bindings.cp310-win_amd64.pyd" - fallback_module = "ddbc_bindings.cp39-win_amd64.pyd" + with warnings.catch_warnings(): + warnings.simplefilter("error") + module_path = find_module_path(str(tmp_path), "cp310", "amd64", ".pyd") - # Capture stdout to verify warning format - f = io.StringIO() - with contextlib.redirect_stdout(f): - print(f"Warning: Using fallback module file {fallback_module} instead of {expected_module}") + assert module_path == str(tmp_path / expected_module) + assert capsys.readouterr().out == "" - output = f.getvalue() - assert "Warning: Using fallback module file" in output - assert fallback_module in output - assert expected_module in output +def test_ddbc_bindings_warning_fallback_scenario(tmp_path, capsys): + """The fallback module pick is reported through warnings, not printed to stdout (GH-726).""" -def test_ddbc_bindings_no_module_found_error(): - """Test error when no ddbc_bindings module is found (Lines 110-112).""" + expected_module = "ddbc_bindings.cp310-amd64.pyd" + fallback_module = "ddbc_bindings.cp310-arm64.pyd" + (tmp_path / fallback_module).write_bytes(b"") + (tmp_path / "other_file.txt").write_bytes(b"") + + with pytest.warns(RuntimeWarning, match="Using fallback module file") as record: + module_path = find_module_path(str(tmp_path), "cp310", "amd64", ".pyd") + + assert module_path == str(tmp_path / fallback_module) + message = str(record[0].message) + assert fallback_module in message + assert expected_module in message + # Nothing may be written to stdout: callers that pipe script output must not + # receive a stray warning line prepended to their data. + assert capsys.readouterr().out == "" + + +def test_fallback_never_returns_wrong_python_version(tmp_path): + """A binary for another Python version must not be selected (GH-726). + + The cpXY tags are version specific, so a cp39 build cannot load under + cp310; picking it turns a clean ImportError into a DLL load failure + from deeper inside importlib. + """ + (tmp_path / "ddbc_bindings.cp39-amd64.pyd").write_bytes(b"") + + with pytest.raises(ImportError): + find_module_path(str(tmp_path), "cp310", "amd64", ".pyd") + + +def test_fallback_pick_is_deterministic(tmp_path): + """With several same version candidates the lexically first is chosen.""" + (tmp_path / "ddbc_bindings.cp310-win32.pyd").write_bytes(b"") + (tmp_path / "ddbc_bindings.cp310-arm64.pyd").write_bytes(b"") + + with pytest.warns(RuntimeWarning): + module_path = find_module_path(str(tmp_path), "cp310", "amd64", ".pyd") + + assert module_path == str(tmp_path / "ddbc_bindings.cp310-arm64.pyd") + + +def test_ddbc_bindings_no_module_found_error(tmp_path): + """Test error when no ddbc_bindings module is found.""" - # Test the error message format that would be used python_version = "cp310" architecture = "x64" extension = ".pyd" - expected_error = f"No ddbc_bindings module found for {python_version}-{architecture} with extension {extension}" + # Files with the wrong extension or the wrong prefix must not be picked up + (tmp_path / "ddbc_bindings.cp310-x64.so").write_bytes(b"") + (tmp_path / "other_file.pyd").write_bytes(b"") + + with pytest.raises(ImportError) as exc_info: + find_module_path(str(tmp_path), python_version, architecture, extension) - # Verify the error message format is correct + expected_error = str(exc_info.value) assert "No ddbc_bindings module found for" in expected_error assert python_version in expected_error assert architecture in expected_error assert extension in expected_error +@pytest.mark.parametrize( + "interpreter_platform, host_machine, expected_raw_arch, expected_module_arch", + [ + # x64 interpreter on a Windows ARM64 host: platform.machine() reports the + # host CPU, but the installed wheel is win_amd64 (GH-726) + ("win-amd64", "ARM64", "amd64", "amd64"), + # native x64 interpreter on an x64 host + ("win-amd64", "AMD64", "amd64", "amd64"), + # native ARM64 interpreter + ("win-arm64", "ARM64", "arm64", "arm64"), + # 32 bit interpreter on a 64 bit host; the build names that artifact + # with the win32 token (pybind/build.bat, pybind/CMakeLists.txt) + ("win32", "AMD64", "win32", "win32"), + ], +) +def test_windows_architecture_follows_interpreter_not_host( + monkeypatch, interpreter_platform, host_machine, expected_raw_arch, expected_module_arch +): + """On Windows the loader must derive the architecture from the interpreter build.""" + + import sysconfig + + monkeypatch.setattr(sysconfig, "get_platform", lambda: interpreter_platform) + monkeypatch.setattr(platform, "machine", lambda: host_machine) + monkeypatch.setattr(platform, "system", lambda: "Windows") + + assert get_interpreter_architecture("windows") == expected_raw_arch + assert get_module_architecture("windows") == expected_module_arch + + +def test_expected_extension_matches_loader_on_windows_arm64_host(monkeypatch): + """The test helper and the loader agree on an x64 interpreter on an ARM64 host (GH-726).""" + + import sysconfig + + monkeypatch.setattr(sysconfig, "get_platform", lambda: "win-amd64") + monkeypatch.setattr(platform, "machine", lambda: "ARM64") + monkeypatch.setattr(platform, "system", lambda: "Windows") + + python_version = f"cp{sys.version_info.major}{sys.version_info.minor}" + expected_name = f"ddbc_bindings.{python_version}-{get_module_architecture('windows')}.pyd" + + assert expected_name == f"ddbc_bindings.{python_version}-amd64.pyd" + assert DependencyTester().get_expected_python_extension().name == expected_name + + +def test_non_windows_architecture_uses_platform_machine(monkeypatch): + """Linux keeps using platform.machine(); macOS always resolves to universal2.""" + + import sysconfig + + # A misleading sysconfig value must be ignored outside Windows + monkeypatch.setattr(sysconfig, "get_platform", lambda: "win-amd64") + monkeypatch.setattr(platform, "machine", lambda: "aarch64") + + assert get_interpreter_architecture("linux") == "aarch64" + assert get_module_architecture("linux") == "arm64" + + monkeypatch.setattr(platform, "machine", lambda: "x86_64") + assert get_interpreter_architecture("linux") == "x86_64" + assert get_module_architecture("linux") == "x86_64" + assert get_module_architecture("darwin") == "universal2" + + class TestOdbcPackageSplit: """Coverage for the standalone ``mssql-python-odbc`` driver package.