Cmake build

[papadop]

Here is a cmake build for blitz. Minimally tested on linux (but I have in the past compiled on MacOS and Windows, so I have good hopes). There are probably some polishing to be done (notably make test which does not build dependencies and the hao-he benchmark which does not compile due to the removal of vector.h), but most of the functionality is there.

To use it:

1. Go to a new directory.
2. cmake path-to-the-source-directory
3. make (or make lib)
4. make check-{testsuite,benchmarks,examples}

This PR also contains a trivial correction for iter.cpp (elapsedSeconds->elapsed) which was needed to make teh test pass. check-testsuite and check-examples pass. check-benchmarks currently fails due to the hao-he test. I do not know whether we have to correct the test or remove it. So I left it untouched for now.

[citibeth]

Wow... that's quite a tour de force. But now I'm getting cold feet because it's 112 new files. I was hoping for something simpler. (I did a CMake build for Blitz in a proprietary setting for Windows, using far fewer files. Unfortunately, I cannot access or share that build. Then again, the build I made was a lot more minimal, just enough to get it working on Windows without re-generating code that someone else had already generated).

Overall, the mantra should be simplify, simplify. SOme things I can see right now I would want to remove:

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.

[papadop]

I just had a fast reading of your comments. I totally agree that it's more complicated than necessary nowadays. Please remember, that this was done years ago, at that time I wanted to provide the same functionality as with autotools. I just spent half an hour to separate this from other changes...

As for the goals of:

• requiring C++-11
• removing most of the testing macros (accidentally HAVE_STRING and HAVE_STRINGS might be the ones that we need to keep -- it is C no C++ and related to operating systems IIRC).
• cleaning up the commented stuff in CMakeLists,txt (this was mostly reminders for eventual TODO work).

I totally agree with that (I would even go further and use #pragma once everywhere instead of include guards...). Then I'm not sure about the best strategy as doing all the work before merging might delay it for a while. I'll try to do some of the simple simplifications quickly, but my time is counted, and I'm unsure I can do all of your requests in the limited time I have. Can we find some middle ground ?

• I'll make some initial cleanup and some of the suggested changes (the simplest ones).
• We merge, so that the work remains synchronized with other changes and we can gather comments from users. Eventually, the testers can be extended to include a cmake build.
• We do the remaining changes incrementally (and actually it would be nice to make them in both autotools and cmake world simultaneously).

I'll try to provide the initial cleanup over the week-end...

[citibeth]

I totally agree with that (I would even go further and use #pragma once everywhere instead of include guards...)

AFAIK, #pragma once is not standard. I would prefer we just stick with the C++11 standard, avoid using extensions.

. Then I'm not sure about the best strategy as doing all the work before merging might delay it for a while.

I'm not sure either. I can see arguments on both sides: a) Merge now because it works, and it doesn't conflict with / change any existing files b) Merge later because what we have now isn't what we eventually want.

Now that I put it that way, I'd lean toward merging now. Even if the CMake build is totally broken (which I assume it is not), the merge STILL won't affect anything else. I think I'll be comfortable merging once the integration tests build and run successfully with CMake. ( @slayoo can you do this or tell us how to?)

I'll try do some of the simple simplifications quickly, but my time is counted, and I'm unsure I can do all of your requests in the limited time I have. Can we find some middle ground ?

• I'll make some initial cleanup and some of the suggested changes (the simplest ones).
• We merge, so that the work remains synchronized with other changes and we can gather comments from users. Eventually, the testers can be extended to include a cmake build.
• We do the remaining changes incrementally (and actually it would be nice to make them in both autotools and cmake world simultaneously).

I'll try to provide the initial cleanup over the week-end...

OK let's just stick with simple clean-up. Get rid of commented-out autotools code and the (one) vestigal file, make the doc/ directory optional or disable it, and (if there's time) move single-use macros into the file where they are used. Does that work?

[papadop]

OK. I think I did the required changes... By the way, I noticed various things.

• The manual directory is mostly an old version of the doc directory. There are few differences in codes files though. I think this is confusing. The manual directory should be removed. At the very least, the all the Makefile.am(s) in this hierarchy should be removed.
• There are bunch of .cvsignore remaining that should be suppressed.... That's an easy cleanup to do.

[youngmit]

@papadop Thanks for your work on this; I'm really excited for how much simpler Blitz++ can get once the CMake configuration is ready to replace autotools. I assume that is the plan?

1. Get a functional CMake configuration that can do everything that the autotools config can do (that we decide we wish to preserve, anyways)
2. Deprecate/remove the autotools build system
3. Remove all of the preprocessor macros that guard features that are standard as of C++11
4. Simplify the CMake config to not have to set all of those macros in the first place. e.g. remove the lion's share of the cxx_tests stuff.

edit: Sorry, used to rst

[slayoo]

will try this week to add Travis and Appveyor entries to this PR so the cmake build is tested during CI on Linux, OSX and Windows

[citibeth]

Awesome!

On Sun, Mar 3, 2019 at 3:48 PM Sylwester Arabas notifications@github.com wrote:

will try this week to add Travis and Appveyor entries to this PR so the cmake build is tested during CI on Linux, OSX and Windows

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/pull/97#issuecomment-469062595, or mute the thread https://github.com/notifications/unsubscribe-auth/AB1cd4IbDWPwlLo4Q6T_xlEtWhcCoHWsks5vTDUzgaJpZM4bFK09 .

[emmenlau]

Hi, just a "thumbs up" for cmake support! Is this currently still being considered? For what its worth, I agree with @citibeth that merging sooner is better than merging later, and iterative improvements can be made when more people use/test the cmake support.

[slayoo]

apologies for the delay and thanks for the reminder. Here's a first try to add CMake build to travis: https://github.com/papadop/blitz-1/pull/2

[slayoo]

Appveyor builds are green, but these do not include test runs yet

[papadop]

Yes I know... But given that @citibeth was only asking for travis, that it now works (even if perfectible), tests more things than the autotools build and is actually faster, I think it would be nice to merge now and to do the appveyor stuff in another branch. I a little afraid of the time needed to make the appveyor build succesful (windows + have to recover from travis burns for now).

[slayoo]

I fully support merging now. Would "squash and merge" be a good choice (instead of bringing all the travis-struggle commits into the history)?

[slayoo]

Plus, there is a question of when to remove the autotools files from the repo?

[papadop]

Well, removing the autotools is a no-no for now. At least, appveyor build needs to be up and running. Long term, if maintaining two build systems in sync is too difficult, why not. But let's get people gain experience with the cmake build, and take time to polish the build (for example on all the other architectures not tested on travis/appveyor... which is mostly unexplored land for now).

[slayoo]

(appveyor did not use autotools previously - there are Visual Studio project files in the repo which were used, but should also be not needed when we are happy with CMake)

[papadop]

As for squash, I have no experience with it. I would have say "why" if there was many pure cmake related commits, but given that most commits are only for pure travis debugging, why not...

[papadop]

OK. That leaves the argument of taking time to get some feedback for the cmake build. Not that I want to insist on keeping the autotools build, just to make sure that people can fallback on the most used build in case something goes wrong. Plus I want to take time to verify that no autotools change creeped in after my cmake work (which was a while ago) and is not taken into account with the cmake build.

So let's keep the autotools build for now, and have the following strategy (if everyone agrees with it): 1) Advertise better the cmake build (to get more feedback). 2) After a while announce that the cmake way is the preferred one (autotools will be deprecated). 3) Remove the autotools (and also why not VS) files.

But, we need to agree on the plan and on a calendar !!!

[slayoo]

Also, for the record, I've added an item mentioning your CMake contribution here: https://github.com/blitzpp/blitz/wiki/Credits

[papadop]

Again, can we have an issue asking for such enhancements... As said above, the problem is much more complicated than only testing for pdflatex (in theory any other latex implementation could work too)... Note that the autotool build does the same thing, and almost silently cancels some components when tools are missing.... or otherwise I would have not being obliged to add that many packages to the travis build. Look for the missing keyword at configuration time in the builds...

The cmake build already does much more testing than the autotools one. This is a net improvement. Let's already merge that and and create issues (not PR, sorry) for tracking required further enhancements.

[citibeth]

@papadop Looks like the easiest way to get docs built will be to figure out the correct dependencies (including versions); and then make a Spack package for it, that will build all the correct versions of Doxygen, etc. At that point I'm happy with a single documentation CMake option that either builds everything or nothing. As long as the Spack build knows how to build all the required dependencies, then we at least have a way to generate docs that doesn't require humans to go to a lot of installation effort just to build docs for one package.

[citibeth]

@slayoo If you can check this over and ensure it works with Travis, I'm happy to merge.

Before removing Autotools and Visual Studio, I want to make sure this works on Windows. CMake can generate VS builds. But the CMake files need to be tweaked differently to enable Windows functionality; at least in my experience.

[slayoo]

Re Doxygen, I'm a bit puzzled - IIUC this has nothing to do with the "good old blitz docs" that can be generated into the pdf.

It would be great to have the travis script upload somewhere the generated docs, and it's worth the effort, but of course there is no reason to wait for it in context of this PR - just created a separate issue: https://github.com/blitzpp/blitz/issues/122

[slayoo]

Re Windows: currently (i.e. with the branch behind this PR), CMake generates VS project files on Windows and compilation under VS is executed, no test execution happens, I've added an issue for it: #126

[slayoo]

@citibeth any opinion on merge vs. squash-and-merge?

@lutorm, @pguio any objections against merging it as is and continue development of further enhancements in separate branches?

I'm trying to go through all the comments above and create separate issues for the discussed features, I'll also try out the CMake build on my machine this evening and report back

[citibeth]

On Mon, May 6, 2019 at 11:41 AM Sylwester Arabas notifications@github.com wrote:

@citibeth https://github.com/citibeth any opinion on merge vs. squash-and-merge?

Squash-and-merge is normally preferred.

[slayoo]

@trophime, @tillea - if I'm not mistaken, you're listed as Blitz++ packagers in Debian. This PR includes initial version of CMake build scripts for Blitz++ intended as a replacement for autotools scripts. All thanks to @papadop!

We're in need of testing it and getting feedback from packagers - if possible, please test and report feedback. thanks!

[slayoo]

I've noticed that repeating "make check-testsuite" repeats compilation, while one would expect only the test execution to repeat (as there was no code change)? Is it intentional?

[emmenlau]

I've noticed that repeating "make check-testsuite" repeats compilation, while one would expect only the test execution to repeat (as there was no code change)? Is it intentional?

Does this hold true after a second, third and fourth time? I've seen that on complex projects, cmake can take one or two iterations until the cache does not change anymore.

[slayoo]

I've noticed that repeating "make check-testsuite" repeats compilation, while one would expect only the test execution to repeat (as there was no code change)? Is it intentional?

Does this hold true after a second, third and fourth time? I've seen that on complex projects, cmake can take one or two iterations until the cache does not change anymore.

yes, reproduced a few times with various intervals (but just on one machine)

[slayoo]

not sure if that matters, but in the generated install_manifest.txt file, the bzconfig.h entry is listed twice, which results in an error message when uninstalling:

$ sudo xargs rm < install_manifest.txt
rm: cannot remove '/usr/include/blitz/bzconfig.h': No such file or directory

[citibeth]

Sounds to me like a big.

On Tue, May 7, 2019 at 17:08 Sylwester Arabas notifications@github.com wrote:

not sure if that matters, but in the generated install_manifest.txt file, the bzconfig.h entry is listed twice, which results in an error message when uninstalling:

$ sudo xargs rm < install_manifest.txt rm: cannot remove '/usr/include/blitz/bzconfig.h': No such file or directory

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/blitzpp/blitz/pull/97#issuecomment-490256053, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOVY57TDEV3IDWK4EWTYNTPUHVVNANCNFSM4GYUVU6Q .

[emmenlau]

Thanks to @papadop and all!