Simplify blosc build configuration by turning it into an object library

external/blosc/CMakeLists.txt

@@ -24,34 +24,24 @@
 # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
 # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # THE SOFTWARE.
-#
-# Precondition
-#   Expects caller to be <top>/tiledb/CMakeLists.txt. CMake (as of 3.20) does
-#   not have a way of retrieving the caller's directories, nor of passing
-#   argument for the callee in add_subdirectory(). Here we avoid premature
-#   modularization and hard-code the caller.
-# Outputs, all variables set in parent scope
-#   TileDB_blosc_SOURCES
-#   TileDB_blosc_INCLUDE_DIRS
-#   TileDB_blosc_COMPILE_OPTIONS
 
 set(SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
 
-list(APPEND SOURCES
+add_library(blosc
+  OBJECT
   ${SOURCE_DIR}/src/shuffle.c
   ${SOURCE_DIR}/src/shuffle-generic.c
   ${SOURCE_DIR}/src/bitshuffle-stub.c
 )
+target_include_directories(blosc PUBLIC ${SOURCE_DIR}/include)
+
 if(COMPILER_SUPPORTS_AVX2)
-  list(APPEND SOURCES
-    ${CMAKE_CURRENT_SOURCE_DIR}/src/shuffle-avx2.c
-    )
+  target_sources(blosc PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src/shuffle-avx2.c)
 endif()
+
 try_compile(SSE2_DETECTED ${CMAKE_CURRENT_BINARY_DIR} ${SOURCE_DIR}/cmake/detect-sse2.c)
 if(SSE2_DETECTED)
-  list(APPEND SOURCES
-    ${CMAKE_CURRENT_SOURCE_DIR}/src/shuffle-sse2.c
-    )
+  target_sources(blosc PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/src/shuffle-sse2.c)
 endif()
 
 
@@ -63,56 +53,15 @@ if (CMAKE_USE_PTHREADS_INIT AND NOT MSVC)
   message(DEBUG "Using system-provided pthread")
 elseif(CMAKE_USE_WIN32_THREADS_INIT OR MSVC)
   message(DEBUG "Using blosc-provided pthread substitute for Win32")
-  list(APPEND SOURCES
-    ${SOURCE_DIR}/src/win32/pthread.c
-  )
+  target_sources(blosc PRIVATE ${SOURCE_DIR}/src/win32/pthread.c)
 else()
   message(FATAL_ERROR "Thread package found, but not a supported one")
 endif()
 
 # The blosc files are external and we ignore their warnings.
 # -w is common to gcc and MSVC
-set(TILEDB_BLOSC_COMPILE_OPTIONS "-w")
+target_compile_options(blosc PUBLIC "-w")
 # If we found pthreads, activate it by defining PTHREAD_AVAILABLE.
 if (CMAKE_USE_PTHREADS_INIT AND NOT MSVC)
-  string(APPEND TILEDB_BLOSC_COMPILE_OPTIONS ";-DPTHREAD_AVAILABLE")
+  target_compile_options(blosc PUBLIC "-DPTHREAD_AVAILABLE")
 endif()
-
-#
-# Set the outputs
-#
-# CMake makes it difficult to have modular build files. We're trying hard
-# anyway. We can't get good encapsulation, but that doesn't mean we can't have
-# any.
-#
-# We can't return our results as an object library because (a) it's too new and
-# (b) it doesn't even work as we need it to. As of CMake 3.20, when targetA
-# depends on an object library target objB, $<TARGET_OBJECTS:objB> is not
-# incorporated into $<TARGET_OBJECTS:targetA>, meaning objB isn't encapsulated.
-# It would be an unacceptable maintenance burden to add objB everywhere
-# $<TARGET_OBJECTS:targetA> is used. So instead of using a library, we set
-# variables TileDB_blosc_SOURCES and TileDB_blosc_INCLUDE_DIRS in the parent
-# scope, where they are added to existing lists.
-#
-# blosc requires it's own, distinct compilation options. If we were able to use
-# a library, that would be the end of it. Instead we need to set compilation
-# options in the caller context. A draft of the present code had used the
-# following statement to set these properties in the absence of using a library.
-#
-#   set_source_files_properties(
-#    ${SOURCES}
-#    DIRECTORY ${CMAKE_SOURCE_DIR}/tiledb
-#    PROPERTIES COMPILE_OPTIONS "${TILEDB_BLOSC_COMPILE_OPTIONS}"
-#
-# The DIRECTORY option was introduced in CMake 3.18, too new for ubuntu-20.04,
-# which ships with 3.16. Note that even then there's no "DIRECTORY PARENT"
-# syntax available. Instead we return a variable holding the options and then
-# set them in the caller.
-#
-# There's one cheat here. We're not passing a link option here. At most it would
-# be for pthread, and that's already being linked.
-#
-set(TileDB_blosc_SOURCES ${SOURCES} PARENT_SCOPE)
-set(TileDB_blosc_INCLUDE_DIRS ${SOURCE_DIR}/include PARENT_SCOPE)
-set(TileDB_blosc_COMPILE_OPTIONS ${TILEDB_BLOSC_COMPILE_OPTIONS} PARENT_SCOPE)
-set(TileDB_blosc_LINK_OPTIONS ${CMAKE_THREAD_LIBS_INIT} PARENT_SCOPE)

external/include/blosc/tiledb-shuffle.h

@@ -35,7 +35,7 @@
 #define TILEDB_SHUFFLE_H
 
 // This blosc header contains an extern "C" statement, so we don't need to.
-#include "shuffle.h"
+#include "external/blosc/include/shuffle.h"
 
 namespace blosc {
   const auto &shuffle = blosc_internal_shuffle;

tiledb/CMakeLists.txt

@@ -401,16 +401,11 @@ list(APPEND TILEDB_EXTERNALS_SOURCES
 
 #
 # Delegate configuration and detection of platform-specific code generation to
-# the subdirectory. For why the separation isn't clean, see comments there.
+# the subdirectory.
 #
 add_subdirectory(
   ${CMAKE_CURRENT_SOURCE_DIR}/../external/blosc
   ${CMAKE_CURRENT_BINARY_DIR}/../external/blosc)
-list(APPEND TILEDB_EXTERNALS_INCLUDE_DIRS ${TileDB_blosc_INCLUDE_DIRS})
-list(APPEND TILEDB_EXTERNALS_SOURCES ${TileDB_blosc_SOURCES})
-set_source_files_properties(
-  ${TileDB_blosc_SOURCES}
-  PROPERTIES COMPILE_OPTIONS "${TileDB_blosc_COMPILE_OPTIONS}")
 
 ############################################################
 # Build core objects as a reusable object library
@@ -424,6 +419,7 @@ add_library(TILEDB_CORE_OBJECTS OBJECT
 # List of libraries to be linked to TILEDB_CORE_OBJECTS.
 set(TILEDB_CORE_OBJECTS_LIBS
   baseline
+  blosc
 )
 
 list(TRANSFORM TILEDB_CORE_OBJECTS_LIBS PREPEND "$<TARGET_OBJECTS:" OUTPUT_VARIABLE TILEDB_CORE_OBJECTS_LIBS_SOURCES)
@@ -495,9 +491,6 @@ add_library(TILEDB_CORE_OBJECTS_ILIB INTERFACE)
 # object store definitions
 target_link_libraries(TILEDB_CORE_OBJECTS_ILIB INTERFACE configuration_definitions)
 
-# blosc link dependencies
-target_link_libraries(TILEDB_CORE_OBJECTS_ILIB INTERFACE ${TileDB_blosc_LINK_OPTIONS})
-
 # Find OpenSSL first in case it's needed for S3 or Azure
 if (NOT WIN32)
   find_package(OpenSSL REQUIRED)

tiledb/sm/filter/CMakeLists.txt

@@ -58,25 +58,9 @@ conclude(object_library)
 #
 # `byteshuffle_filter` object library
 #
-# `byteshuffle_filter` depends on blosc, so we need to add its subdirectory. At
-# the present time, that subdirectory is also being added in the main build.
-# Adding the same directory twice causes an error. Thus, in the interim, we use
-# `blosc-alt` for the binary. This also means that we're compiling blosc
-# separately for the main build and for the unit. These separation is temporary
-# for the duration of converting the main build into a unit-dependent one.
-#
 commence(object_library byteshuffle_filter)
-    # Dependency on external/blosc
-    cmake_path(APPEND TILEDB_SOURCE_ROOT "external/blosc" OUTPUT_VARIABLE BLOSC_SOURCE_ROOT)
-    cmake_path(APPEND CMAKE_BINARY_DIR "external/blosc-alt" OUTPUT_VARIABLE BLOSC_BINARY_ROOT)
-    add_subdirectory(${BLOSC_SOURCE_ROOT} ${BLOSC_BINARY_ROOT})
-    this_target_sources(byteshuffle_filter.cc ${TileDB_blosc_SOURCES})
-    this_target_object_libraries(baseline buffer filter)
-    # [As of CMake 3.21] Setting private options on object libraries does not set
-    # the corresponding properties on sources in the object library.
-    set_source_files_properties(byteshuffle_filter.cc PROPERTIES INCLUDE_DIRECTORIES "${TILEDB_EXTERNAL_INCLUDE};${TileDB_blosc_INCLUDE_DIRS}")
-    set_source_files_properties(${TileDB_blosc_SOURCES} PROPERTIES INCLUDE_DIRECTORIES "${TileDB_blosc_INCLUDE_DIRS}")
-    set_source_files_properties(${TileDB_blosc_SOURCES} PROPERTIES COMPILE_OPTIONS "${TileDB_blosc_COMPILE_OPTIONS}")
+    this_target_sources(byteshuffle_filter.cc)
+    this_target_object_libraries(baseline blosc buffer filter)
 conclude(object_library)
 
 #

tiledb/sm/filter/byteshuffle_filter.cc

@@ -36,7 +36,7 @@
 #include "tiledb/sm/enums/filter_type.h"
 #include "tiledb/sm/tile/tile.h"
 
-#include "blosc/tiledb-shuffle.h"
+#include "external/include/blosc/tiledb-shuffle.h"
 
 using namespace tiledb::common;