Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1 cmake #3

Merged
merged 20 commits into from
Nov 1, 2020
Merged

1 cmake #3

merged 20 commits into from
Nov 1, 2020

Conversation

apthorpe
Copy link
Contributor

This PR provides CMake support and a (rather brittle) workaround for recent meson versions changing the build directory location for the zofu library and not properly installing Fortran .mod files.

CMake is supported in two ways, first in building, documenting, packaging, and installing Zofu itself, and second, by providing helper functions to allow CMake to detect and use and existing Zofu installation and to assist in compiling Zofu-based unit tests and adding them to CTest's test plan. More information is available under contrib/cmake/README.md and in the .cmake files themselves, especially FindZOFU.cmake and ZOFUhelper.cmake.

This has been tested with meson 0.55.3. The main issue with not properly installing Fortran .mod files is that earlier versions of meson wrote .mod files under ./build/zofu@sha. Version 0.55.3 renames this build directory according to the zofu library filename, such as

  • build/libzofu.a.p/
  • build/libzofu.so.p/
  • build/libzofu.dll.p/ and

probably build/libzofu.dylib.p/ on OSX (I haven't tested on OSX yet...)

The following has been added to meson.build.

  # Uncomment one of these:
  # mod_path = join_paths('build', 'libzofu.dll.p')
  # mod_path = join_paths('build', 'libzofu.a.p')
  # mod_path = join_paths('build', 'libzofu.so.p')
  mod_path = join_paths('build', 'zofu@sha')
  install_subdir(mod_path, ... )

Several failed attempts at dynamically setting mod_path are commented out in meson.build; they are retained for information purposes but can be deleted.

A final note: I've tested the CMake build process for Zofu on both Windows and Linux. An MPI-enabled build was attempted on Linux; it found the MPI libraries and module files but failed because the .mod files did not match the more recent compiler I was using. MPI was installed as a Linux Mint package and not built from source, so the error is understandable. I suspect it would have succeeded if I was using the vendor-supplied version of gfortran but I haven't confirmed that.

@acroucher
Copy link
Owner

Thanks Bob. For installing the .mod files, I have just been trying this:

  if meson.version() < '0.55'
    mod_dir = 'zofu@sha'
  else
    mod_dir = zofu.full_path().split('/').get(-1) + '.p'
  endif
  mod_path = join_paths('build', mod_dir)
  install_subdir(mod_path,
                 install_dir: module_install_dir,
                 strip_directory: true,
                 exclude_files: zofu_objs)

So for Meson 0.55 or later, it gets the full path of the library from .full_path(), removes the directories and adds .p on the end to get the module file directory.

I think this should work for any platform. Of course the whole thing is still far from ideal and only a workaround until such time as Meson (hopefully) has support for installing Fortran modules "properly".

I've tested it on an Ubuntu 18 VM , with various versions of Meson from 0.54.3 up to the latest 0.55.3. I have confirmed the behaviour was changed in Meson 0.55.0.

I don't have access to any non-Linux machines though. Could you try this on your Windows setup?

@apthorpe
Copy link
Contributor Author

This change seems to work on OSX and Windows; the only issue I've noticed is that if you use a different build directory than build, (say meson buildx; ninja -C buildx) modules are sourced from the wrong directory.

This is minor; I was testing with multiple different build directories simultaneously but most users will follow the instructions and use build as the build directory so will never see this.

The important part is that mod_dir is being set correctly based on the library name + '.p' for Windows and OSX.

@acroucher
Copy link
Owner

Hmm, no I'd call that a bug. The build directory name shouldn't be hard-coded in there. I'll fix that.

@acroucher
Copy link
Owner

It would be good to incorporate the build instructions for both Meson and CMake (which you have added in contrib/cmake/README.md) into the main README.md. Or at least a summary of them, without quite so much detail?

Your readme also says "Due to issues with the meson/ninja build process, the Fortran .mod files must be manually copied to the installation directory". Hopefully that is no longer the case, so we could delete that bit?

@acroucher
Copy link
Owner

acroucher commented Oct 27, 2020

I just tried a CMake build of Zofu on an Ubuntu 18 machine, and got the following error:

CMake Error at CMakeLists.txt:165 (install): install TARGETS given no RUNTIME DESTINATION for executable target "ZOFUDRIVER".

This is CMake 3.10.2. Any idea what the problem is?

What I did was mkdir build-cmake; cd build-cmake; cmake ..

@apthorpe
Copy link
Contributor Author

I have an idea what might be causing that. I've only checked with the latest CMake so let me try a test; I'm guessing this is a simple mod.

@acroucher
Copy link
Owner

Thanks, that seems to have fixed the problem. I have just added a new Github action so that the CI will also do a CMake build and run the unit tests on push.

@acroucher
Copy link
Owner

I've just added some README instructions for building, testing and installing with CMake.

It could perhaps do with some more material on how to configure the CMake build for e.g. debug/release, specifying different directories for lib install, include dir for modules etc. Do you think you could add something along those lines?

@apthorpe
Copy link
Contributor Author

I'll add some more info about configuring with CMake; I've been juggling a few other projects but I should have time this weekend.

@apthorpe
Copy link
Contributor Author

I added more configuration to CMakeLists.txt to allow users to configure where artifacts are placed under install tree and added more detailed documentation to the top-level README.md as well as to contrib/cmake/README.md. I don't believe that specifying Release or Debug changes the compilation options in CMakeLists.txt but adding that is straightforward.

@acroucher
Copy link
Owner

Thanks Bob, looking good... I think we are ready to merge this, and release as v.1.1.

@acroucher acroucher merged commit 5671456 into acroucher:master Nov 1, 2020
@acroucher
Copy link
Owner

I had to make one more change, as the CI unit tests were failing- Meson 0.56 has just been released and it looks like you can't compare meson.version() with a string any more. I found there is actually a version_compare() function which you are meant to use, so I've changed it to do that.

@apthorpe apthorpe deleted the 1-cmake branch November 2, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants