Skip to content
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.

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

fix: Ensure that test suite passes with system-provided SQLite on MacOS #1499

Merged
merged 3 commits into from
Apr 5, 2024

Conversation

paleolimbot
Copy link
Member

@paleolimbot paleolimbot commented Jan 30, 2024

I opened up the C drivers and ran the tests and got a failing test: when the driver compiles against system SQLite on MacOS, extension loading is not allowed (the symbols aren't even provided nor defined in the header). I added a define to make sure the R package could build on MacOS, but the test needs it too.

I also added come CMake to check this...I also tried check_sybmol_exists but this seems to require setting CMAKE_REQUIRED_LIBRARIES/includes which seems like it might have some global impact that we don't want. Checking for the text in the header does the trick for me (and is maybe less likely to result in accidentally building the library without extension support).

The other failing test I saw before seems to have been cleared up by the SQLite driver refactor!

The .cache in the .gitignore is because clangd seems to put a lot of files there.

@lidavidm
Copy link
Member

lidavidm commented Mar 1, 2024

It looks like CI is passing here?

@paleolimbot
Copy link
Member Author

I forgot to circle back here...I'm pretty sure all of our MacOS CI checks don't use system libsqlite. This is pretty much only an issue if you're a contributor to the SQLite driver who isn't working in a conda environment (which is probably just me, so pretty low priority).

@lidavidm lidavidm removed this from the ADBC Libraries 0.11.0 milestone Mar 19, 2024
@paleolimbot paleolimbot marked this pull request as ready for review April 5, 2024 18:21
@lidavidm lidavidm merged commit dff48ec into apache:main Apr 5, 2024
60 checks passed
@paleolimbot paleolimbot deleted the c-sqlite-macos branch April 16, 2024 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants