CMake cleanup

[slayoo]

@papadop, @citibeth - please comment on which of the things mentioned below are still TODO:

1. Remove anything having to do with Pkgconfig. That is Autotools-related stuff, and it only works if everyone uses it --- which everyone never did.

2. Building the doc/ directory should be optional. Doc building requires a lot of extra build dependencies, and people should be able to build without first installing all that stuff. Most people will just use pre-built docs online. (This is true for almost any package that comes with docs).

3. Put all options in the top-level CMakeLists.txt file. That way it's easy to see what options this build has. (eg: FORTRAN_BENCHMARKS in in benchmarks/CMakeLists.txt)

4. Similarly, put all find_***() stuff in the top-level, so we can easily see what the dependencies are.

5. Remove commented-out Autotools code.

6. Remove bin/CMakeLists.txt, and the line that includes it.

7. The original Autotools build checked a zillion C++ features and configured accordingly. I would like to consider whether some of those checks are no longer required, for (reasonably) modern compilers. It is now 15+ years since Blitz++ originally was released, and plenty has changed. I don't care to support ancient compilers going forward; and people who DO use those compilers can always use the existing Autotools build. If we can remove checks, then that is super. (Later, we can remove the corresponding #ifdefs) For example... we can probably get rid of BZ_HAVE_STRINGS_H and BZ_HAVE_STRING_H. And a lot more...

8. Can we have a way to put the Blitz version number into the config.h file, coming from the top-level CMakeLists.txt file?

9. Is it possible to remove compiler-specific configuration (BlitzconfigFileName.cmake)? Configurations for specific compilers or OS's have fallen out of favor; instead, feature-based configuration options (like those discussed above) are preferred. In any case, can we remove support for compilers that no longer exist?

10. CheckCXXFeature: Again, let's get rid of checks for things that are now standardized. For example, HAVE_EXCEPTIONS, HAVE_NAMESPACES, HAVE_RTTI, HAVE_MEMBER_CONSTANTS, HAVE_OLD_FOR_SCOPING, HAVE_EXPLICIT, HAVE_BOOL,... Most of these (90%?) we can get rid of because they are now standard in C++. I would say, we should require C++11 (even though the current Blitz API is not C++11 compliant; I'm still not interested in having this CMake build work on any pre-C++11 compilers).

11. Let's try to put things in the file in which they are used. For example, CHECK_ALL_CXX_FEATURES is only used once, and that's in blitz/CMakeLists.txt. The macro should therefore be defined in that file, rather than in cmake/CheckCXXFeature.cmake. In general, any macro that's used in only one file should be inlined into that file.

12. cmake/Win32Compat.cmake: I'm happy not supporting CMake 2.6 any more. I'd be fine with requiring at least CMake 3.0. So we can get rid of extra code in the build required to support old CMake versions.

13. cmake/XXXConfig.cmake.in: Is there any way to avoid the situation in which the CMake build auto-generates part of itself? If we are to keep this kind of complexity, I would want a good reason.

14. Remove commented-out Autotools code from manual/CMakeLists/txt.

Originally posted by @citibeth in https://github.com/blitzpp/blitz/pull/97#issuecomment-466412856

[MicheleBe]

Hi, interesting post, if you don't mind I can make a couple of suggestion from my esperience in trying to move all my build to Modern cmake. Point 4. I suggest to put all the find in a file like third dependiencies that not only find the package but creates a target (imported or to be compiled separately) in order to link the blitz package towards it. These permits to use the so called superbuild pattern inwhich we can also build the depependencies if they are not available.

I usually do something like

find_package(XXX REQUIRED)

if (XXX_FOUND AND NOT TARGET XXX::XXX)
message("Add target XXX::XXX")
    add_library(XXX::XXX INTERFACE IMPORTED)
    set_target_properties(XXX::XXX PROPERTIES
        INTERFACE_INCLUDE_DIRECTORIES "${XXX_INCLUDE_DIRS}"
        INTERFACE_LINK_DIRECTORIES "${XXX_LIBRARY_DIRS}")
endif()

to wrap find that does not create directly targets

point 8. There are several solution,, what I prefer is the following:

In BZ_Config.h.in file

#define BZ_VERSION_MAJOR @BZ_VERSION_MAJOR@
#define BZ_VERSION_MINOR @BZ_VERSION_MINOR@
#define BZ_VERSION_PATCH @BZ_VERSION_PATCH@

and in the cmake file the value are defined:

set(BZ_VERSION_MAJOR 1 CACHE STRING "MAIN Version for BLITZ" FORCE)

and the file is generated by cmake though the configure command

configure_file (
  "${CMAKE_CURRENT_LIST_DIR}/BZ_Config.h.in"
  "${CMAKE_CURRENT_BINARY_DIR}/BZ_Config.h"
  )

Point12. Modern cmake now consider only version greater than 3.5. if you want to find package through Pkgconfig it is also possible to support cmake starting from 2.8. previous version should be consider obsolete.

[papadop]

I will put comment incrementally one by one... 1 - Remove pkgconfig : I do not think this is wise. Even if we build blitz with cmake, people might want to use blitz in an autotools based project (or just with there own makefile). In this case, pkgconfig files are still useful (and much easier to use than cmake config files).

[papadop]

2. This is already the case... This is option BUILD_DOC which defaults to OFF... Are you referring to something else ??

[papadop]

3 & 4: There are advantages and drawbacks in doing that. I usually prefer to have options and find where they are used because its more modular... If you remove/change the directory (or directory content), you directly also remove/see-what-to-change and do not have to remember visit various different files... and I like this principle of locality. I agree that finding options easily would be nice and currently requires a grep or launching cmake gui. Overall, I'd prefer to have options (and actually options we want user to use as there might be options for developpers) directly in the README.md

Find find***() stuff, the same locality argument works... But I tend to "uplevel" find that are common to several subdirs to show that this is some common stuff (I'm not even sure this is the right thing to do as I do not believe there is a significant coherence or efficiency advantage, but at least it factorizes the find_ code).

[papadop]

5. Yes, but this is already done ? No. The only thing I see is a comment at the top of the CMakeLists.txt. It's not autotools code but related and could be suppressed after comparing the situation with the Makefile.am at the same place.

[papadop]

6. Already done.

[papadop]

7 & 9 & 10. A big yes... But I think that the goal was not only for CMake but for autotools too (making your comment stating that people using the old compiler should be using the autotools build). Globally, I think it is better to keep the same overall scheme between autotools and cmake. We do not want to have too many differences.

In this view (removing old compiler support for both autotools and cmake): I wanted first to cleanup the code (i.e. remove the macro stuff in the code and the check and bzconfig.h generation simultaneously). The proposed strategy would be fine too, but there is a chance, we leave a piece in place. Can we split the task among us C++ feature by C++ feature ? That would be more incremental...

[papadop]

8. Yes. I though it was already done. If it's not, it is a bug and is easy to do (probably as easy as adding a set(BZ_VERSION value) in the top CMakeLists. I'm sure it was there at some point, but was probably removed by error. On the other hand, the code does not use it at all (but user may). I'd also advocate for a version number which can be easily tested (i.e. not a string 1.10.2 but a number, C++ uses a date tag which is not very meaningful but easily testable. Of course, we can have macros for both...

[papadop]

11. Well, I'd advocate for suppressing the macro. The blitz code uses it only at one place, but I use it (or used it) in other packages, so this eases the maintenance work. Putting you idea to put everything in the file using it (and following the rule recursively) would make a huge file and I do not believe it would be clearer.

But again, my preferred goal is to suppress it (and from autotools too).