Re: [hatari-devel] Hatari default / release build types

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Am Thu, 21 Aug 2025 17:07:47 +0300
schrieb Eero Tamminen <oak@xxxxxxxxxxxxxx>:

> Hi,
> 
> On 21.8.2025 16.18, Nicolas Pomarède wrote:
> > Le 21/08/2025 à 15:04, Eero Tamminen a écrit :  
> >> It may be better to default to (optimized) debug build with asserts 
> >> enabled, and only do release builds with asserts disabled[2], but 
> >> that's up to Nicolas.
> >>
> >> (I don't know how Nicolas does releases.)  
> > 
> > For release, I use "./configure" or "./configure --cross-compile- 
> > win64_64". I must admit I rarely add "--enable-debug" flag to "./ 
> > configure" in my usual "pipeline"  
> 
> I had forgotten that there was a "configure" wrapper script...

We're at least using --enable-debug for the CI jobs that don't produce
artifacts for the end users, so we got some coverage there.

> > But if we want to spot possible assert errors, maybe the automated 
> > builds on framagit could have "--enable-debug" ? This way people who 
> > download build's artefact (such as zip archive for windows) would see them.  
> 
> That maps to CMake "Debug" build type:
> https://github.com/hatari/hatari/blob/main/configure#L53
> 
> I did not find from current CMake documentation even what the current 
> build types (are supposed to) map to:
> https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html
> 
> But when testing my (older) CMake 3.25.1 version, the standard values 
> seem to correspond to:
> - Debug: "-g" [1]
> - MinSizeRel: "-Os -DNDEBUG"
> - RelWithDebInfo: "-O2 -g -DNDEBUG"
> - Release: "-O3 -DNDEBUG"
> 
> Meaning that debug builds are no good because:
> - Resulting binary is really slow
> - Compiler misses most of warnings
>    (as code analysis is done by its optimization passes)
> 
> And none of the others are good either. One would need to create an 
> extra build type to CMakeLists.txt, for example:
> - RelDebug: "O3 -g"

We could do something like this to enable assert() in the RelWithDebInfo
builds at least:

diff --git a/CMakeLists.txt b/CMakeLists.txt
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -174,6 +174,10 @@ check_large_file()
 # CPP Definitions:
 # ################
 
+# We still want to have assert() in RelWithDebInfo builds:
+string(REPLACE " -DNDEBUG" ""
+       CMAKE_C_FLAGS_RELWITHDEBINFO "${CMAKE_C_FLAGS_RELWITHDEBINFO}")
+
 # Test for large file support:
 execute_process(COMMAND getconf LFS_CFLAGS
                 OUTPUT_VARIABLE DETECTED_LFS_CFLAGS
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -63,7 +63,7 @@ build-mingw:
     - cd ../build
     - VERSION=$(git describe)
     - cmake -DCMAKE_TOOLCHAIN_FILE=cmake/Toolchain-mingw32-win64_64.cmake
-            -DENABLE_WERROR:BOOL=1 ..
+            -DENABLE_WERROR:BOOL=1 -DCMAKE_BUILD_TYPE:STRING=RelWithDebInfo ..
     - make -j$(nproc)
     - cd ..
     # And finally create the package

WDYT?

 Thomas



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/