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

CMake: pkg-config Fallback #133

Merged
merged 3 commits into from Jul 20, 2017
Merged

Conversation

ax3l
Copy link
Contributor

@ax3l ax3l commented Jul 14, 2017

Try to find installs of libzmq that were performed with autotools with a pkg-config fallback. Close #132

@herbrechtsmeier we are trying to add a pkg-config fallback for cppzmq's CMakeLists.txt to find libzmq installs that were done with autotools and thus have no ZeroMQ cmake module around.

Do you have an idea how we can re-introduce the targets you are using for the libs when found via pkg-config?

@herbrechtsmeier
Copy link

I'm unsure if pkg-config is a good solution. But you could use the variables instead of the targets. But you need to call find_path to detect the include path.

I would recommand a FindZeroMQ.cmake and append its path to the CMAKE_MODULE_PATH. This could be installed by this project and could be used from the CMakeLists.txt and the cppzmqConfig.cmake.in. Otherwise you have to add the pkg-config fallback to both files.

You could also use the add_library function together with the IMPORTED signature. Therefore you have to copy and adapt more or less the code from the generated target files of libzmq.

@ax3l
Copy link
Contributor Author

ax3l commented Jul 17, 2017

Thanks a lot for the feedback!

I guess the first option is unlikely to happen since it was already dismissed in #110 & #127.

You could also use the add_library function together with the IMPORTED signature. Therefore you have to copy and adapt more or less the code from the generated target files of libzmq.

thanks, likely a

 add_library(libzmq SHARED IMPORTED)
 set_property(TARGET libzmq PROPERTY IMPORTED_LOCATION ZeroMQ_LIBRARY)
 add_library(libzmq-static STATIC IMPORTED)
 set_property(TARGET libzmq-static PROPERTY IMPORTED_LOCATION ZeroMQ_STATIC_LIBRARY)

will already solve it.

@ax3l ax3l changed the title [WIP] CMake: pkg-config Fallback CMake: pkg-config Fallback Jul 17, 2017
@ax3l
Copy link
Contributor Author

ax3l commented Jul 17, 2017

@bluca it works! :)

export PKG_CONFIG_PATH=$LIBZMQ_PREFIX/lib/pkgconfig

cmake ..
make install

and off it goes :)

@bluca
Copy link
Member

bluca commented Jul 17, 2017

Great stuff, thanks!
@herbrechtsmeier is this solution fine for you?

@herbrechtsmeier
Copy link

Are you sure that it is correct to use the variables without ${} inside the set_property function?

You don't touch the package configuration cppzmqConfig.cmake.in. Have you test to compile an external package which use find_package(cppzmq)?

Additionally you need to add the header path to the targets.

@ax3l
Copy link
Contributor Author

ax3l commented Jul 18, 2017

Thanks again for the review!

Are you sure that it is correct to use the variables without ${} inside the set_property function?

True, following this example page it should use ${}. Updated it, nevertheless I did not see a difference in the install dir.

You don't touch the package configuration cppzmqConfig.cmake.in.

I didn't think that's not necessary when looking into it, but checking the installed cppzmqConfig.cmake shows we need to add the same logic there, yes! -> updated!

Have you test to compile an external package which use find_package(cppzmq)?

I just tested make install so far. This installed the two header files and the cmake files.

I now tested also a dependent project with a main.cpp and a CMakeLists.txt as follows:

# project
cmake_minimum_required(VERSION 3.0.0)
project(ZeroMQexample)

# dependencies
find_package(cppzmq REQUIRED)

# executable
add_executable(main main.cpp)

target_link_libraries(main cppzmq)

which still fails while looking for a ZeroMQConfig.cmake since the installed cppzmqConfig.cmake is not yet using the same logic. -> added! -> now it only fails on the headers. -> ok, also fixed now! :)

Additionally you need to add the header path to the targets.

Sure, can you show me how please? :)
I thought that is already done in the foreach (target cppzmq cppzmq-static) step.
update: ah, you are talking about the libzmq C-headers! (zmq.h)
-> updated, works now! :)

Try to find installs of `libzmq` that were performed with
autotools with a `pkg-config` fallback.
@herbrechtsmeier
Copy link

Instead of the duplicated code I would recommend to move the code into its own cmake file and install and include this file in both files.

Even better would be a FindZeroMQ.cmake module. This would reduce the changes inside the CMakeLists.txt and cppzmqConfig.cmake.in to a list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}) before the find_package / find_dependency call and an additional ${CMAKE_CURRENT_BINARY_DIR}/FindZeroMQ.cmake line inside the install(FILES ...) function.

I just released that there is already a FindZeroMQ.cmake inside an other ZeroMQ project which works without pkg-config:
https://github.com/zeromq/azmq/blob/master/config/FindZeroMQ.cmake

@bluca
Copy link
Member

bluca commented Jul 18, 2017

Isn't the point of a lot of recent cmake changes in libzmq to remove the need for dependencies to ship a findzeromq file?

@herbrechtsmeier
Copy link

@bluca The FindZeroMQ file is only needed for the autotools build system.

@bluca
Copy link
Member

bluca commented Jul 18, 2017

Rather than shipping additional files which look like are needed by cmake but are actually for autotools, isn't it better to just use pkg-config which autotools already ships?

move the new logic to a simple file so we
do not add code duplication
@ax3l
Copy link
Contributor Author

ax3l commented Jul 18, 2017

Instead of the duplicated code I would recommend to move the code into its own cmake file and install and include this file in both files.

Sure, I just removed the code duplication.

@ax3l
Copy link
Contributor Author

ax3l commented Jul 18, 2017

Even better would be a FindZeroMQ.cmake module

One could also make this a module, but in such case rather a FindZeroMQFallback.cmake module since one would not want to overwrite the default behavior to look for an existing "real" libzmq cmake module on find_package.

I think the solution for now is a good bridge between the autotools and the CMake world, so dependent projects can build safely with CMake.

For the long run, it's not a good idea to support two build systems in libzmq itself because one will always be bridging back and forth if the installs are not fully identically.

@herbrechtsmeier
Copy link

The FindZeroMQ.cmake is needed by cmake and not by autotools.

The problem is that libzmq support different build systems and the autotools build don't install any cmake support. Hence all follow-up project needs do deal with the different build systems. Best would be to remove the autotools support from libzmq or let autotools install a FindZeroMQ.cmake.

Because of its limitations it isn't recommended to use pkg-config inside a cmake project.

@herbrechtsmeier
Copy link

@ax3l You are right. You need to call find_package(ZeroMQ CONFIG) to bypass the FindZeroMQ.cmake module.

@bluca
Copy link
Member

bluca commented Jul 18, 2017

Sorry, autotools is not going anywhere. Also work was recently done to remove the findzeromq file, so that's not coming back either.

pkgconfig is universal and well supported, so I would suggest sticking with that as the fallback, when the cmake files are not found.

@ax3l
Copy link
Contributor Author

ax3l commented Jul 18, 2017

I think we can live with that, now that we have a marvelous bridge in place 🦄

@ax3l
Copy link
Contributor Author

ax3l commented Jul 19, 2017

@bluca @herbrechtsmeier are you ok with the current state of the PR for a merge? :)

CMake targets / interfaces need at least CMake 3.0
@herbrechtsmeier
Copy link

@bluca What is the reason to support two build systems with different outputs? Why the autotools build don't provide also a native CMake support?

With this patch you use different code to detected libzmq inside czmq and cppzmq. The first project use a find package and the second project bypass the find package function but do similar things. This could be avoid with a common cmake support inside libzmq.

What do you mean by pkg-config is universal and well supported? There is a reason why cmake avoid the usage of pkg-config inside its modules.

@ax3l
Copy link
Contributor Author

ax3l commented Jul 20, 2017

@herbrechtsmeier I do understand your pain but discussing this here goes off-topic for this PR. Is it ok for you to just open a proposal in the issues of https://github.com/zeromq/libzmq ? :)

@bluca
Copy link
Member

bluca commented Jul 20, 2017

  • We have to support multiple build systems, not only 2 (for example visual studio, and soon meson hopefully) because no single one can fit in all use cases, and more simply some users have different preferences and we have to respect that. Not everyone likes CMake for example.
    An autotools-generated distributable tarball is the preferred delivery method for distributions, for embedded devices and for bootstrapping new architectures, as only a compiler is needed and the autotools-generated configure scripts are self-contained.
    The tarball can't provide generated CMake files because that would require adding CMake to the toolchain and running it.

  • I will add the same fallback to zproject soon, so that CZMQ and others can use it too. Given it's just a fallback in case the main method fails, which uses native CMake functions.

  • All distributions and *nix systems support pkg-config, even OSX. Even 10 years old versions of Solaris. We have to support all of them, and CMake is often not available, especially the latests versions that are required here. Again a distributable tarball that ships a pkg-config file will just work.
    And it's the preferred method for all Linux distributions.

@bluca bluca merged commit a89d35b into zeromq:master Jul 20, 2017
@herbrechtsmeier
Copy link

Isn't visual studio supported by CMake? How many distributions don't offer a CMake package? CMake even supports bare metal cross compilation. Additionally the packages are relocatable and support dependency tracking. And nobody needs to use CMake for its project even if this project use CMake.

But even if you support different build systems you should support a common build system interface for follow up projects. You should really add CMake support to autotools. Otherwise you should remove the CMake package generation and only support pkg-config. But even this leads to a cmake file if it is called libzmqPkgConfigFallback.cmake or FindZereMQ.cmake. Additionally if the "autotools-generated distributable tarball is the preferred delivery method for distributions" the fallback will be the main interface.

Autotools could provide generated CMake files without CMake because it only needs to replace some variables inside a ZeroMQConfig.cmake text file like it does for pkg-config.

This project requires a three year old CMake version. Attentionally the pkg-config file could also be generated by CMake.

With your changes you bring the FindZeroMQ.cmake file back. You only rename the file and use an include instead of the find_package(ZeroMQ) function. Additionally you distribute the file over different projects.

I could open a new issue in libzmq but this makes only sense if there is interests in an autotools support for CMake.

@ax3l ax3l deleted the fix-pkgConfigAutotools branch July 21, 2017 07:41
@ax3l
Copy link
Contributor Author

ax3l commented Jul 21, 2017

@bluca just to stay on topic of the PR: can you ping me pls when a v4.2.2 of cppzmq is released so I can add a package to spack which we are using to build/ship our downstream project that brought me here? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants