From 74c33652914ff923f7a8ab0e7c3ed81a4ed76458 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 5 Aug 2025 16:59:34 +0530 Subject: [PATCH 1/5] REFACTOR: Shift get_driver_path from Python to ddbc bindings --- mssql_python/helpers.py | 39 ----------- mssql_python/pybind/ddbc_bindings.cpp | 97 ++++++++++++++++++++++----- tests/test_000_dependencies.py | 62 ++++++++++++----- 3 files changed, 125 insertions(+), 73 deletions(-) diff --git a/mssql_python/helpers.py b/mssql_python/helpers.py index 267ede75..e1125b8f 100644 --- a/mssql_python/helpers.py +++ b/mssql_python/helpers.py @@ -146,45 +146,6 @@ def detect_linux_distro(): return distro_name -def get_driver_path(module_dir, architecture): - """ - Get the platform-specific ODBC driver path. - - Args: - module_dir (str): Base module directory - architecture (str): Target architecture (x64, arm64, x86, etc.) - - Returns: - str: Full path to the ODBC driver file - - Raises: - RuntimeError: If driver not found or unsupported platform - """ - - platform_name = platform.system().lower() - normalized_arch = normalize_architecture(platform_name, architecture) - - if platform_name == "windows": - driver_path = Path(module_dir) / "libs" / "windows" / normalized_arch / "msodbcsql18.dll" - - elif platform_name == "darwin": - driver_path = Path(module_dir) / "libs" / "macos" / normalized_arch / "lib" / "libmsodbcsql.18.dylib" - - elif platform_name == "linux": - distro_name = detect_linux_distro() - driver_path = Path(module_dir) / "libs" / "linux" / distro_name / normalized_arch / "lib" / "libmsodbcsql-18.5.so.1.1" - - else: - raise RuntimeError(f"Unsupported platform: {platform_name}") - - driver_path_str = str(driver_path) - - # Check if file exists - if not driver_path.exists(): - raise RuntimeError(f"ODBC driver not found at: {driver_path_str}") - - return driver_path_str - def sanitize_connection_string(conn_str: str) -> str: """ diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 49c7c7af..8cd0fdf0 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -637,20 +637,80 @@ std::string GetLastErrorMessage() { #endif } -// Function to call Python get_driver_path function -std::string GetDriverPathFromPython(const std::string& moduleDir, const std::string& architecture) { - try { - py::module_ helpers = py::module_::import("mssql_python.helpers"); - py::object get_driver_path = helpers.attr("get_driver_path"); - py::str result = get_driver_path(moduleDir, architecture); - return std::string(result); - } catch (const py::error_already_set& e) { - LOG("Python error in get_driver_path: {}", e.what()); - ThrowStdException("Failed to get driver path from Python: " + std::string(e.what())); - } catch (const std::exception& e) { - LOG("Error calling get_driver_path: {}", e.what()); - ThrowStdException("Failed to get driver path: " + std::string(e.what())); - } + +/* + * DRIVER PATH RESOLUTION: C++ Only Implementation + * + * This function handles all ODBC driver path resolution in C++ rather than calling + * Python helpers to avoid circular import issues on Alpine Linux. + * + * BACKGROUND: + * Originally, driver path resolution was handled by a Python function in helpers.py + * (get_driver_path) that was called from C++ during module initialization. This worked + * fine on most platforms but caused a critical circular import issue specifically on + * Alpine Linux. + * + * THE ALPINE PROBLEM: + * 1. Alpine Linux uses musl libc instead of glibc (used by Ubuntu/RHEL/macOS) + * 2. musl's dynamic loader has stricter rules about circular dependencies during + * module initialization + * 3. The circular import chain was: + * - ddbc_bindings.cpp starts loading + * - LoadDriverOrThrowException() calls GetDriverPathFromPython() + * - This tries to import mssql_python.helpers + * - helpers.py tries to import ddbc_bindings (not fully loaded yet) + * - Alpine/musl fails with circular import error + * - Ubuntu/macOS/glibc systems handle this gracefully + * + * THE SOLUTION: + * By implementing driver path resolution entirely in C++, we: + * 1. Eliminate all Python dependencies during critical driver initialization + * 2. Avoid the circular import issue completely on all platforms +*/ +std::string GetDriverPathCpp(const std::string& moduleDir) { + namespace fs = std::filesystem; + fs::path basePath(moduleDir); + + std::string platform; + std::string arch; + + // Detect architecture + #if defined(__aarch64__) || defined(_M_ARM64) + arch = "arm64"; + #elif defined(__x86_64__) || defined(_M_X64) || defined(_M_AMD64) + arch = "x86_64"; // maps to "x64" on Windows + #else + throw std::runtime_error("Unsupported architecture"); + #endif + + // Detect platform and set path + #ifdef __linux__ + if (fs::exists("/etc/alpine-release")) { + platform = "alpine"; + } else if (fs::exists("/etc/redhat-release") || fs::exists("/etc/centos-release")) { + platform = "rhel"; + } else { + platform = "ubuntu"; + } + + fs::path driverPath = basePath / "libs" / "linux" / platform / arch / "lib" / "libmsodbcsql-18.5.so.1.1"; + return driverPath.string(); + + #elif defined(__APPLE__) + platform = "macos"; + fs::path driverPath = basePath / "libs" / platform / arch / "lib" / "libmsodbcsql.18.dylib"; + return driverPath.string(); + + #elif defined(_WIN32) + platform = "windows"; + // Normalize x86_64 to x64 for Windows naming + if (arch == "x86_64") arch = "x64"; + fs::path driverPath = basePath / "libs" / platform / arch / "lib" / "msodbcsql18.dll"; + return driverPath.string(); + + #else + throw std::runtime_error("Unsupported platform"); + #endif } DriverHandle LoadDriverOrThrowException() { @@ -662,8 +722,11 @@ DriverHandle LoadDriverOrThrowException() { std::string archStr = ARCHITECTURE; LOG("Architecture: {}", archStr); - // Use Python function to get the correct driver path for the platform - std::string driverPathStr = GetDriverPathFromPython(moduleDir, archStr); + // Use only C++ function for driver path resolution + // Not using Python function since it causes circular import issues on Alpine Linux + // and other platforms with strict module loading rules. + std::string driverPathStr = GetDriverPathCpp(moduleDir); + fs::path driverPath(driverPathStr); LOG("Driver path determined: {}", driverPath.string()); @@ -2448,7 +2511,7 @@ PYBIND11_MODULE(ddbc_bindings, m) { // Expose the C++ functions to Python m.def("ThrowStdException", &ThrowStdException); - m.def("get_driver_path", &GetDriverPathFromPython, "Get platform-specific ODBC driver path"); + m.def("GetDriverPathCpp", &GetDriverPathCpp, "Get the path to the ODBC driver"); // Define parameter info class py::class_(m, "ParamInfo") diff --git a/tests/test_000_dependencies.py b/tests/test_000_dependencies.py index 08d16fef..8404ae52 100644 --- a/tests/test_000_dependencies.py +++ b/tests/test_000_dependencies.py @@ -9,6 +9,9 @@ import sys from pathlib import Path +from mssql_python.ddbc_bindings import normalize_architecture +from mssql_python.helpers import detect_linux_distro + class DependencyTester: """Helper class to test platform-specific dependencies.""" @@ -164,8 +167,31 @@ def get_expected_python_extension(self): return self.module_dir / extension_name + def get_expected_driver_path(self): + platform_name = platform.system().lower() + normalized_arch = normalize_architecture(platform_name, self.normalized_arch) + + if platform_name == "windows": + driver_path = Path(self.module_dir) / "libs" / "windows" / normalized_arch / "msodbcsql18.dll" + + elif platform_name == "darwin": + driver_path = Path(self.module_dir) / "libs" / "macos" / normalized_arch / "lib" / "libmsodbcsql.18.dylib" + + elif platform_name == "linux": + distro_name = detect_linux_distro() + driver_path = Path(self.module_dir) / "libs" / "linux" / distro_name / normalized_arch / "lib" / "libmsodbcsql-18.5.so.1.1" + + else: + raise RuntimeError(f"Unsupported platform: {platform_name}") + + driver_path_str = str(driver_path) + + # Check if file exists + if not driver_path.exists(): + raise RuntimeError(f"ODBC driver not found at: {driver_path_str}") + + return driver_path_str -# Create global instance for use in tests dependency_tester = DependencyTester() @@ -314,21 +340,6 @@ def test_python_extension_imports(self): except Exception as e: pytest.fail(f"Failed to import or use ddbc_bindings: {e}") - - def test_helper_functions_work(self): - """Test that helper functions can detect platform correctly.""" - try: - from mssql_python.helpers import get_driver_path - - # Test that get_driver_path works for current platform - driver_path = get_driver_path(str(dependency_tester.module_dir), dependency_tester.normalized_arch) - - assert Path(driver_path).exists(), \ - f"Driver path returned by get_driver_path does not exist: {driver_path}" - - except Exception as e: - pytest.fail(f"Failed to use helper functions: {e}") - # Print platform information when tests are collected def pytest_runtest_setup(item): @@ -349,4 +360,21 @@ def test_ddbc_bindings_import(): import mssql_python.ddbc_bindings assert True, "ddbc_bindings module imported successfully." except ImportError as e: - pytest.fail(f"Failed to import ddbc_bindings: {e}") \ No newline at end of file + pytest.fail(f"Failed to import ddbc_bindings: {e}") + + + +def test_get_driver_path_from_ddbc_bindings(): + """Test the GetDriverPathCpp function from ddbc_bindings.""" + try: + import mssql_python.ddbc_bindings as ddbc + module_dir = dependency_tester.module_dir + + driver_path = ddbc.GetDriverPathCpp(str(module_dir)) + + # The driver path should be same as one returned by the Python function + expected_path = dependency_tester.get_expected_driver_path() + assert driver_path == str(expected_path), \ + f"Driver path mismatch: expected {expected_path}, got {driver_path}" + except Exception as e: + pytest.fail(f"Failed to call GetDriverPathCpp: {e}") From e4bf7b8488b23d2d540707effaede8ac91da5142 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 5 Aug 2025 17:04:41 +0530 Subject: [PATCH 2/5] Fix windows path Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 8cd0fdf0..324b42c4 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -705,7 +705,7 @@ std::string GetDriverPathCpp(const std::string& moduleDir) { platform = "windows"; // Normalize x86_64 to x64 for Windows naming if (arch == "x86_64") arch = "x64"; - fs::path driverPath = basePath / "libs" / platform / arch / "lib" / "msodbcsql18.dll"; + fs::path driverPath = basePath / "libs" / platform / arch / "msodbcsql18.dll"; return driverPath.string(); #else From dcabb6644bb2eb9b1d2fef22d8bd4e86e9cce8fb Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 5 Aug 2025 17:06:10 +0530 Subject: [PATCH 3/5] fix ubuntu path Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- mssql_python/pybind/ddbc_bindings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 324b42c4..1ee64864 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -690,7 +690,7 @@ std::string GetDriverPathCpp(const std::string& moduleDir) { } else if (fs::exists("/etc/redhat-release") || fs::exists("/etc/centos-release")) { platform = "rhel"; } else { - platform = "ubuntu"; + platform = "debian_ubuntu"; } fs::path driverPath = basePath / "libs" / "linux" / platform / arch / "lib" / "libmsodbcsql-18.5.so.1.1"; From 7aa619ed86b74cdbeb9fd24f389a7246b277db28 Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 5 Aug 2025 17:20:59 +0530 Subject: [PATCH 4/5] removed detect_linux_distro as well --- mssql_python/helpers.py | 33 ---------------------------- tests/test_000_dependencies.py | 39 ++++++++++++++++++---------------- 2 files changed, 21 insertions(+), 51 deletions(-) diff --git a/mssql_python/helpers.py b/mssql_python/helpers.py index e1125b8f..f15365c9 100644 --- a/mssql_python/helpers.py +++ b/mssql_python/helpers.py @@ -114,39 +114,6 @@ def add_driver_name_to_app_parameter(connection_string): return ";".join(modified_parameters) + ";" -def detect_linux_distro(): - """ - Detect Linux distribution for driver path selection. - - Returns: - str: Distribution name ('debian_ubuntu', 'rhel', 'alpine', etc.) - """ - import os - - distro_name = "debian_ubuntu" # default - - try: - if os.path.exists("/etc/os-release"): - with open("/etc/os-release", "r") as f: - content = f.read() - for line in content.split("\n"): - if line.startswith("ID="): - distro_id = line.split("=", 1)[1].strip('"\'') - if distro_id in ["ubuntu", "debian"]: - distro_name = "debian_ubuntu" - elif distro_id in ["rhel", "centos", "fedora"]: - distro_name = "rhel" - elif distro_id == "alpine": - distro_name = "alpine" - else: - distro_name = distro_id # use as-is - break - except Exception: - pass # use default - - return distro_name - - def sanitize_connection_string(conn_str: str) -> str: """ Sanitize the connection string by removing sensitive information. diff --git a/tests/test_000_dependencies.py b/tests/test_000_dependencies.py index 8404ae52..f76a7413 100644 --- a/tests/test_000_dependencies.py +++ b/tests/test_000_dependencies.py @@ -10,7 +10,6 @@ from pathlib import Path from mssql_python.ddbc_bindings import normalize_architecture -from mssql_python.helpers import detect_linux_distro class DependencyTester: @@ -60,23 +59,26 @@ def _normalize_architecture(self): def _detect_linux_distro(self): """Detect Linux distribution for driver path selection.""" distro_name = "debian_ubuntu" # default - + ''' + #ifdef __linux__ + if (fs::exists("/etc/alpine-release")) { + platform = "alpine"; + } else if (fs::exists("/etc/redhat-release") || fs::exists("/etc/centos-release")) { + platform = "rhel"; + } else { + platform = "ubuntu"; + } + + fs::path driverPath = basePath / "libs" / "linux" / platform / arch / "lib" / "libmsodbcsql-18.5.so.1.1"; + return driverPath.string(); + ''' try: - if os.path.exists("/etc/os-release"): - with open("/etc/os-release", "r") as f: - content = f.read() - for line in content.split("\n"): - if line.startswith("ID="): - distro_id = line.split("=", 1)[1].strip('"\'') - if distro_id in ["ubuntu", "debian"]: - distro_name = "debian_ubuntu" - elif distro_id in ["rhel", "centos", "fedora"]: - distro_name = "rhel" - elif distro_id == "alpine": - distro_name = "alpine" - else: - distro_name = distro_id - break + if (Path("/etc/alpine-release").exists()): + distro_name = "alpine" + elif (Path("/etc/redhat-release").exists() or Path("/etc/centos-release").exists()): + distro_name = "rhel" + else: + distro_name = "debian_ubuntu" except Exception: pass # use default @@ -178,7 +180,7 @@ def get_expected_driver_path(self): driver_path = Path(self.module_dir) / "libs" / "macos" / normalized_arch / "lib" / "libmsodbcsql.18.dylib" elif platform_name == "linux": - distro_name = detect_linux_distro() + distro_name = self._detect_linux_distro() driver_path = Path(self.module_dir) / "libs" / "linux" / distro_name / normalized_arch / "lib" / "libmsodbcsql-18.5.so.1.1" else: @@ -192,6 +194,7 @@ def get_expected_driver_path(self): return driver_path_str +# Create global instance for use in tests dependency_tester = DependencyTester() From d56329eff9b7f58a542ed42d617a113769acc0bc Mon Sep 17 00:00:00 2001 From: Gaurav Sharma Date: Tue, 5 Aug 2025 17:24:34 +0530 Subject: [PATCH 5/5] changed comment --- mssql_python/pybind/ddbc_bindings.cpp | 42 +++++++++------------------ 1 file changed, 14 insertions(+), 28 deletions(-) diff --git a/mssql_python/pybind/ddbc_bindings.cpp b/mssql_python/pybind/ddbc_bindings.cpp index 1ee64864..1b37b8f0 100644 --- a/mssql_python/pybind/ddbc_bindings.cpp +++ b/mssql_python/pybind/ddbc_bindings.cpp @@ -639,34 +639,20 @@ std::string GetLastErrorMessage() { /* - * DRIVER PATH RESOLUTION: C++ Only Implementation - * - * This function handles all ODBC driver path resolution in C++ rather than calling - * Python helpers to avoid circular import issues on Alpine Linux. - * - * BACKGROUND: - * Originally, driver path resolution was handled by a Python function in helpers.py - * (get_driver_path) that was called from C++ during module initialization. This worked - * fine on most platforms but caused a critical circular import issue specifically on - * Alpine Linux. - * - * THE ALPINE PROBLEM: - * 1. Alpine Linux uses musl libc instead of glibc (used by Ubuntu/RHEL/macOS) - * 2. musl's dynamic loader has stricter rules about circular dependencies during - * module initialization - * 3. The circular import chain was: - * - ddbc_bindings.cpp starts loading - * - LoadDriverOrThrowException() calls GetDriverPathFromPython() - * - This tries to import mssql_python.helpers - * - helpers.py tries to import ddbc_bindings (not fully loaded yet) - * - Alpine/musl fails with circular import error - * - Ubuntu/macOS/glibc systems handle this gracefully - * - * THE SOLUTION: - * By implementing driver path resolution entirely in C++, we: - * 1. Eliminate all Python dependencies during critical driver initialization - * 2. Avoid the circular import issue completely on all platforms -*/ + * Resolve ODBC driver path in C++ to avoid circular import issues on Alpine. + * + * Background: + * On Alpine Linux, calling into Python during module initialization (via pybind11) + * causes a circular import due to musl's stricter dynamic loader behavior. + * + * Specifically, importing Python helpers from C++ triggered a re-import of the + * partially-initialized native module, which works on glibc (Ubuntu/macOS) but + * fails on musl-based systems like Alpine. + * + * By moving driver path resolution entirely into C++, we avoid any Python-layer + * dependencies during critical initialization, ensuring compatibility across + * all supported platforms. + */ std::string GetDriverPathCpp(const std::string& moduleDir) { namespace fs = std::filesystem; fs::path basePath(moduleDir);