Drivers are unusable when statically linked with CMake

[jtanx]

What happened?

Taking the postgres driver as an example:

• The CMake package adbc_driver_postgresql links to adbc_driver_common and a few other vendored dependencies like nanoarrow. Only adbc_driver_postgresql is installed; the other libraries are missing. This leads to a linking error since the symbols provided in the other libraries are missing.
• For reasons I do not understand, the driver unnecessarily redefines the entirety of the ADBC driver manager functions (e.g. AdbcDatabaseGetOption). This leads to a duplicate symbol error when you try to link both the ADBC driver manager library and the postgresql driver together.
• Furthermore, PostgresqlDriverInit is not declared in any installed header (the driver does not install any headers?), which complicates using it with AdbcDriverManagerDatabaseSetInitFunc. This is less egregious as it's not that hard to just declare the function signature manually, but it is still odd that it's not provided.
• The way vendored dependencies are handled is also problematic if linking statically - no attempt is made to first search for these packages before using the vendored dependencies. This can lead to conflicts if I am also using e.g. the fmt library myself from a third party source.

Stack Trace

No response

How can we reproduce the bug?

• Use CMake
• Build statically
• Install the static libs
• Try to use the libraries statically from another project

Environment/Setup

C driver version 1.4.0

[lidavidm]

There's an existing PR to unvendor dependencies: #2546

[lidavidm]

The redefinition is intentional, but maybe we should provide an option to omit it. We could also optionally install headers (frankly, the expected method of usage is via the driver manager so this is all untrodden ground)

[jtanx]

If the expected method of usage is via the driver manager, why is anything redefined at all? Shouldn't only e.g. PostgresqlDriverInit be exposed from a driver?

[paleolimbot]

Shouldn't only e.g. PostgresqlDriverInit be exposed from a driver?

I have also always wondered this 🙂

[jtanx]

Woops, I made a mistake in my original post; to be clear adbc_driver_common is not installed, so it's not just a problem with vendored dependencies.

We could also optionally install headers (frankly, the expected method of usage is via the driver manager so this is all untrodden ground)

Rereading this, I slightly misunderstood. I'm not saying to install the existing headers with everything exposed, I'm saying that there should be a header that exposes PostgresqlDriverInit and nothing else - indeed I don't see why any other symbol from a driver should be exported at all (and I also still think that redefining the functions don't make sense at all).

[lidavidm]

You can link the postgres driver and use the Adbc* functions.

[jtanx]

As in linking just the postgres driver and not the driver manager? Seems like an over optimisation?

[paleolimbot]

You loose a bit of documentation but it's pretty easy to use a macro to just call the driver callbacks:

https://github.com/OSGeo/gdal/blob/c7006dbfb1c8a3f9030f46642d17bb28b34b0144/ogr/ogrsf_frmts/adbc/ogradbclayer.cpp#L24

At the very least, an opt-out of getting any symbols other than the init function seems helpful (I also happen to think turning them off by default might lead to less accidental problems as people build more ADBC drivers into things).

[lidavidm]

Making all the symbols opt-in and adding headers sounds fine. @jtanx does that solve your needs?

[jtanx]

Thanks, yep that (plus the change to vendored deps) sounds good

[jtanx]

Oh, adbc_driver_common will also need to be installed