Rewrite buildsystem using autotools

[fingolfin]

This also addresses issue #35 partially, and fixes a bunch of compiler warnings.

Resolves #30
Closes #31
Closes #29

[fingolfin]

Question for @DominikBernhardt and @gaehler: when one did just make with the old build system, it always went and extracted tables/qcatalog.tar.gz if no directory tables/qcatalog exists. Right now, I am not doing this in the new build system and hence have to do it "manually" for Travis and AppVeyor. I can of course easily add back this extraction process. I am just wondering: should I? Is this something one expects every user will want to (resp.: should) do this? Let me know what you prefer, then I can implement it that way.

Other than that, this PR is slowly getting close to being ready. Thanks to @DominikBernhardt many of the fixes created while working on this are already merged. Once the current round of PRs is merged (PRs #43, #44, #45, #46), I will probably submit a few more, and then finally this PR should become ready for merging.

[DominikBernhardt]

I'm not quite sure about this one and would be interested to hear what @gaehler has to say about this, especially because a main use of Carat is the bundle into the GAP package?

[gaehler]

In my GAP package, I unpack the qcatalog with --exclude=qcatalog/dim6. This seems like a good compromise, as most of the footprint on disk comes from dimension 6. The test files all run without dimension 6.

[fingolfin]

@gaehler but --exclude= is a GNU tar extension, so it may not be available everywhere. If unpacking everything is a concern, one could split that tarball into two, one containing just dim6, the other containing everything else.

Anyway, for now, I changed it to do exactly what the old build system did, namely extract the whole thing when one just does make.

[fingolfin]

There is another question: right now I place all binaries into bin/carat/. I only did it this way because the files in tst seem to expect that. But I think it would make more sense to put them directly into bin. Thoughts?

[gaehler]

There used to be a time when GAP put its binaries into architecture specific subdirectories, and one could install GAP for several architectures in parallel, without duplicating the library and data tables. I tried to match this capability also in CARAT (bin/carat is actually only a link to one of the architecture specific directories, so that test files could remain generic). Since that capability of GAP is gone, there is indeed no need to keep in CARAT.

[fingolfin]

Actually, that capability of GAP (to support multiple architectures or in general, build variants, from a single source tree) is not at all gone. It just stopped using a home cooked, brittle, and severely limited mechanism that nobody else used, and which was mostly focused on users of NFS volumes shared between machines with different OSes (something rather rare these days in my experience, although of course ymmv), and switched to mechanisms that are well-supported and well-known elsewhere, namely out-of-tree builds. (Disclaimer: I am heavily biased on this, as I designed and wrote the new build system; that said, I also have 20 years of experience writing and maintaining all kinds of build systems for cross platform software, on multiple operating systems and architectures, so I do think I am qualified to comment on this... ;-).

That said, GAP still generates bin/ARCHNAME/ dirs, and also supports these for GAP packages, mainly for backwards compatibility.

But all this is really irrelevant for the standalone tool carat we are looking at here. What GAP does really should have no impact on it, after all. By the way, the new build system in this PR already does allow for out-of-tree builds (I use that compare builds with clang and gcc, with and without debug flags and optimizations; i.e., right now four configs; each done once on a Mac, once on a Linux machine).

Of course I can still rig things so that all binaries end up in bin/${host}/, but I fail to see what this gains for people using standalone carat. Perhaps for the GAP package carat, it would be useful; but that package could also easily run the regular build system of the standalone carat; and then copy or symlink the binaries into the desired final location.

[gaehler]

I was just explaining how this extra architecture directory came about, and already agreed to get rid of it. For the wrong reason, perhaps, but still...

[gaehler]

Regarding the qcatalog, I would then prefer separate archives for dimensions 1-5 and dimension 6, and to unpack only the former by default. When building all packages in the GAP distribution, CARAT gets unpacked for very many user, only few of which actually want to use it. Dimension 6 is another 180 MB, which is not negligible.

[DominikBernhardt]

I agree with the decision to split the archives into to and only unpacking dimension 1-5 by default.
Also, I agree with @fingolfin to put binary files directly into bin.

[fingolfin]

@gaehler indeed you did -- my apologies, I got a bit carried away there :-/. Anyway, I'll make those changes tomorrow.

BTW, while I found a couple more crashing bugs, and fixed them, and got the tests squeaky clean in valgrind memcheck, they still fail on Windows: in 32 bit builds the failures are caused by segfaults that I unfortunately can't really explain. In 64 bit mode, it prints messages of the following form (note that I adjusted the message in this PR to get some more insights, namely the value of found):

this feature (found = 0) hasn't been implemented yet

[fingolfin]

From my point of view, this PR is ready to be merged. While I did not yet look into splitting up qcatalog.tar.gz, I feel that this is independent from this PR: The PR replicates the current behavior (which is to extract all files). I am happy to work on changing this in a new PR tomorrow; but merging this PR now would allow me to continue work on various other changes which all depend on this PR (simply because they need to modify the build system).

The main issue then is compatibility with the GAP package carat (henceforth caratGAP). Clearly this PR will require changes there. But I hope not to big a change:

• if caratGAP extracts carat.tgz and then partially extracts qcatalog.tar.gz before invoking the carat build system, this will prevent carat from extracting qcatalog again, as is the case now, so that's fine
• invoking the build system is a bit different: instead of

cd carat; make TOPDIR="$(TOPDIR)" CC="$(CC)" CFLAGS="$(FLAGS) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS)"
```
I think something like this *should* work (not yet tested), provided GMPDIR is added to the `config.carat` file:
```
cd carat && ./configure && make --with-gmp=$GMPDIR
```
- ... what more? I mean, I could look into making a PR for `caratGAP` to take care of all that, but I don't want to clash with any work @gaehler might be doing right now.

- it may also be possible to avoid using `CARAT_DIR`, and thus the kernel extension in `caratGAP`, by teaching all `carat` binaries to "locate themselves", as GAP does, and then search relative to the binaries: I.e. look in `path_to_binary/../tables`. Something in this vein is needed if one wants to properly support `make install` in `carat` (which I would like to). I can look into provide a PR for that to `carat`, which then would allow `caratGAP` to drop the `CARAT_DIR` stuff.

[gaehler]

I can do the necessary adjustments to caratGAP, which shouldn't be hard. The plan is to provide GMPDIR to CARAT only if GAP uses the GMP bundled with it, or if GAP was configured with some specific GMP. Otherwise, I assume there must be some GMP installed at some standard place, where CARAT's configure can find it. Also, I assume CARAT's configure (or Makefile) will allow me to add some further CFLAGS, if so desired.

Regarding your results on Windows, I am a bit puzzled. I compiled the version just before this PR on my Windows 10 notebook without the flag -DDIAG1. On 32bit, all tests pass, and on 64bit, only two commands crash, maybe even for a related reason. So, on Windows there is quite some regression, it seems.

[fingolfin]

@gaehler Hmm, that's weird. But I am not sure I quite follow what you are comparing. I can think of the following

1. current master on AppVeyor
2. current master on your laptop
3. current master on AppVeyor with DIAG1 removed
4. current master on your laptop with DIAG1 removed
5. this PR on AppVeyor (which also removes DIAG1)
6. this PR on your laptop (which also removes DIAG1)

From what you write, my understanding is that 4 passes for you in 32bit mode, and "almost" passes in 64 bit mode. Correct?

Your PR #29 ostensibly tests 3, except that it's based on an older master. Perhaps you could rebase it so we can see what happens with the AppVeyor 32bit tests then?

It would be really interesting for me to know what the outcome of 6 is. Could you perhaps test this?

Finally, I actually want to remove all of the m_alloc code. I had a look, and fail to see any value in it. There are much better tools available today, in particular valgrind, which found a ton of bugs in carat for me, none of which m_alloc was able to find.

[fingolfin]

I should add that for 1. and 5., I am currently seeing identical results, so that doesn't look like a regression to me, more like "setting DIAG1 doesn't matter". But of course that's in conflict with what we see in PR #29, which really puzzles me.

[gaehler]

Yes, I did variant 4 on my laptop. To test 3, I'd rather make a new PR (one-line change in AppVeyor script), than revive the old PR #29 (which I left just for documentation). 6 would interest me, too, but for this I need some instructions how to check this out to my machine. Regarding m_alloc, I never used it, because I didn't really trust it. Anyway, it's a debugging tool, not for production. It was written with a particular architecture in mind, and certainly would have to be reworked to make it portable. I doubt that this effort would be well invested.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@84157db). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #36   +/-   ##
=========================================
  Coverage          ?   59.62%           
=========================================
  Files             ?      269           
  Lines             ?    28666           
  Branches          ?        0           
=========================================
  Hits              ?    17091           
  Misses            ?    11575           
  Partials          ?        0

Impacted Files	Coverage Δ
functions/Datei/get_data_dir.c	100% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 84157db...b409388. Read the comment docs.

[fingolfin]

I figured out what caused the regression on Windows: I had to add back the cd tables/symbol && make to the build system; apparently symlinks in git don't work well on AppVeyor. sigh. Anyway, was easy enough to fix.

Build times on AppVeyor are higher now, though, because now we actually collect coverage information there, which we did not before due to a buggy/fragile way to add the --coverage flag to CFLAGS. That's in the past now, though.

@gaehler regarding building / testing this PR on your computer: you could clone my fork of carat https://github.com/fingolfin/carat somewhere, and then in there switch to the mh/autotools branch and test from there.

[fingolfin]

... and I managed to fix the Windows 64 bit issues, thanks to Valgrind!

[gaehler]

I can confirm that on my Windows laptop, PR #36 now passes all tests, both on Win32 and Win64.
On Linux (64bits) too, of course.

[fingolfin]

Shall we merge this now?