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

Issues with Findassimp w/ current master #376

Closed
xEnVrE opened this issue Mar 30, 2021 · 10 comments · Fixed by #378
Closed

Issues with Findassimp w/ current master #376

xEnVrE opened this issue Mar 30, 2021 · 10 comments · Fixed by #378

Comments

@xEnVrE
Copy link
Contributor

xEnVrE commented Mar 30, 2021

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
Copy link
Contributor Author

xEnVrE commented Mar 30, 2021

cc @traversaro

@traversaro
Copy link
Member

Point 1. Is a bug, should be fixed by #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 #229, unfortunate that logic was broken on Windows for modern assimp version, so an additional logic was added in #371 and a bug fix added in #373 .

@traversaro
Copy link
Member

traversaro commented Mar 30, 2021

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
Extra CMake Modules for YARP and friends. Contribute to robotology/ycm development by creating an account on GitHub.

@xEnVrE
Copy link
Contributor Author

xEnVrE commented Mar 30, 2021

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
Copy link
Member

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
Copy link
Contributor Author

xEnVrE commented Mar 30, 2021

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
Copy link
Member

traversaro commented Mar 30, 2021

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 point 1. in #376 .

@xEnVrE
Copy link
Contributor Author

xEnVrE commented Mar 30, 2021

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
Copy link
Member

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
Copy link
Contributor Author

xEnVrE commented Mar 30, 2021

Thank you @traversaro, now it is working.

I will open a PR.

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

Successfully merging a pull request may close this issue.

3 participants