This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-adbc.git
The following commit(s) were added to refs/heads/main by this push:
new 69ba923fa feat(c/driver_manager): don't ignore invalid manifests
(#3399)
69ba923fa is described below
commit 69ba923fad12ed8a70e5bf0ebb5ef284387c40ad
Author: David Li <[email protected]>
AuthorDate: Tue Sep 9 08:27:37 2025 +0900
feat(c/driver_manager): don't ignore invalid manifests (#3399)
Fixes #3398.
---
.pre-commit-config.yaml | 6 ++
c/driver_manager/adbc_driver_manager.cc | 92 ++++++++++++++--
c/driver_manager/adbc_driver_manager_test.cc | 124 +++++++++++++++++++++-
ci/scripts/run_cgo_drivermgr_check.sh | 16 ++-
go/adbc/drivermgr/adbc_driver_manager.cc | 92 ++++++++++++++--
python/adbc_driver_manager/tests/test_manifest.py | 106 ++++++++++++++++++
6 files changed, 408 insertions(+), 28 deletions(-)
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 05ce5478c..8441d6a62 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -117,6 +117,12 @@ repos:
pass_filenames: true
files: '^c/include/arrow-adbc/[^/]*\.h$'
entry: "./ci/scripts/run_cgo_drivermgr_check.sh"
+ - id: check-cgo-adbc-impl
+ name: Ensure CGO adbc_driver_manager.cc is syncd
+ language: script
+ pass_filenames: true
+ files: '^c/driver_manager/adbc_driver_manager\.cc$'
+ entry: "./ci/scripts/run_cgo_drivermgr_check.sh"
# https://infra.apache.org/github-actions-policy.html
- id: check-pin
name: Ensure GitHub Actions and pre-commit hooks are pinned to a
specific SHA
diff --git a/c/driver_manager/adbc_driver_manager.cc
b/c/driver_manager/adbc_driver_manager.cc
index ed8946e52..3169fd480 100644
--- a/c/driver_manager/adbc_driver_manager.cc
+++ b/c/driver_manager/adbc_driver_manager.cc
@@ -69,6 +69,7 @@ enum class SearchPathSource {
kUnset,
kDoesNotExist,
kDisabled,
+ kOtherError,
};
using SearchPaths = std::vector<std::pair<SearchPathSource,
std::filesystem::path>>;
@@ -106,6 +107,9 @@ void AddSearchPathsToError(const SearchPaths& search_paths,
std::string& error_m
case SearchPathSource::kDisabled:
error_message += "not enabled at build time: ";
break;
+ case SearchPathSource::kOtherError:
+ // Don't add any prefix
+ break;
}
error_message += path.string();
}
@@ -327,13 +331,22 @@ AdbcStatusCode LoadDriverFromRegistry(HKEY root, const
std::wstring& driver_name
}
#endif // _WIN32
+/// \return ADBC_STATUS_NOT_FOUND if the manifest does not contain a driver
+/// path for this platform, ADBC_STATUS_INVALID_ARGUMENT if the manifest
+/// could not be parsed, ADBC_STATUS_OK otherwise (`info` will be populated)
AdbcStatusCode LoadDriverManifest(const std::filesystem::path& driver_manifest,
DriverInfo& info, struct AdbcError* error) {
toml::table config;
try {
config = toml::parse_file(driver_manifest.native());
} catch (const toml::parse_error& err) {
- SetError(error, err.what());
+ // Despite the name, this exception covers IO errors too. Hence, we can't
+ // differentiate between bad syntax and other I/O error.
+ std::string message = "Could not open manifest. ";
+ message += err.what();
+ message += ". Manifest: ";
+ message += driver_manifest.string();
+ SetError(error, std::move(message));
return ADBC_STATUS_INVALID_ARGUMENT;
}
@@ -410,7 +423,7 @@ AdbcStatusCode LoadDriverManifest(const
std::filesystem::path& driver_manifest,
}
SetError(error, "Driver path not defined in manifest '"s +
driver_manifest.string() +
"'. `Driver.shared` must be a string or table"s);
- return ADBC_STATUS_NOT_FOUND;
+ return ADBC_STATUS_INVALID_ARGUMENT;
}
SearchPaths GetEnvPaths(const char_type* env_var) {
@@ -425,6 +438,8 @@ SearchPaths GetEnvPaths(const char_type* env_var) {
std::wstring path_var;
path_var.resize(required_size);
_wgetenv_s(&required_size, path_var.data(), required_size, env_var);
+ // Remove null terminator
+ path_var.resize(required_size - 1);
auto path = Utf8Encode(path_var);
#else
const char* path_var = std::getenv(env_var);
@@ -602,13 +617,21 @@ struct ManagedLibrary {
return FindDriver(driver_path, load_options, additional_search_paths,
info, error);
}
+ /// \return ADBC_STATUS_NOT_FOUND if the driver shared library could not be
+ /// found (via dlopen) or if a manifest was found but did not contain a
+ /// path for the current platform, ADBC_STATUS_INVALID_ARGUMENT if a
+ /// manifest was found but could not be parsed, ADBC_STATUS_OK otherwise
+ ///
+ /// May modify search_paths to add error info
AdbcStatusCode SearchPathsForDriver(const std::filesystem::path& driver_path,
- const SearchPaths& search_paths,
DriverInfo& info,
+ SearchPaths& search_paths, DriverInfo&
info,
struct AdbcError* error) {
+ SearchPaths extra_debug_info;
for (const auto& [source, search_path] : search_paths) {
if (source == SearchPathSource::kRegistry || source ==
SearchPathSource::kUnset ||
source == SearchPathSource::kDoesNotExist ||
- source == SearchPathSource::kDisabled) {
+ source == SearchPathSource::kDisabled ||
+ source == SearchPathSource::kOtherError) {
continue;
}
std::filesystem::path full_path = search_path / driver_path;
@@ -616,19 +639,63 @@ struct ManagedLibrary {
// check for toml first, then dll
full_path.replace_extension(".toml");
if (std::filesystem::exists(full_path)) {
- auto status = LoadDriverManifest(full_path, info, nullptr);
+ OwnedError intermediate_error;
+
+ auto status = LoadDriverManifest(full_path, info,
&intermediate_error.error);
if (status == ADBC_STATUS_OK) {
- return Load(info.lib_path.c_str(), search_paths, error);
+ // Don't pass attempted_paths here; we'll generate the error at a
higher level
+ status = Load(info.lib_path.c_str(), {}, &intermediate_error.error);
+ if (status == ADBC_STATUS_OK) {
+ return status;
+ }
+ std::string message = "found ";
+ message += full_path.string();
+ if (intermediate_error.error.message) {
+ message += " but: ";
+ message += intermediate_error.error.message;
+ } else {
+ message += " could not load the driver it specified";
+ }
+ extra_debug_info.emplace_back(SearchPathSource::kOtherError,
+ std::move(message));
+ search_paths.insert(search_paths.end(), extra_debug_info.begin(),
+ extra_debug_info.end());
+ return status;
+ } else if (status == ADBC_STATUS_INVALID_ARGUMENT) {
+ // The manifest was invalid. Don't ignore that!
+ search_paths.insert(search_paths.end(), extra_debug_info.begin(),
+ extra_debug_info.end());
+ if (intermediate_error.error.message) {
+ std::string error_message = intermediate_error.error.message;
+ AddSearchPathsToError(search_paths, error_message);
+ SetError(error, std::move(error_message));
+ }
+ return status;
}
+ // Should be NOT_FOUND otherwise
+ std::string message = "found ";
+ message += full_path.string();
+ if (intermediate_error.error.message) {
+ message += " but: ";
+ message += intermediate_error.error.message;
+ } else {
+ message += " which did not define a driver for this platform";
+ }
+
+ extra_debug_info.emplace_back(SearchPathSource::kOtherError,
std::move(message));
}
- full_path.replace_extension(""); // remove the .toml extension
- auto status = Load(full_path.c_str(), search_paths, nullptr);
+ // remove the .toml extension; Load will add the DLL/SO/DYLIB suffix
+ full_path.replace_extension("");
+ // Don't pass error here - it'll be suppressed anyways
+ auto status = Load(full_path.c_str(), {}, nullptr);
if (status == ADBC_STATUS_OK) {
return status;
}
}
+ search_paths.insert(search_paths.end(), extra_debug_info.begin(),
+ extra_debug_info.end());
return ADBC_STATUS_NOT_FOUND;
}
@@ -677,7 +744,8 @@ struct ManagedLibrary {
#endif // ADBC_CONDA_BUILD
auto status = SearchPathsForDriver(driver_path, search_paths, info,
error);
- if (status == ADBC_STATUS_OK) {
+ if (status != ADBC_STATUS_NOT_FOUND) {
+ // If NOT_FOUND, then keep searching; if OK or INVALID_ARGUMENT, stop
return status;
}
}
@@ -704,7 +772,7 @@ struct ManagedLibrary {
auto user_paths = GetSearchPaths(ADBC_LOAD_FLAG_SEARCH_USER);
status = SearchPathsForDriver(driver_path, user_paths, info, error);
- if (status == ADBC_STATUS_OK) {
+ if (status != ADBC_STATUS_NOT_FOUND) {
return status;
}
search_paths.insert(search_paths.end(), user_paths.begin(),
user_paths.end());
@@ -728,7 +796,7 @@ struct ManagedLibrary {
auto system_paths = GetSearchPaths(ADBC_LOAD_FLAG_SEARCH_SYSTEM);
status = SearchPathsForDriver(driver_path, system_paths, info, error);
- if (status == ADBC_STATUS_OK) {
+ if (status != ADBC_STATUS_NOT_FOUND) {
return status;
}
search_paths.insert(search_paths.end(), system_paths.begin(),
system_paths.end());
@@ -753,6 +821,8 @@ struct ManagedLibrary {
#endif // _WIN32
}
+ /// \return ADBC_STATUS_NOT_FOUND if the driver shared library could not be
+ /// found, ADBC_STATUS_OK otherwise
AdbcStatusCode Load(const char_type* library, const SearchPaths&
attempted_paths,
struct AdbcError* error) {
std::string error_message;
diff --git a/c/driver_manager/adbc_driver_manager_test.cc
b/c/driver_manager/adbc_driver_manager_test.cc
index b17ec236d..0fe83ca65 100644
--- a/c/driver_manager/adbc_driver_manager_test.cc
+++ b/c/driver_manager/adbc_driver_manager_test.cc
@@ -620,11 +620,44 @@ TEST_F(DriverManifest, ManifestDriverMissing) {
// Attempt to load the driver
ASSERT_THAT(AdbcFindLoadDriver(filepath.string().data(), nullptr,
ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver,
&error),
- IsStatus(ADBC_STATUS_NOT_FOUND, &error));
+ IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error));
+
+ ASSERT_THAT(error.message, ::testing::HasSubstr("Driver path not defined in
manifest"));
+ ASSERT_THAT(error.message,
+ ::testing::HasSubstr("`Driver.shared` must be a string or
table"));
+ ASSERT_TRUE(std::filesystem::remove(filepath));
+}
+TEST_F(DriverManifest, ManifestDriverMissingAdbcDatabase) {
+ // Similar test as above but with AdbcDatabaseInit path and using the
+ // additional search path.
+ // Create a manifest without the "Driver" section
+ auto filepath = temp_dir / "sqlite.toml";
+ toml::table manifest_without_driver = simple_manifest;
+ manifest_without_driver.erase("Driver");
+
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << manifest_without_driver;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "driver", "sqlite",
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDriverManagerDatabaseSetLoadFlags(&database.value,
+ ADBC_LOAD_FLAG_DEFAULT,
&error),
+ IsOkStatus(&error));
+ std::string search_path = temp_dir.string();
+ ASSERT_THAT(AdbcDriverManagerDatabaseSetAdditionalSearchPathList(
+ &database.value, search_path.data(), &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
+ IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error));
ASSERT_THAT(error.message, ::testing::HasSubstr("Driver path not defined in
manifest"));
ASSERT_THAT(error.message,
::testing::HasSubstr("`Driver.shared` must be a string or
table"));
+
ASSERT_TRUE(std::filesystem::remove(filepath));
}
@@ -643,7 +676,7 @@ TEST_F(DriverManifest, ManifestDriverInvalid) {
// Attempt to load the driver
ASSERT_THAT(AdbcFindLoadDriver(filepath.string().data(), nullptr,
ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver,
&error),
- IsStatus(ADBC_STATUS_NOT_FOUND, &error));
+ IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error));
ASSERT_THAT(error.message, ::testing::HasSubstr("Driver path not defined in
manifest"));
ASSERT_THAT(error.message,
@@ -702,6 +735,93 @@ TEST_F(DriverManifest, ManifestWrongArch) {
ASSERT_TRUE(std::filesystem::remove(filepath));
}
+TEST_F(DriverManifest, ManifestDriverMissingArchAdbcDatabase) {
+ // Similar test as above but with AdbcDatabaseInit path and using the
+ // additional search path.
+ // Create a manifest without the "Driver" section
+ auto filepath = temp_dir / "sqlite.toml";
+ toml::table manifest_without_driver = simple_manifest;
+ manifest_without_driver.erase("Driver");
+ manifest_without_driver.insert("Driver",
+ toml::table{
+ {"shared",
+ toml::table{
+ {"non-existent",
"path/to/bad/driver.so"},
+ {"windows-alpha64",
"path/to/bad/driver.so"},
+ }},
+ });
+
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << manifest_without_driver;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "driver", "sqlite",
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDriverManagerDatabaseSetLoadFlags(&database.value,
+ ADBC_LOAD_FLAG_DEFAULT,
&error),
+ IsOkStatus(&error));
+ std::string search_path = temp_dir.string();
+ ASSERT_THAT(AdbcDriverManagerDatabaseSetAdditionalSearchPathList(
+ &database.value, search_path.data(), &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
+ IsStatus(ADBC_STATUS_NOT_FOUND, &error));
+ ASSERT_THAT(error.message, ::testing::HasSubstr("sqlite.toml but:"));
+ ASSERT_THAT(error.message,
+ ::testing::HasSubstr("Architectures found: non-existent
windows-alpha64"));
+
+ ASSERT_TRUE(std::filesystem::remove(filepath));
+}
+
+TEST_F(DriverManifest, ManifestDriverPointsNowhere) {
+ // Similar test as above but with AdbcDatabaseInit path and using the
+ // additional search path.
+ // Create a manifest without the "Driver" section
+ auto filepath = temp_dir / "sqlite.toml";
+ toml::table manifest_without_driver = simple_manifest;
+ manifest_without_driver.erase("Driver");
+ // The idea is that we can find the manifest, but not the driver it points
to.
+ manifest_without_driver.insert("Driver", toml::table{
+ {"shared",
+ toml::table{
+ {"linux_arm64",
"adbc-goosedb"},
+ {"linux_amd64",
"adbc-goosedb"},
+ {"macos_arm64",
"adbc-goosedb"},
+ {"macos_amd64",
"adbc-goosedb"},
+ {"windows_arm64",
"adbc-goosedb"},
+ {"windows_amd64",
"adbc-goosedb"},
+ }},
+ });
+
+ std::ofstream test_manifest_file(filepath);
+ ASSERT_TRUE(test_manifest_file.is_open());
+ test_manifest_file << manifest_without_driver;
+ test_manifest_file.close();
+
+ adbc_validation::Handle<struct AdbcDatabase> database;
+ ASSERT_THAT(AdbcDatabaseNew(&database.value, &error), IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseSetOption(&database.value, "driver", "sqlite",
&error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDriverManagerDatabaseSetLoadFlags(&database.value,
+ ADBC_LOAD_FLAG_DEFAULT,
&error),
+ IsOkStatus(&error));
+ std::string search_path = temp_dir.string();
+ ASSERT_THAT(AdbcDriverManagerDatabaseSetAdditionalSearchPathList(
+ &database.value, search_path.data(), &error),
+ IsOkStatus(&error));
+ ASSERT_THAT(AdbcDatabaseInit(&database.value, &error),
+ IsStatus(ADBC_STATUS_NOT_FOUND, &error));
+ ASSERT_THAT(error.message, ::testing::HasSubstr("sqlite.toml but:"));
+ // Message is platform-specific but something like "dlopen() failed:
+ // adbc-goosedb: cannot open shared object file..."
+ ASSERT_THAT(error.message, ::testing::HasSubstr("adbc-goosedb"));
+
+ ASSERT_TRUE(std::filesystem::remove(filepath));
+}
+
TEST_F(DriverManifest, ManifestArchPathEmpty) {
auto filepath = temp_dir / "sqlite.toml";
toml::table manifest_without_driver = simple_manifest;
diff --git a/ci/scripts/run_cgo_drivermgr_check.sh
b/ci/scripts/run_cgo_drivermgr_check.sh
index 946ab0906..5e8fb87a5 100755
--- a/ci/scripts/run_cgo_drivermgr_check.sh
+++ b/ci/scripts/run_cgo_drivermgr_check.sh
@@ -32,10 +32,18 @@ main() {
for f in "$@"; do
fn=$(basename $f)
- if ! diff -q "$f" "go/adbc/drivermgr/arrow-adbc/$fn" &>/dev/null; then
- >&2 echo "OUT OF SYNC: $f differs from go/adbc/drivermgr/arrow-adbc/$fn"
- popd
- return 1
+ if [[ "$fn" == "adbc_driver_manager.cc" ]]; then
+ if ! diff -q "$f" "go/adbc/drivermgr/$fn" &>/dev/null; then
+ >&2 echo "OUT OF SYNC: $f differs from go/adbc/drivermgr/$fn"
+ popd
+ return 1
+ fi
+ else
+ if ! diff -q "$f" "go/adbc/drivermgr/arrow-adbc/$fn" &>/dev/null; then
+ >&2 echo "OUT OF SYNC: $f differs from
go/adbc/drivermgr/arrow-adbc/$fn"
+ popd
+ return 1
+ fi
fi
done
diff --git a/go/adbc/drivermgr/adbc_driver_manager.cc
b/go/adbc/drivermgr/adbc_driver_manager.cc
index ed8946e52..3169fd480 100644
--- a/go/adbc/drivermgr/adbc_driver_manager.cc
+++ b/go/adbc/drivermgr/adbc_driver_manager.cc
@@ -69,6 +69,7 @@ enum class SearchPathSource {
kUnset,
kDoesNotExist,
kDisabled,
+ kOtherError,
};
using SearchPaths = std::vector<std::pair<SearchPathSource,
std::filesystem::path>>;
@@ -106,6 +107,9 @@ void AddSearchPathsToError(const SearchPaths& search_paths,
std::string& error_m
case SearchPathSource::kDisabled:
error_message += "not enabled at build time: ";
break;
+ case SearchPathSource::kOtherError:
+ // Don't add any prefix
+ break;
}
error_message += path.string();
}
@@ -327,13 +331,22 @@ AdbcStatusCode LoadDriverFromRegistry(HKEY root, const
std::wstring& driver_name
}
#endif // _WIN32
+/// \return ADBC_STATUS_NOT_FOUND if the manifest does not contain a driver
+/// path for this platform, ADBC_STATUS_INVALID_ARGUMENT if the manifest
+/// could not be parsed, ADBC_STATUS_OK otherwise (`info` will be populated)
AdbcStatusCode LoadDriverManifest(const std::filesystem::path& driver_manifest,
DriverInfo& info, struct AdbcError* error) {
toml::table config;
try {
config = toml::parse_file(driver_manifest.native());
} catch (const toml::parse_error& err) {
- SetError(error, err.what());
+ // Despite the name, this exception covers IO errors too. Hence, we can't
+ // differentiate between bad syntax and other I/O error.
+ std::string message = "Could not open manifest. ";
+ message += err.what();
+ message += ". Manifest: ";
+ message += driver_manifest.string();
+ SetError(error, std::move(message));
return ADBC_STATUS_INVALID_ARGUMENT;
}
@@ -410,7 +423,7 @@ AdbcStatusCode LoadDriverManifest(const
std::filesystem::path& driver_manifest,
}
SetError(error, "Driver path not defined in manifest '"s +
driver_manifest.string() +
"'. `Driver.shared` must be a string or table"s);
- return ADBC_STATUS_NOT_FOUND;
+ return ADBC_STATUS_INVALID_ARGUMENT;
}
SearchPaths GetEnvPaths(const char_type* env_var) {
@@ -425,6 +438,8 @@ SearchPaths GetEnvPaths(const char_type* env_var) {
std::wstring path_var;
path_var.resize(required_size);
_wgetenv_s(&required_size, path_var.data(), required_size, env_var);
+ // Remove null terminator
+ path_var.resize(required_size - 1);
auto path = Utf8Encode(path_var);
#else
const char* path_var = std::getenv(env_var);
@@ -602,13 +617,21 @@ struct ManagedLibrary {
return FindDriver(driver_path, load_options, additional_search_paths,
info, error);
}
+ /// \return ADBC_STATUS_NOT_FOUND if the driver shared library could not be
+ /// found (via dlopen) or if a manifest was found but did not contain a
+ /// path for the current platform, ADBC_STATUS_INVALID_ARGUMENT if a
+ /// manifest was found but could not be parsed, ADBC_STATUS_OK otherwise
+ ///
+ /// May modify search_paths to add error info
AdbcStatusCode SearchPathsForDriver(const std::filesystem::path& driver_path,
- const SearchPaths& search_paths,
DriverInfo& info,
+ SearchPaths& search_paths, DriverInfo&
info,
struct AdbcError* error) {
+ SearchPaths extra_debug_info;
for (const auto& [source, search_path] : search_paths) {
if (source == SearchPathSource::kRegistry || source ==
SearchPathSource::kUnset ||
source == SearchPathSource::kDoesNotExist ||
- source == SearchPathSource::kDisabled) {
+ source == SearchPathSource::kDisabled ||
+ source == SearchPathSource::kOtherError) {
continue;
}
std::filesystem::path full_path = search_path / driver_path;
@@ -616,19 +639,63 @@ struct ManagedLibrary {
// check for toml first, then dll
full_path.replace_extension(".toml");
if (std::filesystem::exists(full_path)) {
- auto status = LoadDriverManifest(full_path, info, nullptr);
+ OwnedError intermediate_error;
+
+ auto status = LoadDriverManifest(full_path, info,
&intermediate_error.error);
if (status == ADBC_STATUS_OK) {
- return Load(info.lib_path.c_str(), search_paths, error);
+ // Don't pass attempted_paths here; we'll generate the error at a
higher level
+ status = Load(info.lib_path.c_str(), {}, &intermediate_error.error);
+ if (status == ADBC_STATUS_OK) {
+ return status;
+ }
+ std::string message = "found ";
+ message += full_path.string();
+ if (intermediate_error.error.message) {
+ message += " but: ";
+ message += intermediate_error.error.message;
+ } else {
+ message += " could not load the driver it specified";
+ }
+ extra_debug_info.emplace_back(SearchPathSource::kOtherError,
+ std::move(message));
+ search_paths.insert(search_paths.end(), extra_debug_info.begin(),
+ extra_debug_info.end());
+ return status;
+ } else if (status == ADBC_STATUS_INVALID_ARGUMENT) {
+ // The manifest was invalid. Don't ignore that!
+ search_paths.insert(search_paths.end(), extra_debug_info.begin(),
+ extra_debug_info.end());
+ if (intermediate_error.error.message) {
+ std::string error_message = intermediate_error.error.message;
+ AddSearchPathsToError(search_paths, error_message);
+ SetError(error, std::move(error_message));
+ }
+ return status;
}
+ // Should be NOT_FOUND otherwise
+ std::string message = "found ";
+ message += full_path.string();
+ if (intermediate_error.error.message) {
+ message += " but: ";
+ message += intermediate_error.error.message;
+ } else {
+ message += " which did not define a driver for this platform";
+ }
+
+ extra_debug_info.emplace_back(SearchPathSource::kOtherError,
std::move(message));
}
- full_path.replace_extension(""); // remove the .toml extension
- auto status = Load(full_path.c_str(), search_paths, nullptr);
+ // remove the .toml extension; Load will add the DLL/SO/DYLIB suffix
+ full_path.replace_extension("");
+ // Don't pass error here - it'll be suppressed anyways
+ auto status = Load(full_path.c_str(), {}, nullptr);
if (status == ADBC_STATUS_OK) {
return status;
}
}
+ search_paths.insert(search_paths.end(), extra_debug_info.begin(),
+ extra_debug_info.end());
return ADBC_STATUS_NOT_FOUND;
}
@@ -677,7 +744,8 @@ struct ManagedLibrary {
#endif // ADBC_CONDA_BUILD
auto status = SearchPathsForDriver(driver_path, search_paths, info,
error);
- if (status == ADBC_STATUS_OK) {
+ if (status != ADBC_STATUS_NOT_FOUND) {
+ // If NOT_FOUND, then keep searching; if OK or INVALID_ARGUMENT, stop
return status;
}
}
@@ -704,7 +772,7 @@ struct ManagedLibrary {
auto user_paths = GetSearchPaths(ADBC_LOAD_FLAG_SEARCH_USER);
status = SearchPathsForDriver(driver_path, user_paths, info, error);
- if (status == ADBC_STATUS_OK) {
+ if (status != ADBC_STATUS_NOT_FOUND) {
return status;
}
search_paths.insert(search_paths.end(), user_paths.begin(),
user_paths.end());
@@ -728,7 +796,7 @@ struct ManagedLibrary {
auto system_paths = GetSearchPaths(ADBC_LOAD_FLAG_SEARCH_SYSTEM);
status = SearchPathsForDriver(driver_path, system_paths, info, error);
- if (status == ADBC_STATUS_OK) {
+ if (status != ADBC_STATUS_NOT_FOUND) {
return status;
}
search_paths.insert(search_paths.end(), system_paths.begin(),
system_paths.end());
@@ -753,6 +821,8 @@ struct ManagedLibrary {
#endif // _WIN32
}
+ /// \return ADBC_STATUS_NOT_FOUND if the driver shared library could not be
+ /// found, ADBC_STATUS_OK otherwise
AdbcStatusCode Load(const char_type* library, const SearchPaths&
attempted_paths,
struct AdbcError* error) {
std::string error_message;
diff --git a/python/adbc_driver_manager/tests/test_manifest.py
b/python/adbc_driver_manager/tests/test_manifest.py
new file mode 100644
index 000000000..0b4e62bfa
--- /dev/null
+++ b/python/adbc_driver_manager/tests/test_manifest.py
@@ -0,0 +1,106 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import pytest
+
+import adbc_driver_manager.dbapi
+
+
[email protected]
+def test_manifest_indirect(tmp_path, monkeypatch) -> None:
+ with (tmp_path / "testdriver.toml").open("w") as sink:
+ sink.write(
+ """
+name = "my driver"
+version = "0.1.0"
+
+[Driver]
+shared = "adbc_driver_sqlite"
+ """
+ )
+
+ monkeypatch.setenv("ADBC_DRIVER_PATH", str(tmp_path))
+
+ with adbc_driver_manager.dbapi.connect(driver="testdriver") as conn:
+ with conn.cursor() as cursor:
+ cursor.execute("SELECT sqlite_version()")
+ assert cursor.fetchone() is not None
+
+
+def test_manifest_indirect_unknown_driver(tmp_path, monkeypatch) -> None:
+ with (tmp_path / "testdriver2.toml").open("w") as sink:
+ sink.write(
+ """
+name = "my driver"
+version = "0.1.0"
+
+[Driver]
+shared = "adbc_driver_goosedb"
+ """
+ )
+
+ monkeypatch.setenv("ADBC_DRIVER_PATH", str(tmp_path))
+
+ with pytest.raises(
+ adbc_driver_manager.dbapi.Error, match="adbc_driver_goosedb"
+ ) as excinfo:
+ with adbc_driver_manager.dbapi.connect(driver="testdriver2"):
+ pass
+
+ assert "ADBC_DRIVER_PATH: " + str(tmp_path) in str(excinfo.value)
+ assert "found {}".format(str(tmp_path / "testdriver2.toml")) in
str(excinfo.value)
+
+
+def test_manifest_indirect_missing_platform(tmp_path, monkeypatch) -> None:
+ with (tmp_path / "testdriver3.toml").open("w") as sink:
+ sink.write(
+ """
+name = "my driver"
+version = "0.1.0"
+
+[Driver.shared]
+msdos_itanium64 = "adbc_driver_sqlite"
+ """
+ )
+
+ monkeypatch.setenv("ADBC_DRIVER_PATH", str(tmp_path))
+
+ with pytest.raises(adbc_driver_manager.dbapi.Error) as excinfo:
+ with adbc_driver_manager.dbapi.connect(driver="testdriver3"):
+ pass
+
+ assert "ADBC_DRIVER_PATH: " + str(tmp_path) in str(excinfo.value)
+ assert "found {}".format(str(tmp_path / "testdriver3.toml")) in
str(excinfo.value)
+ assert "Architectures found: msdos_itanium64" in str(excinfo.value)
+
+
+def test_manifest_bad(tmp_path, monkeypatch) -> None:
+ with (tmp_path / "testdriver4.toml").open("w") as sink:
+ sink.write(
+ """
+name = "my driver"
+version = "0.1.0"
+ """
+ )
+
+ monkeypatch.setenv("ADBC_DRIVER_PATH", str(tmp_path))
+
+ with pytest.raises(
+ adbc_driver_manager.dbapi.Error, match="Driver path not defined in
manifest"
+ ):
+ with adbc_driver_manager.dbapi.connect(driver="testdriver4"):
+ pass
