kou commented on code in PR #2918:
URL: https://github.com/apache/arrow-adbc/pull/2918#discussion_r2125132489
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +128,175 @@ struct OwnedError {
}
};
+std::filesystem::path UserConfigDir() {
+ std::filesystem::path config_dir;
+#if defined(_WIN32)
+ auto dir = std::getenv("AppData");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "ADBC/drivers";
+ }
+#elif defined(__APPLE__)
+ auto dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "Library/Application Support/ADBC";
+ }
+#elif defined(__linux__)
+ auto dir = std::getenv("XDG_CONFIG_HOME");
+ if (!dir) {
+ dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ }
+ } else {
+ config_dir = std::filesystem::path(dir);
+ }
+
+ if (!config_dir.empty()) {
+ config_dir /= "adbc";
+ }
+#endif
+
+ return config_dir;
+}
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::string lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::string_view subkey) noexcept
+ : root_(root), key_(nullptr), is_open_(false) {
+ auto result = RegOpenKeyExA(root_, subkey.data(), 0, KEY_READ, &key_);
Review Comment:
It may be better that we use `xxxW()` functions such as `RegOpenKeyExW()`
not `xxxA()` functions for newly written code.
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +128,175 @@ struct OwnedError {
}
};
+std::filesystem::path UserConfigDir() {
+ std::filesystem::path config_dir;
+#if defined(_WIN32)
+ auto dir = std::getenv("AppData");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "ADBC/drivers";
+ }
+#elif defined(__APPLE__)
+ auto dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "Library/Application Support/ADBC";
+ }
+#elif defined(__linux__)
+ auto dir = std::getenv("XDG_CONFIG_HOME");
+ if (!dir) {
+ dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ }
+ } else {
+ config_dir = std::filesystem::path(dir);
+ }
+
+ if (!config_dir.empty()) {
+ config_dir /= "adbc";
+ }
+#endif
+
+ return config_dir;
+}
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::string lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::string_view subkey) noexcept
+ : root_(root), key_(nullptr), is_open_(false) {
+ auto result = RegOpenKeyExA(root_, subkey.data(), 0, KEY_READ, &key_);
+ if (result == ERROR_SUCCESS) {
+ is_open_ = true;
+ }
+ }
+
+ ~RegistryKey() {
+ if (is_open_ && key_ != nullptr) {
+ RegCloseKey(key_);
+ key_ = nullptr;
+ is_open_ = false;
+ }
+ }
+
+ HKEY key() const { return key_; }
+ bool is_open() const { return is_open_; }
+
+ std::string GetString(const std::string& name, const std::string&
default_value = "") {
+ if (!is_open_) return default_value;
+
+ DWORD type = REG_SZ;
+ DWORD size = 0;
+ auto result = RegQueryValueExA(key_, name.c_str(), nullptr, &type,
nullptr, &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ if (type != REG_SZ) return default_value;
+
+ std::string value(size, '\0');
+ result = RegQueryValueExA(key_, name.c_str(), nullptr, &type,
+ reinterpret_cast<LPBYTE>(value.data()), &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ return value;
+ }
+
+ private:
+ HKEY root_;
+ HKEY key_;
+ bool is_open_;
+};
+
+AdbcStatusCode LoadDriverFromRegistry(HKEY root, const std::string&
driver_name,
+ DriverInfo& info, struct AdbcError*
error) {
+ static const LPCSTR kAdbcDriverRegistry = "SOFTWARE\\ADBC\\Drivers";
+ RegistryKey drivers_key(root, kAdbcDriverRegistry);
+ if (!drivers_key.is_open()) {
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ RegistryKey dkey(drivers_key.key(), driver_name);
+ if (!dkey.is_open()) {
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ info.driver_name = dkey.GetString("name");
+ info.entrypoint = dkey.GetString("entrypoint");
+ info.version = dkey.GetString("version");
+ info.source = dkey.GetString("source");
+ info.lib_path = dkey.GetString("driver");
+ if (info.lib_path.empty()) {
+ SetError(error, "Driver path not found in registry");
+ return ADBC_STATUS_NOT_FOUND;
+ }
+ return ADBC_STATUS_OK;
+}
+#endif
+
+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());
+ return ADBC_STATUS_INVALID_STATE;
+ }
+
+ info.manifest_file = driver_manifest.string();
+ info.driver_name = config["name"].value_or(""s);
+ info.entrypoint = config["entrypoint"].value_or(""s);
+ info.version = config["version"].value_or(""s);
+ info.source = config["source"].value_or(""s);
+ auto lib = config.at_path("Driver.shared").value<std::string>();
+ if (!lib) {
+ SetError(error, "Driver path not found in manifest");
Review Comment:
We may be able to add more information to this error message for easy to
debug.
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -138,6 +320,176 @@ struct ManagedLibrary {
// release() from the DLL - how to handle this?
}
+ AdbcStatusCode GetDriverInfo(const std::string_view driver_name,
+ const uint16_t load_options, DriverInfo& info,
+ struct AdbcError* error) {
+ if (driver_name.empty()) {
+ SetError(error, "Driver name is empty");
+ return ADBC_STATUS_INVALID_STATE;
+ }
+
+ std::filesystem::path driver_path(driver_name);
+ const bool allow_relative_paths =
+ load_options & static_cast<uint16_t>(kAdbcAllowRelativePaths);
+ if (driver_path.has_extension()) {
+ if (driver_path.is_relative() && !allow_relative_paths) {
+ SetError(error, "Driver path is relative and relative paths are not
allowed");
+ return ADBC_STATUS_INVALID_STATE;
+ }
+
+ // if the extension is .toml, attempt to load the manifest
+ // erroring if we fail
+#ifdef _WIN32
+ if (strncasecmp(driver_path.extension().string().c_str(), ".toml", 5) ==
0) {
+#else
+ if (driver_path.extension() == ".toml") {
+#endif
+ auto status = LoadDriverManifest(driver_path, info, error);
+ if (status == ADBC_STATUS_OK) {
+ return Load(info.lib_path.c_str(), error);
+ }
+ return status;
+ }
+
+ // if the extension is not .toml, then just try to load the provided
+ // path as if it was an absolute path to a driver library
+ return Load(driver_path.string().c_str(), error);
+ }
+
+ if (driver_path.is_absolute()) {
+ // if we have an absolute path without an extension, first see if
there's a
+ // toml file with the same name.
+ driver_path.replace_extension(".toml");
+ if (std::filesystem::exists(driver_path)) {
+ auto status = LoadDriverManifest(driver_path, info, error);
+ if (status == ADBC_STATUS_OK) {
+ return Load(info.lib_path.c_str(), error);
+ }
+ }
+
+ // otherwise just try to load the provided path as if it was an absolute
path
+ return Load(driver_name.data(), error);
+ }
+
+ if (driver_path.has_extension()) {
+ if (driver_path.is_relative() &&
+ (load_options & static_cast<uint16_t>(kAdbcAllowRelativePaths)) ==
0) {
+ SetError(error, "Driver path is relative and relative paths are not
allowed");
+ return ADBC_STATUS_INVALID_STATE;
+ }
+
+#if defined(_WIN32)
+ static const std::string kPlatformLibrarySuffix = ".dll";
+ if (strncasecmp(driver_path.extension().string().c_str(),
+ kPlatformLibrarySuffix.c_str(),
+ kPlatformLibrarySuffix.size()) == 0) {
Review Comment:
`.DLLX` may be matched too.
It may be better that we have a helper function that compares a path
extension.
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -138,6 +320,176 @@ struct ManagedLibrary {
// release() from the DLL - how to handle this?
}
+ AdbcStatusCode GetDriverInfo(const std::string_view driver_name,
+ const uint16_t load_options, DriverInfo& info,
+ struct AdbcError* error) {
+ if (driver_name.empty()) {
+ SetError(error, "Driver name is empty");
+ return ADBC_STATUS_INVALID_STATE;
+ }
+
+ std::filesystem::path driver_path(driver_name);
+ const bool allow_relative_paths =
+ load_options & static_cast<uint16_t>(kAdbcAllowRelativePaths);
+ if (driver_path.has_extension()) {
+ if (driver_path.is_relative() && !allow_relative_paths) {
+ SetError(error, "Driver path is relative and relative paths are not
allowed");
+ return ADBC_STATUS_INVALID_STATE;
+ }
+
+ // if the extension is .toml, attempt to load the manifest
+ // erroring if we fail
+#ifdef _WIN32
+ if (strncasecmp(driver_path.extension().string().c_str(), ".toml", 5) ==
0) {
Review Comment:
Does this match with `.tomlX` too?
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +128,175 @@ struct OwnedError {
}
};
+std::filesystem::path UserConfigDir() {
+ std::filesystem::path config_dir;
+#if defined(_WIN32)
+ auto dir = std::getenv("AppData");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "ADBC/drivers";
+ }
+#elif defined(__APPLE__)
+ auto dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "Library/Application Support/ADBC";
+ }
+#elif defined(__linux__)
+ auto dir = std::getenv("XDG_CONFIG_HOME");
+ if (!dir) {
+ dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ }
+ } else {
+ config_dir = std::filesystem::path(dir);
+ }
+
+ if (!config_dir.empty()) {
+ config_dir /= "adbc";
+ }
+#endif
+
+ return config_dir;
+}
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::string lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::string_view subkey) noexcept
+ : root_(root), key_(nullptr), is_open_(false) {
+ auto result = RegOpenKeyExA(root_, subkey.data(), 0, KEY_READ, &key_);
+ if (result == ERROR_SUCCESS) {
+ is_open_ = true;
+ }
Review Comment:
```suggestion
is_open_ = (result == ERROR_SUCCESS);
```
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +128,175 @@ struct OwnedError {
}
};
+std::filesystem::path UserConfigDir() {
+ std::filesystem::path config_dir;
+#if defined(_WIN32)
+ auto dir = std::getenv("AppData");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "ADBC/drivers";
+ }
+#elif defined(__APPLE__)
+ auto dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "Library/Application Support/ADBC";
+ }
+#elif defined(__linux__)
+ auto dir = std::getenv("XDG_CONFIG_HOME");
+ if (!dir) {
+ dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ }
+ } else {
+ config_dir = std::filesystem::path(dir);
+ }
+
+ if (!config_dir.empty()) {
+ config_dir /= "adbc";
+ }
+#endif
+
+ return config_dir;
+}
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::string lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::string_view subkey) noexcept
+ : root_(root), key_(nullptr), is_open_(false) {
+ auto result = RegOpenKeyExA(root_, subkey.data(), 0, KEY_READ, &key_);
+ if (result == ERROR_SUCCESS) {
+ is_open_ = true;
+ }
+ }
+
+ ~RegistryKey() {
+ if (is_open_ && key_ != nullptr) {
+ RegCloseKey(key_);
+ key_ = nullptr;
+ is_open_ = false;
+ }
+ }
+
+ HKEY key() const { return key_; }
+ bool is_open() const { return is_open_; }
+
+ std::string GetString(const std::string& name, const std::string&
default_value = "") {
+ if (!is_open_) return default_value;
+
+ DWORD type = REG_SZ;
+ DWORD size = 0;
+ auto result = RegQueryValueExA(key_, name.c_str(), nullptr, &type,
nullptr, &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ if (type != REG_SZ) return default_value;
+
+ std::string value(size, '\0');
+ result = RegQueryValueExA(key_, name.c_str(), nullptr, &type,
+ reinterpret_cast<LPBYTE>(value.data()), &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ return value;
+ }
+
+ private:
+ HKEY root_;
+ HKEY key_;
+ bool is_open_;
+};
+
+AdbcStatusCode LoadDriverFromRegistry(HKEY root, const std::string&
driver_name,
+ DriverInfo& info, struct AdbcError*
error) {
+ static const LPCSTR kAdbcDriverRegistry = "SOFTWARE\\ADBC\\Drivers";
+ RegistryKey drivers_key(root, kAdbcDriverRegistry);
+ if (!drivers_key.is_open()) {
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ RegistryKey dkey(drivers_key.key(), driver_name);
+ if (!dkey.is_open()) {
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ info.driver_name = dkey.GetString("name");
+ info.entrypoint = dkey.GetString("entrypoint");
+ info.version = dkey.GetString("version");
+ info.source = dkey.GetString("source");
+ info.lib_path = dkey.GetString("driver");
+ if (info.lib_path.empty()) {
+ SetError(error, "Driver path not found in registry");
+ return ADBC_STATUS_NOT_FOUND;
+ }
+ return ADBC_STATUS_OK;
+}
+#endif
+
+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());
+ return ADBC_STATUS_INVALID_STATE;
+ }
+
+ info.manifest_file = driver_manifest.string();
+ info.driver_name = config["name"].value_or(""s);
+ info.entrypoint = config["entrypoint"].value_or(""s);
+ info.version = config["version"].value_or(""s);
+ info.source = config["source"].value_or(""s);
+ auto lib = config.at_path("Driver.shared").value<std::string>();
+ if (!lib) {
+ SetError(error, "Driver path not found in manifest");
+ return ADBC_STATUS_NOT_FOUND;
+ }
+ info.lib_path = lib.value();
+
+ return ADBC_STATUS_OK;
+}
+
+std::vector<std::filesystem::path> GetSearchPaths(const uint16_t levels) {
+ std::vector<std::filesystem::path> paths;
+ if (levels & static_cast<uint16_t>(kAdbcSearchEnvironment)) {
+ // Check the ADBC_DRIVER_PATH environment variable
+ const char* env_path = std::getenv("ADBC_CONFIG_PATH");
Review Comment:
We may need to update `ADBC_DRIVER_PATH` in the comment or
`ADBC_CONFIG_PATH` in `std::getenv()`.
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +128,175 @@ struct OwnedError {
}
};
+std::filesystem::path UserConfigDir() {
+ std::filesystem::path config_dir;
+#if defined(_WIN32)
+ auto dir = std::getenv("AppData");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "ADBC/drivers";
+ }
+#elif defined(__APPLE__)
+ auto dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ config_dir /= "Library/Application Support/ADBC";
+ }
+#elif defined(__linux__)
+ auto dir = std::getenv("XDG_CONFIG_HOME");
+ if (!dir) {
+ dir = std::getenv("HOME");
+ if (dir) {
+ config_dir = std::filesystem::path(dir);
+ }
+ } else {
+ config_dir = std::filesystem::path(dir);
+ }
+
+ if (!config_dir.empty()) {
+ config_dir /= "adbc";
+ }
+#endif
+
+ return config_dir;
+}
+
// Driver state
+struct DriverInfo {
+ std::string manifest_file;
+ std::string driver_name;
+ std::string lib_path;
+ std::string entrypoint;
+
+ std::string version;
+ std::string source;
+};
+
+#ifdef _WIN32
+class RegistryKey {
+ public:
+ RegistryKey(HKEY root, const std::string_view subkey) noexcept
+ : root_(root), key_(nullptr), is_open_(false) {
+ auto result = RegOpenKeyExA(root_, subkey.data(), 0, KEY_READ, &key_);
+ if (result == ERROR_SUCCESS) {
+ is_open_ = true;
+ }
+ }
+
+ ~RegistryKey() {
+ if (is_open_ && key_ != nullptr) {
+ RegCloseKey(key_);
+ key_ = nullptr;
+ is_open_ = false;
+ }
+ }
+
+ HKEY key() const { return key_; }
+ bool is_open() const { return is_open_; }
+
+ std::string GetString(const std::string& name, const std::string&
default_value = "") {
+ if (!is_open_) return default_value;
+
+ DWORD type = REG_SZ;
+ DWORD size = 0;
+ auto result = RegQueryValueExA(key_, name.c_str(), nullptr, &type,
nullptr, &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ if (type != REG_SZ) return default_value;
+
+ std::string value(size, '\0');
+ result = RegQueryValueExA(key_, name.c_str(), nullptr, &type,
+ reinterpret_cast<LPBYTE>(value.data()), &size);
+ if (result != ERROR_SUCCESS) return default_value;
+ return value;
+ }
+
+ private:
+ HKEY root_;
+ HKEY key_;
+ bool is_open_;
+};
+
+AdbcStatusCode LoadDriverFromRegistry(HKEY root, const std::string&
driver_name,
+ DriverInfo& info, struct AdbcError*
error) {
+ static const LPCSTR kAdbcDriverRegistry = "SOFTWARE\\ADBC\\Drivers";
+ RegistryKey drivers_key(root, kAdbcDriverRegistry);
+ if (!drivers_key.is_open()) {
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ RegistryKey dkey(drivers_key.key(), driver_name);
+ if (!dkey.is_open()) {
+ return ADBC_STATUS_NOT_FOUND;
+ }
+
+ info.driver_name = dkey.GetString("name");
+ info.entrypoint = dkey.GetString("entrypoint");
+ info.version = dkey.GetString("version");
+ info.source = dkey.GetString("source");
+ info.lib_path = dkey.GetString("driver");
+ if (info.lib_path.empty()) {
+ SetError(error, "Driver path not found in registry");
Review Comment:
If we add more information such as driver name to error message, it may be
useful for debug.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]