strange basis_set_project_property error

[ahundt]

This code produces errors that claim to be due to a missing PROPERTY argument:

# Set the search paths for include files
basis_include_directories(
  # External
  ${EIGEN3_INCLUDE_DIR}
  ${OPENCV_INCLUDE_DIRS}

  # All library includes are prefixed with the path to avoid conflicts
  ${CMAKE_CURRENT_SOURCE_DIR}

  # testing
  ${GTEST_INCLUDE_DIRS}

  # CamOdoCalConfig.h
  ${CMAKE_CURRENT_BINARY_DIR}
)

I modified CommonTools.cmake basis_set_project_property to print some debug info with message(STATUS...) as follows:

function (basis_set_project_property)
  CMAKE_PARSE_ARGUMENTS (
    ARGN
      "APPEND"
      "PROJECT"
      "PROPERTY"
    ${ARGN}
  )
  message(STATUS "ARGN:${ARGN}")
  message(STATUS "ARGN_PROPERTY:${ARGN_PROPERTY}")

  if (NOT ARGN_PROJECT)
    set (ARGN_PROJECT "${PROJECT_NAME}")
  endif ()

Here is the cmake output, note how basis_include_directories calls basis_set_project_property, which passes a PROPERTY argument. Furthermore the variable ARGN_PROPERTY is not empty, though I'm not sure it is correct either since all the paths are present too:

-- Found OpenMP.
-- ARGN:PROPERTY;PROJECT_INCLUDE_DIRS;/home/hbr/.linuxbrew/include/eigen3;/home/hbr/.linuxbrew/include;/home/hbr/robonetracker/thirdparty/camodocal;/home/hbr/robonetracker/build/thirdparty/camodocal;/home/hbr/robonetracker/thirdparty/camodocal/include;/home/hbr/robonetracker/thirdparty/camodocal/src/ceres-solver/include;/home/hbr/robonetracker/thirdparty/camodocal/src/ceres-solver/internal;/home/hbr/robonetracker/thirdparty/camodocal/src/ceres-solver/internal/ceres;/home/hbr/robonetracker/thirdparty/camodocal/src/ceres-solver/CHOLMOD_INCLUDE_DIR-NOTFOUND
-- ARGN_PROPERTY:PROJECT_INCLUDE_DIRS;/home/hbr/.linuxbrew/include/eigen3;/home/hbr/.linuxbrew/include;/home/hbr/robonetracker/thirdparty/camodocal;/home/hbr/robonetracker/build/thirdparty/camodocal;/home/hbr/robonetracker/thirdparty/camodocal/include;/home/hbr/robonetracker/thirdparty/camodocal/src/ceres-solver/include;/home/hbr/robonetracker/thirdparty/camodocal/src/ceres-solver/internal;/home/hbr/robonetracker/thirdparty/camodocal/src/ceres-solver/internal/ceres;/home/hbr/robonetracker/thirdparty/camodocal/src/ceres-solver/CHOLMOD_INCLUDE_DIR-NOTFOUND
CMake Error at /home/hbr/.linuxbrew/share/cmake-modules/CommonTools.cmake:1307 (message):
  Missing PROPERTY argument!
Call Stack (most recent call first):
  /home/hbr/.linuxbrew/share/cmake-modules/TargetTools.cmake:287 (basis_set_project_property)
  thirdparty/camodocal/src/ceres-solver/CMakeLists.txt:99 (basis_include_directories)

-- Configuring incomplete, errors occurred!
See also "/home/hbr/robonetracker/build/CMakeFiles/CMakeOutput.log".
See also "/home/hbr/robonetracker/build/CMakeFiles/CMakeError.log".
Makefile:1408: recipe for target 'cmake_check_build_system' failed
make: *** [cmake_check_build_system] Error 1

I'm not sure what to make of it because I don't expect the if statement to fail, and I'm not completely sure if the variables are set correctly.

[ahundt]

It was because of the CHOLMOD_INCLUDE_DIR-NOTFOUND component of the path, so a previous directory was marked as not found then included. I suggest we modify the error case of this code from Missing PROPERTY argument! to provide more accurate/easy to debug information.

Instead of the current behavior it could check that the property argument exists, which says Missing PROPERTY argument! and another which states that the property argument couldn't be used, and prints out more context details and the contents of the variable.

A second possibility is just a single check that says the error is one of the following: property argument not defined, or invalid, plus prints the contents of the property argument and some context of where in the code this is and why it is being set.

What do you think?

[schuhschuh]

Sounds reasonable, the if (NOT ARGN_PROPERTY) check was more meant as if (ARGN_PROPERTY STREQUAL ""), because if (NOT DEFINED ARGN_PROPERTY) will probably not do because cmake_parse_arguments might always define it, even if not parsed (would need to be tested).

I would not necessarily throw an error for an invalid ARGN_PROPERTY argument in basis_set_project_property because the property value should be allowed to be anything and this function can hardly decide whether it makes sense for this property to have such value.

For the basis_include_directories call with invalid path, the underlying CMake command include_directories to which this call is delegated eventually should catch any issue with it. Just as if the user called the CMake command directly with an invalid argument.

[schuhschuh]

Or

list (LENGTH ARGN_PROPERTY len)
if (len EQUAL 0)
  message (FATAL_ERROR "...")
endif ()

but this is probably more expensive than the comparison with an empty string.