sysrepo / sysrepo-cpp

C++ bindings for the sysrepo library
BSD 3-Clause "New" or "Revised" License
7 stars 6 forks source link

Sysrepo subscriber type enum mismatch #1

Closed mstaflex closed 2 years ago

mstaflex commented 2 years ago

When compiling master on https://github.com/sysrepo/sysrepo-cpp/commit/0f6791ec2102d2761c2deff67954ee6eed1cff43 I run into enum conflicts that are picked up by your static asserts:

Step 32/40 : RUN     cmake -D BUILD_TESTING=0 -D WITH_DOCS=0 ..; make; make install
 ---> Running in 84d65f9f5814
-- The CXX compiler identification is GNU 10.2.1
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Git: /usr/bin/git (found version "2.30.2") 
-- Could NOT find Doxygen (missing: DOXYGEN_EXECUTABLE) 
-- Found PkgConfig: /usr/bin/pkg-config (found version "0.29.2") 
-- Checking for modules 'libyang-cpp=alpha;libyang-cpp'
--   Found libyang-cpp, version alpha
--   Found libyang-cpp, version alpha
-- Checking for modules 'sysrepo>=2.0.37;sysrepo'
--   Found sysrepo, version 2.0.53
--   Found sysrepo, version 2.0.53
-- Configuring done
-- Generating done
-- Build files have been written to: /app/sysrepocpp/build
[ 16%] Building CXX object CMakeFiles/sysrepo-cpp.dir/src/Connection.cpp.o
In file included from /app/sysrepocpp/src/Connection.cpp:14:
/app/sysrepocpp/src/utils/enum.hpp:60:62: error: static assertion failed
   60 | static_assert(toSubscribeOptions(SubscribeOptions::NoThread) == SR_SUBSCR_NO_THREAD);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:61:61: error: static assertion failed
   61 | static_assert(toSubscribeOptions(SubscribeOptions::Passive) == SR_SUBSCR_PASSIVE);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:62:62: error: static assertion failed
   62 | static_assert(toSubscribeOptions(SubscribeOptions::DoneOnly) == SR_SUBSCR_DONE_ONLY);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:63:61: error: static assertion failed
   63 | static_assert(toSubscribeOptions(SubscribeOptions::Enabled) == SR_SUBSCR_ENABLED);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:64:60: error: static assertion failed
   64 | static_assert(toSubscribeOptions(SubscribeOptions::Update) == SR_SUBSCR_UPDATE);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:65:63: error: static assertion failed
   65 | static_assert(toSubscribeOptions(SubscribeOptions::OperMerge) == SR_SUBSCR_OPER_MERGE);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:66:67: error: static assertion failed
   66 | static_assert(toSubscribeOptions(SubscribeOptions::ThreadSuspend) == SR_SUBSCR_THREAD_SUSPEND);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [CMakeFiles/sysrepo-cpp.dir/build.make:76: CMakeFiles/sysrepo-cpp.dir/src/Connection.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:85: CMakeFiles/sysrepo-cpp.dir/all] Error 2
make: *** [Makefile:136: all] Error 2
Consolidate compiler generated dependencies of target sysrepo-cpp
[ 16%] Building CXX object CMakeFiles/sysrepo-cpp.dir/src/Connection.cpp.o
In file included from /app/sysrepocpp/src/Connection.cpp:14:
/app/sysrepocpp/src/utils/enum.hpp:60:62: error: static assertion failed
   60 | static_assert(toSubscribeOptions(SubscribeOptions::NoThread) == SR_SUBSCR_NO_THREAD);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:61:61: error: static assertion failed
   61 | static_assert(toSubscribeOptions(SubscribeOptions::Passive) == SR_SUBSCR_PASSIVE);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:62:62: error: static assertion failed
   62 | static_assert(toSubscribeOptions(SubscribeOptions::DoneOnly) == SR_SUBSCR_DONE_ONLY);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:63:61: error: static assertion failed
   63 | static_assert(toSubscribeOptions(SubscribeOptions::Enabled) == SR_SUBSCR_ENABLED);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:64:60: error: static assertion failed
   64 | static_assert(toSubscribeOptions(SubscribeOptions::Update) == SR_SUBSCR_UPDATE);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:65:63: error: static assertion failed
   65 | static_assert(toSubscribeOptions(SubscribeOptions::OperMerge) == SR_SUBSCR_OPER_MERGE);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~
/app/sysrepocpp/src/utils/enum.hpp:66:67: error: static assertion failed
   66 | static_assert(toSubscribeOptions(SubscribeOptions::ThreadSuspend) == SR_SUBSCR_THREAD_SUSPEND);
      |               ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [CMakeFiles/sysrepo-cpp.dir/build.make:76: CMakeFiles/sysrepo-cpp.dir/src/Connection.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:85: CMakeFiles/sysrepo-cpp.dir/all] Error 2
make: *** [Makefile:136: all] Error 2

Comparing this with sysrepo Version 2.0.53 in src/sysrepo_types.h (https://github.com/sysrepo/sysrepo/blob/39bc5adb98026af355febcca5abf27d44ebf6f10/src/sysrepo_types.h#L345) the former values were actually correct.

Only

 /**
     * @brief This option enables the application to re-use an already existing subscription context previously returned
     * from any sr_*_subscribe call instead of requesting the creation of a new one. In that case a single
     * ::sr_unsubscribe call unsubscribes from all subscriptions filed within the context.
     */
    SR_SUBSCR_CTX_REUSE = 1,

was missing from the list.

mstaflex commented 2 years ago

Maybe it would be a good idea to not push directly to master, which indicates a stable state version, but do changes like that on either development branches, or feature branches altogether? :)

syyyr commented 2 years ago

Hi, please update your sysrepo version, they have changed their API. We are generally tracking the devel branch of sysrepo.

https://github.com/sysrepo/sysrepo/blob/devel/src/sysrepo_types.h#L354

syyyr commented 2 years ago

I've updated the required version of sysrepo, I should have done that with the API changes, oh well.

https://gerrit.cesnet.cz/c/CzechLight/sysrepo-cpp/+/5337

mstaflex commented 2 years ago

Ah ok, thx for coming back to quickly.. will try that out :)

mstaflex commented 2 years ago

From the outside perspective that is a bit obscure that the master here tracks devel on sysrepo ;)

syyyr commented 2 years ago

I would have to implement the API changes anyway at some point. Quite a lot of features are implemented in sysrepo-cpp already, but I still consider them a work in progress, so I think that tracking the latest sysrepo is fine. As of now, sysrepo/devel equals sysrepo/master.

Thanks for catching the version, however, I didn't realize that I should increase it!

jktjkt commented 2 years ago

And to provide a bit more of an additional context to what @syyyr has already said -- we're writing these bindings and porting our apps to them at the same time. We still keep changing the APIs a little bit, so there's no API or ABI compatibility just yet. We also bump into some bugs of upstream sysrepo every now and then, so in general it's better for us to follow the upstream development closely -- hence we're targeting their devel branches. There are, however, some technical reasons in our CI/CD stack why we prefer to call our branches master and not devel.