Skip to content
This repository was archived by the owner on Apr 7, 2022. It is now read-only.

Conversation

@t-b
Copy link
Contributor

@t-b t-b commented Oct 25, 2019

We do require the header for compilation but do currently not check for
it. The code snippet ensures that we get a recent
version with zmq::socket_t::disconnect method. This snipped is copied
from cppTango/6402affe (Use feature tests for checking if we have
zmq::socket::disconnect, 2019-10-23).

I'm not very familiar with autotools, but that works here.

@bourtemb
Copy link
Member

bourtemb commented Oct 28, 2019

Thanks for creating this PR and looking into this. I am still a newbie with the autotools.

It looks like this AC_CHECK_HEADERS is looking for system headers zmq.hpp and will complain if it is not found in the include system directories.
configure should work even if the user didn't install libzmq via its Linux Distribution packaging system and if the user installed libzmq and cppzmq in a custom directory and use the --with-zmq configure option to specify the zmq install root directory, or using PKG_CONFIG_PATH environment variable to point to the zmq root directory.
It looks like this AC_CHECK_HEADERS(zmq.hpp, ...) test is failing in this use case, even though libzmq >= 4.0.5 test passed.

@bourtemb
Copy link
Member

But maybe here I am doing the wrong assumption that zmq.hpp include file is installed in the same directory as libzmq.h file (which was the case in my test)?

We do require the header for compilation but do currently not check for
it. The code snippet ensures that we get a recent
version with zmq::socket_t::disconnect method. This snipped is copied
from cppTango/6402affe (Use feature tests for checking if we have
zmq::socket::disconnect, 2019-10-23).

Saving and restoring CXXFLAGS allows us to honour a custom zmq-prefix
which makes it possible to have libzmq and cppzmq in different
locations.

So the following now works:

git clone https://github.com/zeromq/libzmq
cd libzmq
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=$HOME/libzmq -DENABLE_DRAFTS=OFF ..
make
make install

git clone https://github.com/zeromq/cppzmq
cd cppzmq
mkdir build
cd build
export CMAKE_PREFIX_PATH=$HOME/libzmq/share/cmake/ZeroMQ
cmake -DCMAKE_INSTALL_PREFIX=$HOME/cppzmq -DENABLE_DRAFTS=OFF -DCPPZMQ_BUILD_TESTS=OFF ..
make
make install

./configure --with-zmq=$HOME/libzmq CXXFLAGS="-I $HOME/cppzmq/include"
make
@t-b
Copy link
Contributor Author

t-b commented Oct 29, 2019

@bourtemb I've fixed one issue regarding different locations for libzmq and cppzmq. The commit message has detailed instructions for testing. This works here on debian stretch.

@t-b t-b force-pushed the check-for-correct-zmq-hpp branch from 4407f27 to c6a3601 Compare October 29, 2019 15:24
@bourtemb
Copy link
Member

I propose the following patch to support some additional older versions of zmq.hpp file which were already supporting the disconnect method but didn't have a zmq::context_t default constructor:

--- a/assets/configure.ac
+++ b/assets/configure.ac
@@ -205,7 +205,7 @@ AC_CHECK_HEADERS(zmq.hpp, [], [AC_MSG_ERROR([Couldn't find a compatible zmq.hpp]
 
 int main(int, char**)
 {
-  zmq::context_t c;
+  zmq::context_t c(1);
   zmq::socket_t s(c, ZMQ_REQ);
   s.disconnect("some endpoint");
 }])

This could be useful for people who try to use the zmq.hpp file which was provided in some previous Tango distributions (this file is still compatible):
https://github.com/tango-controls/cppTango/blob/cppapi_Release_9_2_5/cppapi/client/zmq.hpp

@t-b
Copy link
Contributor Author

t-b commented Oct 30, 2019

@bourtemb Sounds good. I'll make that change here and in the cppTango repo as well.

We used to ship a version of zmg.hpp with cppTango, see 5144cb80 (Add
again zmq.hpp before tagging release 9.2.5 to be consistent with 9.2.5
version from SVN Sourceforge (#348), 2017-01-11) in
tango-controls/cppTango.

And in that version there was no default constructor for zmq::context_t
available. The constructor taking an integer parameter is still
available in newer versions so we can just use that here.
@t-b
Copy link
Contributor Author

t-b commented Oct 31, 2019

@bourtemb Good point. Done.

Copy link
Member

@bourtemb bourtemb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks!

@t-b t-b merged commit f82d34b into tango-controls:master Oct 31, 2019
@t-b t-b deleted the check-for-correct-zmq-hpp branch October 31, 2019 18:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants