Clean build system and installation #213

Merged
merged 105 commits into from Apr 4, 2016

Conversation

Projects
None yet
8 participants
@tammoippen
Contributor

tammoippen commented Jan 29, 2016

In trac we have a milestone Clean build system and installation:

The build system of NEST has grown over the years and is currently containing custom checks for many systems not in existence anymore. This milestone is about fixing and cleaning the build system.

In order to solve the contained tickets, I rewrote the build system from scratch
using cmake for different reasons:

  • Very good cross platform support, where autotools is very much rooted in GNU Li/Unix based operating systems.
  • It has very good support for cross compilation using toolchain files for the different targets/compiler combinations.
  • As a "meta"-build system you can generate various "target"-build systems: Unix Makefiles, Ninja and project files for many different IDE's. Every developer can work with his preferred tool.

Every build configuration, that was possible before, should be possible with the cmake version, too. I tried to keep most of the work-flows intact. For NEST installation details, please see INSTALL.

The trac tickets that should be solved by this PR:

  • trac.523 "make dist target includes compilation byproducts with libltdl" as libltdl will not be generated during configuring / building and the system libltdl will be used.
  • trac.768 "Use pkgconfig for library detection wherever possible" as cmake has a find_package infrastructure which is "simple" to extend. Internally, they often use pkgconfig.
  • trac.775 "Change OpenMP configuration option --with-openmp to take the flag as argument" is enabled.
  • trac.791 "Remove --enable-bluegene ./configure flag" as the corresponding toolchain sets all necessary options.
  • trac.865 "check cython version in configure" as we check for Cythen version now.
  • trac.923 "Do have gcc/g++ default guess when setting compiler options" as cmake has support for many compilers included. I tried to use as few as possible default flags in the CMakeLists.txt files

Partly or not solved trac issues from this milestone:

  • trac.654 "Move all examples from all over the source tree to the examples directory" partially solved, as all examples are installed, but in the same directory structure as before.
  • trac.781 "NEST build system doesn't respect --docdir" NOT solved, but can be adjusted by modifying INSTALL_DOC_DIR to be a user option.
  • trac.904 "Review examples" did NOT happen (although all examples are installed now).

The GitHub Issues affected by this:

  • #152 Documentation will be skipped when cross compiling.
  • #56 Both, dynamic and static building of user modules is enabled with cmake. It might also be possible to build a user module as a cmake add_subdirectory, but this requires additional work.

I propose as reviewers @jougs, @apeyser, @heplesser, but again I also welcome @nest/nest-developers to review this PR (especially those with cmake experiences).

tammoippen added some commits Nov 17, 2015

Unify implementation:
* all managers inherit publicly
* use of `std::vectors...` etc.
Make a BlueGeneQ file and
* update library usage (readline, ltdl)
* adjust used compile options
Add CPack definitions...
creates targets:
* make package # creates a binary package
* make package_source # creates a packages from the source

also adding custom target `make dist` which calls `make package_source` for script compatibility
also let doc be generated in Source as `doc/Makefile.am` suggests

create new rcsinfo.sli before...
Clean up for BlueGene
* extend and simplify FindGSL
* rm old BluGene* Platforms, as they are included in cmake 2.8 already
* add BlueGeneQ_GCC and rename BlueGeneQ- *_XLC
* Adjust creation of objects on BG/Q XLC
Install doc and more examples, setup.py's and other stuff
* use variables for bin
* corretly initiate nest_vars.sh
Refactor toolchain files for BGQ
* Some nicer var setting for bluegeneq
* Add  -DNDEBUG to RELWITHDEBUGINFO on BGQ
Cleanup & Music & Make Python work again
* have SET in lower case
* group targets in base CMakeLists.txt
* have WITH_DEBUG option and default values for WITH_OPTIMIZE
* have the None build type as default
* include and link libneurosim from base CMakeLists.txt
* Find Music and build with Music
* rm selfinclude from config.h and add copyright head
* do not link against Python-lib, it breaks with OS X at least

All tests run through.
Cleanup & MyModule &
* use FATAL_ERRORs
* Make MyModule compile again and have CMakeLists.txt for it
* surround filenames with "" to prevent spaces to make problems (stile filenames with spaces will be tricky)
* calculate the HELPDIRS in the cmakelists
* improve nest-config: --configure-bluegene actually means that we build static libs
* Add naming instructions for CMakeLists.txt in MyModule
* Allow static/dynamic linking of external user modules. Plugin loading was possible before already.
* Correctly initiate a module when dynamically linked.
* Add second RPath for modules.
@apeyser

This comment has been minimized.

Show comment
Hide comment
@apeyser

apeyser Mar 11, 2016

Contributor

So, I can build on JQ with under the latest pull:

cmake -DCMAKE_TOOLCHAIN_FILE=../cmake/Platform/BlueGeneQ_XLC.cmake       -DCMAKE_INSTALL_PREFIX:PATH=../install       -Dwith-python=/bgsys/local/python3/3.4.2/bin/python3 -Dwith-readline=OFF -Dstatic-libraries=ON -DPYTHON_LIBRARY=/bgsys/local/python3/3.4.2/lib/libpython3.4m.a -DPYTHON_INCLUDE_DIR=/bgsys/local/python3/3.4.2/include/python3.4m ..

but on import load, I get an empty module. I think we do have to build with dynamic libraries to get cmake to properly build the nest.so for python to import. It's not that nest must be dynamically built, it's that the pynest part must be dynamically built, and cmake like autotools doesn't know how to distinguish them. I still need to investigate what cmake is putting out, which allows the import to happen "successfully" without actually loading any symbols.

Contributor

apeyser commented Mar 11, 2016

So, I can build on JQ with under the latest pull:

cmake -DCMAKE_TOOLCHAIN_FILE=../cmake/Platform/BlueGeneQ_XLC.cmake       -DCMAKE_INSTALL_PREFIX:PATH=../install       -Dwith-python=/bgsys/local/python3/3.4.2/bin/python3 -Dwith-readline=OFF -Dstatic-libraries=ON -DPYTHON_LIBRARY=/bgsys/local/python3/3.4.2/lib/libpython3.4m.a -DPYTHON_INCLUDE_DIR=/bgsys/local/python3/3.4.2/include/python3.4m ..

but on import load, I get an empty module. I think we do have to build with dynamic libraries to get cmake to properly build the nest.so for python to import. It's not that nest must be dynamically built, it's that the pynest part must be dynamically built, and cmake like autotools doesn't know how to distinguish them. I still need to investigate what cmake is putting out, which allows the import to happen "successfully" without actually loading any symbols.

CMakeLists.txt
+option( with-ps-arrays "Use PS array construction semantics. [default=ON]" ON )
+
+# add user modules
+option( external-modules "External modules to be linked in. Divided by ';'. [default=OFF]" OFF )

This comment has been minimized.

@heplesser

heplesser Mar 14, 2016

Contributor

Suggestion: "External NEST modules to be linked in, separated by ';'."

@heplesser

heplesser Mar 14, 2016

Contributor

Suggestion: "External NEST modules to be linked in, separated by ';'."

CMakeLists.txt
+ endif ()
+ foreach ( lib ${with-libraries} )
+ if ( EXISTS "${lib}" )
+ link_libraries( "${lib}" )

This comment has been minimized.

@heplesser

heplesser Mar 14, 2016

Contributor

Eclipse tells me that link_libraries is deprecated and that one should use target_link_libraries instead, not sure how important this is.

@heplesser

heplesser Mar 14, 2016

Contributor

Eclipse tells me that link_libraries is deprecated and that one should use target_link_libraries instead, not sure how important this is.

CMakeLists.txt
+# Find GSL
+set( HAVE_GSL OFF )
+set( HAVE_GSL_1_2 OFF )
+set( HAVE_GSL_1_11 OFF )

This comment has been minimized.

@heplesser

heplesser Mar 14, 2016

Contributor

I wonder if we should still differentiate between these GSL versions. V 1.11 was released in 2008. HAVE_GSL should only be true if the version is at least 1.11.

@heplesser

heplesser Mar 14, 2016

Contributor

I wonder if we should still differentiate between these GSL versions. V 1.11 was released in 2008. HAVE_GSL should only be true if the version is at least 1.11.

This comment has been minimized.

@jougs

jougs Mar 17, 2016

Contributor

I agree with @heplesser and think we should use the opportunity to get rid of this distinction. This also brings some simplifications of #ifdefs in the code.

@jougs

jougs Mar 17, 2016

Contributor

I agree with @heplesser and think we should use the opportunity to get rid of this distinction. This also brings some simplifications of #ifdefs in the code.

@tammoippen

This comment has been minimized.

Show comment
Hide comment
@tammoippen

tammoippen Mar 18, 2016

Contributor

I improved compilation on BG/Q with the last commits. Please see INSTALL for more detailed instructions. I can confirm building a working dynamic NEST and PyNEST with this on JUQUEEN.

@heplesser I saw the deprecation flag, but ignored it as it keeps the code cleaner. This email clarifies the deprecation flag. Maybe I should go through the code and see, where which dependancy is actually used.

Contributor

tammoippen commented Mar 18, 2016

I improved compilation on BG/Q with the last commits. Please see INSTALL for more detailed instructions. I can confirm building a working dynamic NEST and PyNEST with this on JUQUEEN.

@heplesser I saw the deprecation flag, but ignored it as it keeps the code cleaner. This email clarifies the deprecation flag. Maybe I should go through the code and see, where which dependancy is actually used.

@tammoippen

This comment has been minimized.

Show comment
Hide comment
@tammoippen

tammoippen Mar 18, 2016

Contributor

@heplesser All link_libraries are replaced by corresponding target_link_libraries calls.

Further, I reorganized and formatted the CMake files according to the last meeting.

Contributor

tammoippen commented Mar 18, 2016

@heplesser All link_libraries are replaced by corresponding target_link_libraries calls.

Further, I reorganized and formatted the CMake files according to the last meeting.

tammoippen added some commits Mar 21, 2016

External review comments
* cleanup CPack ignore
* cleanup unnecessary comments
* improve status messages and comments
* break longe function calls over more lines (~80 columns)
* improve CMake documentation for example/MyModule
@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Mar 25, 2016

Contributor

@tammoippen I tried to build MyModule and ran into some problems.

On first try, I did not specify which compiler to use when running cmake for mymodule, and got the following message

CMake Error at CMakeLists.txt:138 (message):
  NEST compiler and CMAKE compiler differ.  Please specify with
  `-DCMAKE_CXX_COMPILER=/usr/local/bin/g++-5` .

Couldn't CMake pick up the compiler information from nest-config? Might need additions to nest-config.

With the compiler flag in place, compilation went well, but CMake chose /usr/local/llib/nest as install target, not the NEST installation location, even though $NEST_INSTALL_DIR/bin/nest-config --prefix returns the correct prefix. I needed to specify the prefix explicitly with -DCMAKE_INSTALL_PREFIX:PATH=...

With that in place, everything works (OSX 10.10, g++-5 from Homebrew).

Contributor

heplesser commented Mar 25, 2016

@tammoippen I tried to build MyModule and ran into some problems.

On first try, I did not specify which compiler to use when running cmake for mymodule, and got the following message

CMake Error at CMakeLists.txt:138 (message):
  NEST compiler and CMAKE compiler differ.  Please specify with
  `-DCMAKE_CXX_COMPILER=/usr/local/bin/g++-5` .

Couldn't CMake pick up the compiler information from nest-config? Might need additions to nest-config.

With the compiler flag in place, compilation went well, but CMake chose /usr/local/llib/nest as install target, not the NEST installation location, even though $NEST_INSTALL_DIR/bin/nest-config --prefix returns the correct prefix. I needed to specify the prefix explicitly with -DCMAKE_INSTALL_PREFIX:PATH=...

With that in place, everything works (OSX 10.10, g++-5 from Homebrew).

@w-klijn

This comment has been minimized.

Show comment
Hide comment
@w-klijn

w-klijn Mar 29, 2016

So I did my review and the comments have been processed. I guess I should now give a 👍

w-klijn commented Mar 29, 2016

So I did my review and the comments have been processed. I guess I should now give a 👍

@tammoippen

This comment has been minimized.

Show comment
Hide comment
@tammoippen

tammoippen Mar 29, 2016

Contributor

@heplesser Commit 3ca304d should solve the compiler selection: CMAKE_CXX_COMPILER must be set before the call to project(...). I rearranged the call to ${NEST_CONFIG} --compiler before the project(...) and this solves it on my machine.

With the compiler flag in place, compilation went well, but CMake chose /usr/local/llib/nest as install target, not the NEST installation location, even though $NEST_INSTALL_DIR/bin/nest-config --prefix returns the correct prefix. I needed to specify the prefix explicitly with -DCMAKE_INSTALL_PREFIX:PATH=...

I cannot reproduce this. Sometimes the build directory is confused from previous failed runs. Try to

  1. run make rebuild_cache or
  2. remove the content of the build directory and run the cmake command again.
Contributor

tammoippen commented Mar 29, 2016

@heplesser Commit 3ca304d should solve the compiler selection: CMAKE_CXX_COMPILER must be set before the call to project(...). I rearranged the call to ${NEST_CONFIG} --compiler before the project(...) and this solves it on my machine.

With the compiler flag in place, compilation went well, but CMake chose /usr/local/llib/nest as install target, not the NEST installation location, even though $NEST_INSTALL_DIR/bin/nest-config --prefix returns the correct prefix. I needed to specify the prefix explicitly with -DCMAKE_INSTALL_PREFIX:PATH=...

I cannot reproduce this. Sometimes the build directory is confused from previous failed runs. Try to

  1. run make rebuild_cache or
  2. remove the content of the build directory and run the cmake command again.
@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Mar 29, 2016

Contributor

@tammoippen The compiler is now detected properly. I have also understood the problem with the prefix. The relevant code is

# If no `CMAKE_INSTALL_PREFIX` is given, set the NEST_PREFIX.
if ( "${CMAKE_INSTALL_PREFIX}" STREQUAL "/usr/local" )
  # no prefix given?
  set( CMAKE_INSTALL_PREFIX "${NEST_PREFIX}" CACHE STRING "Install path prefix, prepended onto install directories." FORCE )
endif ()

Note that unless CMAKE_INSTALL_PREFIX is equal to /usr/local, the prefix will not be set to the value obtained from nest_config. This is the case on my system, and that is why I ran into problems.

I would suggest that we just drop the check and unconditionally use the value in NEST_PREFIX, obtained from nest_config: The module shall be installed together with the NEST for which it is compiled.

Contributor

heplesser commented Mar 29, 2016

@tammoippen The compiler is now detected properly. I have also understood the problem with the prefix. The relevant code is

# If no `CMAKE_INSTALL_PREFIX` is given, set the NEST_PREFIX.
if ( "${CMAKE_INSTALL_PREFIX}" STREQUAL "/usr/local" )
  # no prefix given?
  set( CMAKE_INSTALL_PREFIX "${NEST_PREFIX}" CACHE STRING "Install path prefix, prepended onto install directories." FORCE )
endif ()

Note that unless CMAKE_INSTALL_PREFIX is equal to /usr/local, the prefix will not be set to the value obtained from nest_config. This is the case on my system, and that is why I ran into problems.

I would suggest that we just drop the check and unconditionally use the value in NEST_PREFIX, obtained from nest_config: The module shall be installed together with the NEST for which it is compiled.

@tammoippen

This comment has been minimized.

Show comment
Hide comment
@tammoippen

tammoippen Mar 30, 2016

Contributor

@heplesser Commit 87891f2 sets the install path unconditionally. In the developer space for writing modules, we should then also rewrite point 4. of Building MyModule in #234.

Contributor

tammoippen commented Mar 30, 2016

@heplesser Commit 87891f2 sets the install path unconditionally. In the developer space for writing modules, we should then also rewrite point 4. of Building MyModule in #234.

@heplesser

This comment has been minimized.

Show comment
Hide comment
@heplesser

heplesser Mar 30, 2016

Contributor

@tammoippen Excellent, now it works, so 👍!

Contributor

heplesser commented Mar 30, 2016

@tammoippen Excellent, now it works, so 👍!

@jougs jougs merged commit 2f2e79a into nest:master Apr 4, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jougs

This comment has been minimized.

Show comment
Hide comment
@jougs

jougs Apr 4, 2016

Contributor

I've just closed the related trac tickets 523, 768, 775, 791, 865, 923.

@tammoippen, can you please check the status of trac tickets 654, 781 and 904 and close them after creating follow-up tickets here with the remaining issues? Please also have a look at #152 and #56. Many thanks

Contributor

jougs commented Apr 4, 2016

I've just closed the related trac tickets 523, 768, 775, 791, 865, 923.

@tammoippen, can you please check the status of trac tickets 654, 781 and 904 and close them after creating follow-up tickets here with the remaining issues? Please also have a look at #152 and #56. Many thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment