Skip to content

Commit

Permalink
Fix bootstrap usage of WARNINGS_AS_ERRORS
Browse files Browse the repository at this point in the history
In contrast to the default for CMake files, `Release` is used as the
default in `CMakeFiles.txt` which causes the `bootstrap` script to do a
release build with development flags, in particular `-Werror`. Since
warnings are triggered in a release build, this cause the build to
fail while a debug build works fine.

This commit fixes this by removing the `-Werror` flag (by setting
`WARNINGS_AS_ERRORS` to `OFF`) on anything that is not a debug build
and also disable the warnings that (currently) trigger the warnings in
the release build.

The commit also changes some of the GitHub workflows to run without
`WARNINGS_AS_ERRORS` since it should always work without this option
regardless of build type (on release build this should be disabled, on
debug builds this should be enabled).  But it is set to `ON` for the
full release and debug builds to ensure that we do not generate any
warnings, which will capture new surfacing warnings.

Fixes timescale#2770
  • Loading branch information
mkindahl committed Feb 5, 2021
1 parent 954c775 commit 44fac26
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 8 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/coverity.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ jobs:
- name: Build TimescaleDB
run: |
PATH="$GITHUB_WORKSPACE/coverity/bin:$PATH"
./bootstrap -DCMAKE_BUILD_TYPE=Release -DPG_SOURCE_DIR=~/$PG_SRC_DIR -DPG_PATH=~/$PG_INSTALL_DIR -DWARNINGS_AS_ERRORS=OFF
./bootstrap -DCMAKE_BUILD_TYPE=Release -DPG_SOURCE_DIR=~/$PG_SRC_DIR -DPG_PATH=~/$PG_INSTALL_DIR
cov-build --dir cov-int make -C build
- name: Upload report
Expand Down
14 changes: 11 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,15 @@ if (NOT CMAKE_C_COMPILER_ID IN_LIST SUPPORTED_COMPILERS)
message(FATAL_ERROR "Unsupported compiler ${CMAKE_C_COMPILER_ID}. Supported compilers are: ${SUPPORTED_COMPILERS}")
endif ()

# Option to treat warnings as errors when compiling (default on)
option(WARNINGS_AS_ERRORS "Make compiler warnings into errors (default ON)" ON)
# Option to treat warnings as errors when compiling (default on for
# debug builds, off for all other build types)
if (CMAKE_BUILD_TYPE STREQUAL Debug)
message(STATUS "CMAKE_BUILD_TYPE matches Debug")
option(WARNINGS_AS_ERRORS "Make compiler warnings into errors (default ON)" ON)
else()
message(STATUS "CMAKE_BUILD_TYPE does not match Debug")
option(WARNINGS_AS_ERRORS "Make compiler warnings into errors (default ON)" OFF)
endif()

if (WARNINGS_AS_ERRORS)
if (CMAKE_C_COMPILER_ID MATCHES "GNU|Clang|AppleClang")
Expand All @@ -116,7 +123,8 @@ if(CMAKE_C_COMPILER_ID MATCHES "GNU|AppleClang|Clang")
# These flags are supported on all compilers.
add_compile_options(
-Wempty-body -Wvla -Wall -Wmissing-prototypes -Wpointer-arith
-Werror=vla -Wendif-labels -fno-strict-aliasing -fno-omit-frame-pointer)
-Werror=vla -Wendif-labels -Wno-stringop-truncation
-fno-strict-aliasing -fno-omit-frame-pointer)

# These flags are just supported on some of the compilers, so we
# check them before adding them.
Expand Down
18 changes: 14 additions & 4 deletions scripts/gh_matrix_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,18 @@
def build_debug_config(overrides):
# llvm version and clang versions must match otherwise
# there will be build errors this is true even when compiling
# with gcc as clang is used to compile the llvm parts
# with gcc as clang is used to compile the llvm parts.
#
# Strictly speaking, WARNINGS_AS_ERRORS=ON is not needed here, but
# we add it as a precation. Intention is to have at least one
# release and one debug build with WARNINGS_AS_ERRORS=ON so that we
# capture warnings generated due to changes in the code base or the
# compiler.
base_config = dict({
"name": "Debug",
"build_type": "Debug",
"pg_build_args": "--enable-debug --enable-cassert",
"tsdb_build_args": "-DCODECOVERAGE=ON",
"tsdb_build_args": "-DCODECOVERAGE=ON -DWARNINGS_AS_ERRORS=ON",
"installcheck_args": "IGNORES='bgw_db_scheduler'",
"coverage": True,
"llvm_config": "llvm-config-9",
Expand All @@ -52,13 +58,17 @@ def build_debug_config(overrides):
base_config.update(overrides)
return base_config

# We build this release configuration with WARNINGS_AS_ERRORS=ON to
# make sure that we can build with -Werrors even for release
# builds. This will capture some cases where warnings are generated
# for release builds but not for debug builds.
def build_release_config(overrides):
base_config = build_debug_config({})
release_config = dict({
"name": "Release",
"build_type": "Release",
"pg_build_args": "",
"tsdb_build_args": "-DWARNINGS_AS_ERRORS=OFF",
"tsdb_build_args": "-DWARNINGS_AS_ERRORS=ON",
"coverage": False,
})
base_config.update(release_config)
Expand All @@ -70,7 +80,7 @@ def build_apache_config(overrides):
apache_config = dict({
"name": "ApacheOnly",
"build_type": "Release",
"tsdb_build_args": "-DAPACHE_ONLY=1 -DWARNINGS_AS_ERRORS=OFF",
"tsdb_build_args": "-DAPACHE_ONLY=1",
"pg_build_args": "",
"coverage": False,
})
Expand Down

0 comments on commit 44fac26

Please sign in to comment.