search

conan-io / conan

Conan - The open-source C and C++ package manager
https://conan.io
MIT License
8.26k stars 981 forks source link

[feature] cmake_find_package[_multi] generators: provide a way to create non namespaced imported targets #7615

Closed SpaceIm closed 3 years ago

SpaceIm commented 4 years ago

Currently, cmake_find_package and cmake_find_package_multi generators always create namespaced imported targets in generated module/config files. While not considered as best practice, it's not uncommon that official CMake config file of a library create non-namespaced imported targets (and quite uncommon for module files because official ones are usually provided by Kitware).

This is a non-exhaustive list of CCI recipes whose official config file creates non-namespaced targets, to show you that it's not just a corner case:

ade
amqp-cpp
antlr4-cppruntime
arcus
arduinojson
argtable3
asyncplusplus
celero
cereal
charls
cjson
cppzmq
cryptopp
cwalk
czmq
dcmtk
debug_assert
docopt.cpp
easy_profiler
effolkronium-random
embree3
evix2
ezc3d
fast-cdr
fast-dds
fcl
foonathan-memory
freetype
g3log
gcem
gdcm
gflags
glfw
glslang
gtsam
hana
jinja2cpp
json-schema-validator
jsoncpp
kangaru
leptonica
libccd
libde265
libe57format
libgeotiff
libkml
libmediainfo
libpng
libsigcpp
libtins
libwebsockets
libyaml
libzen
litehtml
log4cxx
mbits-args
mfast
mpark-variant
msgpack
octomap
onnx
opencascade
opencolorio
opencv
opengv
openjpeg
openmesh
parallel-hashmap
pro-mdnsd
rapidcheck
rapidjson
raylib
read-excel
rxcpp
sfml
spirv-cross
spirv-tools
sqlitecpp
symengine
szip
tensorpipe
tesseract
tinyobjloader
tlx
tmx
type_safe
utfcpp
wslay
xproperty
xsimd
xtensor
xtl
yaml-cpp
zeromq

It's worth noting that a library may have an official module and config file, and one might create namespaced targets but not the other. libpng and freetype are good examples.

library cmake target in module file cmake target in config file
freetype Freetype::Freetype freetype
libpng PNG::PNG png
memsharded commented 4 years ago

Hi @SpaceIm

First, it is sad to discover this CMake lack of consistency (though not very surprising). Second, I agree, this is not a corner case, I thought it was going to be just a very few recipes, but it is clear I was wrong. I think Conan needs to provide a native and idiomatic way to define these targets too. Not sure they can be introduced without breaking, we might need to wait until Conan 2.0 (which will start very soon), but lets have a look and think about possible approaches.

Regarding the multiple module/config definitions, I am not so sure. I would say that this seems too much, and Conan should provide only one of them (I guess the modern module one). Lets try to think about the possible approaches in Conan 1.30.

SSE4 commented 4 years ago

@SpaceIm do you have an idea how the syntax to describe non-namespaced targets may look like? e.g. assigning an empty string, or None, or something else? example would be helpful.

SpaceIm commented 4 years ago

@SSE4 I don't know how it could be implemented with existing cpp_info properties without breaking current behaviour. For example, setting self.cpp_info.names["cmake_find_package"] = "" would prevent defining global target name::name?

madebr commented 4 years ago

Having multiple namespaces would also be useful. e.g. thrift generates thrift::thrift, thriftz::thriftz, thriftnb::thriftnb, thriftqt5::thriftqt5.

In general, the cmake_find_package generators should assume that everything can be modified. (e.g. the name of a components is not always the same as the name of the imported target)

SpaceIm commented 4 years ago

Difference between component name and underlying imported target can be emulated, but not multiple namespace (openexr also has multiple namespaces).

aleksa2808 commented 4 years ago

Agreeing with these points, confusingly CMake target names can be ANYTHING (and with my relatively brief CMake experience I've seen almost every type of target name in active use). Some packages like GTest even define things like GTest::GTest and then GTest::Main which as far as I know cannot be modeled with components right now.

SSE4 commented 4 years ago

unfortunately, I don't see how it could be implemented with the current CppInfo model. I suppose CppInfo should be extended, for instance:

# generates "libzmq-static" (no namespace)
cpp_info.names["cmake_find_package_multi"] = "libzmq-static"
cpp_info.namespaces["cmake_find_package_multi"] = ""
# alternatively: cpp_info.namespaces["cmake_find_package_multi"] = None

# generates "thriftz::thriftz"
cpp_info.components["thriftz"].names["cmake_find_package_multi"] = "thriftz"
cpp_info.components["thriftz"].namespaces["cmake_find_package_multi"] = "thriftz"

# generates "thriftnb::thriftnb"
cpp_info.components["thriftnb"].names["cmake_find_package_multi"] = "thriftnb"
cpp_info.components["thriftnb"].namespaces["cmake_find_package_multi"] = "thriftnb"

# generates "OpenEXR::IlmThread"
cpp_info.components["IlmThread"].names["cmake_find_package_multi"] = "IlmThread"
cpp_info.components["IlmThread"].namespaces["cmake_find_package_multi"] = "OpenEXR"

# generates "Imath::ImathConfig"
cpp_info.components["ImathConfig"].names["cmake_find_package_multi"] = "ImathConfig"
cpp_info.components["ImathConfig"].namespaces["cmake_find_package_multi"] = "Imath"
danimtb commented 3 years ago

I have created this PR https://github.com/conan-io/conan/pull/8130 to try a new approach using build modules for this kind of thing. I feel that CMake has so many different moving parts that it is a bit messy to keep adding things to the cpp_info model. WDYT?

SpaceIm commented 3 years ago

@danimtb, looking at the PR, I don't really understand how a robust recipe could be written, ie providing non namespaced targets for cmake_find_package and cmake_find_package_multi generators only, since it doesn't make sense for others generators. Indeed, recipes in your tests seem to depend on generator used (a custom cmake file must be written at package or consume time, linking to... a target generated by conan, but how to know which one is availabe?), while recipe should be generic.

===================== EDIT: or maybe it could work with something like this injected in build_modules for each target:

if(NOT target AND namespace::target)
  add_library(target INTERFACE IMPORTED)
  target_link_libraries(target INTERFACE namespace::target)
endif()

=====================

It seems also to require lot of boilerplate code. package_info() of several recipes are already quite complex (example: https://github.com/conan-io/conan-center-index/blob/40b28fa770140bc83a5de01a860e6dbc7c9ac0f3/recipes/dcmtk/all/conanfile.py#L174).

liarokapisv commented 3 years ago

Thanks for working on this, this is a very important feature!

Just a reminder: :: in cmake is nothing more than an utility for more readable Imported/Alias targets. Target names can be anything, using :: is just a hint that the target is meant to be consumed as an imported target (or alias for compatibility with add_subdirectory usage). Depending on the policy and version cmake may or may not also perform validation checks to see that it really is an imported target or alias. As far as conan is concerned it should be just a normal substring in a target name.

Now it's good cmake practice to use :: to add some form of namespacing but this should not be enforced. Eg I have encountered libraries with no namespaces and libraries with the same namespaces spanning multiple projects.

madebr commented 3 years ago

@jgsogo Does #8130 really add non-namespaced targets? I don't see an example doing that, unless I didn't look well enough.

jgsogo commented 3 years ago

I'll write an example with a POC (it was closed with the PR)