fix(rust/driver_manager): use platform library filenames in search paths#4155
Open
VersusFacit wants to merge 1 commit intoapache:mainfrom
Open
fix(rust/driver_manager): use platform library filenames in search paths#4155VersusFacit wants to merge 1 commit intoapache:mainfrom
VersusFacit wants to merge 1 commit intoapache:mainfrom
Conversation
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
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.
Summary
Fix Rust
driver_managersearch-path loading for bare driver names.This came up while testing local ADBC driver development through additional search paths. We've used this plenty over the past year or so. However, now
search_path_list()inrust/driver_manager/src/search.rswas joining a bare driver name likeadbc_driver_bigquerydirectly into the search directory and attempting to load that path as-is. On macOS, that meant trying/path/to/adbc_driver_bigqueryinstead of/path/to/libadbc_driver_bigquery.dylib.This change applies
libloading::library_filename(...)before loading from additional search paths so path-based search matches the existing by-name loading behavior.This also, I believe, aligns the Rust behavior with the existing C++ driver-manager search-path logic in
c/driver_manager/adbc_driver_manager_driver_loading.cc, whereManagedLibrary::SearchPathsForDriver()removes the manifest suffix and relies onLoad(...)to add the platform library suffix during path-based search.If there's a different intent for this package, do let me know. Happy to iterate if required 🙏
Testing
cargo +1.86 test -p adbc_driver_manager test_load_additional_path_with_platform_library_filenameI smoke tested this with my local project which was looking for the extension-less filename, then once more began to find my drivers built in arrow-adbc's go subrepo.