search

robotology / ycm-cmake-modules

YCM (YCM CMake modules) is a collection of various useful CMake modules.
http://robotology.github.io/ycm-cmake-modules
Other
50 stars 22 forks source link

Issues with Findassimp w/ current master #376

Closed xEnVrE closed 3 years ago

xEnVrE commented 3 years ago

I am using Ubuntu 18.04, libassimp-dev 4.1.0~dfsg-3 and CMake 3.20.

When using YCM 0.12.1 I can successfully use the following CMakeLists.txt (and build)

cmake_minimum_required(VERSION 3.20)

project(test)

find_package(YCM)
find_package(assimp)

add_library(test_lib src/test.cpp)
target_link_libraries(test_lib PUBLIC assimp::assimp)

Instead, using the latest master two issues occur:

  1. find_package_handle_standard_args cannot be found in

https://github.com/robotology/ycm/blob/32c38ef8d68d77ee2fc96c4dae4338b06bea457e/find-modules/Findassimp.cmake#L84

:/test_ycm_assimp/build$ cmake ..
-- The C compiler identification is GNU 7.5.0
-- The CXX compiler identification is GNU 7.5.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found YCM: /home/hsp-user/robot-install/share/cmake/YCM (found version "0.12.20210302.8-20210302.8+git32c38ef")
CMake Error at /home/hsp-user/robot-install/share/YCM/find-modules/Findassimp.cmake:84 (find_package_handle_standard_args):
  Unknown CMake command "find_package_handle_standard_args".
Call Stack (most recent call first):
  CMakeLists.txt:6 (find_package)

-- Configuring incomplete, errors occurred!
  1. apparently it is now required to include (FindPackageHandleStandardArgs) before being able to use such command. After adding it to Findassimp.cmake however I get this: 
    
:/test_ycm_assimp/build$ cmake ..
-- The C compiler identification is GNU 7.5.0
-- The CXX compiler identification is GNU 7.5.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found YCM: /home/hsp-user/robot-install/share/cmake/YCM (found version "0.12.20210302.8-20210302.8+git32c38ef")
-- Found assimp: /usr/lib/x86_64-linux-gnu/cmake/assimp-4.1/assimp-config.cmake (found version "4.1.0") 
-- Configuring done
CMake Error at CMakeLists.txt:8 (add_library):
Target "test_lib" links to target "assimp::assimp" but the target was not
found.  Perhaps a find_package() call is missing for an IMPORTED target, or
an ALIAS target is missing?

-- Generating done CMake Generate step failed. Build files cannot be regenerated correctly.



Am I doing something wrong?
xEnVrE commented 3 years ago

cc @traversaro

traversaro commented 3 years ago

Point 1. Is a bug, should be fixed by https://github.com/robotology/ycm/pull/377 . Point 2. is a bug upstream in Ubuntu, the idea of vendoring the Findassimp.cmake in YCM was done exactly to workaround that in https://github.com/robotology/ycm/pull/229, unfortunate that logic was broken on Windows for modern assimp version, so an additional logic was added in https://github.com/robotology/ycm/pull/371 and a bug fix added in https://github.com/robotology/ycm/pull/373 .

traversaro commented 3 years ago

To understand which workaround we need to add, can post the content of /usr/lib/x86_64-linux-gnu/cmake/assimp-4.1/assimp-config.cmake and related files on your system, so we understand the workaround logic we need to add in https://github.com/robotology/ycm/blob/master/find-modules/Findassimp.cmake#L61 .

GitHub
robotology/ycm
Extra CMake Modules for YARP and friends. Contribute to robotology/ycm development by creating an account on GitHub.
xEnVrE commented 3 years ago

can post the content of /usr/lib/x86_64-linux-gnu/cmake/assimp-4.1/assimp-config.cmake and related files on your system

I am not sure I understood correctly! Do you want me to post here the content of the file from my Ubuntu installation?

traversaro commented 3 years ago

can post the content of /usr/lib/x86_64-linux-gnu/cmake/assimp-4.1/assimp-config.cmake and related files on your system

I am not sure I understood correctly! Do you want me to post here the content of the file from my Ubuntu installation?

Yes!

xEnVrE commented 3 years ago

Here they are:

/usr/lib/x86_64-linux-gnu/cmake/assimp-4.1/assimp-config.cmake

# - Find Assimp Installation
#
# Users can set the following variables before calling the module:
#  ASSIMP_DIR - The preferred installation prefix for searching for ASSIMP. Set by the user.
#
# ASSIMP_ROOT_DIR - the root directory where the installation can be found
# ASSIMP_CXX_FLAGS - extra flags for compilation
# ASSIMP_LINK_FLAGS - extra flags for linking
# ASSIMP_INCLUDE_DIRS - include directories
# ASSIMP_LIBRARY_DIRS - link directories
# ASSIMP_LIBRARIES - libraries to link plugins with
# ASSIMP_Boost_VERSION - the boost version assimp was compiled with
get_filename_component(_PREFIX "${CMAKE_CURRENT_LIST_FILE}" PATH)
get_filename_component(_PREFIX "${_PREFIX}" PATH)
get_filename_component(_PREFIX "${_PREFIX}" PATH)
get_filename_component(ASSIMP_ROOT_DIR "${_PREFIX}" PATH)

if( MSVC )
  # in order to prevent DLL hell, each of the DLLs have to be suffixed with the major version and msvc prefix
  if( MSVC70 OR MSVC71 )
    set(MSVC_PREFIX "vc70")
  elseif( MSVC80 )
    set(MSVC_PREFIX "vc80")
  elseif( MSVC90 )
    set(MSVC_PREFIX "vc90")
  elseif( MSVC10 )
    set(MSVC_PREFIX "vc100")
  elseif( MSVC11 )
    set(MSVC_PREFIX "vc110")
  elseif( MSVC12 )
    set(MSVC_PREFIX "vc120")
  elseif( MSVC14 )
    set(MSVC_PREFIX "vc140")
  else()
    set(MSVC_PREFIX "vc150")
  endif()
  set(ASSIMP_LIBRARY_SUFFIX "-${MSVC_PREFIX}-mt" CACHE STRING "the suffix for the assimp windows library" FORCE)
else()
  set(ASSIMP_LIBRARY_SUFFIX "" CACHE STRING "the suffix for the openrave libraries" FORCE)
endif()

set( ASSIMP_CXX_FLAGS ) # dynamically linked library
if( WIN32 )
  # for visual studio linking, most of the time boost dlls will be used
  set( ASSIMP_CXX_FLAGS " -DBOOST_ALL_DYN_LINK -DBOOST_ALL_NO_LIB")
endif()
set( ASSIMP_LINK_FLAGS "" )
set( ASSIMP_LIBRARY_DIRS "${ASSIMP_ROOT_DIR}/lib/x86_64-linux-gnu")
set( ASSIMP_INCLUDE_DIRS "${ASSIMP_ROOT_DIR}/include")
set( ASSIMP_LIBRARIES assimp${ASSIMP_LIBRARY_SUFFIX})
set( ASSIMP_LIBRARIES ${ASSIMP_LIBRARIES})

# search for the boost version assimp was compiled with
#set(Boost_USE_MULTITHREAD ON)
#set(Boost_USE_STATIC_LIBS OFF)
#set(Boost_USE_STATIC_RUNTIME OFF)
#find_package(Boost ${ASSIMP_Boost_VERSION} EXACT COMPONENTS thread date_time)
#if(Boost_VERSION AND NOT "${Boost_VERSION}" STREQUAL "0")
#   set( ASSIMP_INCLUDE_DIRS "${ASSIMP_INCLUDE_DIRS}" ${Boost_INCLUDE_DIRS})
#else(Boost_VERSION AND NOT "${Boost_VERSION}" STREQUAL "0")
#   message(WARNING "Failed to find Boost ${ASSIMP_Boost_VERSION} necessary for assimp")
#endif(Boost_VERSION AND NOT "${Boost_VERSION}" STREQUAL "0")

# the boost version assimp was compiled with
set( ASSIMP_Boost_VERSION ".")

# for compatibility with pkg-config
set(ASSIMP_CFLAGS_OTHER "${ASSIMP_CXX_FLAGS}")
set(ASSIMP_LDFLAGS_OTHER "${ASSIMP_LINK_FLAGS}")

MARK_AS_ADVANCED(
  ASSIMP_ROOT_DIR
  ASSIMP_CXX_FLAGS
  ASSIMP_LINK_FLAGS
  ASSIMP_INCLUDE_DIRS
  ASSIMP_LIBRARIES
  ASSIMP_Boost_VERSION
  ASSIMP_CFLAGS_OTHER
  ASSIMP_LDFLAGS_OTHER
  ASSIMP_LIBRARY_SUFFIX
)

/usr/lib/x86_64-linux-gnu/cmake/assimp-4.1/assimp-config-version.cmake

set( PACKAGE_VERSION "4.1.0" )
if( "${PACKAGE_FIND_VERSION}" VERSION_EQUAL "4.1.0")
  set(PACKAGE_VERSION_EXACT 1)                                                                                                                                                                 
endif()                                                                                                                                                                                        
if( "${PACKAGE_FIND_VERSION_MAJOR}.${PACKAGE_FIND_VERSION_MINOR}" EQUAL "4.1.0" )                                                                                                              
  set(PACKAGE_VERSION_COMPATIBLE 1)                                                                                                                                                            
elseif( "${PACKAGE_FIND_VERSION_MAJOR}" EQUAL "4" )                                                                                                                                            
  # for now backward compatible if minor version is less                                                                                                                                       
  if( ${PACKAGE_FIND_VERSION_MINOR}  LESS 1 )                                                                                                                                                  
    set(PACKAGE_VERSION_COMPATIBLE 1)                                                                                                                                                          
  endif()                                                                                                                                                                                      
endif()                                                                                                                                                                                        
set( ASSIMP_STATIC_LIB "")
traversaro commented 3 years ago

So apparently this version of the config file does not provided imported targets. Perhaps the best strategy is to add the following logic before https://github.com/robotology/ycm/pull/377/files#diff-0678d0da3d35c8ff51d6b1836194654c132b4230b5d5c74c036d46583e0f06d0R84 : 

if(NOT TARGET assimp::assimp)
  add_library(assimp::assimp UNKNOWN IMPORTED)
  set_target_properties(assimp::assimp PROPERTIES
    INTERFACE_INCLUDE_DIRECTORIES "${ASSIMP_INCLUDE_DIRS}")
  set_property(TARGET assimp::assimp APPEND PROPERTY
    IMPORTED_LOCATION "${ASSIMP_LIBRARY_DIRS}/${ASSIMP_LIBRARIES}")
endif()

Can you try it and check if it works, and in that case open a PR? Thanks!

GitHub
Fix missing FindPackageHandleStandardArgs include in Findassimp.cmake by traversaro · Pull Request #377 · robotology/ycm
Fix point 1. in #376 .
xEnVrE commented 3 years ago

With that addition, the cmake call is working and the build process for the tiny example posted above is working.

However, a library, using YCM, that used to work now is not compiling anymore. Since I am not sure this is a problem of the library I will post here the output of the build process because maybe this depends on the addition that we just made in the Findassimp.cmake.

Trying to compile superimpose-mesh-lib/devel I get, after all the single compilation units are processed,

make[2]: *** No rule to make target '/usr/lib/lib/x86_64-linux-gnu/assimp', needed by 'lib/libSuperimposeMesh.so.0.11.100'.  Stop.
traversaro commented 3 years ago

Yes the problem is that /usr/lib/lib/x86_64-linux-gnu/assimp is not an actual library, a possible fix is:

if(NOT TARGET assimp::assimp)
  add_library(assimp::assimp UNKNOWN IMPORTED)
  set_target_properties(assimp::assimp PROPERTIES
    INTERFACE_INCLUDE_DIRECTORIES "${ASSIMP_INCLUDE_DIRS}")
  find_library(_ycm_ASSIMP_LIBRARY NAMES assimp libassimp PATHS ${ASSIMP_LIBRARY_DIRS})
  mark_as_advanced(_ycm_ASSIMP_LIBRARY)
  set_property(TARGET assimp::assimp APPEND PROPERTY
    IMPORTED_LOCATION "${_ycm_ASSIMP_LIBRARY}")
endif()
xEnVrE commented 3 years ago

Thank you @traversaro, now it is working.

I will open a PR.