ci: Add debug build to CI matrix #83
Conversation
ZacBlanco
changed the title
ZacBlanco
force-pushed
the
ci-debug-build
branch
3 times, most recently
from
35257ff to
4aa809c
Compare
ZacBlanco
force-pushed
the
ci-debug-build
branch
6 times, most recently
from
844ab2b to
d9e86a8
Compare
conanfile.py
Outdated
| tc.cache_variables["MAX_LINK_JOBS"] = num_link_job | ||
|
|
||
| if self.options.get_safe("ci_debug_minsize"): | ||
| tc.cache_variables["CMAKE_CXX_FLAGS_DEBUG"] = "" |
There was a problem hiding this comment.
No need to touch this?
CMAKE_CXX_FLAGS_DEBUG flag will controlled by conan build -s build_type=xxx
To use the Release dependencies:
conan install . -sbuild_type=$${DEPENDENCY_BUILD_TYPE:-${BUILD_TYPE}}
It will work, I guess.
There was a problem hiding this comment.
In my testing I found CMAKE_CXX_FLAGS_DEBUG is always set to "-g". In order to create a "debug" build but without the debug info, I needed to remove the "-g" argument from the compilation flags. I found if I just set this variable to "" then -g is removed from the compilation command. This variable is set even in non-debug builds. It just only gets used when the build type is set to Debug.
This was the solution I came up with to strip debug info, but I'm happy to do it another way if you have a suggestion
ZacBlanco
force-pushed
the
ci-debug-build
branch
2 times, most recently
from
14f73ac to
ac59340
Compare
conanfile.py
Outdated
| "enable_test": [True, False], | ||
| "build_benchmark": ["off", "basic", "on"], | ||
| "enable_coverage": [True, False], | ||
| "ci_debug_minsize": [True, False], |
There was a problem hiding this comment.
The conan options are the attributes of a library, and these options will exposed to the user (gluten/presto ...)
From my point of view, "ci_debug_minsize" should not be an option.
It is only for CI. Use an environment variable?
btw, build_benchmark / enable_coverage also should be removed. (I will try to remove them later)
There was a problem hiding this comment.
Got it. I think that makes sense.
I changed the strategy to modify the cmake variable by instead configuring the profile with the following value in the [conf] section:
tools.cmake.cmaketoolchain:extra_variables={"CMAKE_CXX_FLAGS_DEBUG": ""}
This avoids having any specific logic in the build conanfile for CI, which I feel is a much better solution. Configuring the environment seems better to me anyways.
ZacBlanco
force-pushed
the
ci-debug-build
branch
5 times, most recently
from
bb0a4e7 to
de3ebdf
Compare
- debug info makes the build very slow due to limited disk perf - debug info is not really required in CI. We just want the code and tests to run as if debug is enabled (-O0)
- BOLT_BUILD_TIME added as a cached variable. This prevents inadvertent changes to version.h which causes recompilation every time we call a `make` command - version.h is now generated to _build/<BUILD_TYPE>/gen_cpp/bolt/version/version.h - In the case you use the makefile to configure, calling the same target twice in a row will no longer delete the CMakeCache.txt - With the version.h changes, CI builds for release and release_spark profiles will now improve in time by 2-4 minutes for every job due to not recompiling. - changes benchmarks-build to build-only with a matrix using benchmarks and debug_with_test - Added IN_CI variable to force dependencies to be built in release mode as opposed to debug mode purely for CI. This is used in conjunction with the `-s &:...` arguments to the conan build
ZacBlanco
force-pushed
the
ci-debug-build
branch
from
de3ebdf to
24d5bae
Compare
3 participants
What problem does this PR solve?
Adds the debug and test build to the CI matrix
Type of Change
Description
Adds debug mode build to CI builds.
Performance Impact
Release Note
N/A
Checklist (For Author)
Breaking Changes