Skip to content

feat(c/driver_manager,rust/driver_manager): handle virtual environments in driver manager #3320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zeroshade merged 33 commits into from
Aug 27, 2025
Merged

feat(c/driver_manager,rust/driver_manager): handle virtual environments in driver manager #3320

Show file tree
Hide file tree
Changes from 31 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
View file
Open in desktop
Copy link
Copy Markdown
Member

@lidavidm lidavidm Aug 22, 2025

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

@zeroshade zeroshade Aug 22, 2025

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

@paleolimbot paleolimbot Aug 22, 2025

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
140 changes: 109 additions & 31 deletions c/driver_manager/adbc_driver_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ std::wstring Utf8Decode(const std::string& str) {

#else
using char_type = char;
#endif
#endif // _WIN32

// Driver state
struct DriverInfo {
Expand Down Expand Up @@ -239,7 +239,7 @@ AdbcStatusCode LoadDriverFromRegistry(HKEY root, const std::wstring& driver_name
}
return ADBC_STATUS_OK;
}
#endif
#endif // _WIN32

AdbcStatusCode LoadDriverManifest(const std::filesystem::path& driver_manifest,
DriverInfo& info, struct AdbcError* error) {
Expand Down Expand Up @@ -273,14 +273,39 @@ AdbcStatusCode LoadDriverManifest(const std::filesystem::path& driver_manifest,
return ADBC_STATUS_OK;
}

std::vector<std::filesystem::path> GetEnvPaths(const char_type* env_var) {
#ifdef _WIN32
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);
auto path = Utf8Encode(path_var);
#else
const char* path_var = std::getenv(env_var);
if (!path_var) {
return {};
}
std::string path(path_var);
#endif
return InternalAdbcParsePath(path);
}

std::vector<std::filesystem::path> GetSearchPaths(const AdbcLoadFlags levels) {
std::vector<std::filesystem::path> paths;
if (levels & ADBC_LOAD_FLAG_SEARCH_ENV) {
#ifdef _WIN32
static const wchar_t* env_var = L"ADBC_CONFIG_PATH";
#else
static const char* env_var = "ADBC_CONFIG_PATH";
#endif
// Check the ADBC_CONFIG_PATH environment variable
const char* env_path = std::getenv("ADBC_CONFIG_PATH");
if (env_path) {
paths = InternalAdbcParsePath(env_path);
}
paths = GetEnvPaths(env_var);
}

if (levels & ADBC_LOAD_FLAG_SEARCH_USER) {
Expand All @@ -305,7 +330,7 @@ std::vector<std::filesystem::path> GetSearchPaths(const AdbcLoadFlags levels) {
if (std::filesystem::exists(system_config_dir)) {
paths.push_back(system_config_dir);
}
#endif
#endif // defined(__APPLE__)
}

return paths;
Expand All @@ -319,7 +344,7 @@ bool HasExtension(const std::filesystem::path& path, const std::string& ext) {
_wcsnicmp(path_ext.data(), wext.data(), wext.size()) == 0;
#else
return path.extension() == ext;
#endif
#endif // _WIN32
}

/// A driver DLL.
Expand All @@ -344,9 +369,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 @@ -405,7 +431,7 @@ struct ManagedLibrary {
static const std::string kPlatformLibrarySuffix = ".dylib";
#else
static const std::string kPlatformLibrarySuffix = ".so";
#endif
#endif // defined(_WIN32)
if (HasExtension(driver_path, kPlatformLibrarySuffix)) {
info.lib_path = driver_path;
return Load(driver_path.c_str(), error);
Expand All @@ -418,7 +444,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,27 +472,52 @@ 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;
}

{
// First search the paths in the env var `ADBC_CONFIG_PATH`.
// Then search the runtime application-defined additional search paths.
auto search_paths = GetSearchPaths(load_options & ADBC_LOAD_FLAG_SEARCH_ENV);
search_paths.insert(search_paths.end(), additional_search_paths.begin(),
additional_search_paths.end());

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

@ianmcook ianmcook Aug 23, 2025

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

@zeroshade zeroshade Aug 23, 2025

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

@ianmcook ianmcook Aug 23, 2025

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

@zeroshade zeroshade Aug 25, 2025

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 •
edited
Loading

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

@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.

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

#if ADBC_CONDA_BUILD
// Then, if this is a conda build, search in the conda environment if
// it is activated.
if (load_options & ADBC_LOAD_FLAG_SEARCH_ENV) {
#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.
if (load_options & ADBC_LOAD_FLAG_SEARCH_ENV) {
auto search_paths = GetSearchPaths(ADBC_LOAD_FLAG_SEARCH_ENV);
const wchar_t* conda_name = L"CONDA_PREFIX";
#else
const char* conda_name = "CONDA_PREFIX";
#endif // _WIN32
auto venv = GetEnvPaths(conda_name);
if (!venv.empty()) {
for (const auto& venv_path : venv) {
search_paths.push_back(venv_path / "etc" / "adbc");
}
}
}
#endif // ADBC_CONDA_BUILD

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

// We searched environment paths and additional search paths (if they
// exist), so now search the rest.
#ifdef _WIN32
// On Windows, check registry keys, not just search paths.
if (load_options & ADBC_LOAD_FLAG_SEARCH_USER) {
// Check the user registry for the driver
// Check the user registry for the driver.
auto status =
LoadDriverFromRegistry(HKEY_CURRENT_USER, driver_path.native(), info, error);
if (status == ADBC_STATUS_OK) {
Expand All @@ -481,7 +532,7 @@ struct ManagedLibrary {
}

if (load_options & ADBC_LOAD_FLAG_SEARCH_SYSTEM) {
// Check the system registry for the driver
// Check the system registry for the driver.
auto status =
LoadDriverFromRegistry(HKEY_LOCAL_MACHINE, driver_path.native(), info, error);
if (status == ADBC_STATUS_OK) {
Expand All @@ -498,17 +549,17 @@ struct ManagedLibrary {
info.lib_path = driver_path;
return Load(driver_path.c_str(), error);
#else
// Otherwise, search the configured paths
auto search_paths = GetSearchPaths(load_options);
// Otherwise, search the configured paths.
auto search_paths = GetSearchPaths(load_options & ~ADBC_LOAD_FLAG_SEARCH_ENV);
auto status = SearchPaths(driver_path, search_paths, info, error);
if (status == ADBC_STATUS_NOT_FOUND) {
// If we reach here, we didn't find the driver in any of the paths
// so let's just attempt to load it as default behavior
// so let's just attempt to load it as default behavior.
info.lib_path = driver_path;
return Load(driver_path.c_str(), error);
}
return status;
#endif
#endif // _WIN32
}

AdbcStatusCode Load(const char_type* library, struct AdbcError* error) {
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 @@ -1043,7 +1095,7 @@ std::filesystem::path InternalAdbcUserConfigDir() {
if (!config_dir.empty()) {
config_dir /= "adbc";
}
#endif
#endif // defined(_WIN32)

return config_dir;
}
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
Loading
Loading