conan-io / conan

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

[feature] Add executables to cpp_info model #7240

Closed danimtb closed 1 month ago

danimtb commented 4 years ago

Coming from https://github.com/conan-io/conan-center-index/pull/1717#discussion_r441501353 Related to #5090

In order to allow a better integration with executables, cpp_info could model those with an extra field similar to the current cpp_info.libs. Something similar to the proposal at #5090 with cpp_info.exes = [] that will allow adding <NAME>_EXECUTABLE being <NAME> the name of the component or similar

cc/ @madebr

madebr commented 4 years ago

Things that become possible:

A potential issue on Macos is the requirement to set DYLD_LIBRARY_PATH in the same call. See https://github.com/conan-io/conan-center-index/pull/1179#issuecomment-621339628.

jgsogo commented 4 years ago

Hi. I'm not opposed to adding a cpp_info.exes field to the package information (probably it is needed), but we need to be very cautious about adding variables and targets.

Usually (always?) in the CMakeLists.txt files we use these executables from the requirements (protoc, doxygen,...) to execute something in the process of building, that is, the executable works in the build machine while we are generating binaries for the host machine.

Take the well-know protobuf/protoc example and this graph as a reference:

image

Here, the CMakeLists.txt of my_pkg will contain something as follows:

CMakeLists.txt

project(my_pkg)

find_package(protobuf)  # Provides targets 'protobuf::libprotobuf' and proposed 'protobuf::protoc'

protobuf_generate_cpp(....)  # <-- Here we want to use protoc
add_library(my_pkg ...)
target_link_libraries(my_pkg protobuf::libprotobuf)

It is clear that our library my_pkg want to link with the target protobuf::libprotobuf provided by find_package(protobuf), that is the library that has been compiled for the host machine and it is available via the Findprotobuf.cmake file that Conan has generated.

But, the protoc executable we need here is not the one from the Findprotobuf.cmake (that belongs to the host context, may not run in this build machine), the protoc we want belong to the build context, it is in a different package and we don't have Findprotobuf-build-context.cmake for it! We can use and run that protoc only because Conan injected the PATH to that executable in the environment before running CMake.

IMO we should encourage raw executables over targets (protoc over protobuf::protoc) in CMakeLists.txt, otherwise we might be blocking cross-building scenarios.


There are two challenges with this approach:

About the problem with shared libraries (it is not only Macos and SIP, the problem still exists for the rest of OS), I'm still thinking about how to deal with it. Some ideas, but I need to make up my mind before sharing them.

jgsogo commented 4 years ago

BTW, my last approach requires cpp_info.exes to list the executables. My long paragraph was only about how to deal with them inside CMake, not about the initial proposal of this issue. 😉

steffenroeber commented 4 years ago

Another use case of that is automoc/autouic/autorcc of Qt. Currently it is not possible to use automoc with conan qt recipes and cmake_find_package(_multi) generator. Automoc needs a target like Qt5::moc that points to the moc.exe.

madebr commented 4 years ago

Adding executable components to the tree would avoid adding executable-only requirements to libraries. e.g. let's assume a package provides a library and an executable. This packages also requires 2 dependencies: one is used by the library and the other is used by the executable. Right now, both dependencies need to be listed as requirement for the library.

def package_info(self):
    self.cpp_info.components["lib"].libs = ["library"]
    self.cpp_info.components["lib"].requires = ["libdep::libdep", "exedep::exedep"]

Even though the exedep::exedep requirement is only used by the executable. When executable components would be introduced, this could be expressed in code as:

def package_info(self):
    self.cpp_info.components["lib"].libs = ["library"]
    self.cpp_info.components["lib"].requires = ["libdep::libdep"]
    self.cpp_info.components["exe"].executable = "program"
    self.cpp_info.components["exe"].requires = ["lib", "exedep::exedep"]
jgsogo commented 4 years ago

Hi! Qt is a really good example and a challenging one for the cross-building scenario. First step will be to add the executables to the cpp_info, but then we need to understand how to work with them from within CMake. This is the same scenario as protoc.

The problem with these scenarios arises when we enter the cross-building with the two contexts: host and build. If we are cross-building to ARM a package that depends on Qt, we want:

Here, libraries and the moc executable come from completely different packages. The native building is just a concrete scenario of the general cross-building one.

The consequence of this is that you shouldn't be using Qt::moc inside the CMakeLists.txt because it won't be the executable you want. You should use moc.exe and the environment outside CMake should be responsible of adding to the PATH the moc.exe that is in your graph as a build_requires.

steffenroeber commented 4 years ago

If only moc.exe is in PATH, automoc will not work. automoc needs a target called Qt5::moc

jgsogo commented 4 years ago

I've been having a quick look to the Qt docs and to Google. I've found this question in Stackoverflow (_"CMake CMAKEAUTOMOC in cross compilation") with the answer I'm copy/pasting here for reference:

I finally found the solution thanks to this post: https://stackoverflow.com/questions/36570791/how-can-i-use-cmakes-automoc-feature-with-a-custom-qt-package. As I assumed the QT_MOC_EXECUTABLE isn't use directly by AUTOMOC.

Before first qt find_package following lines must be added:

   set(QT_MOC_EXECUTABLE /usr/bin/moc)
   add_executable(Qt5::moc IMPORTED)
   set_property(TARGET Qt5::moc PROPERTY IMPORTED_LOCATION ${QT_MOC_EXECUTABLE})

The issue here was that not only the variable QT_MOC_EXECUTABLE has to be set to proper value but finally the automoc uses just Qt5:moc target which must be declared before any qt package will be included in CMakeList.txt file.

This same issue is with other qt tools so more generic option will be:

   file(GLOB Qt_bin /usr/bin)
   find_program(QT_MOC_EXECUTABLE qt_moc moc PATHS ${Qt_bin})
   add_executable(Qt5::moc IMPORTED)
   set_property(TARGET Qt5::moc PROPERTY IMPORTED_LOCATION ${QT_MOC_EXECUTABLE})

One cannot use the Qt5::moc provided by the package in a general scenario. I still don't know how we can handle this situation in Conan, the recipe for Qt is WIP in ConanCenter, and next major Conan version will use the two profiles as the default way to work. By that time (or before), we need to have answered these questions.

danimtb commented 4 years ago

Somehow related to https://github.com/conan-io/conan-center-index/pull/2740/files#r481107458

ericLemanissier commented 4 years ago

also: https://github.com/conan-io/conan/issues/7444

ohanar commented 4 years ago

@jgsogo Ignoring the various Qt specific variables (which should be handled by build_modules), I guess I don't really see Conan having the same issue as Qt's builtin cmake scripts. They only have (and can assume) the single distribution of Qt for those scripts, so it makes sense that they don't work for cross compilation -- although thankfully that seems to be easily worked around with a toolchain per the stack overflow post.

I guess the way I see it working for conan is that the generated cmake scripts would have the library targets from the requires and the executable targets from the build_requires. If a package has a common require and build_require (e.g. Qt), these two would have to be merged. Of course this needs something like #7324 so that you don't pull an executable's potentially conflicting library requirements into cmake.

I don't really know how capable other generators are at handling executables, so I'm not really sure what is needed to be considered there.

Also, as a note with Qt: to cross-compile Qt, you need Qt on the build system, i.e. Qt is a build_requires of Qt when cross-compiling. I don't know if conan will handle this at the moment, although I see no reason why conan shouldn't properly handle this (once the build and host graphs are disjoint).

jgsogo commented 4 years ago

Hi, @ohanar , thanks for your comments.

I guess the way I see it working for conan is that the generated cmake scripts would have the library targets from the requires and the executable targets from the build_requires

Yes, this would be a way to go, merging two different nodes (the one from the build-requires and the one from the requires) into a single generated FindXXXX.cmake file. This can be very challenging (of course not before a new graph model for Conan 2.0), but still, it has some drawbacks: executables from host might be used by an emulator to run testing (I haven't checked if it works with targets, but it should), so not always these targets can refer to the build ones.

Then I step back and think if it is those CMake's functions/macros the ones that are broken, and CMake targets should always be the artifacts from the host context and those functions should never use them to run protoc or any executable. Cmake would need something else, those scripts should try to find the executable somehow (in the PATH) first and then fallback to the target defined inside the library only if it is not a cross-compiling scenario. I'm not sure if here we are trying to workaround something broken in 3rd party scripts... instead of patching those scripts to do the right thing.

SpaceIm commented 3 years ago

FYI, here is a POC of executables imported targets in geographiclib recipe, working with conan 1.32.0 https://github.com/conan-io/conan-center-index/pull/4295/commits/815254d31f7f345688a7d6c18e58c8c6b2146cd0

lasote commented 2 years ago

There is another use case that might be valid for this cpp_info new model, the COM objects from @thorntonryan

https://github.com/conan-io/conan/pull/11601#issuecomment-1243880002

Importantly the consuming application doesn't link directly against these DLLs. Instead, COM objects are created in a variety of ways and can come from within the same process, a different process, or even a different computer entirely. Ordinarily the COM servers are registered on a machine using regsvr32 and the consuming application loads them at runtime via COM machinery. Registration free techniques also exist for consumers to leverage.

Fortunately, our needs are simple. We really just need a CMake target that knows where these DLLs live, that way we can feed them into other custom CMake tooling that produces language projections (i.e. C++ header files) for our COM libraries based on the interface definitions (i.e. type library) embedded inside the DLLs.

Prior to CMakeDeps, we were creating our own IMPORTED targets for these COM servers and using IMPORTED_LOCATION to describe where the DLLs sat on disk. This way we had a CMake target that knew where the files were.

But since CMakeDeps doesn't really have a way to model COM servers, when the resulting CMake targets are created, they don't know where these DLLs live, and so we have to work a little harder to set custom properties that tell us their location.

I'm honestly not sure what's the best approach to take. I transitioned off my CMakeDeps spike back to cmake generator, but IIRC CMakeDeps is producing INTERFACE targets instead of IMPORTED ones. And we have some questions about correct usage of reserved CMake properties like INTERFACE* or IMPORTED*.

I'm assuming that however you model exes will result in a CMake target that knows where that exe sits on disk, that way you can use the CMake target in CMake commands, etc. If so that's exactly the sort of thing we're looking to do with our COM servers.

samuel-emrys commented 2 years ago

@jgsogo said..

IMO we should encourage raw executables over targets (protoc over protobuf::protoc) in CMakeLists.txt, otherwise we might be blocking cross-building scenarios.

Not considering cross compilation

Stepping away from the cross compilation problem where the build and host profiles are different for a second, how would this work for a more vanilla use case, where a package only distributes an executable? The use case I have in mind in particular is sphinx-build, when used to generate documentation - example in https://github.com/conan-io/conan/pull/11601.

Using the target, an exemplar of what I've been using is:

find_package(sphinx REQUIRED)

# Sphinx configuration
set(SPHINX_SOURCE ${CMAKE_CURRENT_SOURCE_DIR})
set(SPHINX_BUILD ${CMAKE_CURRENT_BINARY_DIR}/sphinx)
set(SPHINX_INDEX_FILE ${SPHINX_BUILD}/index.html)

add_custom_command(
  OUTPUT ${SPHINX_INDEX_FILE}
  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/index.rst
  COMMAND sphinx::sphinx-build -b html ${SPHINX_SOURCE} ${SPHINX_BUILD}
  WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
  COMMENT "Generating documentation with Sphinx")

With your proposal, without using the cmake target, the following change is made:

-   COMMAND sphinx::sphinx-build -b html ${SPHINX_SOURCE} ${SPHINX_BUILD}
+  COMMAND sphinx-build -b html ${SPHINX_SOURCE} ${SPHINX_BUILD}

Do you see any danger here that an ambiguity has been introduced as to the necessity/purpose of the find_package(sphinx) directive?

To be explicit about what that ambiguity is:

Considering Cross Compilation

Coming back to the problem of cross compilation, as you say, the only executable we would ever be interested in here would be the executable exported in the build environment. Wouldn't it then make sense to:

include(${CMAKE_CURRENT_LIST_DIR}/mypkg-build-config.cmake)

where the contents of mypkg-build-config.cmake is something approximating:

if(NOT TARGET mypkg::exe)
    add_executable(mypkg::exe IMPORTED)
    set_target_properties(mypkg::exe PROPERTIES IMPORTED_LOCATION /home/user/.conan/data/mypkg/0.1.0/_/_/package/<package id>/bin/mypkg_exe)
endif()

Then, the exe target mypkg::exe would only exist when a package is used in the build context, not solely in the host context.

This appears to be the pattern that's already being used for different build_type values, so it would be consistent with the current approach, just adding the additional dimension of build vs host profile rather than just build type.

I can't think of a use case for including an executable in the host context, but if it were needed, that could also be accounted for by making it a component property or something, where you have to choose either build or host context, which would switch which mypkg-{,build-}config.cmake it's generated in. Alternatively, perhaps it could make it into the new requirements modelling:

def requirements(self):
    # default exes=False, this indicates that exes should be 
    # available in host context
    self.requires("protobuf/x.y.z", exes=True) 

def build_requirements(self):
    # default exes=True. Validation would need to occur
    # to ensure that only one profile has exes=True for each
    # package required in both
    self.tool_requires("protobuf/x.y.z", exes=False) 

Perhaps this is what you were trying to get at in your comments above and I just misunderstood. Regardless, I think that this would be a reasonable first step to modelling these executable requirements, even if it's not perfect. Having an implementation modelling build executables would help us more readily evaluate the importance of modelling executables in the host context. This can be an iterative process - we don't need to have a perfect solution before we do anything. We don't need perfect to get in the way of useful.

User interface

I think, as a user, using the target is a much more preferable interface to hoping that conan injects the PATH correctly behind the scenes so that the program name resolves correctly. Using the target is much more traceable way of ensuring that there won't be any conflict with system executables, and is generally the way that CMake expects executables to be used, particularly because of the way it resolves/substitutes paths when executable targets are used with add_custom_command.

samuel-emrys commented 1 year ago

@jcar87 @memsharded @lasote how is this going? Any updates to share?

samuel-emrys commented 8 months ago

@memsharded is this on your horizon/roadmap at all?

peakschris commented 6 months ago

What is the current status of this? In the absence of this feature, what is the current best practice for supplying package_info for an executable?

memsharded commented 6 months ago

@peakschris the thing is that this model hasn't been prioritized because for the majority of cases, it is not necessary.

Conan is already automatically adding dependencies packages bindirs from tools_requires to the PATH, LD_LIBRARY_PATH, etc, as necessary. So it is not necessary to do anything specific to use those executables from consumers, a normal self.run("mydep_exe") will work, or even if mydep_exe is ran inside a build script, it will still work.

peakschris commented 6 months ago

@memsharded ok, I can see that.

My case is I'm using Bazel to consume conan packages, and bazel requires executables to be wrapped in a sh_binary rule before they are used in the toolchain. E.g:

sh_binary(
    name = "flatc.exe",
    srcs = ["bin/flatc.exe"],
    visibility = ["//visibility:public"],
)

The bazel/conan integration already creates wrapper rules for each library. I would guess that it would need some metadata for each binary in order to allow it to create these wrapper rules for executables.

memsharded commented 1 month ago

We are releasing in Conan 2.9 a completely new CMakeDeps generator in https://github.com/conan-io/conan/pull/16964 that has closed this ticket, with many pending features and fixes:

Current known pending functionality (to be added soon):

The new CMakeDeps generator is intended for testing and validation only, being a transparent replacement of the old one, so it is behind a new conf. To use it, use the -c tools.cmake.cmakedeps:new=will_break_next, and that will use the new generator instead of the old one. Note the will_break_next value means exactly that, that value will change in next release to force a break, so no one can depend on this generator in production yet.

Your feedback is very important

As this is a major change, we will only remove the conf gate when we get confirmation from users that it works and solve the issues. Please try the new generator for your project, and let us know if it works. If it doesn't, please re-open this ticket and let us know what failed. Thanks very much!