Switch default build type back to RelWithDebInfo for now
Some checks failed
make test / ubuntu (push) Has been cancelled
make test / ubuntu-32bit-static-pcre2 (push) Has been cancelled
make test / ubuntu-asan (push) Has been cancelled
make test / macos (push) Has been cancelled
Rust checks / rustfmt (push) Has been cancelled
Rust checks / clippy (push) Has been cancelled

A release build is recommended to most users (to avoid occasional slowness)
whereas developers may prefer debug builds for shorter build times and more
accurate debug information.

There are more users of "make install" than developers, so I think the
default should be optimized for users, i.e. an optimized build. I think
that's in line with what most of our peer projects do.

Even if developers don't know about the -DCMAKE_BUILD_TYPE=Debug
trick, they will likely be able to iterate quickly by using "cargo
{build,check,clippy,test}" and rust-analyzer, all of which use a debug
configuration by default, irrespective of cmake. Granted, users will need
to use cmake to run system tests. If a task needs a lot of iterations,
one can always convert the system test to a script that can be run with
target/build/fish. For building & running all system tests, the release
build takes 30% longer, so not that much.

Here are my build/test times and binary sizes; with debug:

    $ time ninja -C build-Debug/
    ________________________________________________________
    Executed in   25.30 secs    fish           external
       usr time   68.33 secs  676.00 micros   68.32 secs
       sys time   11.34 secs   41.00 micros   11.34 secs
    $ du -h build-Debug/fish
    43M	    build-Debug/fish
    $ time ninja -C build-Debug/ test
    ________________________________________________________
    Executed in  193.96 secs    fish           external
       usr time  182.84 secs    1.53 millis  182.83 secs
       sys time   30.97 secs    0.00 millis   30.97 secs

with release

    $ time ninja -C build-RelWithDebInfo/
    ________________________________________________________
    Executed in  106.80 secs    fish           external
       usr time  164.98 secs  631.00 micros  164.98 secs
       sys time   11.62 secs   41.00 micros   11.62 secs
    $ du -h build-RelWithDebInfo/fish
    4.6M	build-RelWithDebInfo/fish
    $ time ninja -C build-RelWithDebInfo/ test
    ________________________________________________________
    Executed in  249.87 secs    fish           external
       usr time  260.25 secs    1.43 millis  260.25 secs
       sys time   29.86 secs    0.00 millis   29.86 secs

Tangentially related, the numbers with "lto = true" deleted.  This seems
like a nice compromise for a default but I don't know much about the other
benefits of lto.

    $ time ninja -C build-RelWithDebInfo-thin-lto/
    ________________________________________________________
    Executed in   35.50 secs    fish           external
       usr time  196.93 secs    0.00 micros  196.93 secs
       sys time   13.00 secs  969.00 micros   13.00 secs
    $ du -h build-RelWithDebInfo-thin-lto/fish
    5.5M	build-RelWithDebInfo-thin-lto/fish
    $ time ninja -C build-RelWithDebInfo-thin-lto/ test
    ________________________________________________________
    Executed in  178.62 secs    fish           external
       usr time  287.48 secs  976.00 micros  287.48 secs
       sys time   28.75 secs  115.00 micros   28.75 secs

Alternative solution: have no default at all, and error out until the user
chooses a build type.
This commit is contained in:
Johannes Altmanninger 2024-10-19 21:13:41 +02:00
parent dccc3349f0
commit 31d7f197b1
2 changed files with 1 additions and 3 deletions

View File

@ -235,8 +235,6 @@ CMake remains for now because cargo is unable to install the many asset files th
Some smaller changes:
- The default build configuration has changed to "Debug".
Please pass ``-DCMAKE_BUILD_TYPE=RelWithDebInfo`` if you want to build a package.
- Xcode support has been removed (:issue:`9924`).
- fish no longer links against the (n)curses library, opting to read the terminfo database via the terminfo crate.
This means hashed terminfo databases are no longer supported (from our research, they are basically unmaintained and unused).

View File

@ -5,7 +5,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_LIST_DIR}/cmake")
project(fish LANGUAGES C)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
set(DEFAULT_BUILD_TYPE "Debug")
set(DEFAULT_BUILD_TYPE "RelWithDebInfo")
# Generate Xcode schemas (but not for tests).
set(CMAKE_XCODE_GENERATE_SCHEME 1)