CHORE: uninstall system unixODBC before tests to validate bundled…#658
Draft
jahnvi480 wants to merge 3 commits into
Draft
CHORE: uninstall system unixODBC before tests to validate bundled…#658jahnvi480 wants to merge 3 commits into
jahnvi480 wants to merge 3 commits into
Conversation
📊 Code Coverage Report
Diff CoverageDiff: main...HEAD, staged and unstaged changesNo lines with coverage information in this diff. 📋 Files Needing Attention📉 Files with overall lowest coverage (click to expand)mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.pybind.ddbc_bindings.cpp: 76.3%
mssql_python.__init__.py: 77.3%
mssql_python.row.py: 77.6%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%🔗 Quick Links
|
Revert the 'uninstall system unixODBC' step (it only ran on the x86_64 runner, whose dylib is already correct, and it broke the benchmark by removing unixODBC that pyodbc needs). Add test_macos_dylibs_have_no_absolute_dependency_paths in test_000_dependencies.py. It runs 'otool -L' on every bundled dylib in both libs/macos/arm64 and libs/macos/x86_64 and fails if a sibling bundled library is referenced via an absolute path instead of @loader_path/@rpath. Because otool reads Mach-O load commands of any architecture, the x86_64 macOS CI lane now catches the arm64-only packaging bug behind GitHub #656.
Verified against the shipped binaries that: - arm64/libmsodbcsql.18.dylib hardcodes /opt/homebrew/lib/libodbcinst.2.dylib (the GH #656 break); x86_64 gets rewritten to @loader_path at build time because configure_dylibs.sh only fixes ARCH=\ on the Intel runner. - ddbc_bindings dlopens libmsodbcsql.18.dylib DIRECTLY, so libodbc.2.dylib is off the runtime load path (and is never touched by configure_dylibs.sh). The previous test scanned every dylib, so it would have flagged the dead libodbc.2.dylib and could never go green even after the real fix. Replace it with test_macos_driver_load_chain_is_relocatable, which starts at the driver and follows only its bundled sibling deps -- matching what dlopen resolves at runtime, ignoring external openssl, and catching the arm64 bug from an x86_64 runner via otool.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Work Item / Issue Reference
Summary
This pull request adds a new step to the macOS test pipeline to ensure that the bundled ODBC driver libraries are used during testing, rather than any system-installed versions. This helps verify that the built wheel is truly self-contained and does not rely on Homebrew-provided
unixODBCormsodbcsql18.Test pipeline improvements:
unixODBCandmsodbcsql18via Homebrew, remove any leftover driver-manager dylibs from standard Homebrew locations, and verify that the bundled ODBC driver dependencies are used by checking withotool -Lon the driver library. (eng/pipelines/pr-validation-pipeline.yml)