From 21d9e71136e991dc5fbb48087fa4f8d71d20db62 Mon Sep 17 00:00:00 2001 From: Julia Dark <24235303+jp-dark@users.noreply.github.com> Date: Thu, 10 Oct 2024 15:05:26 -0400 Subject: [PATCH] [c++] Set warnings-as-errors to be off by default (#3159) * Set `TILEDBSOMA_ENABLE_WERROR` to be `OFF` by default. * Allow user to set `TILEDBSOMA_ENABLE_WERROR` as an environment variable. This can be overridden by a user provided value. * Fix CMake code checking if `TILEDBSOMA_ENABLE_WERROR` is `OFF`. * Update the build script to take `werror` as an argument. * Explicitly set `werror=true` for CI. --- .github/workflows/libtiledb-ci.yml | 2 +- .github/workflows/python-ci-packaging.yml | 1 + .github/workflows/r-python-interop-testing.yml | 4 ++-- libtiledbsoma/CMakeLists.txt | 11 ++++++++--- scripts/bld | 8 ++++++++ 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/.github/workflows/libtiledb-ci.yml b/.github/workflows/libtiledb-ci.yml index fa8f2d619d..9bbfa562fc 100644 --- a/.github/workflows/libtiledb-ci.yml +++ b/.github/workflows/libtiledb-ci.yml @@ -19,7 +19,7 @@ jobs: - name: Checkout TileDB-SOMA uses: actions/checkout@v4 - name: Build libTileDB-SOMA - run: TILEDBSOMA_COVERAGE="--coverage" ./scripts/bld --no-tiledb-deprecated=true + run: TILEDBSOMA_COVERAGE="--coverage" ./scripts/bld --no-tiledb-deprecated=true --werror=true - name: Run libTileDB-SOMA unittests run: ctest --test-dir build/libtiledbsoma -C Release --verbose --rerun-failed --output-on-failure - name: Upload coverage to Codecov diff --git a/.github/workflows/python-ci-packaging.yml b/.github/workflows/python-ci-packaging.yml index c470117b00..a85db66d09 100644 --- a/.github/workflows/python-ci-packaging.yml +++ b/.github/workflows/python-ci-packaging.yml @@ -92,6 +92,7 @@ jobs: -D OVERRIDE_INSTALL_PREFIX=OFF \ -D DOWNLOAD_TILEDB_PREBUILT=OFF \ -D TILEDB_REMOVE_DEPRECATIONS=ON \ + -D TILEDBSOMA_ENABLE_WERROR=ON \ -D FORCE_BUILD_TILEDB=OFF cmake --build build-libtiledbsoma -j $(nproc) cmake --build build-libtiledbsoma --target install-libtiledbsoma diff --git a/.github/workflows/r-python-interop-testing.yml b/.github/workflows/r-python-interop-testing.yml index 3cb006f4c7..e481205615 100644 --- a/.github/workflows/r-python-interop-testing.yml +++ b/.github/workflows/r-python-interop-testing.yml @@ -80,7 +80,7 @@ jobs: # run: cd apis/r && Rscript -e "options(bspm.version.check=TRUE); install.packages('tiledb', repos = c('https://eddelbuettel.r-universe.dev/bin/linux/jammy/4.3/', 'https://cloud.r-project.org'))" - name: Build and install libtiledbsoma - run: sudo scripts/bld --prefix=/usr/local --no-tiledb-deprecated=true && sudo ldconfig + run: sudo scripts/bld --prefix=/usr/local --no-tiledb-deprecated=true --werror=true && sudo ldconfig - name: Install R-tiledbsoma run: | @@ -100,7 +100,7 @@ jobs: cache-dependency-path: ./apis/python/setup.py - name: Install tiledbsoma - run: pip -v install -e apis/python[dev] -C "--build-option=--no-tiledb-deprecated" + run: pip -v install -e apis/python[dev] -C "--build-option=--no-tiledb-deprecated " - name: Show Python package versions run: | diff --git a/libtiledbsoma/CMakeLists.txt b/libtiledbsoma/CMakeLists.txt index 5d3d6d90be..7ee8e227ec 100644 --- a/libtiledbsoma/CMakeLists.txt +++ b/libtiledbsoma/CMakeLists.txt @@ -39,12 +39,17 @@ set(CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} "${CMAKE_CURRENT_SOURCE_DIR}/cmake/Mo set(TILEDBSOMA_CMAKE_INPUTS_DIR "${CMAKE_CURRENT_SOURCE_DIR}/cmake/inputs") +# Check for and set environment variables. +if (NOT DEFINED TILEDBSOMA_ENABLE_WERROR AND DEFINED $ENV{TILEDBSSOMA_ENABLE_WERROR}) + set(TILEDBSOMA_ENABLE_WERROR "$ENV{TILEDBSOMA_ENABLE_WERROR}") +endif() + # TileDB-SOMA Options option(TILEDBSOMA_BUILD_PYTHON "Build Python API bindings" ON) option(TILEDBSOMA_BUILD_CLI "Build tiledbsoma CLI tool" ON) option(TILEDBSOMA_BUILD_STATIC "Build a static library; otherwise, shared library" OFF) option(TILEDBSOMA_ENABLE_TESTING "Enable tests" ON) -option(TILEDBSOMA_ENABLE_WERROR "Enables the -Werror flag during compilation." ON) +option(TILEDBSOMA_ENABLE_WERROR "Enables the -Werror flag during compilation." OFF) # Superbuild option must be on by default. option(SUPERBUILD "If true, perform a superbuild (builds all missing dependencies)." ON) @@ -168,7 +173,7 @@ if(MSVC) set(TILEDBSOMA_COMPILE_OPTIONS /W4 /wd4101 /wd4146 /wd4244 /wd4251 /wd4456 /wd4457 /wd4702 /wd4800 /wd4996) # Warnings as errors: - if(TILEDBSOMA_ENABLE_WERROR) + if(${TILEDBSOMA_ENABLE_WERROR}) set(TILEDBSOMA_WERROR_OPTION /WX) else() set(TILEDBSOMA_WERROR_OPTION "") @@ -199,7 +204,7 @@ else() set(TILEDBSOMA_COMPILE_OPTIONS -Wall -Wextra) - if(TILEDBSOMA_ENABLE_WERROR) + if(${TILEDBSOMA_ENABLE_WERROR}) set(TILEDBSOMA_WERROR_OPTION -Werror) else() set(TILEDBSOMA_WERROR_OPTION "") diff --git a/scripts/bld b/scripts/bld index 5008e188dc..a1355b302d 100755 --- a/scripts/bld +++ b/scripts/bld @@ -13,6 +13,7 @@ prefix="" tiledb="" cmake_verbose="false" no_tiledb_deprecated="false" +werror="false" while test $# != 0; do case "$1" in @@ -21,6 +22,7 @@ while test $# != 0; do --tiledb=*) tiledb=$(arg "$1");; --cmake-verbose=*) cmake_verbose=$(arg "$1");; --no-tiledb-deprecated=*) no_tiledb_deprecated=$(arg "$1");; + --werror=*) werror=$(arg "$1");; esac shift done @@ -46,6 +48,8 @@ if [ "$(uname -m)" == "aarch64" ]; then extra_opts+=" -DDOWNLOAD_TILEDB_PREBUILT=OFF" fi + + # NOTE: set to true to debug the cmake build if [ "$cmake_verbose" = "true" ]; then # This is _incredibly_ helpful in that it reveals the actual compile lines etc which make itself @@ -61,12 +65,16 @@ if [ "$cmake_verbose" = "true" ]; then # Also (pro-tip), set nproc=1 to get a more deterministic ordering of output lines. nproc=1 +elif [ "$werror" = "true" ]; then + extra_opts+=" -DTILEDBSOMA_ENABLE_WERROR=ON" + fi if [ "$no_tiledb_deprecated" = "true" ]; then extra_opts+=" -DTILEDB_REMOVE_DEPRECATIONS=ON" fi + # set installation path if [ -n "${prefix}" ]; then extra_opts+=" -DCMAKE_INSTALL_PREFIX=${prefix} -DOVERRIDE_INSTALL_PREFIX=OFF"