cmake-basis / BASIS

CMake BASIS makes it easy to create sharable software and libraries that work together. This is accomplished by combining and documenting some of the best practices and utilities available. This project supplies a fully integrated suite of functionality to make the whole process seamless!
https://cmake-basis.github.io
Other
48 stars 10 forks source link

dependencies such as "sch-core" look for package sch with version core #608

Closed ahundt closed 7 years ago

ahundt commented 7 years ago

I'm not sure if this is due to basis or not, but I recently updated to the latest version and I'm trying to add the package sch-core to the list of OPTIONAL_DEPENDENCIES in a call to basis_project() within BasisProject.cmake.

I get the following error:

-- Looking for sch core (optional)...
CMake Error at /usr/local/share/modules/CommonTools.cmake:56 (_find_package):
  find_package called with invalid argument "core"
Call Stack (most recent call first):
  /usr/local/share/modules/CommonTools.cmake:677 (find_package)
  /usr/local/share/modules/ProjectTools.cmake:2135 (basis_find_package)
  /usr/local/share/modules/ProjectTools.cmake:2422 (basis_find_packages)
  /usr/local/share/modules/ProjectTools.cmake:2696 (basis_project_begin)
  modules/grl/CMakeLists.txt:84 (basis_project_impl)

-- Looking for sch core (optional)... - not found
CMake Error at /usr/local/share/modules/CommonTools.cmake:826 (message):

  Module grl was requested to be build with sch version core.  Please ensure
  that the package is installed in a standard system location or set
  DEPENDS_sch_DIR to the installation prefix (i.e., root directory of the
  installation).  To disable features which require this optional dependency,
  set the WITH_sch option to OFF and try again.

  The DEPENDS_sch_DIR variable can alternatively be set to the directory
  containing a schConfig.cmake or sch-config.cmake file.  If no such file
  exists, contact either the developer of this project or CMake BASIS to
  provide a Findsch.cmake file.

Is this a CMake thing or a BASIS thing? If it is BASIS I'm not sure this new behavior is ideal, hopefully there is a simple workaround like putting sch-core in quotes "sch-core"?

schuhschuh commented 7 years ago

Hm, project/module names were initially meant to be upper camel case only without dashes or underscores as separators. I thus didn't consider this case. The package name is split into <name>-<version> here. Multiple versions are specified as <name>-<version1>|<version2>|.... I don't actually think this is such a new feature. Only the support of multiple alternative versions is newer. The difference now is just that the new regex no longer specifically matches for major, minor, and patch version numbers to the right of the dash. Hence the error now. But from the doc string of the function you can see that a dash was always meant as separator of package name from package version... it should be relatively straightforward to make the regex more specific again, though, i.e., require the first version to match the pattern <major>.<minor>.<patch> and to consider just everything after the pipe | as versions anyway (or match them more precisely as well...).

See https://github.com/cmake-basis/BASIS/commit/8e3a06fd1a4611816352deb40365c49b03f58b58#diff-3fc3d3020b05f0acc1a7998aa105690eL114 .

schuhschuh commented 7 years ago

Here's the regex that should work: (.*)-([0-9](\.[0-9])?(\.[0-9])?(\|[0-9](\.[0-9])?(\.[0-9])?)*)

Example: sch-core-1.2|5|3.4.1 is split into:

Match 1
Full match  0-20    `sch-core-1.2|5|3.4.1`
Group 1.    0-8 `sch-core`
Group 2.    9-20    `1.2|5|3.4.1`

(https://regex101.com/)

schuhschuh commented 7 years ago

@ahundt Can you confirm that it works with the just submitted PR? I've just tested the new regex with a small CMake script, it seems to work fine. Not tested it within BASIS itself, though.

ahundt commented 7 years ago

I tried installing from HEAD and got configuration errors: https://gist.github.com/181371a60bc3a5795e5ce77f94eccc93

ahundt commented 7 years ago

Here is the key section:


CMake Error at /usr/local/Cellar/vtk6/6.3.0/lib/cmake/vtk-6.3/vtkModuleAPI.cmake:120 (message):
  Requested modules not available:

    vtkRenderingOpenGL2
Call Stack (most recent call first):
  /usr/local/lib/cmake/ITK-4.10/Modules/ITKVtkGlue.cmake:36 (vtk_module_config)
  /usr/local/lib/cmake/ITK-4.10/ITKModuleAPI.cmake:54 (include)
  /usr/local/lib/cmake/ITK-4.10/ITKModuleAPI.cmake:26 (itk_module_load)
  /usr/local/lib/cmake/ITK-4.10/ITKModuleAPI.cmake:84 (_itk_module_config_recurse)
  /usr/local/lib/cmake/ITK-4.10/ITKConfig.cmake:73 (itk_module_config)
  src/cmake/CommonTools.cmake:56 (_find_package)
  src/cmake/FindITK.cmake:108 (FIND_PACKAGE)
  src/cmake/CommonTools.cmake:56 (_find_package)
  src/cmake/CommonTools.cmake:367 (find_package)
  src/cmake/ProjectTools.cmake:1694 (basis_find_package)
  src/cmake/ProjectTools.cmake:1909 (basis_find_packages)
  src/cmake/ProjectTools.cmake:2159 (basis_project_begin)
  CMakeLists.txt:66 (basis_project_impl)
schuhschuh commented 7 years ago

Well, seems there's something odd about either your ITK or VTK installation.

Warning: You have unlinked kegs in your Cellar
Leaving kegs unlinked can lead to build-trouble and cause brews that depend on
those kegs to fail to run properly once built. Run `brew link` on these:
  matplotlib
  pillow
  protobuf
  vtk

This is unrelated to the issue here and not caused by BASIS. You should be able to build it without ITK, which is only used by one test application that you probably don't use.

ahundt commented 7 years ago

aha, you're right. Did that and it works! Thanks!

ahundt commented 7 years ago

That turned up another bug in the HAVE_blah definitons:

<command line>:23:17: warning: ISO C99 requires whitespace after the macro name
      [-Wc99-extensions]
#define HAVE_sch-core 1

Apparently we should do something like substitute HAVE_sch-core with HAVE_sch_core to meet the C/C++ standard, in other words the dash could become an underscore there.

ahundt commented 7 years ago

Actually, would the regex fail on something like spdlog-0.11.0 or boost-1.63.0?

(.*)-([0-9](\.[0-9])?(\.[0-9])?(\|[0-9](\.[0-9])?(\.[0-9])?)*)

am I reading things correctly when I think it seems like only one consecutive digit is permitted?

schuhschuh commented 7 years ago

You're right, the regex for each version number component should be [0-9]+ rather. Only digits are allowed here because CMake itself does not allow version numbers to contain alphanumeric characters. For if (a VERSION_* b) the documentation states "Component-wise integer version number comparison".