Skip to content

Commit

Permalink
Fix importing TileDB in CMake versions prior to 3.18. (#4671)
Browse files Browse the repository at this point in the history
#4528 introduced a single `TileDB::tiledb`exported CMake target for
TileDB with either static or dynamic linkage. For compatibility with
previous versions, the targets `TileDB::tiledb_shared` or
`TileDB::tiledb_static` were also defined depending on the linkage, as
`ALIAS`es to `TileDB::tiledb`.

As it turns out however, we cannot use `ALIAS` targets, because they are
always declared in the global scope prior to CMake 3.18 and if
`find_package(TileDB)` is not called in the top-level `CMakeLists.txt`
file, it will fail with `add_library cannot create ALIAS target
"TileDB::tiledb_shared" because target "TileDB::tiledb" is imported but
not globally visible.`.

Nor can we switch to using `IMPORTED INTERFACE` targets and linking them
to `TileDB::tiledb`, because it would bring a minor breaking change[^1].
Because `TileDB::tiledb_shared` would become an `INTERFACE` library, it
does not have an `IMPORTED_LOCATION` anymore, which would cause [calls
to
`install_target_libs(TileDB::tiledb_shared)`](https://github.com/TileDB-Inc/TileDB-VCF/blob/5bcc79b07935ac540c56bf6ed9ee0f5d60bf247e/libtiledbvcf/cmake/Modules/FindTileDB_EP.cmake#L121)
to fail.

Thankfully there is another solution. We set the
[`EXPORT_NAME`](https://cmake.org/cmake/help/latest/prop_tgt/EXPORT_NAME.html)
of the `tiledb` target to either `tiledb_shared` or `tiledb_static`
depending on the linkage, and define `TileDB::tiledb` as an `IMPORTED
INTERFACE` target[^2] to the linkage-specific target. This maintains
full compatibility.

[^1]: Something similar is the "breaking build system change" I talked
about in
#4408 (comment).
After removing the `install_target_libs` calls from this repository, the
change in Curl did not afffect us and we could update much more easily.
[^2]: In this opposite case the unified target _must_ be an `IMPORTED
INTERFACE`. We cannot get the `IMPORTED_LOCATION` of `TileDB::tiledb`,
but since the target is new this is not a breaking change.

---
TYPE: BUILD
DESC: Fix importing TileDB in CMake versions prior to 3.18.

(cherry picked from commit ad0371f)
  • Loading branch information
teo-tsirpanis authored and github-actions[bot] committed Feb 5, 2024
1 parent 16b321d commit b3115ec
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 4 deletions.
12 changes: 8 additions & 4 deletions cmake/inputs/Config.cmake.in
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,14 @@ endif()
include("${CMAKE_CURRENT_LIST_DIR}/@[email protected]")
check_required_components("@PROJECT_NAME@")

if(@BUILD_SHARED_LIBS@ AND NOT TARGET TileDB::tiledb_shared) # BUILD_SHARED_LIBS AND NOT TARGET TileDB::tiledb_shared
add_library(TileDB::tiledb_shared ALIAS TileDB::tiledb)
elseif(NOT TARGET TileDB::tiledb_static)
add_library(TileDB::tiledb_static ALIAS TileDB::tiledb)
if(NOT TARGET TileDB::tiledb)
if(TARGET TileDB::tiledb_shared)
add_library(TileDB::tiledb INTERFACE IMPORTED)
set_target_properties(TileDB::tiledb PROPERTIES INTERFACE_LINK_LIBRARIES TileDB::tiledb_shared)
elseif(TARGET TileDB::tiledb_static)
add_library(TileDB::tiledb INTERFACE IMPORTED)
set_target_properties(TileDB::tiledb PROPERTIES INTERFACE_LINK_LIBRARIES TileDB::tiledb_static)
endif()
endif()

# Define a convenience all-caps variable
Expand Down
9 changes: 9 additions & 0 deletions tiledb/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,15 @@ endif()
# the value of the BUILD_SHARED_LIBS variable.
add_library(tiledb $<TARGET_OBJECTS:TILEDB_CORE_OBJECTS>)

# Export the target as either tiledb_shared or tiledb_static for compatibility.
# The exported config will create the unified tiledb target that links to either
# of them.
if(BUILD_SHARED_LIBS)
set_target_properties(tiledb PROPERTIES EXPORT_NAME tiledb_shared)
else()
set_target_properties(tiledb PROPERTIES EXPORT_NAME tiledb_static)
endif()

file(READ "${CMAKE_CURRENT_SOURCE_DIR}/sm/c_api/tiledb_version.h" ver)

string(REGEX MATCH "TILEDB_VERSION_MAJOR ([0-9]*)" _ ${ver})
Expand Down

0 comments on commit b3115ec

Please sign in to comment.