Adds support for modern CMake

[jiverson002]

The primary purpose of this PR is to incorporate modern CMake (>=3.1) best practices into GKlib. As a result, a reorganization of the directory structure as well as a few minor code cleanups were required. A summary of those changes follows:

• Moves all header files into a separate include directory.

• The test directory was renamed to apps, to allow for future unit tests to be written and included in a directory named test.

• Makes building of the GKlib apps disabled by default if the GKlib library is not the top CMake project and enabled if it is. This way, when GKlib is a dependency of another project, the apps are not build by default. An option, GKLIB_BUILD_APPS, was added to allow projects depending on GKlib to also build the apps.

• The win32 shims should only be included when compiling in a Windows environment. Upstream checks this by testing the CMake variable MSVC. However, this PR uses generator expressions and the more general test of the CMake PLATFORM_ID being equal to Windows.

• All options and other configurable behavior of the CMakeLists.txt is implemented using generator-expressions.

• Replaces the following project defined macros:

  • __OPENMP__ -> _OPENMP
  • WIN32 -> _WIN32

  with predefined macros for conforming systems.

• Adds the file GKlibSystem.cmake which includes lots of nice default settings for different things. Along with this, the CMake variable GKLIB_SYSTEM is added. This variable, when defined, will point to a CMake script, like GKlibSystem.cmake, where compiler options and the like can be specified. This file can be included using the CMake variable CMAKE_PROJECT_GKlib_INCLUDE at configure time. This allows the user of the build system to have a file where they manage common CMake settings without affecting the core CMake logic and thus imposing those settings on other users of this build.

• Adds support for testing successful compilation (w/ and w/o apps) on Travis using Linux, Windows, and Apple

• Incorporates changes from pkg-petsc

There are two sample projects here and here, which show how GKlib can be incorporated into a project using more modern CMake features. The add_subdirectory() will still work as well, but these examples highlight some of the convenience of modern C/C++ tooling. The first example uses only CMake, while the second uses the C/C++ package manage Conan.

[gruenich]

I love this merge request and would like to see most of to get merged. But I find it difficult to review all changes, name the controversial ones from the obvious ones, and in the Git history it will be not obvious, why a line was modified.

Would you mind to split your commit in at least a dozen smaller ones?

Two notes I could already figure out:

• Why having names _WIN32 and _OPENMP. Usually you have the prefix of the project name to have unique variable names. Like GKlib_WIN32 and GKlib_OPENMP.
• You increased the required CMake version from 2.8 to 3.1. I am not against it, but is has to be clear, that this will cut of some users. I think it is ok, CMake 3.1 is from December 2014.

[jiverson002]

I would be happy to split it up. I envision this split then being the following:

1. CMake changes - this includes bump to CMake 3.1 and reorganization of the directory structure.
2. Travis changes - this adds support for test compiling on Travis platform - should have been separate from the beginning since this may not be necessary with GitHub Actions, although I am less familiar with that.
3. pkg-petsc changes.
4. Other changes to files, like renaming of macros, changes from <> to "" for includes, etc.

However that is only four indivisible units. My guess is that you would like to see (1) above broken up further, since the others are quite small and self-contained. In this case, I am not sure how much I can break that down, since many of those changes are interdependent. If I really tried, I might be able to extract changes that apply to the feature availability checks. I could also break (4) into separate commits, one for each change.

Can you elaborate on what you have in mind?

[jiverson002]

I split the commit into three commits as follows:

1. CMake upgrade
2. Travis support
3. Code cleanup

I left out incorporation of pkg-petsc changes as well as the renaming of the WIN32 macro. After looking into the WIN32 macro more, it appears that there could be more improvements than the simple ones that I had originally made in terms of handling the Windows platform; enough that they may warrant their own pull-request.

[gruenich]

Why the change? I think the old form was correct. You use quotation marks for relative paths to the current file and angle brackets for files from the include path.

[jiverson002]

The #include documentation for GCC states:

Both user and system header files are included using the preprocessing directive ‘#include’. It has two variants:

#include <file>

This variant is used for system header files. It searches for a file named file in a standard list of system directories. You can prepend directories to this list with the -I option (see Invocation).

#include "file"

This variant is used for header files of your own program. It searches for a file named file first in the directory containing the current file, then in the quote directories and then the same directories used for <file>. You can prepend directories to the list of quote directories with the -iquote option.

The Microsoft docs say something similar.

What distinguishes the two syntax forms is that the quote form first checks the directory of the current file, then falls back to the angle bracket form if nothing is found. With this behavior in mind, I had used the quote form to distinguish headers that were part of the project, despite the fact that the behavior was actually falling back to the angle bracket form for any files in the src and apps directories. My rationale is that this allows the developer to quickly identify headers distributed with the project versus those provided by the system.

With that in mind, one could argue that in src and apps, these should be using angle bracket form.

[gruenich]

Thanks for quoting the specs. I was arguing like you said in the last sentence and I don't see an advantage in changing it. I wouldn't change it, but it probably is a matter of style.

[jiverson002]

I am okay with either. Since it is not my project, I will defer to the primary maintainers if they have a preference. I will gladly change it back to using <> where appropriate.

[gruenich]

I pretty much like your changes. Especially the move in the readme from make, make config etc. to proper CMake. Fingers crossed, that Mr. Karypis will merge it!

[hadim]

xref to the conda forge recipe as it could be useful: conda-forge/staged-recipes#12547

[jiverson002]

I added a missing RUNTIME clause in the install command in the main CMakeLists.txt.

[LecrisUT]

Hi @jiverson002, I plan to address these again in a form like KarypisLab/METIS#79. I plan to do more structural changes first to make the projects importable one in another. Could you then help address the other issues like the naming conventions on top of that?