Support out-of-source build with modern CMake

lichray commented 4 years ago

Rehash the build system to support:

Out-of-source build
Ninja
Multi-configuration build with Ninja

Out-of-source build

We used to have a "build" directory that mixes sources and binary, which causes users not able to clean up this directory easily. In this patch, we lift CMakeLists.txt to the project root directory and move template files (*.in) to files/. Now you can choose an arbitrary directory as the build directory:

> cmake -B build -DCMAKE_BUILD_TYPE=Debug  # still using "build" in this example
> make -C build

This directory contains only generated build artifacts so that you can rm -fr the whole thing to start over.

Ninja

Ninja is a very fast parallel build system.

> cmake -G Ninja -B build -DCMAKE_BUILD_TYPE=Debug
> ninja -C build

Multi-configuration build with Ninja

It is often annoying to switch the build between Debug and Release. With CMake >= 3.17, you can hold both in the same build directory:

> cmake -G "Ninja Multi-Config" -B build
> ninja -C build              # default -- do debug build
> ninja -C build -f build-Release.ninja  # do release build

The debug build produces the nvi binary under build/Debug/, and the release build produces binary under build/Release/.

CC @leres, would you like to give this a try?

leres commented 4 years ago

This would have saved me some time when I created the editors/nvi2 port last week; the FreeBSD ports system uses ninja with cmake by default and I couldn't easily get it working.

Minor nit, my brain had trouble parsing rm -fr, it is very much conditioned to expect rm -rf.

I see that various includes are still added to the source tree as part of the build (e.g. cl/extern.h). I guess I don't understand what out-of-source build means, I was expecting that the source tree could be on a read/only filesystem, not that I do it that way.

I see many changes to CMakeLists.txt and I get a lot of new warnings, e.g:

% rm -rf build && cmake -B build && make -C build
[...]
[ 14%] Building C object CMakeFiles/nvi.dir/cl/cl_term.c.o
/var/tmp/nvi2.git-cmake/cl/cl_term.c:430:4: warning: add explicit braces to
      avoid dangling else [-Wdangling-else]
                        else
                        ^
/var/tmp/nvi2.git-cmake/cl/cl_term.c:435:4: warning: add explicit braces to
      avoid dangling else [-Wdangling-else]
                        else
                        ^
2 warnings generated.

I assume these are due to less tolerant compiler flags (which I'm in favor of -- I use -Werror with many of my projects). But I do not see the warnings when I build with ninja:

rm -rf build && cmake -G "Ninja Multi-Config" -B build && ninja -C build
rm -rf build && cmake -G "Ninja Multi-Config" -B build && ninja -C build -f build-Release.ninja

I have limited experience with ninja so maybe this is expected.

Anyway this looks nice.

lichray commented 4 years ago

This would have saved me some time when I created the editors/nvi2 port last week;

Hmm, not sure why do we need this port; to update more easily using pkg? The /usr/bin/vi command in FreeBSD is already nvi2, with wide chars and iconv turned on.

I see that various includes are still added to the source tree as part of the build (e.g. cl/extern.h). I guess I don't understand what out-of-source build means, I was expecting that the source tree could be on a read/only filesystem, not that I do it that way.

Your expectation is right, but the status-quo of nvi source code (#include "extern.h" in the headers) doesn't expect this. On the other hand, generating sources in the source tree simplifies importing the code to other source trees, such as the FreeBSD base.

I see many changes to CMakeLists.txt and I get a lot of new warnings, e.g: [...]

This is because when using a single-config generator, CMAKE_BUILD_TYPE is unset; there is no default build type. In this situation, -DCMAKE_C_FLAGS=... will be in effect and so that a user can play with customized flags, such as -Werror. I pushed a change to issue a warning if neither is set.

It's a piece of common but weird knowledge to CMake users... FreeBSD ports system sets CMAKE_BUILD_TYPE to Release by default.

But I do not see the warnings when I build with ninja:

rm -rf build && cmake -G "Ninja Multi-Config" -B build && ninja -C build
rm -rf build && cmake -G "Ninja Multi-Config" -B build && ninja -C build -f build-Release.ninja

The multi-config generator defaults to Debug build.

leres commented 4 years ago

Hmm, not sure why do we need this port; to update more easily using pkg? The /usr/bin/vi command in FreeBSD is already nvi2, with wide chars and iconv turned on.

The only reason I'm interested in a nvi port is to gain the expandtab option. If FreeBSD vi tracks your github project then maybe one day it will catch up with this feature but that is many months away (at best) and doing python dev work with the base vi is just too painful.

lichray commented 4 years ago

The only reason I'm interested in a nvi port is to gain the expandtab option. If FreeBSD vi tracks your github project then maybe one day it will catch up with this feature but that is many months away (at best) [...]

Haha, yes.

leres commented 4 years ago

Thanks for this change, it makes the FreeBSD port more minimal.

lichray / nvi2