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

Modernize cmake; make cmake compatible with git submoduling #103

Merged
merged 6 commits into from
Sep 17, 2020
Merged

Modernize cmake; make cmake compatible with git submoduling #103

merged 6 commits into from
Sep 17, 2020

Conversation

jlblancoc
Copy link
Contributor

This PR does:

  • Add install(EXPORT ... to cmake scripts to allow better use of exported targets.
  • Add build options to allow not building parts of the projects (defaults=as it was before)

@ethzasl-jenkins
Copy link

Can one of the admins verify this patch?

@pomerlef
Copy link
Collaborator

pomerlef commented Jun 1, 2020

ok to test

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Show resolved Hide resolved
@jlblancoc
Copy link
Contributor Author

(TODO: write a test of use with find_package(libnabo))

@jlblancoc
Copy link
Contributor Author

Updates:

  • Updated README.
  • Added a new cmake-usage example of how easy it's now to integrate the library since it uses cmake "exported targets":

https://github.com/ethz-asl/libnabo/blob/7d998e64e12bda8fac497c3af75b55673d47ab40/examples/libnabo-cmake-example/CMakeLists.txt#L1-L11

@YoshuaNava
Copy link

LGTM.

@YoshuaNava
Copy link

YoshuaNava commented Sep 3, 2020

Two last comments:

  • On which platform did you test the changes?

  • @pomerlef Do you know why the Jenkins pipeline is failing? (I don't have access to the build server details)

@jlblancoc
Copy link
Contributor Author

On which platform did you test the changes?

Ubutu 20.04 and 18.04 (amd64).

@YoshuaNava
Copy link

Oki.

All good from my side, let's wait for details on the pipeline.

@YoshuaNava
Copy link

Updates:

  • Updated README.
  • Added a new cmake-usage example of how easy it's now to integrate the library since it uses cmake "exported targets":

https://github.com/ethz-asl/libnabo/blob/7d998e64e12bda8fac497c3af75b55673d47ab40/examples/libnabo-cmake-example/CMakeLists.txt#L1-L11

Thank you for taking the time to do this, it's very cool 👍

@pomerlef
Copy link
Collaborator

pomerlef commented Sep 3, 2020

It's not specific to a given build, they all have the following errors:

In file included from /home/jenkins/workspace/libnabo/compiler/clang/label/ubuntu-trusty/nabo/nabo_private.h:35:0,
                 from /home/jenkins/workspace/libnabo/compiler/clang/label/ubuntu-trusty/nabo/kdtree_cpu.cpp:32:

/home/jenkins/workspace/libnabo/compiler/clang/label/ubuntu-trusty/nabo/nabo.h:35:22: fatal error: Eigen/Core: No such file or directory
 #include "Eigen/Core"
                      ^
compilation terminated.
In file included from /home/jenkins/workspace/libnabo/compiler/clang/label/ubuntu-trusty/nabo/nabo_private.h:35:0,
                 from /home/jenkins/workspace/libnabo/compiler/clang/label/ubuntu-trusty/nabo/brute_force_cpu.cpp:32:

/home/jenkins/workspace/libnabo/compiler/clang/label/ubuntu-trusty/nabo/nabo.h:35:22: fatal error: Eigen/Core: No such file or directory
 #include "Eigen/Core"
                      ^
compilation terminated.
In file included from /home/jenkins/workspace/libnabo/compiler/clang/label/ubuntu-trusty/nabo/nabo.cpp:32:0:

/home/jenkins/workspace/libnabo/compiler/clang/label/ubuntu-trusty/nabo/nabo.h:35:22: fatal error: Eigen/Core: No such file or directory
 #include "Eigen/Core"
                      ^
compilation terminated.

@pomerlef
Copy link
Collaborator

ok to test

@pomerlef
Copy link
Collaborator

Same errors

@jlblancoc
Copy link
Contributor Author

Same errors

I'll try to reproduce it... I think it may be related to the outdated version of Eigen in trusty.

@jlblancoc
Copy link
Contributor Author

Ok, there was a missing cmake command that made the python module to fail to build.

It's fixed now, please verify and merge at your convenience.

@pomerlef
Copy link
Collaborator

Alright, I'm not sure how to handle:

default Expected — Waiting for status to be reported

Can someone from ETH how this repo was set up?

@michaelpantic
Copy link

Alright, I'm not sure how to handle:

default Expected — Waiting for status to be reported

Can someone from ETH how this repo was set up?

I had a look, there seems to be old build hooks in the config. Removed unreachable URLS. Let's see if it works again.

@michaelpantic
Copy link

@pomerlef fixed. Seems to have been a reference to a status check that does not exist anymore.

@pomerlef
Copy link
Collaborator

@michaelpantic thanks for the investigation!

@pomerlef pomerlef merged commit 99e58e4 into norlab-ulaval:master Sep 17, 2020
@peci1
Copy link
Contributor

peci1 commented Sep 18, 2020

What am I doing wrong? I'm unable to compile libpointmatcher, and neither can I compile the cmake example in libnabo:

$ cd src/libnabo/examples/libnabo-cmake-example/build
$ cmake ..
-- The C compiler identification is GNU 7.5.0
-- The CXX compiler identification is GNU 7.5.0
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Configuring done
-- Generating done
-- Build files have been written to: /media/data/subt/mapper/src/libnabo/examples/libnabo-cmake-example/build
$ make
Scanning dependencies of target libnabo-cmake-example
[ 50%] Building CXX object CMakeFiles/libnabo-cmake-example.dir/trivial.cpp.o
/media/data/subt/mapper/src/libnabo/examples/libnabo-cmake-example/trivial.cpp:3:10: fatal error: nabo/nabo.h: Directory or file does not exist
 #include "nabo/nabo.h"
          ^~~~~~~~~~~~~
compilation terminated.

Looking at libnabo-targets-release.cmake , there's not a word about include directories:

#----------------------------------------------------------------
# Generated CMake target import file for configuration "Release".
#----------------------------------------------------------------

# Commands may need to know the format version.
set(CMAKE_IMPORT_FILE_VERSION 1)

# Import target "libnabo::nabo" for configuration "Release"
set_property(TARGET libnabo::nabo APPEND PROPERTY IMPORTED_CONFIGURATIONS RELEASE)
set_target_properties(libnabo::nabo PROPERTIES
  IMPORTED_LINK_INTERFACE_LANGUAGES_RELEASE "CXX"
  IMPORTED_LINK_INTERFACE_LIBRARIES_RELEASE "gomp"
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib/libnabo.a"
  )

list(APPEND _IMPORT_CHECK_TARGETS libnabo::nabo )
list(APPEND _IMPORT_CHECK_FILES_FOR_libnabo::nabo "${_IMPORT_PREFIX}/lib/libnabo.a" )

# Commands beyond this point should not need to know the version.
set(CMAKE_IMPORT_FILE_VERSION)

@peci1
Copy link
Contributor

peci1 commented Sep 18, 2020

This change to the generated libnabo-targets-release.cmake helps building both the example and libpointmatcher:

set_target_properties(libnabo::nabo PROPERTIES
  IMPORTED_LINK_INTERFACE_LANGUAGES_RELEASE "CXX"
  IMPORTED_LINK_INTERFACE_LIBRARIES_RELEASE "gomp"
  IMPORTED_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib/libnabo.a"
+  INTERFACE_INCLUDE_DIRECTORIES "/media/data/subt/mapper/devel/include;/usr/include/eigen3"
  )

@jlblancoc
Copy link
Contributor Author

Can you please update to the latest version? It works straight away for me... and the automatically generated targets file has:

set_target_properties(libnabo::nabo PROPERTIES
  INTERFACE_INCLUDE_DIRECTORIES "/usr/include/eigen3;/home/jlblanco/code/libnabo"
)

@peci1
Copy link
Contributor

peci1 commented Sep 18, 2020

Let's continue in #106. I am using up-to-date master version.

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.

6 participants