Le dim. 9 janv. 2022 à 08:27, Gerhard Sittig <gerhard.sit...@gmx.net> a écrit : > > On Mon, 2022-01-03 at 08:32 +0100, Fabrice Fontaine wrote: > > > > Support glibmm 2.68 which has been released one year ago and is the > > first stable release in the glibmm-2.68 ABI series: > > https://gitlab.gnome.org/GNOME/glibmm/-/blob/2.68.2/NEWS > > > > As TimeVal is not available with glibmm 2.68, use DateTime which is > > available since version 2.26 > > As mentioned earlier I'm not familiar with C++, and my autotools > foo is rather weak. So take that response with a grain of salt. > > > --- a/README > > +++ b/README > > @@ -63,7 +63,7 @@ Requirements for the C++ bindings: > > - doxygen (required for building the bindings, not only for C++ API docs!) > > - graphviz (optional, only needed for the C++ API docs) > > - Python (2 or 3) executable (development files are not needed) > > - - glibmm-2.4 (>= 2.32.0) > > + - glibmm-2.4 (>= 2.32.0) or glibmm-2.68 (>= 2.68.0) > > Mention preferred versions before "also acceptable" fallbacks > perhaps? Checks probably also should reflect preference. glibmm-2.4 is preferred over glibmm-2.68 in the README and in configure.ac (see below). I'll update the README. > > > header->feed_version = 1; > > - header->starttime.tv_sec = start_time.tv_sec; > > - header->starttime.tv_usec = start_time.tv_usec; > > + header->starttime.tv_sec = start_time.to_unix(); > > + header->starttime.tv_usec = start_time.get_microsecond(); > > auto packet = g_new(struct sr_datafeed_packet, 1); > > Is this source code change compatible with either version? The > one that you target anew, as well as the versions that current > mainline already supports? A library that has only been around > for one year is rather new an external dependency, given how many > platforms the sigrok project supports and that execution on older > systems is a stated goal. Consider users' and co-developers' > needs as well as distributors' processes. > > If the data type change and corresponding source update works for > glibmm-2.4 already, then I suggest to separate it from the 2.68 > upgrade. Simplifies review and test, and thus mainline integration. As stated in the commit message, DateTime is available since version 2.26. I'll split the patch. > > > --- a/configure.ac > > +++ b/configure.ac > > @@ -407,9 +407,14 @@ AS_IF([test "x$HAVE_CXX11" != x1], > > [SR_APPEND([sr_cxx_missing], [', '], ['C++11'])]) > > > > # The C++ bindings need glibmm. > > -SR_PKG_CHECK([glibmm], [SR_PKGLIBS_CXX], [glibmm-2.4 >= 2.32.0]) > > +SR_GLIBMM_REQUIRES=glibmm-2.4 > > +SR_PKG_CHECK([glibmm], [SR_PKGLIBS_CXX], [$SR_GLIBMM_REQUIRES >= 2.32.0]) > > +AS_IF([test "x$sr_have_glibmm" != xyes], > > + [SR_GLIBMM_REQUIRES=glibmm-2.68 > > + SR_PKG_CHECK([glibmm], [SR_PKGLIBS_CXX], [$SR_GLIBMM_REQUIRES >= > > 2.68.0])]) > > AS_IF([test "x$sr_have_glibmm" != xyes], > > [SR_APPEND([sr_cxx_missing], [', '], [glibmm])]) > > +AC_SUBST(SR_GLIBMM_REQUIRES) > > > > # The C++ bindings use Doxygen to parse libsigrok symbols. > > AC_CHECK_PROG([HAVE_DOXYGEN], [doxygen], [yes], [no]) > > This logic confuses me, can you come up with phrases that better > reflect what's happening? Ideally search in order of preference, That's what I'm doing, glibmm-2.4 is checked and then glibmm-2.68 (if needed). > search once then use whatever was found. The "conditional" check > for something newer after something acceptable yet older(?) was > found is counter intuitive. The fact that something is assigned I think that you misread the conditional: I'm searching for something newer (glibmm-2.68) if glibmm-2.4 was *not* found: AS_IF([test "x$sr_have_glibmm" != xyes], I'll add comments even if it means paraphrasing the code. > before the check actually happens is surprising. How robust is > that phrase, have you checked it in different configurations, > with new and with old libs and with none at all available? Some > machines could provide both versions while you seem to prefer the > newer implementation. I'm not preferring the newer version but the older as explained above. It works fine even when glibmm-2.4 and glibmm-2.68 are both installed. If none is installed, C++ binding is disabled as expected. > > > virtually yours > Gerhard Sittig > -- > If you don't understand or are scared by any of the above > ask your parents or an adult to help you. > > > _______________________________________________ > sigrok-devel mailing list > sigrok-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/sigrok-devel Best Regards,
Fabrice _______________________________________________ sigrok-devel mailing list sigrok-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/sigrok-devel
