lidavidm commented on code in PR #2918:
URL: https://github.com/apache/arrow-adbc/pull/2918#discussion_r2151120349
##########
c/driver_manager/adbc_driver_manager_test.cc:
##########
@@ -332,7 +343,249 @@ TEST(AdbcDriverManagerInternal,
AdbcDriverManagerDefaultEntrypoint) {
"C:\\System32\\proprietary_engine.dll",
}) {
SCOPED_TRACE(driver);
- EXPECT_EQ("AdbcProprietaryEngineInit",
::AdbcDriverManagerDefaultEntrypoint(driver));
+ EXPECT_EQ("AdbcProprietaryEngineInit",
+ ::InternalAdbcDriverManagerDefaultEntrypoint(driver));
+ }
+
+ for (const auto& driver : {
+ "driver_example",
+ "libdriver_example.so",
+ }) {
+ SCOPED_TRACE(driver);
+ EXPECT_EQ("AdbcDriverExampleInit",
+ ::InternalAdbcDriverManagerDefaultEntrypoint(driver));
}
}
+
+TEST(AdbcDriverManagerInternal, InternalAdbcParsePath) {
+ // Test parsing a path of directories
+#ifdef __WIN32
+ static const char* const delimiter = ";";
+#else
+ static const char* const delimiter = ":";
+#endif
+
+ std::vector<std::string> paths = {
+ "/usr/lib/adbc/drivers", "/usr/local/lib/adbc/drivers",
+ "/opt/adbc/drivers", "/home/user/.config/adbc/drivers",
+ "/home/\":foo:\"/bar",
+ };
+
+ std::ostringstream joined;
+ std::copy(paths.begin(), paths.end(),
+ std::ostream_iterator<std::string>(joined, delimiter));
+
+ auto output = InternalAdbcParsePath(joined.str());
+ EXPECT_EQ(output.size(), paths.size());
+
+ for (size_t i = 0; i < paths.size(); ++i) {
+ EXPECT_EQ(output[i].string(), paths[i]);
+ }
+}
+
+class DriverManifest : public ::testing::Test {
+ public:
+ void SetUp() override {
+ std::memset(&driver, 0, sizeof(driver));
+ std::memset(&error, 0, sizeof(error));
+
+#ifndef ADBC_DRIVER_MANAGER_TEST_LIB
+ GTEST_SKIP() << "ADBC_DRIVER_MANAGER_TEST_LIB is not defined. "
+ "This test requires a driver library to be specified.";
+#else
+ driver_path = std::filesystem::path(ADBC_DRIVER_MANAGER_TEST_LIB);
+ if (!std::filesystem::exists(driver_path)) {
+ GTEST_SKIP() << "Driver library does not exist: " << driver_path;
+ }
+
+ simple_manifest = toml::table{
+ {"name", "SQLite3"},
+ {"publisher", "arrow-adbc"},
+ {"version", "X.Y.Z"},
+ {"ADBC",
+ toml::table{
+ {"version", "1.1.0"},
+ }},
+ {"Driver",
+ toml::table{
+ {"shared",
+ toml::table{
+ {current_arch(), driver_path.string()},
+ }},
+ }},
+ };
+
+ auto temp_path = std::filesystem::temp_directory_path();
+ temp_path /= "adbc_driver_manager_test";
+
+ ASSERT_TRUE(std::filesystem::create_directories(temp_path));
+ temp_dir = temp_path;
+#endif
+ }
+
+ void TearDown() override {
+ if (error.release) {
+ error.release(&error);
+ }
+
+ if (driver.release) {
+ ASSERT_THAT(driver.release(&driver, &error), IsOkStatus(&error));
+ ASSERT_EQ(driver.private_data, nullptr);
+ ASSERT_EQ(driver.private_manager, nullptr);
+ }
+
+ driver_path.clear();
+ if (std::filesystem::exists(temp_dir)) {
+ std::filesystem::remove_all(temp_dir);
+ }
+ }
+
+ protected:
+ void set_config_path(const char* path) {
+#ifdef _WIN32
+ ASSERT_TRUE(SetEnvironmentVariable("ADBC_CONFIG_PATH", path));
+#else
+ setenv("ADBC_CONFIG_PATH", path, 1);
+#endif
+ }
+
+ void unset_config_path() { set_config_path(""); }
Review Comment:
we only use snake case for trivial accessors, fwiw
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -632,6 +985,36 @@ static const char kDefaultEntrypoint[] = "AdbcDriverInit";
// Other helpers (intentionally not in an anonymous namespace so they can be
tested)
+std::vector<std::filesystem::path> AdbcParsePath(const std::string_view& path)
{
+ std::vector<std::filesystem::path> result;
+ if (path.empty()) {
+ return result;
+ }
+
+#ifdef _WIN32
+ constexpr char delimiter = ';';
+#else
+ constexpr char delimiter = ':';
+#endif // _WIN32
+
+ bool in_quotes = false;
Review Comment:
This still isn't resolved FWIW
##########
c/driver_manager/current_arch.h:
##########
@@ -0,0 +1,84 @@
+// 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.
+
+#pragma once
+
+#include <string>
+
+const std::string& current_arch() {
Review Comment:
Put it in a namespace? Also could use string_view and make it constexpr
##########
c/driver_manager/adbc_driver_manager.cc:
##########
@@ -114,7 +128,228 @@ struct OwnedError {
}
};
+std::filesystem::path UserConfigDir() {
+ std::filesystem::path config_dir;
+#if defined(_WIN32)
+ size_t required_size;
+ _wgetenv_s(&required_size, NULL, 0, L"AppData");
+ if (required_size == 0) {
+ return config_dir;
+ }
+
+ std::wstring app_data_value;
+ app_data_value.resize(required_size);
+ _wgetenv_s(&required_size, app_data_value.data(), required_size, L"AppData");
+ std::filesystem::path dir(app_data_value);
+ if (!dir.empty()) {
+ 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) /= ".config";
+ }
+ } else {
+ config_dir = std::filesystem::path(dir);
+ }
+
+ if (!config_dir.empty()) {
+ config_dir /= "adbc";
+ }
+#endif
+
+ return config_dir;
+}
+
+#ifdef _WIN32
+using char_type = wchar_t;
+
+std::string utf8_encode(const std::wstring& wstr) {
Review Comment:
```suggestion
std::string Utf8Encode(const std::wstring& wstr) {
```
##########
c/driver_manager/current_arch.h:
##########
@@ -0,0 +1,84 @@
+// 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.
+
+#pragma once
+
+#include <string>
+
+const std::string& current_arch() {
Review Comment:
```suggestion
const std::string& CurrentArch() {
```
##########
c/driver_manager/current_arch.h:
##########
@@ -0,0 +1,84 @@
+// 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.
+
+#pragma once
+
+#include <string>
+
+const std::string& current_arch() {
Review Comment:
Also where was this sourced from?
##########
c/driver_manager/adbc_driver_manager_test.cc:
##########
@@ -15,19 +15,29 @@
// specific language governing permissions and limitations
// under the License.
+#if defined(_WIN32)
+#include <windows.h>
+#endif
+
#include <gmock/gmock.h>
#include <gtest/gtest.h>
+#include <stdlib.h>
+#include <algorithm>
+#include <filesystem> // NOLINT [build/c++17]
Review Comment:
We should just disable the lint in cpplint, it seems rather outdated anyways
--
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]
