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

Add CMake option to bundle all static dependencies #5347

Draft
wants to merge 1 commit into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions bootstrap
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Configuration:
['"${default_dependency}"']
--linkage specify the linkage of tiledb. Defaults to shared.
[static|shared]
--enable-static-bundle when building with static linkage, create a libtiledb_bundled.a
--remove-deprecations build TileDB without any deprecated APIs
--disable-werror disables use of -Werror during build
--disable-cpp-api disables building of the TileDB C++ API
Expand Down Expand Up @@ -109,6 +110,7 @@ tiledb_arrow_tests="OFF"
tiledb_experimental_features="OFF"
tiledb_build_webp="ON"
tiledb_tests_aws_s3_config="OFF"
tiledb_static_bundle="OFF"
enable_multiple=""
while test $# != 0; do
case "$1" in
Expand All @@ -119,6 +121,7 @@ while test $# != 0; do
--dependency=*) dir=`arg "$1"`
dependency_dir="$dir";;
--linkage=*) linkage=`arg "$1"`;;
--enable-static-bundle) tiledb_static_bundle="ON";;
--force-build-all-deps) echo "Argument '--force-build-all-deps' has no effect and will be removed in a future version. Vcpkg builds all dependencies by default, please consult the guide in doc/dev/BUILD.md or vcpkg's documentation to see how to provide your own dependencies.";;
--remove-deprecations) tiledb_remove_deprecations="ON";;
--disable-werror) tiledb_werror="OFF";;
Expand Down Expand Up @@ -247,6 +250,7 @@ ${cmake} -DCMAKE_BUILD_TYPE=${build_type} \
-DTILEDB_SANITIZER="${sanitizer}" \
-DTILEDB_EXPERIMENTAL_FEATURES=${tiledb_experimental_features} \
-DTILEDB_TESTS_AWS_S3_CONFIG=${tiledb_tests_aws_s3_config} \
-DTILEDB_STATIC_BUNDLE=${tiledb_static_bundle} \
${tiledb_disable_avx2} \
${vcpkg_base_triplet} \
"${source_dir}" || die "failed to configure the project"
Expand Down
37 changes: 37 additions & 0 deletions scripts/bundle-libs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
#!/usr/bin/env python3

import shlex
import os.path
import subprocess as sp

VCPKG_DIR = "vcpkg_installed"
Copy link
Member

Choose a reason for hiding this comment

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

Bundling all static libraries in vcpkg_installed is fragile (assumes all dependencies are being obtained via vcpkg, would need extra logic for Windows and debug libraries) and inefficient (we don't necessarily depend on all libraries in that folder).


def vcpkg_deps():
if not os.path.isdir(VCPKG_DIR):
print("Unable to find `{VCPKG_DIR}` directory")
exit(2)
for entry in os.listdir(VCPKG_DIR):
libdir = os.path.join(VCPKG_DIR, entry, "lib")
print(libdir)
if not os.path.isdir(libdir):
continue
for entry in os.listdir(libdir):
if os.path.splitext(entry)[1] != ".a":
continue
yield os.path.join(libdir, entry)


def main():
libs = ["tiledb/libtiledb.a"]
libs.extend(vcpkg_deps())
for lib in libs:
if not os.path.isfile(lib):
print("Path failed: {}", lib)
exit(2)

cmd = "armerge --output libtiledb_bundled.a " + " ".join(libs)
sp.check_call(cmd, shell=True)


if __name__ == "__main__":
main()
25 changes: 25 additions & 0 deletions tiledb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -940,3 +940,28 @@ add_custom_target(install-tiledb
COMMAND ${CMAKE_COMMAND} --build . --target install --config $<CONFIG>
WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
)

# Create a static library that includes libtiledb and all of its dependencies
message(STATUS "CHECKING FOR STATIC BUNDLE")
if (NOT BUILD_SHARED_LIBS AND TILEDB_STATIC_BUNDLE)
# We check for the availability of armerge here rather than in the
# bundle-libs.py script.
find_program(ARMERGE armerge)
if (NOT ARMERGE)
message(FATAL_ERROR "Failed to find 'armerge' for building static bundle.")
endif()
Comment on lines +949 to +952
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
find_program(ARMERGE armerge)
if (NOT ARMERGE)
message(FATAL_ERROR "Failed to find 'armerge' for building static bundle.")
endif()
find_program(ARMERGE armerge REQUIRED)

Alternatively we could use libtool and lib on Windows.


add_custom_target(static_bundle
ALL
COMMENT "Bundle tiledb and all static library dependencies into a single archive."
COMMAND "${CMAKE_SOURCE_DIR}/scripts/bundle-libs.py"
WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
DEPENDS tiledb
BYPRODUCTS libtiledb_bundled.a
)

install(
FILES ${PROJECT_BINARY_DIR}/libtiledb_bundled.a
DESTINATION ${CMAKE_INSTALL_LIBDIR}
)
endif()
Comment on lines +944 to +967
Copy link
Member

Choose a reason for hiding this comment

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

I have noticed a recent CMake change that might help us precisely get all transitive static dependencies of a target. Here's my (unsuccessful) experiment, annotated with open questions:

# Create a static library that includes libtiledb and all of its dependencies
if (TILEDB_STATIC_BUNDLE)
    if(POLICY CMP0166)
      # TARGET_PROPERTY evaluates link properties transitively over private dependencies of static libraries.
      # Hmm, it doesn't have any effect? I'm not seeing azure-core-cpp for example in the generated Ninja file.
      cmake_policy(SET CMP0166 NEW)
    else()
      message(FATAL_ERROR "TILEDB_STATIC_BUNDLE requires CMake 3.30 or newer")
    endif()

    # Incrementally build generator expression with the library files of all of our dependencies
    set(static_deps_genex $<TARGET_PROPERTY:LINK_LIBRARIES>)
    # Expand expressions like $<TARGET_PROPERTY:tgt,INTERFACE_LINK_LIBRARIES>
    set(static_deps_genex $<GENEX_EVAL:${static_deps_genex}>)
    # Wrap each dependency target in a $<TARGET_LINKER_FILE:dep>.
    # TODO: What happens on system libraries? We might need to transform dep to
    # $<IF:$<TARGET_EXISTS:dep>,$<TARGET_LINKER_FILE:dep>,dep>. This could be done
    # with a regex replacement which I tried, but still couldn't andwer the question below.
    # The million-dollar question: How to escape the dollar?? Is it even possible?
    set(static_deps_genex $<LIST:TRANSFORM,${static_deps_genex},PREPEND,$<TARGET_LINKER_FILE:>)
    set(static_deps_genex $<LIST:TRANSFORM,${static_deps_genex},APPEND,$<ANGLE-R>>)
    # Evaluate the genex again. We go from $<TARGET_LINKER_FILE:a>;$<TARGET_LINKER_FILE:b>
    # to liba.a;libb.a
    set(static_deps_genex $<GENEX_EVAL:${static_deps_genex}>)

    # We can simplify the developer experience and post-process the existing libtiledb.a.
    # This is optional and should be done if we can resolve the concern below.
    add_custom_command(TARGET tiledb
        POST_BUILD
        # TODO: Use libtool or armerge on non-Windows.
        COMMAND lib /out:$<TARGET_FILE:tiledb> $<TARGET_FILE:tiledb> ${static_deps_genex}
        COMMAND_EXPAND_LISTS
    )

    # TODO: Clear all link_libraries of the tiledb target.
    # This sounds fragile. All our PUBLIC link_libraries are currently link-only and removing them
    # is safe, but if it stops being true, how to pick them out? There is already such target
    # in configuration_definitions, but that's undocumented and should not be exposed either way.
endif()

Loading