boost-cmake / boost

Cmake-based build of boost
Boost Software License 1.0
58 stars 24 forks source link

Compilation fails without python #6

Open dutow opened 7 years ago

dutow commented 7 years ago

When CMake can't find python, the boost::python package is correctly skipped.

However, the parameter library still tries to link with it.

This causes several errors after the configuration phase such as:

CMake Error at bcm/share/bcm/cmake/BCMFuture.cmake:68 (_add_library):
  Target "log-compile-current_function_support" links to target
  "boost::python" but the target was not found.  Perhaps a find_package()
  call is missing for an IMPORTED target, or an ALIAS target is missing?
Call Stack (most recent call first):
  bcm/share/bcm/cmake/BCMTest.cmake:104 (add_library)
  libs/log/test/CMakeLists.txt:62 (bcm_test)

An easy way to reproduce this is, for example, to do a 64 build on a machine with only the 32-bit python libraries present.

Commenting out the line I referenced solved the problem in this case, and I was able to build the project - so I think it should be conditional.

And also, other, optionally built libraries referenced in this way could cause similar issues.

ricejasonf commented 6 years ago

I'm having the same issue trying to cross-compile in a Docker container with no python installed.

It seems most of the targets are tests. I'm tempted to call out running tests by default as a bad practice because the target can't always be executed on the system that is compiling it.

With some guidance, I would be interested in trying to make a patch to fix this.

EDIT: I see there is already a PR for this: https://github.com/boost-cmake/boost/pull/9

ricejasonf commented 6 years ago

Here is a list of failed targets that are NOT tests that fail because they link against boost::python:

via the cmake errors:

CMake Error at bcm/share/bcm/cmake/BCMFuture.cmake:68 (_add_library):
  Target "boost_graph" links to target "boost::python" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  libs/graph/CMakeLists.txt:53 (add_library)

CMake Error at bcm/share/bcm/cmake/BCMFuture.cmake:68 (_add_library):
  Target "boost_log_setup" links to target "boost::python" but the target was
  not found.  Perhaps a find_package() call is missing for an IMPORTED
  target, or an ALIAS target is missing?
Call Stack (most recent call first):
  libs/log/CMakeLists.txt:231 (add_library)

CMake Error at bcm/share/bcm/cmake/BCMFuture.cmake:68 (_add_library):
  Target "boost_log" links to target "boost::python" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  libs/log/CMakeLists.txt:53 (add_library)

CMake Error at bcm/share/bcm/cmake/BCMFuture.cmake:68 (_add_library):
  Target "boost_graph" links to target "boost::python" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  libs/graph/CMakeLists.txt:53 (add_library)

CMake Error at bcm/share/bcm/cmake/BCMFuture.cmake:68 (_add_library):
  Target "boost_log" links to target "boost::python" but the target was not
  found.  Perhaps a find_package() call is missing for an IMPORTED target, or
  an ALIAS target is missing?
Call Stack (most recent call first):
  libs/log/CMakeLists.txt:53 (add_library)
pfultz2 commented 6 years ago

It seems most of the targets are tests. I'm tempted to call out running tests by default as a bad practice because the target can't always be executed on the system that is compiling it.

The tests do not run by default, and the test should by default still configure even with missing targets. This is because it bcm_test_link_libraries check if target exists first. If this is not happening, then it needs to be fixed.

ricejasonf commented 6 years ago

I tried a little digging.

It looks like bcm_test_link_libraries calls bcm_shadow_exists which calls add_library which I think is actually a macro that calls this _add_library thing that I can't make heads or tails of, but that is where it explodes.

_add_library does end up getting called with these args: _bcm_shadow_target_boost::python;INTERFACE;IMPORTED;GLOBAL

Could this be the problem? I'm not sure what this shadow target stuff is.

EDIT: This is more accurate. sorry

ERROR_add_library _bcm_shadow_target_boost::python;INTERFACE;IMPORTED;GLOBAL
   Called from: [6] /opt/src/bcm/share/bcm/cmake/BCMFuture.cmake
                [5] /opt/src/bcm/share/bcm/cmake/BCMFuture.cmake
                [4] /opt/src/bcm/share/bcm/cmake/BCMPkgConfig.cmake
                [3] /opt/src/bcm/share/bcm/cmake/BCMPkgConfig.cmake
                [2] /opt/src/bcm/share/bcm/cmake/BCMDeploy.cmake
                [1] /opt/src/libs/parameter/CMakeLists.txt
pfultz2 commented 6 years ago

It looks like bcm_test_link_libraries calls bcm_shadow_exists which calls add_library which I think is actually a macro that calls this _add_library thing that I can't make heads or tails of, but that is where it explodes.

So I need to write a document explaining some of the design rationale. Here some background.

The library overrides add_library which calls _add_library to call the original cmake function. This is overriden to inject the properties such as FIND_PACKAGE_NAME for imported targets so it can track what find_package call was used to get the imported target. This feature for cmake hasn't been implemented natively yet, which is why it overrides add_library.

The shadow targets are used to emulate TARGET_EXISTS generator expressions. This is done with a generator expression instead of a if(TARGET) so it can know if the target exists even if its added later by add_subdirectory in the superproject. To do this it creates a custom property on a shadow target, something like this:

# Custom property to check if target exists
define_property(TARGET PROPERTY "INTERFACE_TARGET_EXISTS"
    BRIEF_DOCS "True if target exists"
    FULL_DOCS "True if target exists"
)

# Create shadow target to notify that the target exists
macro(shadow_notify TARGET)
    if(NOT TARGET _shadow_target_${TARGET})
        add_library(_shadow_target_${TARGET} INTERFACE IMPORTED GLOBAL)
    endif()
    set_target_properties(_shadow_target_${TARGET} PROPERTIES INTERFACE_TARGET_EXISTS 1)
endmacro()
# Check if target exists by querying the shadow target
macro(shadow_exists OUT TARGET)
    if(NOT TARGET _shadow_target_${TARGET})
        add_library(_shadow_target_${TARGET} INTERFACE IMPORTED GLOBAL)
        set_target_properties(_shadow_target_${TARGET} PROPERTIES INTERFACE_TARGET_EXISTS 0)
    endif()
    set(${OUT} "$<TARGET_PROPERTY:_shadow_target_${TARGET},INTERFACE_TARGET_EXISTS>")
endmacro()

This creates two functions, and a custom property to track if the target exists or not. The shadow_notify will create a shadow target if it doesn't exists and then set the INTERFACE_TARGET_EXISTS property to true if it does exists.

The shadow_exists target will create a generator expression to query if the target exists. It does this by creating a new shadow target if doesn't exists with the INTERFACE_TARGET_EXISTS property set to 0. Then it will create a generator expression that will query the INTERFACE_TARGET_EXISTS property on the shadow target. The shadow target will always exists, and if the actual target exists then the property will be set to 1 by shadow_notify.

The bcm_deploy function will call notify on the shadow target to let the build know the target exists. However, it looks like this comes from the pkg config generation, which also checks that the target exists before it gets the pkg config name property.

_add_library does end up getting called with these args: _bcm_shadow_target_boost::python;INTERFACE;IMPORTED;GLOBAL

The :: is a problem. A shadow target can't have a :: in the name, and there was a fix for that here. However, that fix hasn't been update on here yet.

ricejasonf commented 6 years ago

Perhaps that is a rabbit hole. It appears that EXCLUDE_LIBS isn't used outside of the root CMakeLists.txt.

I tried excluding several libraries and it created more explosions like this one:

CMake Error at bcm/share/bcm/cmake/BCMFuture.cmake:68 (_add_library):
  Target "_bcm_internal_tests-boost_disjoint_sets" links to target
  "boost::graph" but the target was not found.  Perhaps a find_package() call
  is missing for an IMPORTED target, or an ALIAS target is missing?
Call Stack (most recent call first):
  bcm/share/bcm/cmake/BCMTest.cmake:43 (add_library)
  libs/disjoint_sets/test/CMakeLists.txt:1 (include)
pfultz2 commented 6 years ago

Perhaps that is a rabbit hole. It appears that EXCLUDE_LIBS isn't used outside of the root CMakeLists.txt.

Yes of course, because each project can be built independently without the superproject. So for now, if you exclude a library you need to manually exclude its dependencies. I don't know of an easy way to exclude the dependencies automatically.

I tried excluding several libraries and it created more explosions like this one

Yea that's definitely a bug. The missing dependencies for a test shouldn't cause it to break like that unless BUILD_TESTING is true. I need to add more tests for this in BCM.