Skip to content

Commit

Permalink
[c++] Set warnings-as-errors to be off by default (#3159)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
jp-dark authored Oct 10, 2024
1 parent 4d37aa9 commit 21d9e71
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 6 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/libtiledb-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/python-ci-packaging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/r-python-interop-testing.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand All @@ -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: |
Expand Down
11 changes: 8 additions & 3 deletions libtiledbsoma/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 "")
Expand Down Expand Up @@ -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 "")
Expand Down
8 changes: 8 additions & 0 deletions scripts/bld
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ prefix=""
tiledb=""
cmake_verbose="false"
no_tiledb_deprecated="false"
werror="false"

while test $# != 0; do
case "$1" in
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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"
Expand Down

0 comments on commit 21d9e71

Please sign in to comment.