CMake build infra-stucture #88

bolding · 2020-04-28T11:22:25Z

As part of a project where the CVMix turbulence schemes will be available through GOTM adding support for CMake builds in CVMix is - neat to have.

I've added two CMakeList.txt files and updated README.md.

I've tested on Linux using gfortran 7 and 8 and ifort - as well as on Windows using VisualStudio and ifort.

I suggest to make a --squash merge if possible.

mnlevy1981 · 2020-05-15T17:59:50Z

Sorry for the delay, I'll find some time to look at this early next week. In the meantime, it looks like there's one issue raised by TravisCI:

Check Fortran files for coding standard violations:

* Check for hard tabs: 0 error(s) found

* Check for trailing white space: 1 error(s) found

  ../src/dummy.F90:2: ! This is a known issue when building static/dynamic libraries based on█

* Check length of lines: 0 error(s) found

* Check for case sensitive statements: 0 error(s) found

Fortran errors found: 1

Could you please update this line so it does not end with whitespace?

mnlevy1981 · 2020-05-15T18:02:18Z

Also, it would be nice to have an example of the CMake build in reg_tests/ (and test it with Travis). I'm happy to work with you via google meets or zoom to set that up -- I'm not very familiar with CMake, but I could definitely help you navigate the test system

bolding · 2020-05-16T12:00:20Z

done we have recently started to use editorconfig - very convinient Den fre. 15. maj 2020 kl. 20.00 skrev Michael Levy <notifications@github.com

…

: Sorry for the delay, I'll find some time to look at this early next week. In the meantime, it looks like there's one issue raised by TravisCI: Check Fortran files for coding standard violations: * Check for hard tabs: 0 error(s) found * Check for trailing white space: 1 error(s) found ../src/dummy.F90:2: ! This is a known issue when building static/dynamic libraries based on█ * Check length of lines: 0 error(s) found * Check for case sensitive statements: 0 error(s) found Fortran errors found: 1 Could you please update this line so it does not end with whitespace? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#88 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVA42PKTQ2YB3SRB6AJVL3RRV7KPANCNFSM4MSXWAZA> .

-- Karsten Bolding karsten@bolding-bruggeman.com +4564422058

bolding · 2020-05-16T12:09:15Z

Sounds good to have an online meeting - just write when it fits you Doing it the CMake way would be to use the embedded CTest framework - but that would likely require some changes as now the testing is based on shell scripts and NCl for plotting. Would not really work on most computers. One thing you'll have to decide is the include directory for the Fortran module files when an installation is done. Line 74 in the master CMakelLists.txt. I've added '/cvmix' so default on Linux will be /usr/local/include/cvmix - feel free to change. Den fre. 15. maj 2020 kl. 20.02 skrev Michael Levy <notifications@github.com

…

: Also, it would be nice to have an example of the CMake build in reg_tests/ (and test it with Travis). I'm happy to work with you via google meets or zoom to set that up -- I'm not very familiar with CMake, but I could definitely help you navigate the test system — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#88 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACVA42L3B6MUSPJEIMJ6ARTRRV7TRANCNFSM4MSXWAZA> .

-- Karsten Bolding karsten@bolding-bruggeman.com +4564422058

mnlevy1981 · 2020-05-19T20:51:56Z

Thanks for getting the travis tests passing again!

Doing it the CMake way would be to use the embedded CTest framework - but
that would likely require some changes as now the testing is based on shell
scripts and NCl for plotting. Would not really work on most computers.

I wasn't envisioning a wholesale switch to CTest, but rather updating reg_tests/common/parse_inputs.sh to allow -cm and --cmake to set CMAKE_BUILD=TRUE and then update reg_tests/common/build.sh to call cmake instead of make -f $CVMix/src/Makefile CVMIX_ROOT=$CVMix ${USE_NETCDF} if CMAKE_BUILD=TRUE.

You could actually test this locally:

$ cd reg_tests/Bryan-Lewis/
$ ./Bryan-Lewis-test.sh --cmake # should build with CMake and generate *.out as output
./Bryan-Lewis-test.sh --cmake --netcdf # should build with CMake and generate *.nc as output (it looks like you have some support for building with netCDF, but maybe the user needs to provide some additional info?)

The last piece of the puzzle would be updating .travis.yml -- it looks like cmake is already available, so we just need to figure out the best way to implement the test. I think adding

  script:
    - cd bld; ./cvmix_setup gfortran $(dirname $(dirname $(which nc-config)))
    - cd ../CVMix_tools; ./run_test_suite.sh --already-ran-setup
+   - cd ../src; make allclean
+   - cd ../reg_tests/Bryan-Lewis/; ./Bryan-Lewis-test.sh --cmake

would be sufficient for this branch, and I'll probably update run_test_suite.sh in a future PR to rerun all the tests with CMake.

Sounds good to have an online meeting - just write when it fits you
...
Den fre. 15. maj 2020 kl. 20.02 skrev Michael Levy <notifications@github.com

It looks like you're eight hours ahead of me (that email was generated at 12:02 local time), but I'm happy to chat in person. My mornings are spoken for this week, and this coming Monday is a US holiday, but we could aim for 8:30a/4:30p or 9:00a/5:00p any other day next week. My email is mlevy [at] ucar.edu.

I'll check out your branch and play around with things I mentioned above -- if I get anywhere, I'll submit a PR back to you that should update this PR if you merge it in.

mnlevy1981

As mentioned in one of my comments, I needed to use the CMakeLists.txt file in the root directory rather than src (macos 10.14; gfortran 9.3 and cmake 3.17.2 both via homebrew)

$ cmake ${CVMIX}/src -DCVMIX_BUILD_DRIVER=on
CMake Warning (dev) in CMakeLists.txt:
  No project() command is present.  The top-level CMakeLists.txt file must
  contain a literal, direct call to the project() command.  Add a line of
  code such as

    project(ProjectName)

  near the top of the file, but after cmake_minimum_required().

  CMake is pretending there is a "project(Project)" command on the first
  line.
This warning is for project developers.  Use -Wno-dev to suppress it.

...

-- Configuring done
CMake Error: Cannot determine link language for target "cvmix_drivers".
CMake Error: CMake can not determine linker language for target: cvmix_drivers
CMake Error: Cannot determine link language for target "cvmix_io".
CMake Error: CMake can not determine linker language for target: cvmix_io
-- Generating done
CMake Generate step failed.  Build files cannot be regenerated correctly.

also, is there a way to have gfortran be the default compiler? Or, better yet, take advantage of the existing CVMix build system to query bld/.CVMix_env? I have the pgi compiler installed on my system, but was not expecting it to be used by cmake:

$ cmake ${CVMIX} -DCVMIX_BUILD_DRIVER=on
-- The Fortran compiler identification is PGI 19.10.0
-- Check for working Fortran compiler: /opt/pgi/osx86-64/19.10/bin/pgf95
-- Check for working Fortran compiler: /opt/pgi/osx86-64/19.10/bin/pgf95 - works
-- Detecting Fortran compiler ABI info
-- Detecting Fortran compiler ABI info - done
-- Checking whether /opt/pgi/osx86-64/19.10/bin/pgf95 supports Fortran 90
-- Checking whether /opt/pgi/osx86-64/19.10/bin/pgf95 supports Fortran 90 - yes
-- Setting default build type to 'Release'.  Set CMAKE_BUILD_TYPE variable to change build types.
-- Configuring done
-- Generating done
-- Build files have been written to: ${CVMIX}

mnlevy1981 · 2020-05-19T22:47:47Z

README.md

+In this recipe CVMIX\_BASE s a convinience environmental variable pointing to 
+the CVMix source directory. It is possible to execute the following commands 


I think of "the CVMix source directory" as ${CVMIX_ROOT}/src, but when I tested this out, I needed to point to ${CVMIX_ROOT} instead.

For me - the CVMix source directory is what is cloned from GitHub. I'm fine with calling it CVMIX_ROOT instead. The main thing is that the CMakeLists.txt file defining the project is used.

CMake checks the FC environment variable. Otherwise the one in the PATH is used - if any.

I don't think it is a good idea to mix the build systems and let CMake use bld/.CVMix_env. This will for sure not work on Windows :-)

For me - the CVMix source directory is what is cloned from GitHub. I'm fine with calling it CVMIX_ROOT instead.

Cool, I'll make this change in bolding#1

I don't think it is a good idea to mix the build systems and let CMake use bld/.CVMix_env.

fair enough; I was caught off-guard by this, but that's just due to my lack of experience with CMake

mnlevy1981 · 2020-05-19T22:50:41Z

README.md

+providing the full path.
+
+
+1. mkdir ~/build/cvmix


We already build CVMix outside of the source directory, can we recommend people use the existing bld/ directory? Maybe we can create an empty bld/cmake_build/directory and direct folks to build there? I.e. create bld/cmake_build/.gitignore that contains

* !.gitignore

The choice of build directory is up to the user - suggesting/recommending bld/ is fine with me.

When CVMix will be used as a submodule - as is the case with GOTM - it is the GOTM CMake configuration that sets the build folder.

suggesting/recommending bld/ is fine with me.

Cool, I'll make this change in bolding#1 too

mnlevy1981 · 2020-05-19T22:51:05Z

README.md

+make
+```
+
+and instllation by:


"installation" (missing the first "a")

Also, what does installation do if you do not specify -DCMAKE_INSTALL_PREFIX?

typo is fixed

Platform dependent
https://cmake.org/cmake/help/latest/variable/CMAKE_INSTALL_PREFIX.html

If not Windows we can introduce GNUInstallDirs if you want.

src/dummy.F90

mnlevy1981 · 2020-05-19T22:54:09Z

README.md

+VisualStudio and the Intel Fortran compiler but NetCDF support has not been
+tested.
+
+In this recipe CVMIX\_BASE s a convinience environmental variable pointing to 


"is a convenient" rather than "s a convinience"

At this point, it's pretty klunky -- no netcdf support, forces compiler to be gfortran. But it should get cmake tested in travis, which is a good start.

Travis CI provides cmake 3.9.2 (we are using an old linux distribution so the apt-get versions of gfortran and libnetcdf-dev play nicely together); hoping there aren't actually features introduced in 3.10 in the system...

Also modified build.sh and run.sh to follow directions laid out in the README (created cmake_bin/ for build target, build.sh calls "make install" and run.sh runs from cmake_bin/bin/)

bolding added 7 commits April 28, 2020 08:42

support for building the CVMix library and the CVMix driver using CMake

c3e4866

CMake build instructions added to README.md

29bf5b3

update documentation

393ab26

improved support for NetCDF

0f005b0

fixed compilation with VisualStudio

4dd8764

updated README.md

a323cef

README.md - a few typos

f5818f5

removed trailing blank

6d7bb49

mnlevy1981 requested changes May 19, 2020

View reviewed changes

mnlevy1981 added 4 commits May 19, 2020 21:33

Create directory for cmake build

1dbf897

Allow reg_tests to use cmake

1cb7ba2

At this point, it's pretty klunky -- no netcdf support, forces compiler to be gfortran. But it should get cmake tested in travis, which is a good start.

Lowering required version

b583d7d

Travis CI provides cmake 3.9.2 (we are using an old linux distribution so the apt-get versions of gfortran and libnetcdf-dev play nicely together); hoping there aren't actually features introduced in 3.10 in the system...

Clean up install instructions in README

4c61476

Also modified build.sh and run.sh to follow directions laid out in the README (created cmake_bin/ for build target, build.sh calls "make install" and run.sh runs from cmake_bin/bin/)

mnlevy1981 mentioned this pull request May 26, 2020

Cmake updates bolding/CVMix-src#1

Merged

bolding and others added 4 commits May 27, 2020 08:02

updated README

3019c4e

Merge branch 'master' into cmake_updates

30de4dc

Clean up more trailing whitespace

04cc49f

Merge branch 'mnlevy1981-cmake_updates'

d416bde

mnlevy1981 merged commit 9423197 into CVMix:master Jun 22, 2020

mnlevy1981 mentioned this pull request Jun 22, 2020

New build system? #40

Open

mnlevy1981 mentioned this pull request Jun 30, 2020

Langmuir turbulence induced entrainment ESCOMP/POP2-CESM#31

Merged

		In this recipe CVMIX\_BASE s a convinience environmental variable pointing to
		the CVMix source directory. It is possible to execute the following commands

CMake build infra-stucture #88

CMake build infra-stucture #88

Uh oh!

Conversation

bolding commented Apr 28, 2020

Uh oh!

mnlevy1981 commented May 15, 2020

Uh oh!

mnlevy1981 commented May 15, 2020

Uh oh!

bolding commented May 16, 2020 via email

Uh oh!

bolding commented May 16, 2020 via email

Uh oh!

mnlevy1981 commented May 19, 2020

Uh oh!

mnlevy1981 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants