Update cmake

[jurajsic]

This PR updates CMake configuration files to more modern way. More specifically:

• building tests can now be turned off by using -DBUILD_TESTING:BOOL=OFF while running cmake and they are not built when mata is not the main project (i.e. it is submodule or similar)
• examples are built trough CMake, can be turned off by -DMATA_BUILD_EXAMPLES:BOOL=OFF and they are not built when mata is not the main project
• dependencies (CUDD, RE2, simlib) are built as object libraries, i.e. they do not create a library file (.a) but are directly bundled into libmata.a, which also means I could delete the script that used to bundle all .a files together
• linking dependencies is now done "correctly"
• Catch2 is now fetched from its git repository, it will make it easier to move to version 3
• updated Makefile so that we can write only make + added release-lib target which does not build tests or examples
• updated python binding, so that it builds only release-lib + builds to build-python directory
• added uninstall target, that will remove all files installed using the install target

There are still some things I am undecided about:

• Should we use CCACHE (see links in the main CMakeLists.txt)? VATA used it, but I am not sure if it helps in any way.
• Should Catch2 be fetched? It slows down the configuration.

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (6b5322d) 70.86% compared to head (74fa0f3) 70.86%.
Report is 1 commits behind head on devel.

Additional details and impacted files

@@           Coverage Diff           @@
##            devel     #315   +/-   ##
=======================================
  Coverage   70.86%   70.86%           
=======================================
  Files          33       33           
  Lines        4191     4191           
  Branches      965      965           
=======================================
  Hits         2970     2970           
  Misses        845      845           
  Partials      376      376           

Files Changed	Coverage Δ
src/re2parser.cc	38.07% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Adda0]

Both debug and release versions compile on my Linux, local coverage is working. The changes overall look good to me.

Should we use CCACHE (see links in the main CMakeLists.txt)? VATA used it, but I am not sure if it helps in any way.

I would keep it. It might help and it does not cause any trouble.

Should Catch2 be fetched? It slows down the configuration.

I would leave it up to the user to have Catch2 installed. Make Catch2 optional. Without Catch2, the tests will not be automatically compiled. But remember that we need Catch2 for testing in GitHub Actions. There, Catch2 needs to be installed/fetched.

Now that the libraries should be linked correctly, could you remove the GCC and CLANG flags disabling warnings in 3rd-party libraries? Look for #pragma gcc diagnostic or #pragma clang diagnostic in 3rd-party files. There are at the start of a file and at the end. Will the warnings from such files be ignored now correctly?

[jurajsic]

Now that the libraries should be linked correctly, could you remove the GCC and CLANG flags disabling warnings in 3rd-party libraries? Look for #pragma gcc diagnostic or #pragma clang diagnostic in 3rd-party files. There are at the start of a file and at the end. Will the warnings from such files be ignored now correctly?

I cannot remove it from header files which are included from mata, for these it will still show the errors. There is a way so that warnings do not show for these headers, but I am not sure which way is less hacky.

[Adda0]

Now that the libraries should be linked correctly, could you remove the GCC and CLANG flags disabling warnings in 3rd-party libraries? Look for #pragma gcc diagnostic or #pragma clang diagnostic in 3rd-party files. There are at the start of a file and at the end. Will the warnings from such files be ignored now correctly?

I cannot remove it from header files which are included from mata, for these it will still show the errors. There is a way so that warnings do not show for these headers, but I am not sure which way is less hacky.

I feared this was the case. Do not concern yourself with them, then. Let us leave them as they are now.

[Adda0]

We will refactor the local coverage further in another PR to export results into an HTML page and more. I would not enlarge this PR any more.

[tfiedor]

I'm quite baffled what you changed in the binding, and I'm wondering if it was even necessary. Otherwise, I see no major nor colonel issues.

[tfiedor]

Why is python built into other directory?

[Adda0]

I agree that placing the build library in the project root might not be the best approach.

But the change was made to be able to build both the main C++ library plus tests for it and python binding simultaneously. Or at least so we do not have to wait for the main library to be built and tested in order to start building the python binding which takes a considerable time when developing. What I want is the following commands to:

• build C++ library
• build C++ library plus tests, examples, integrations tests
• run integration tests
• build Python binding
• build Python binding and run Python binding tests

However, I do not want to run the command to build library + tests and examples, wait for it to finish, and then run building of python binding for tests and wait again. Then change something and run C++ build and have to wait again for running the Python bindings again only after the C++ build finishes.

Ultimately, what I want is to have a command line flag or some easy to use and set parameter for each type of build: release/debug/release with debug information/python binding which would specify where (in which folder) to build the library. So that I can easily run in two terminals immediately one after the other: run_tests_build_examples, run_python_tests. If I do it now, the two GCC instances rewrite each other's files and the whole build fails.

We could put the build-python folder inside build/ as build/bindings/python, for example. Or have a structure like this:

- /build/
    - bindings/
        - python/ # Python bindings build
    - library/
        - debug/ # C++ lib debug build
        - release/ # C++ lib release build
        - release_debug_info/ # C++ lib release with debug information build

When developing, I need to see the results of each of these four builds (both compilation and tests) (run performance tests on release build and all tests to check correctness with release with debug info), without having to rebuild the entire library again for each one because the previous build compiled the library with different options.

[tfiedor]

Hm, it took me some time, but I think I finally get it. You mean, that your main problem is, that you are e.g. (illustrative; don't take me literally) working on some problem, and you want to have library compiled for DEBUG, and binding built for RELEASE, but if these shared the build (as it did so far), they would clash. I think you confused me with the simultaneously (I interpreted it as "parallelly", which I though was quite weird).

Such interpretation is fine by me. Since I mostly worked on binding only, I was fine with "reusing" the build both for binding and libmata.

[Adda0]

You are right, but that is only a part of the problem. Next, I also want to do the same thing as for DEBUG for release (and measure performance testing) and for release with debug info (and run test – the same tests as for DEBUG).

Moreover, what I want is to be able to do some change and decide which of these commands I need to run after this change. Then run the following commands, each in their own terminal window, simultaneously. That is, start all of them in like two seconds, go do some other stuff and then return and have results for all of these builds. Therefore, not only would release and debug overwrite each other, they also overwrite each other because they are (ideally, how I would want them to) run in parallel, indeed.

After discussing this with @jurajsic, we came to the conclusion that we can already set up the library builds exactly as described above by specifying a command line flag BUILD_DIR for make. However, I cannot do that for Python binding yet. Therefore, the solution for now would be to move the default Python binding build directory to build/python or build/bindings/python. But, for this PR, we will revert these changes, build Python to the default build/ folder and I will try to make a PR in the future allowing to specify a custom build folder for Python binding by a command line flag the same way as for C++ library builds.

[jurajsic]

Mata for python bindings should be built in build/ again.

[tfiedor]

I think in our master make we are starting to have quite a lot of targets. We should add some comment to each of them or find some better names, since these names says fucking nothing.

[jurajsic]

I added some comments, please check if they make sense. But for me using a make file in a cmake project is very weird and I would look for a way to get rid of it.

[Adda0]

I am with you on that. It is convenient for GAs, but in general seems weird. But if it works now, let us keep it as it is in the foreseeable future.

[tfiedor]

How do you build it then? I would never ever remove master Makefile, I consider it as a standard that is used everywhere for everything. Things like make install, make doc, etc. are understood by everyone everywhere, and I believe almost every sane person (not considering psychopats and likes) starts with any project by simply typing make. So please, lets keep master Makefile :)

[Adda0]

Like this (taken from Catch2 build system – a CMake project without a Makefile) inside some shell script:

# 2. Configure the full test build
cmake -B build/ -S . -DCMAKE_BUILD_TYPE=Debug --preset all-tests

# 3. Run the actual build
cmake --build build/

The problem with having both CMake and Makefile is that one does not know whether to create a new target in one or the other. If you want to add a new build functionality, what do you do? Add a target to CMake and not to Makefile? Add a target to Makefile, but not to CMake? Add a target to both? It is harder to maintain.

Currently, we use Makefile for shell scripting, essentially. For running CMake commands, not as a build system. This is good enough for now. One day, we may revisit this conversation and think about it again.

[tfiedor]

Once you are as old as us, and every character typed sends pain to you joints, you too will appreciate simple make :D

[Adda0]

Exactly. That is why you have this in a shell script (as in the example above: this is inside a build and test shell script). You type it once, then you only press an up arrow while on the command line to return to the script execution and press enter. The problem with make is, as described above, that you have two build systems and some things are specified in one build system, other things in the second build system.

[jurajsic]

I am not planning to add anything else, so if you have no problems, we can merge this. There is still the question about Catch2: are you ok with having it in FetchContent? It downloads it during configuration, so it slows it down. We could return back to having the header file in 3rdparty or somewhere, as we are still using version 2 here.

[tfiedor]

I am not planning to add anything else, so if you have no problems, we can merge this. There is still the question about Catch2: are you ok with having it in FetchContent? It downloads it during configuration, so it slows it down. We could return back to having the header file in 3rdparty or somewhere, as we are still using version 2 here.

I have many problems, but not with merging this nor with the Catch2.

[Adda0]

We will keep fetching the Catch2 for now. We can modify it if we find it too problematic in the future before upgrading to Catch2 v3. I am going to merge the PR.