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

Strong Windows installation tutorial update #447

Merged
merged 3 commits into from
Feb 12, 2021
Merged

Strong Windows installation tutorial update #447

merged 3 commits into from
Feb 12, 2021

Conversation

maximecharriere
Copy link
Contributor

@maximecharriere maximecharriere commented Feb 12, 2021

Salut l'équipe !

I started using CGAL on Windows a few months ago, and I faced a lot of compilation and installation difficulties.
After weeks of problem solving, I finally wrote an installation tutorial for CGAL and its third party libraries.
You can find it in this Gist:
https://gist.github.com/maximecharriere/5b071d5d07581af58b7333fe72a3e1f7

Part of this tutorial is about libpointmatcher, so I propose to include it in your repository.

My installation logic is slightly different from the current tutorial.
In my tutorial, all necessary environment variables will be configured so that CMake automatically finds all libraries, while in the current tutorial you have to specify the libraries' paths each time to CMake.
It is only for the Eigen library that it is necessary to specify its path to CMake, because libnabo and libpointmatcher's CMakeList.txt does not search in environment variables. (possible improvement in PR 448 ?)

@ethzasl-jenkins
Copy link

Can one of the admins verify this patch?

@maximecharriere
Copy link
Contributor Author

maximecharriere commented Feb 12, 2021

And if you have any suggestions or improvements, I'd love to hear from you!
For example, at the moment libpointmatcher and libnabo require a special installation procedure. So tell me if you know how to simplify the installation.

  1. Libpointmatcher need to be install and not only build, otherwise the following error occurs because yaml-cpp-pm-targets.cmake wont be found.
  CMake Error at C:/dev/libpointmatcher/build/libpointmatcherConfig.cmake:13 (include):
    include could not find load file:
  
      C:/dev/libpointmatcher/build/yaml-cpp-pm-targets.cmake
  Call Stack (most recent call first):
    CMakeLists.txt:93 (find_package)    
  1. Libnabo need to be build in all mode (Debug/Release/RelWithDebInfo/MinSizeRel), otherwise Libnabo will not be found when building a programme using Libpointmatcher in a mode for which Libnabo has not been build for.

@pomerlef
Copy link
Collaborator

ok to test

@pomerlef
Copy link
Collaborator

You will be our Windows expert ;)

That's an interesting behavior that Libnabo needs to be build in all mode. On Ubuntu and Mac, the linking will work independently of the build mode. Otherwise, it would be hard to deal with third party shared library. Is that a limitation from CGAL?

@pomerlef pomerlef merged commit b2dbd09 into norlab-ulaval:master Feb 12, 2021
@maximecharriere maximecharriere deleted the windows/doc branch February 12, 2021 14:21
@maximecharriere
Copy link
Contributor Author

Tks for the merge !


About Libnabo, i don't thinks that's come from CGAL.
When I only build Libnabo (corresponding to make all UNIX command) in Release, the following libnabo-targets.cmake is generated :

...
# Import target "libnabo::nabo" for configuration "Debug"
set_property(TARGET libnabo::nabo APPEND PROPERTY IMPORTED_CONFIGURATIONS DEBUG)
set_target_properties(libnabo::nabo PROPERTIES
  IMPORTED_LINK_INTERFACE_LANGUAGES_DEBUG "CXX"
  IMPORTED_LOCATION_DEBUG "C:/dev/libnabo/build/Debug/nabo.lib"
  )

# 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_LOCATION_RELEASE "C:/dev/libnabo/build/Release/nabo.lib"
  )

# Import target "libnabo::nabo" for configuration "MinSizeRel"
set_property(TARGET libnabo::nabo APPEND PROPERTY IMPORTED_CONFIGURATIONS MINSIZEREL)
set_target_properties(libnabo::nabo PROPERTIES
  IMPORTED_LINK_INTERFACE_LANGUAGES_MINSIZEREL "CXX"
  IMPORTED_LOCATION_MINSIZEREL "C:/dev/libnabo/build/MinSizeRel/nabo.lib"
  )

# Import target "libnabo::nabo" for configuration "RelWithDebInfo"
set_property(TARGET libnabo::nabo APPEND PROPERTY IMPORTED_CONFIGURATIONS RELWITHDEBINFO)
set_target_properties(libnabo::nabo PROPERTIES
  IMPORTED_LINK_INTERFACE_LANGUAGES_RELWITHDEBINFO "CXX"
  IMPORTED_LOCATION_RELWITHDEBINFO "C:/dev/libnabo/build/RelWithDebInfo/nabo.lib"
  )
...

You can see that each build mode has its own path.
I think that afterwards, programs that import libnabo look for nabo.lib in the specified paths corresponding to the config mode, which do not exist since Libnabo was only built in Release.

However, if I install libnabo (corresponding to make install UNIX command) in Release, a libnabo-targets-release.cmake is generated with the following code:

...
# 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_LOCATION_RELEASE "${_IMPORT_PREFIX}/lib/nabo.lib"
  )
...

The path to nabo.lib is always the same, so it won't come into conflict.


Anyway, I think I'll write in the tutorial that you have to install libnabo, and not only to build it. This way it is no longer necessary to build it in all modes.

I will also remove that it's necessary to manually set EIGEN_INCLUDE_DIR variable, since now it is automatically found in the environment variables.
I'm just waiting for the PR110 from libnabo to be merge before updating the tutorial.
Are you the BIG MASTER of libnabo repository ? Can you check this PR about Eigen finding? It's exactly the same that the one you just merged in libpointmatcher.

@pomerlef
Copy link
Collaborator

Not that big of a master, but I've access. I'll have a look.

@pomerlef
Copy link
Collaborator

it is merged

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.

3 participants