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

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

Closed
1 task done
SpaceIm opened this issue Aug 29, 2020 · 12 comments · Fixed by #8130 or #8338
Closed
1 task done
Assignees
Milestone

Comments

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 29, 2020

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

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.

@memsharded memsharded added this to the 1.30 milestone Aug 30, 2020
@SSE4
Copy link
Contributor

SSE4 commented Sep 2, 2020

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

SpaceIm commented Sep 17, 2020

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

madebr commented Sep 25, 2020

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

SpaceIm commented Sep 25, 2020

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

@aleksa2808
Copy link
Contributor

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.

@memsharded memsharded modified the milestones: 1.30, 1.31 Sep 29, 2020
conan-center-bot pushed a commit to conan-io/conan-center-index that referenced this issue Oct 23, 2020
* add args/0.12.1

* Change the recipe to mzdun-args

- Fix test_package issues
- Rework _working_compilers and hand-crafted Version to
  _compilers_minimum_version, tools.Version and tools.check_min_cppstd
- Use cached CMake instance
- Bump library version to one compiling under apple-clang

* Fix last ON/OFF to True/False

* Remove unused code

- Delete fPIC, when building .so
- Limit apple-clang to 10.0 (see [1] for optional, string_view and
  Elementary string conversions)

[1] https://en.cppreference.com/w/cpp/compiler_support/17#C.2B.2B17_library_features

* Fix save on (Deleted) after git mv

* Ban DLLs with MT/MTd and clang 7 with libc++

* Get compilation fixes for the library

* Provide CMake wrapper

* Add support for cmake_find_package[_multi]

* Add FIXME warning about conan-io/conan#7615
@memsharded memsharded modified the milestones: 1.31, 1.32 Oct 29, 2020
@SSE4
Copy link
Contributor

SSE4 commented Nov 7, 2020

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

danimtb commented Dec 2, 2020

I have created this PR #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?

@czoido czoido modified the milestones: 1.32, 1.33 Dec 2, 2020
@SpaceIm
Copy link
Contributor Author

SpaceIm commented Dec 9, 2020

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

liarokapisv commented Dec 15, 2020

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

madebr commented Jan 14, 2021

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

@jgsogo
Copy link
Contributor

jgsogo commented Jan 14, 2021

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

@jgsogo jgsogo reopened this Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment