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 Crc32c to tiledb build via ExternalProject_Add #3455

Merged
merged 8 commits into from Aug 23, 2022
Merged
5 changes: 5 additions & 0 deletions CMakeLists.txt
Expand Up @@ -124,6 +124,7 @@ option(TILEDB_TOOLS "If true, enables building the TileDB tools" OFF)
option(TILEDB_SERIALIZATION "If true, enables building with support for query serialization" OFF)
option(TILEDB_CCACHE "If true, enables use of 'ccache' (if present)" OFF)
option(TILEDB_ARROW_TESTS "If true, enables building the arrow adapter unit tests" OFF)
option(TILEDB_CRC32 "If true, enables building crc32 and a simple linkage test" OFF)
option(TILEDB_WEBP "If true, enables building webp and a simple linkage test" OFF)
option(TILEDB_LOG_OUTPUT_ON_FAILURE "If true, print error logs if dependency sub-project build fails" ON)
option(TILEDB_SKIP_S3AWSSDK_DIR_LENGTH_CHECK "If true, skip check needed path length for awssdk (TILEDB_S3) dependent builds" OFF)
Expand Down Expand Up @@ -432,6 +433,10 @@ if (TILEDB_TESTS)
add_dependencies(tests unit_ast)
add_dependencies(tests unit_serializers)

if (TILEDB_CRC32)
add_dependencies(tests unit_link_crc32)
endif()

if (TILEDB_WEBP)
add_dependencies(tests unit_link_webp)
endif()
Expand Down
5 changes: 5 additions & 0 deletions bootstrap
Expand Up @@ -59,6 +59,7 @@ Configuration:
--enable-tools enables TileDB CLI tools (experimental)
--enable-ccache enables use of ccache (if present)
--enable-arrow-tests enables the compilation of the arrow adapter unit tests
--enable-crc32 enables building of crc32 library and linkage test
--enable-webp enables building of webp library and linkage test
--enable-experimental-features enables experimental TileDB features
--enable=arg1,arg2... same as "--enable-arg1 --enable-arg2 ..."
Expand Down Expand Up @@ -102,6 +103,7 @@ tiledb_tools="OFF"
tiledb_ccache="OFF"
tiledb_arrow_tests="OFF"
tiledb_experimental_features="OFF"
tiledb_build_crc32="OFF"
tiledb_build_webp="OFF"
enable_multiple=""
while test $# != 0; do
Expand Down Expand Up @@ -132,6 +134,7 @@ while test $# != 0; do
--enable-tools) tiledb_tools="ON";;
--enable-ccache) tiledb_ccache="ON";;
--enable-arrow-tests) tiledb_arrow_tests="ON";;
--enable-crc32) tiledb_build_crc32="ON";;
--enable-webp) tiledb_build_webp="ON";;
--enable-experimental-features) tiledb_experimental_features="ON";;
--enable=*) s=`arg "$1"`
Expand All @@ -158,6 +161,7 @@ for en in "${enables[@]}"; do
tools) tiledb_tools="ON";;
ccache) tiledb_ccache="ON";;
arrow-tests) tiledb_arrow_tests="ON";;
crc32) tiledb_build_crc32="ON";;
webp) tiledb_build_webp="ON";;
hdfs) tiledb_hdfs="ON";;
static-tiledb) tiledb_static="ON";;
Expand Down Expand Up @@ -216,6 +220,7 @@ ${cmake} -DCMAKE_BUILD_TYPE=${build_type} \
-DTILEDB_TESTS=${tiledb_tests} \
-DTILEDB_CCACHE=${tiledb_ccache} \
-DTILEDB_ARROW_TESTS=${tiledb_arrow_tests} \
-DTILEDB_CRC32=${tiledb_build_crc32} \
-DTILEDB_WEBP=${tiledb_build_webp} \
-DTILEDB_FORCE_ALL_DEPS=${tiledb_force_all_deps} \
-DSANITIZER="${sanitizer}" \
Expand Down
11 changes: 10 additions & 1 deletion bootstrap.ps1
Expand Up @@ -63,6 +63,9 @@ Enables building of WebP and simple linkage test
.Parameter DisableWerror
Disable use of warnings-as-errors (/WX) during build.

.PARAMETER EnableCrc32
Enables building of Crc32 and simple linkage test

.Parameter DisableCppApi
Disable building the TileDB C++ API.

Expand Down Expand Up @@ -98,6 +101,7 @@ Param(
[switch]$EnableExperimentalFeatures,
[switch]$EnableWebP,
[switch]$EnableBuildDeps,
[switch]$EnableCrc32,
[switch]$DisableWerror,
[switch]$DisableCppApi,
[switch]$DisableTests,
Expand Down Expand Up @@ -210,6 +214,11 @@ if ($EnableBuildDeps.IsPresent) {
$TileDBBuildDeps = "ON"
}

$BuildCrc32="OFF"
if ($EnableCrc32.IsPresent) {
$BuildCrc32="ON"
}

# Set TileDB prefix
$InstallPrefix = $DefaultPrefix
if (![string]::IsNullOrEmpty($Prefix)) {
Expand Down Expand Up @@ -243,7 +252,7 @@ if ($CMakeGenerator -eq $null) {

# Run CMake.
# We use Invoke-Expression so we can echo the command to the user.
$CommandString = "cmake -A X64 -DCMAKE_BUILD_TYPE=$BuildType -DCMAKE_INSTALL_PREFIX=""$InstallPrefix"" -DCMAKE_PREFIX_PATH=""$DependencyDir"" -DMSVC_MP_FLAG=""/MP$BuildProcesses"" -DTILEDB_ASSERTIONS=$AssertionMode -DTILEDB_VERBOSE=$Verbosity -DTILEDB_AZURE=$UseAzure -DTILEDB_S3=$UseS3 -DTILEDB_SERIALIZATION=$UseSerialization -DTILEDB_WERROR=$Werror -DTILEDB_CPP_API=$CppApi -DTILEDB_TESTS=$Tests -DTILEDB_STATS=$Stats -DTILEDB_STATIC=$TileDBStatic -DTILEDB_FORCE_ALL_DEPS=$TileDBBuildDeps -DTILEDB_TOOLS=$TileDBTools -DTILEDB_EXPERIMENTAL_FEATURES=$TileDBExperimentalFeatures -DTILEDB_WEBP=$BuildWebP $GeneratorFlag ""$SourceDirectory"""
$CommandString = "cmake -A X64 -DCMAKE_BUILD_TYPE=$BuildType -DCMAKE_INSTALL_PREFIX=""$InstallPrefix"" -DCMAKE_PREFIX_PATH=""$DependencyDir"" -DMSVC_MP_FLAG=""/MP$BuildProcesses"" -DTILEDB_ASSERTIONS=$AssertionMode -DTILEDB_VERBOSE=$Verbosity -DTILEDB_AZURE=$UseAzure -DTILEDB_S3=$UseS3 -DTILEDB_SERIALIZATION=$UseSerialization -DTILEDB_WERROR=$Werror -DTILEDB_CPP_API=$CppApi -DTILEDB_TESTS=$Tests -DTILEDB_STATS=$Stats -DTILEDB_STATIC=$TileDBStatic -DTILEDB_FORCE_ALL_DEPS=$TileDBBuildDeps -DTILEDB_TOOLS=$TileDBTools -DTILEDB_EXPERIMENTAL_FEATURES=$TileDBExperimentalFeatures -DTILEDB_WEBP=$BuildWebP -DTILEDB_CRC32=$BuildCrc32 $GeneratorFlag ""$SourceDirectory"""
Write-Host $CommandString
Write-Host
Invoke-Expression "$CommandString"
Expand Down
97 changes: 97 additions & 0 deletions cmake/Modules/FindCrc32c_EP.cmake
@@ -0,0 +1,97 @@
#
# FindCrc32c_EP.cmake
#
# The MIT License
#
# Copyright (c) 2022 TileDB, Inc.
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to deal
# in the Software without restriction, including without limitation the rights
# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
# copies of the Software, and to permit persons to whom the Software is
# furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included in
# all copies or substantial portions of the Software.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
# 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.
#
# Finds the Crc32c library, installing with an ExternalProject as necessary.
# This module defines:
# - Crc32c_INCLUDE_DIR, directory containing headers
# - Crc32c_LIBRARIES, the Magic library path
Copy link
Member

Choose a reason for hiding this comment

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

nig: Magic -> Crc

Copy link
Contributor Author

@dhoke4tdb dhoke4tdb Aug 19, 2022

Choose a reason for hiding this comment

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

Addition of the link test is currently subject to same flags as building being activated by bootstrap[.ps1]
If I take away those switches/visibility, how do you want to handle building/link-test enabling?
(The code would be present, but no way to activate it...?)
[edit - Occurs to me 'later' with variable left in place, can invoke cmake directly, although this will require knowledge of any (additional) vars and cmake parameters (on windows, -X A64? others?) that may have to be specified to obtain other 'normal' default settings that may be required (several cmake variables are defaulted to ON by bootstrap.ps1).]

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking of enabling via cmake -D..., but you're right, we don't invoke it directly. For now let's prefix the option with _ so we can use it in CI and remove later once it is implicitly enabled/tested via -DTILEDB_GCS=ON.

# - Crc32c_FOUND, whether Magic has been found
# - The Crc32c::crc32c imported target

# Include some common helper functions.
include(TileDBCommon)

if(TILEDB_CRC32C_EP_BUILT)
find_package(Crc32c CONFIG REQUIRED
HINTS
${TILEDB_EP_INSTALL_PREFIX}/lib/cmake
${TILEDB_DEPS_NO_DEFAULT_PATH})
elseif (NOT TILEDB_FORCE_ALL_DEPS)
# seems no standard findabsl.cmake, so silence warnings with QUIET
find_package(Crc32c QUIET ${TILEDB_DEPS_NO_DEFAULT_PATH})
endif()

# if not yet built add it as an external project
if (NOT TILEDB_CRC32C_EP_BUILT AND NOT Crc32c_FOUND)
if (TILEDB_SUPERBUILD)
message(STATUS "Adding crc32c as an external project")

set(TILEDB_CRC32C_BUILD_VERSION 1.1.2)
set(TILEDB_CRC32C_URL
"https://github.com/google/crc32c/archive/refs/tags/${TILEDB_CRC32C_BUILD_VERSION}.tar.gz")
set(TILEDB_CRC32C_SHA256
"ac07840513072b7fcebda6e821068aa04889018f24e10e46181068fb214d7e56")

ExternalProject_Add(
ep_crc32c
EXCLUDE_FROM_ALL ON
PREFIX "externals"
INSTALL_DIR "${TILEDB_EP_INSTALL_PREFIX}"
URL ${TILEDB_CRC32C_URL}
URL_HASH SHA256=${TILEDB_CRC32C_SHA256}
LIST_SEPARATOR |
CMAKE_ARGS
-DCMAKE_PREFIX_PATH=${TILEDB_EP_INSTALL_PREFIX}
-DCMAKE_INSTALL_PREFIX=${TILEDB_EP_INSTALL_PREFIX}
-DCRC32C_BUILD_TESTS=OFF
-DCRC32C_BUILD_BENCHMARKS=OFF
-DCRC32C_USE_GLOG=OFF
LOG_DOWNLOAD ON
LOG_CONFIGURE ON
LOG_BUILD ON
LOG_INSTALL ON)

list(APPEND TILEDB_EXTERNAL_PROJECTS ep_crc32c)
list(APPEND FORWARD_EP_CMAKE_ARGS
-DTILEDB_CRC32C_EP_BUILT=TRUE
)
else()
message(FATAL_ERROR "Unable to find crc32c")
endif()
endif ()

if (Crc32c_FOUND AND NOT TARGET Crc32c::crc32c)
message(STATUS "Found crc32c, adding imported target: ${Crc32c_LIBRARIES}")
add_library(Crc32c::crc32c UNKNOWN IMPORTED)
set_target_properties(Crc32c::crc32c PROPERTIES
IMPORTED_LOCATION "${Crc32c_LIBRARIES}"
INTERFACE_INCLUDE_DIRECTORIES "${Crc32c_INCLUDE_DIR}"
)
endif()

# If we built a static EP, install it if required.
if (TILEDB_CRC32C_EP_BUILT AND TILEDB_INSTALL_STATIC_DEPS)
install_target_libs(Crc32c::crc32c)
endif()
6 changes: 6 additions & 0 deletions cmake/TileDB-Superbuild.cmake
Expand Up @@ -43,6 +43,7 @@ set(INHERITED_CMAKE_ARGS
-DTILEDB_TOOLS=${TILEDB_TOOLS}
-DTILEDB_SERIALIZATION=${TILEDB_SERIALIZATION}
-DTILEDB_ARROW_TESTS=${TILEDB_ARROW_TESTS}
-DTILEDB_CRC32=${TILEDB_CRC32}
-DTILEDB_WEBP=${TILEDB_WEBP}
-DTILEDB_INSTALL_LIBDIR=${TILEDB_INSTALL_LIBDIR}
-DCMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES}
Expand Down Expand Up @@ -87,6 +88,11 @@ include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindSpdlog_EP.cmake)
include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindZlib_EP.cmake)
include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindZstd_EP.cmake)
include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindMagic_EP.cmake)

if(TILEDB_CRC32)
include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindCrc32c_EP.cmake)
endif()

if(TILEDB_WEBP)
include(${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/FindWebp_EP.cmake)
endif()
Expand Down
3 changes: 3 additions & 0 deletions test/CMakeLists.txt
Expand Up @@ -409,3 +409,6 @@ add_subdirectory(ci)
if(WIN32)
add_subdirectory(performance)
endif()

#add tests related to external items
add_subdirectory(external)
65 changes: 65 additions & 0 deletions test/external/CMakeLists.txt
@@ -0,0 +1,65 @@
#
# test/external/CMakeLists.txt
#
#
# The MIT License
#
# Copyright (c) 2022 TileDB, Inc.
#
# Permission is hereby granted, free of charge, to any person obtaining a copy
# of this software and associated documentation files (the "Software"), to deal
# in the Software without restriction, including without limitation the rights
# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
# copies of the Software, and to permit persons to whom the Software is
# furnished to do so, subject to the following conditions:
#
# The above copyright notice and this permission notice shall be included in
# all copies or substantial portions of the Software.
#
# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
# 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.
#

if(NOT TILEDB_ABSEIL)
message("skipping target unit_link_absl, found NOT TILEDB_ABSEIL")
else()
message("adding target unit_link_absl, have TILEDB_ABSEIL")

add_executable(unit_link_absl EXCLUDE_FROM_ALL src/absl_link_test.cc)

find_package(absl CONFIG REQUIRED
PATHS
${TILEDB_EP_INSTALL_PREFIX}/lib/cmake
${TILEDB_DEPS_NO_DEFAULT_PATH}
)

include(${CMAKE_CURRENT_SOURCE_DIR}/src/absl_library_targets.cmake)

target_link_libraries(unit_link_absl PRIVATE ${TILEDB_ABSL_LIBRARY_TARGETS})

target_include_directories(unit_link_absl PRIVATE ${TILEDB_EP_INSTALL_PREFIX} ${TILEDB_EP_INSTALL_PREFIX}/include )

endif()

if(NOT TILEDB_CRC32)
message("skipping target unit_link_crc32, found NOT TILEDB_CRC32")
else()
message("adding target unit_link_absl, have TILEDB_ABSEIL")

add_executable(unit_link_crc32 EXCLUDE_FROM_ALL src/crc32_link_test.cc)

find_package(Crc32c CONFIG REQUIRED
PATHS
${TILEDB_EP_INSTALL_PREFIX}/lib/cmake
${TILEDB_DEPS_NO_DEFAULT_PATH}
)

target_link_libraries(unit_link_crc32 PRIVATE Crc32c::crc32c)

target_include_directories(unit_link_crc32 PRIVATE ${TILEDB_EP_INSTALL_PREFIX} ${TILEDB_EP_INSTALL_PREFIX}/include )
endif()
45 changes: 45 additions & 0 deletions test/external/src/crc32_link_test.cc
@@ -0,0 +1,45 @@
/**
* @file crc32_lilnk_test.cc
*
* @section LICENSE
*
* The MIT License
*
* @copyright Copyright (c) 2022 TileDB Inc.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* 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.
*
* @section DESCRIPTION
*
* Tests for linkage to Crc32c
*
*/

#include <stdio.h>

#include <include/crc32c/crc32c.h>

int main()
{
uint8_t buf[32];

printf("crc32 of indeterminate buffer is %u\n", crc32c::Crc32c(buf, sizeof(buf)));

return 0 ;
}
15 changes: 15 additions & 0 deletions tiledb/CMakeLists.txt
Expand Up @@ -609,6 +609,20 @@ target_link_libraries(TILEDB_CORE_OBJECTS_ILIB
Zstd::Zstd
libmagic
)

if(TILEDB_CRC32)
find_package(crc32c_EP REQUIRED)
find_package(crc32c CONFIG REQUIRED
HINTS
${TILEDB_EP_INSTALL_PREFIX}/lib/cmake
)

target_link_libraries(TILEDB_CORE_OBJECTS_ILIB
INTERFACE
Crc32c::crc32c
)
endif()

if(TILEDB_WEBP)
find_package(Webp_EP REQUIRED)
target_link_libraries(TILEDB_CORE_OBJECTS_ILIB
Expand Down Expand Up @@ -888,6 +902,7 @@ if (TILEDB_STATIC)
append_dep_lib(GCSSDK::crc32c)
append_dep_lib(spdlog::spdlog)
append_dep_lib(libmagic)
append_dep_lib(Crc32c::crc32c)
ihnorton marked this conversation as resolved.
Show resolved Hide resolved
append_dep_lib(WebP::webp)
endif()

Expand Down