WillAyd commented on code in PR #2738:
URL: https://github.com/apache/arrow-adbc/pull/2738#discussion_r2058250884
##########
c/driver/common/CMakeLists.txt:
##########
@@ -21,6 +21,22 @@ set_target_properties(adbc_driver_common PROPERTIES
POSITION_INDEPENDENT_CODE ON
target_include_directories(adbc_driver_common PRIVATE
"${REPOSITORY_ROOT}/c/include")
target_link_libraries(adbc_driver_common PUBLIC nanoarrow::nanoarrow)
+# For static builds, we need to install the static library here so downstream
+# applications can link to it
+if(ADBC_BUILD_STATIC)
+ if(ADBC_WITH_VENDORED_NANOARROW)
+ message(WARNING "adbc_driver_common is not installed when
ADBC_WITH_VENDORED_NANOARROW is ON. To use the static libraries, for now you
must provide nanoarrow instead of using the vendored copy"
Review Comment:
What is the problem with installing in this case?
##########
c/driver/common/utils.c:
##########
@@ -27,7 +27,7 @@
static size_t kErrorBufferSize = 1024;
-int AdbcStatusCodeToErrno(AdbcStatusCode code) {
+int PrivateAdbcStatusCodeToErrno(AdbcStatusCode code) {
Review Comment:
Nit but maybe this would be better to call Internal instead of Private just
to keep in sync with the arrow repo? I know upstream there is even some
confusion over the different namespace terminology
https://github.com/apache/arrow/issues/45808
##########
docs/source/cpp/static_linking.rst:
##########
@@ -0,0 +1,64 @@
+.. 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.
+
+==============
+Static Linking
+==============
+
+To statically link and use *multiple* drivers, the following limitations hold:
+
+#. You must build with these CMake options [#meson]_:
+
+ - ``-DADBC_BUILD_STATIC=ON``
+ - ``-DADBC_DEFINE_COMMON_ENTRYPOINTS=OFF``
+ - ``-DADBC_WITH_VENDORED_FMT=OFF``
+ - ``-DADBC_WITH_VENDORED_NANOARROW=OFF``
+
+#. You must provide ``fmt`` and ``nanoarrow`` dependencies. (This may be
+ relaxed in the future.)
+#. You must explicitly link all the required transitive dependencies. This
+ is:
+
+ - ``libpq`` for PostgreSQL
+ - ``sqlite3`` for SQLite
+ - ``adbc_driver_common``, ``adbc_driver_framework``, ``fmt``,
+ ``nanoarrow``, and the C++ standard library for either PostgreSQL or
+ SQLite.
+ - To link the C++ standard library, the easiest thing is to just use the
+ C++ linker, even if the code itself is in C, e.g.
+
+ .. code-block:: cmake
+
+ set_target_properties(myapp PROPERTIES LINKER_LANGUAGE CXX)
+
+ - Go-based drivers (BigQuery, Flight SQL, Snowflake) have no transitive
+ dependencies.
+
+#. You cannot link more than a single Go-based driver. See `golang/go#20639
+ <go20639_>`_, `StackOverflow #67243572 <so67243572_>`_, and `StackOverflow
+ #34333107 <so34333107_>`_ for a discussion of the issues involved. (This
+ may be relaxed in the future by providing a way to build a single driver
+ library with all driver implementations included.)
+
+An example of this can be seen with ``cpp_static_test.sh`` and
+``c/integration/static_test`` in the source tree.
+
+.. [#meson] There is currently no documentation for Meson, but in principle
Review Comment:
For Meson I believe you just need to do
```
meson setup -Ddefault_library=static \
-Dc_args="-DADBC_NO_COMMON_ENTRYPOINTS
-Dcpp_args="-DADBC_NO_COMMON_ENTRYPOINTS
...
```
Or, it might be better if we detect whether a shared or static library is
being built in the configuration and define those macros for the user, instead
of requiring them as c{pp}_args. Doesn't need to be done here of course; can be
an easy follow up
```
fmt and nanoarrow are both taken care of by the wrap subsystem, so the user
shouldn't have to do anything extra with those
--
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]
