nats.c

nats.c copied to clipboard

This will add a thin wrapper around the C functions to make the development in C++ less painfull. A python script will generate nats.hpp directly from nats.h with the Clang Python bindings and therefor no additional work is required for maintaining them.

cpp_generator.py needs some cleanup before merging, but I want some feedback about the general idea first.

To test it the installation of clang and mako (pip3 install clang mako) is required.

@paroga Thank you for the contribution. I have to say that I am not a C++ developer so it will be difficult for me to review and judge of the interest of this.

What I can say is that I feel that most users of the NATS C client are actually C++ developers, and I am not sure what difficulty they have encountered (or not) in using this library. I am pinging some users that I know are C++ users to ask their opinion: @rigtorp @pananton @danesh-d @etrochim. Would the change in this PR help you guys, or at least would not break anything in the way you are using the library (being optional, I don't think it would be a problem)?

There is another user: @HannaKraft that has opened a PR in the past for a C++ wrapper (https://github.com/nats-io/nats.c/issues/261), not sure if that would have been equivalent.

In conclusion, myself not being at all familiar with C++, I would like to have other opinions on the value of this proposed change. Thanks!

the three main selling points of my wrapper are:

• "automatic" memory management (e.g. you cannot miss to free a natsMsg* in a callback)
• more natural C++ syntax: nats::Msg msg(...); msg.GetSubject(); instead of natsMsg* msg; natsMsg_GetSubject(msg);
• no need for casting the context pointer (see examples/cpp/subscriber.cpp)

I am not sure what difficulty they have encountered (or not) in using this library

I personally do not see big difficulties. it has a nice and clean API. this wrapper just makes it possible to take advantage of the C++ features (RAII, better type system)

opened a PR in the past for a C++ wrapper (#261)

I saw that PR too, but I did not find any related code

@paroga Thank you for the contribution. I have to say that I am not a C++ developer so it will be difficult for me to review and judge of the interest of this.

What I can say is that I feel that most users of the NATS C client are actually C++ developers, and I am not sure what difficulty they have encountered (or not) in using this library. I am pinging some users that I know are C++ users to ask their opinion: @rigtorp @pananton @danesh-d @etrochim. Would the change in this PR help you guys, or at least would not break anything in the way you are using the library (being optional, I don't think it would be a problem)?

The example usage code looks good. We made some small wrappers for what we used and that's been good enough. My issue is more that nats.c itself is more of a framework than a library causing it to be hard to integrate in our application. Also all the heartbeat and timeout edge case bugs we need to work around.

@paroga You said that you wanted to do more cleanup first, so not merging as-is.

Also, I tried to test this on macOS and did pip3 install clang mako but then when trying to build it seems that the clang library is not found:

[ 32%] Generating ../nats.hpp
Traceback (most recent call last):
  File "/usr/local/lib/python3.7/site-packages/clang/cindex.py", line 4129, in get_cindex_library
    library = cdll.LoadLibrary(self.get_filename())
  File "/usr/local/Cellar/python/3.7.4/Frameworks/Python.framework/Versions/3.7/lib/python3.7/ctypes/__init__.py", line 442, in LoadLibrary
    return self._dlltype(name)
  File "/usr/local/Cellar/python/3.7.4/Frameworks/Python.framework/Versions/3.7/lib/python3.7/ctypes/__init__.py", line 364, in __init__
    self._handle = _dlopen(self._name, mode)
OSError: dlopen(libclang.dylib, 6): image not found

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "bindings/cpp_generator.py", line 387, in <module>
    main()
  File "bindings/cpp_generator.py", line 373, in main
    index = Index.create()
  File "/usr/local/lib/python3.7/site-packages/clang/cindex.py", line 2666, in create
    return Index(conf.lib.clang_createIndex(excludeDecls, 0))
  File "/usr/local/lib/python3.7/site-packages/clang/cindex.py", line 198, in __get__
    value = self.wrapped(instance)
  File "/usr/local/lib/python3.7/site-packages/clang/cindex.py", line 4103, in lib
    lib = self.get_cindex_library()
  File "/usr/local/lib/python3.7/site-packages/clang/cindex.py", line 4134, in get_cindex_library
    raise LibclangError(msg)
clang.cindex.LibclangError: dlopen(libclang.dylib, 6): image not found. To provide a path to libclang use Config.set_library_path() or Config.set_library_file().
make[2]: *** [nats.hpp] Error 1
make[1]: *** [src/CMakeFiles/nats_cpp_bindings.dir/all] Error 2

Also help me understand: is that something users will run in their local install or will we have to commit nats.hpp to this repo? If so, will have to look at dependencies for license purposes.

on Linux libclang.so gets provided by the clang development packages (e.g. libclang-dev. pip installs only the python wrapper for it. sorry for not mentioning that.

is that something users will run in their local install or will we have to commit nats.hpp to this repo?

ideally nats.hpp gets generated during a packaging step for a release. an alternative would be to add the file to the repo and let a CI step check if it's compatible to nats.h

Humm... so I tried on my Linux VM (ubuntu) and first I had to make this change to my cmake file:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -53,7 +53,7 @@ if(NATS_BUILD_WITH_TLS_CLIENT_METHOD)
 endif(NATS_BUILD_WITH_TLS_CLIENT_METHOD)
 
 if(NATS_BUILD_CPP_BINDINGS)
-  find_package(Python3 REQUIRED)
+  find_package(PythonLibs 3)
 endif(NATS_BUILD_CPP_BINDINGS)

apparently this is because of my CMake version.

Then, after installing python, pip3 and then pip3 install as you suggest, I am now getting:

[ 63%] Generating ../nats.hpp
/bin/sh: 1: bindings/cpp_generator.py: Permission denied
src/CMakeFiles/nats_cpp_bindings.dir/build.make:62: recipe for target 'nats.hpp' failed
make[2]: *** [nats.hpp] Error 126
CMakeFiles/Makefile2:1059: recipe for target 'src/CMakeFiles/nats_cpp_bindings.dir/all' failed
make[1]: *** [src/CMakeFiles/nats_cpp_bindings.dir/all] Error 2
Makefile:140: recipe for target 'all' failed
make: *** [all] Error 2

This worries me a little bit if I have to generate the nats.hpp file for each release and I am having these kind of issues. If it was entirely up to the user of the library to generate that, that would be ok, but if I am responsible to maintain it (and update for each release), I would like it to be friction free.

sorry for the troubles. could you tell me which Linux version do you want to use exactly? i just made it work with my system until now (since i wanted to get some general feedback frist), but would like to make it as friction free as possible if it gets accepted. what do you think about an docker container for generating nats.hpp in a cross platform way?

@paroga I run on a iMac with docker terminal a Ubuntu image:

root@6dfbc1beeb84:/home/ivan/cnats/build_linux# lsb_release -a
No LSB modules are available.
Distributor ID:	Ubuntu
Description:	Ubuntu 18.04.3 LTS
Release:	18.04
Codename:	bionic

Having a docker container that would be used to generate the header could work. I don't mind generating manually before each release (if it needs to be committed in this repo), but I would want to have no trouble generating it, that's all :-)

I applied the changes from @paroga to a current version of the code and created a Dockerfile to use the code generation from any platform. I also added the c++ header to doxygen and re-generated the docs.

The code can be found here: https://github.com/hardliner66/nats.c/tree/cpp

To generate the bindings do:

docker build -t nats.c .
docker run -v $(pwd)/src:/src -v $(pwd)/scripts:/scripts -v $(pwd)/src:/dest nats.c

I used two of the existing examples from the getting_started directory and rewrote them with the c++ header.

Biggest Problem right now is, that it wont build. Something somehow gets confused and it tries to use natsMsgList_Destroy in the destructor for nats::Msg, while MsgList seems to be missing completely. I think it somehow can't work with MsgList and overmatches somewhere, which put's the wrong function in the destructor. Haven't checked in detail tho.

The API for Subscribe seems overly verbose as well. Why not just use an abstract type the user can inherit from? It can only have one handle function anyway, might as well use something less dynamic.