Skip to content
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
84cac0c
feat(c/driver_manager,rust/driver_manager): handle virtual environmen…
zeroshade Aug 20, 2025
8d87bee
Update docs/source/format/driver_manifests.rst
zeroshade Aug 20, 2025
4062f95
Update docs/source/format/driver_manifests.rst
zeroshade Aug 20, 2025
1df42a1
Update docs/source/format/driver_manifests.rst
zeroshade Aug 20, 2025
c9b48f0
darn you windows
zeroshade Aug 20, 2025
e513570
fix other references to ADBC_CONFIG_PATH
zeroshade Aug 20, 2025
67a18de
fix lint
zeroshade Aug 20, 2025
69cf5b2
fix rust args
zeroshade Aug 21, 2025
2a427ba
avoid bad move
zeroshade Aug 21, 2025
8375be9
make up your damn mind
zeroshade Aug 21, 2025
824292f
remove VIRTUAL_ENV, add additional search paths
zeroshade Aug 21, 2025
5749c49
fix R build
zeroshade Aug 21, 2025
8314ee2
fix R build
zeroshade Aug 21, 2025
f9aec98
fix R docs
zeroshade Aug 21, 2025
06113f8
add docs to R
zeroshade Aug 21, 2025
245728c
fix typo
zeroshade Aug 21, 2025
0d4021c
only check CONDA_PREFIX during CONDA_BUILD
zeroshade Aug 21, 2025
6b23dfd
cargo conda_build
zeroshade Aug 21, 2025
8148485
updates from comments
zeroshade Aug 22, 2025
86944f5
fix R stuff
zeroshade Aug 22, 2025
f8473e9
R is annoying
zeroshade Aug 22, 2025
4152f1f
maybe this time?
zeroshade Aug 22, 2025
b995130
maybe this time
zeroshade Aug 22, 2025
dc51441
Update docs
ianmcook Aug 23, 2025
e32e72a
Fix indentation in RST
ianmcook Aug 24, 2025
59383a5
fix logic for additoinal_paths
zeroshade Aug 25, 2025
db25d63
re-run pre-commit
zeroshade Aug 25, 2025
3013e78
add #endif comments
zeroshade Aug 25, 2025
07eefc1
dedup some logic
zeroshade Aug 25, 2025
908c695
Improve comments
ianmcook Aug 25, 2025
fd8fc8b
fix ADBC_CONFIG_PATH for wchar
zeroshade Aug 27, 2025
c6f45a8
Update c/driver_manager/adbc_driver_manager.cc
zeroshade Aug 27, 2025
6dfb580
Update c/driver_manager/adbc_driver_manager.cc
zeroshade Aug 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions c/driver_manager/CMakeLists.txt
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may need similar checks in other builds? (R driver manager?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the R Makevars and Makevars.win to check for CONDA_BUILD appropriately

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's very kind of you (there are almost no conda R users so even though this is great I'm not particularly worried about it).

Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ foreach(LIB_TARGET ${ADBC_LIBRARIES})
${REPOSITORY_ROOT}/c/vendor
${REPOSITORY_ROOT}/c/driver)
target_compile_definitions(${LIB_TARGET} PRIVATE ADBC_EXPORTING)
if("$ENV{CONDA_BUILD}" STREQUAL "1")
target_compile_definitions(${LIB_TARGET} PRIVATE ADBC_CONDA_BUILD=1)
else()
target_compile_definitions(${LIB_TARGET} PRIVATE ADBC_CONDA_BUILD=0)
endif()
endforeach()

if(ADBC_BUILD_TESTS)
Expand Down
100 changes: 89 additions & 11 deletions c/driver_manager/adbc_driver_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,35 @@ AdbcStatusCode LoadDriverManifest(const std::filesystem::path& driver_manifest,
return ADBC_STATUS_OK;
}

#if ADBC_CONDA_BUILD
#ifdef _WIN32
std::filesystem::path GetEnvAsPath(const wchar_t* env_var) {
size_t required_size;

_wgetenv_s(&required_size, NULL, 0, env_var);
if (required_size == 0) {
return {};
}

std::wstring path_var;
path_var.resize(required_size);
_wgetenv_s(&required_size, path_var.data(), required_size, env_var);
return std::filesystem::path(path_var);
}
#else
std::filesystem::path GetEnvAsPath(const char* env_var) {
const char* path = std::getenv(env_var);
if (path) {
std::string_view venv(path);
if (!venv.empty()) {
return std::filesystem::path(venv);
}
}
return {};
}
#endif
#endif

std::vector<std::filesystem::path> GetSearchPaths(const AdbcLoadFlags levels) {
std::vector<std::filesystem::path> paths;
if (levels & ADBC_LOAD_FLAG_SEARCH_ENV) {
Expand All @@ -281,6 +310,19 @@ std::vector<std::filesystem::path> GetSearchPaths(const AdbcLoadFlags levels) {
if (env_path) {
paths = InternalAdbcParsePath(env_path);
}

#if ADBC_CONDA_BUILD
#ifdef _WIN32
const wchar_t* conda_name = L"CONDA_PREFIX";
#else
const char* conda_name = "CONDA_PREFIX";
#endif

std::filesystem::path venv = GetEnvAsPath(conda_name);
if (!venv.empty()) {
paths.push_back(venv / "etc" / "adbc");
}
#endif
}

if (levels & ADBC_LOAD_FLAG_SEARCH_USER) {
Expand Down Expand Up @@ -344,9 +386,10 @@ struct ManagedLibrary {
// release() from the DLL - how to handle this?
}

AdbcStatusCode GetDriverInfo(const std::string_view driver_name,
const AdbcLoadFlags load_options, DriverInfo& info,
struct AdbcError* error) {
AdbcStatusCode GetDriverInfo(
const std::string_view driver_name, const AdbcLoadFlags load_options,
const std::vector<std::filesystem::path>& additional_search_paths, DriverInfo& info,
struct AdbcError* error) {
if (driver_name.empty()) {
SetError(error, "Driver name is empty");
return ADBC_STATUS_INVALID_ARGUMENT;
Expand Down Expand Up @@ -418,7 +461,7 @@ struct ManagedLibrary {

// not an absolute path, no extension. Let's search the configured paths
// based on the options
return FindDriver(driver_path, load_options, info, error);
return FindDriver(driver_path, load_options, additional_search_paths, info, error);
}

AdbcStatusCode SearchPaths(const std::filesystem::path& driver_path,
Expand Down Expand Up @@ -446,14 +489,22 @@ struct ManagedLibrary {
return ADBC_STATUS_NOT_FOUND;
}

AdbcStatusCode FindDriver(const std::filesystem::path& driver_path,
const AdbcLoadFlags load_options, DriverInfo& info,
struct AdbcError* error) {
AdbcStatusCode FindDriver(
const std::filesystem::path& driver_path, const AdbcLoadFlags load_options,
const std::vector<std::filesystem::path>& additional_search_paths, DriverInfo& info,
struct AdbcError* error) {
if (driver_path.empty()) {
SetError(error, "Driver path is empty");
return ADBC_STATUS_INVALID_ARGUMENT;
}

{
auto status = SearchPaths(driver_path, additional_search_paths, info, error);
if (status == ADBC_STATUS_OK) {
return status;
}
}

Comment on lines +501 to +490
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is currently implemented, if the user adds additional search paths, are those searched before ADBC_CONFIG_PATH?

I don't think that's the behavior we want. We want them to be searched after ADBC_CONFIG_PATH but before anything else, correct?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we? I thought we wanted the application defined search paths to be searched first? Though I guess it makes sense to search after the env var now that I think of it... I'll fix this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my docs update, I wrote that:

  • ADBC_CONFIG_PATH comes before the user-added search paths
  • $CONDA_PREFIX/etc/adbc comes after the user-added search paths

After you update the implementation, please confirm that that's correct!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the $CONDA_PREFIX/etc/adbc path (if it exists) will be listed as part of the ENV search paths, so it's difficult to insert the user-added paths before that but after the ADBC_CONFIG_PATH paths without a refactor. I'll see what I can do, but it's gonna be weird....

Copy link
Copy Markdown
Member

@ianmcook ianmcook Aug 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Easy but hacky way to do it: If user adds additional search paths AND LOAD_FLAG_SEARCH_ENV flag is set AND ADBC_CONFIG_PATH env var is set, then add $ADBC_CONFIG_PATH to the beginning of the user-added search paths.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, the way you did it looks much cleaner :)

#ifdef _WIN32
// windows is slightly more complex since we also need to check registry keys
// so we can't just grab the search paths only.
Expand Down Expand Up @@ -991,6 +1042,7 @@ struct TempDatabase {
std::string entrypoint;
AdbcDriverInitFunc init_func = nullptr;
AdbcLoadFlags load_flags = ADBC_LOAD_FLAG_ALLOW_RELATIVE_PATHS;
std::string additional_search_path_list;
};

/// Temporary state while the database is being configured.
Expand Down Expand Up @@ -1362,6 +1414,22 @@ AdbcStatusCode AdbcDriverManagerDatabaseSetLoadFlags(struct AdbcDatabase* databa
return ADBC_STATUS_OK;
}

AdbcStatusCode AdbcDriverManagerDatabaseSetAdditionalSearchPathList(
struct AdbcDatabase* database, const char* path_list, struct AdbcError* error) {
if (database->private_driver) {
SetError(error, "Cannot SetAdditionalSearchPathList after AdbcDatabaseInit");
return ADBC_STATUS_INVALID_STATE;
}

TempDatabase* args = reinterpret_cast<TempDatabase*>(database->private_data);
if (path_list) {
args->additional_search_path_list.assign(path_list);
} else {
args->additional_search_path_list.clear();
}
return ADBC_STATUS_OK;
}

AdbcStatusCode AdbcDriverManagerDatabaseSetInitFunc(struct AdbcDatabase* database,
AdbcDriverInitFunc init_func,
struct AdbcError* error) {
Expand Down Expand Up @@ -1399,10 +1467,12 @@ AdbcStatusCode AdbcDatabaseInit(struct AdbcDatabase* database, struct AdbcError*
} else if (!args->entrypoint.empty()) {
status = AdbcFindLoadDriver(args->driver.c_str(), args->entrypoint.c_str(),
ADBC_VERSION_1_1_0, args->load_flags,
args->additional_search_path_list.data(),
database->private_driver, error);
} else {
status = AdbcFindLoadDriver(args->driver.c_str(), nullptr, ADBC_VERSION_1_1_0,
args->load_flags, database->private_driver, error);
status = AdbcFindLoadDriver(
args->driver.c_str(), nullptr, ADBC_VERSION_1_1_0, args->load_flags,
args->additional_search_path_list.data(), database->private_driver, error);
}

if (status != ADBC_STATUS_OK) {
Expand Down Expand Up @@ -2134,6 +2204,7 @@ const char* AdbcStatusCodeMessage(AdbcStatusCode code) {

AdbcStatusCode AdbcFindLoadDriver(const char* driver_name, const char* entrypoint,
const int version, const AdbcLoadFlags load_options,
const char* additional_search_path_list,
void* raw_driver, struct AdbcError* error) {
AdbcDriverInitFunc init_func = nullptr;
std::string error_message;
Expand Down Expand Up @@ -2163,7 +2234,13 @@ AdbcStatusCode AdbcFindLoadDriver(const char* driver_name, const char* entrypoin
info.entrypoint = entrypoint;
}

AdbcStatusCode status = library.GetDriverInfo(driver_name, load_options, info, error);
std::vector<std::filesystem::path> additional_paths;
if (additional_search_path_list) {
additional_paths = InternalAdbcParsePath(additional_search_path_list);
}

AdbcStatusCode status =
library.GetDriverInfo(driver_name, load_options, additional_paths, info, error);
if (status != ADBC_STATUS_OK) {
driver->release = nullptr;
return status;
Expand Down Expand Up @@ -2205,7 +2282,8 @@ AdbcStatusCode AdbcLoadDriver(const char* driver_name, const char* entrypoint,
// but don't enable searching for manifests by default. It will need to be explicitly
// enabled by calling AdbcFindLoadDriver directly.
return AdbcFindLoadDriver(driver_name, entrypoint, version,
ADBC_LOAD_FLAG_ALLOW_RELATIVE_PATHS, raw_driver, error);
ADBC_LOAD_FLAG_ALLOW_RELATIVE_PATHS, nullptr, raw_driver,
error);
}

AdbcStatusCode AdbcLoadDriverFromInitFunc(AdbcDriverInitFunc init_func, int version,
Expand Down
45 changes: 23 additions & 22 deletions c/driver_manager/adbc_driver_manager_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ class DriverManifest : public ::testing::Test {

TEST_F(DriverManifest, LoadDriverEnv) {
ASSERT_THAT(AdbcFindLoadDriver("sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
Not(IsOkStatus(&error)));

std::ofstream test_manifest_file(temp_dir / "sqlite.toml");
Expand All @@ -468,7 +468,7 @@ TEST_F(DriverManifest, LoadDriverEnv) {
SetConfigPath(temp_dir.string().c_str());

ASSERT_THAT(AdbcFindLoadDriver("sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
IsOkStatus(&error));

ASSERT_TRUE(std::filesystem::remove(temp_dir / "sqlite.toml"));
Expand All @@ -478,7 +478,7 @@ TEST_F(DriverManifest, LoadDriverEnv) {

TEST_F(DriverManifest, LoadNonAsciiPath) {
ASSERT_THAT(AdbcFindLoadDriver("sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
Not(IsOkStatus(&error)));

#ifdef _WIN32
Expand All @@ -497,7 +497,7 @@ TEST_F(DriverManifest, LoadNonAsciiPath) {
SetConfigPath(non_ascii_dir.string().c_str());

ASSERT_THAT(AdbcFindLoadDriver("sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
IsOkStatus(&error));

ASSERT_TRUE(std::filesystem::remove(non_ascii_dir / "sqlite.toml"));
Expand All @@ -515,7 +515,7 @@ TEST_F(DriverManifest, DisallowEnvConfig) {

auto load_options = ADBC_LOAD_FLAG_DEFAULT & ~ADBC_LOAD_FLAG_SEARCH_ENV;
ASSERT_THAT(AdbcFindLoadDriver("sqlite", nullptr, ADBC_VERSION_1_1_0, load_options,
&driver, &error),
nullptr, &driver, &error),
Not(IsOkStatus(&error)));

ASSERT_TRUE(std::filesystem::remove(temp_dir / "sqlite.toml"));
Expand Down Expand Up @@ -543,7 +543,7 @@ TEST_F(DriverManifest, ConfigEntrypoint) {
test_manifest_file.close();

ASSERT_THAT(AdbcFindLoadDriver(filepath.string().data(), nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
Not(IsOkStatus(&error)));

ASSERT_TRUE(std::filesystem::remove(filepath));
Expand All @@ -557,7 +557,7 @@ TEST_F(DriverManifest, LoadAbsolutePath) {
test_manifest_file.close();

ASSERT_THAT(AdbcFindLoadDriver(filepath.string().data(), nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
IsOkStatus(&error));

ASSERT_TRUE(std::filesystem::remove(filepath));
Expand All @@ -573,7 +573,7 @@ TEST_F(DriverManifest, LoadAbsolutePathNoExtension) {
auto noext = filepath;
noext.replace_extension(); // Remove the .toml extension
ASSERT_THAT(AdbcFindLoadDriver(noext.string().data(), nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
IsOkStatus(&error));

ASSERT_TRUE(std::filesystem::remove(filepath));
Expand All @@ -585,13 +585,14 @@ TEST_F(DriverManifest, LoadRelativePath) {
test_manifest_file << simple_manifest;
test_manifest_file.close();

ASSERT_THAT(
AdbcFindLoadDriver("sqlite.toml", nullptr, ADBC_VERSION_1_1_0, 0, &driver, &error),
IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error));
ASSERT_THAT(AdbcFindLoadDriver("sqlite.toml", nullptr, ADBC_VERSION_1_1_0, 0, nullptr,
&driver, &error),
IsStatus(ADBC_STATUS_INVALID_ARGUMENT, &error));

ASSERT_THAT(AdbcFindLoadDriver("sqlite.toml", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_ALLOW_RELATIVE_PATHS, &driver, &error),
IsOkStatus(&error));
ASSERT_THAT(
AdbcFindLoadDriver("sqlite.toml", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_ALLOW_RELATIVE_PATHS, nullptr, &driver, &error),
IsOkStatus(&error));

ASSERT_TRUE(std::filesystem::remove("sqlite.toml"));
}
Expand All @@ -609,7 +610,7 @@ TEST_F(DriverManifest, ManifestMissingDriver) {

// Attempt to load the driver
ASSERT_THAT(AdbcFindLoadDriver(filepath.string().data(), nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
IsStatus(ADBC_STATUS_NOT_FOUND, &error));

ASSERT_TRUE(std::filesystem::remove(filepath));
Expand All @@ -634,7 +635,7 @@ TEST_F(DriverManifest, ManifestWrongArch) {

// Attempt to load the driver
ASSERT_THAT(AdbcFindLoadDriver(filepath.string().data(), nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
IsStatus(ADBC_STATUS_NOT_FOUND, &error));

ASSERT_TRUE(std::filesystem::remove(filepath));
Expand All @@ -645,7 +646,7 @@ TEST_F(DriverManifest, ManifestWrongArch) {
#ifdef ADBC_DRIVER_MANAGER_TEST_MANIFEST_USER_LEVEL
TEST_F(DriverManifest, LoadUserLevelManifest) {
ASSERT_THAT(AdbcFindLoadDriver("adbc-test-sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
Not(IsOkStatus(&error)));

auto user_config_dir = InternalAdbcUserConfigDir();
Expand All @@ -662,12 +663,12 @@ TEST_F(DriverManifest, LoadUserLevelManifest) {

// fail to load if flag doesn't have ADBC_LOAD_FLAG_SEARCH_USER
ASSERT_THAT(AdbcFindLoadDriver("adbc-test-sqlite", nullptr, ADBC_VERSION_1_1_0, 0,
&driver, &error),
nullptr, &driver, &error),
Not(IsOkStatus(&error)));

// succeed with default load options
ASSERT_THAT(AdbcFindLoadDriver("adbc-test-sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
IsOkStatus(&error));

ASSERT_TRUE(std::filesystem::remove(user_config_dir / "adbc-test-sqlite.toml"));
Expand All @@ -682,7 +683,7 @@ TEST_F(DriverManifest, LoadUserLevelManifest) {
#ifdef ADBC_DRIVER_MANAGER_TEST_MANIFEST_SYSTEM_LEVEL
TEST_F(DriverManifest, LoadSystemLevelManifest) {
ASSERT_THAT(AdbcFindLoadDriver("adbc-test-sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
Not(IsOkStatus(&error)));

auto system_config_dir = std::filesystem::path("/etc/adbc");
Expand All @@ -699,12 +700,12 @@ TEST_F(DriverManifest, LoadSystemLevelManifest) {

// fail to load if flag doesn't have ADBC_LOAD_FLAG_SEARCH_SYSTEM
ASSERT_THAT(AdbcFindLoadDriver("adbc-test-sqlite", nullptr, ADBC_VERSION_1_1_0, 0,
&driver, &error),
nullptr, &driver, &error),
Not(IsOkStatus(&error)));

// succeed with default load options
ASSERT_THAT(AdbcFindLoadDriver("adbc-test-sqlite", nullptr, ADBC_VERSION_1_1_0,
ADBC_LOAD_FLAG_DEFAULT, &driver, &error),
ADBC_LOAD_FLAG_DEFAULT, nullptr, &driver, &error),
IsOkStatus(&error));

ASSERT_TRUE(std::filesystem::remove(system_config_dir / "adbc-test-sqlite.toml"));
Expand Down
Loading
Loading