Refactor configuration macros

[TysonRayJones]

Supersedes #656

[TysonRayJones]

TODO:

• restore addition to compilation doc from previous (now defunct) PR
• add new cmake options to doc
• restore manual build script (requires sed upon config.h.in to produce config.h)
• test verbose naming still works
• test installation still works
• test multi-build still works

[TysonRayJones]

@otbrown What do you think about this PR, to replace PR #656? Here, I remove all macros from the compiler flags being passed to the QuEST source. They are replaced with cmake options passed to config.h.in which defines the necessary macros. So all QuEST source files which need a macro consult the generated config.h. This addresses all the issues (I think) about installation and/or users overriding macros post-compilation, and seems all round cleaner and more robust. One can consult the generated config.h to learn the compiled configuration, and the macros therein can never be overriden (due to guards therein). Manual compilation (without cmake) will need to manually populate config.h, as you suggested.

It's worth checking whether I broke installation. I removed the MULTI_LIB_HEADERS macro because now this config file is always generated, but wasn't entirely sure about its previous function. I tested installation still works via (within build/)

cmake .. -DCMAKE_INSTALL_PREFIX=$(pwd)
make install

I note it writes the headers to

.../build/quest/include/quest/include/...

where quest/include/quest/include might be indicative of a mistake in the build process - but that existed before this PR (i.e. on devel).

I also note instead performing

cmake .. -DCMAKE_INSTALL_PREFIX=$(pwd)
cmake --install .

produces error

CMake Error at cmake_install.cmake:46 (file):
  file INSTALL cannot find
  "/Users/tysonjones/Desktop/GithubRepos/QuEST/build/min_example": No such
  file or directory.

but I don't really understand what I'm doing since I never install ¯\_(ツ)_/¯.

Let me know what you think!

[TysonRayJones]

Btw, I note

cmake .. -DVERBOSE_LIB_NAME=ON
make install

outputs...

-- Installing: .../quest/lib/libQuEST-fp2+mt.4.1.0.dylib
...
-- Installing: .../quest/include/quest/include/config.h

which was also the case before this PR (on devel), and i.e. unrelated to my changes here.

Is config.h excluding the verbose suffix the correct/intended behaviour? That header is specific to the cmake options (upon which most other headers are dependent).

[otbrown]

Hi Tyson,

LGTM!

I can confirm you didn't break installation -- your test didn't work as setting the install prefix to the build directory is... unexpected. My install directory looks like:

install/
- quest/
  - bin/
    - min_example
  - include/
    - quest.h
    - quest/
      - include/
        - <internal_header_files>
   - lib
     - cmake/
       - QuEST/
         - <cmake_files>
     - libQuEST.so
     - libQuEST.so.4
     - libQuEST.so.4.1.0

which is as expected. The slightly upsetting quest/include/quest/include is the compromise used to accommodate #include "quest/include/header.h" in the source files! This will generally be invisible to the user in any case :)

[otbrown]

Oh! And the config file naming -- that file shouldn't be renamed as then everything else would need to #include "config-whatever.h", which would be a bit of a nightmare. I suspect separate modules (and therefore headers) will still be required for MT vs MPI QuEST in a HPC environment, which was the original use case, but the library renaming at least makes it easier to double-check what a binary is compiled against.

We probably can get it to work such that config headers are distinguished by name, but I would like an actual use-case to be presented before putting the work in as I think the solution would be ugly 😅

[otbrown]

I have PR'd this PR with the fix for #689 and #690 :)

[TysonRayJones]

Wew brilliant, thanks very much! 🎉

The slightly upsetting quest/include/quest/include is the compromise used to accommodate #include "quest/include/header.h"

Aha! Truly I am always bitten by my own previous stubbornness ehehe