From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wido den Hollander Subject: Re: librados.h version numbers Date: Fri, 9 Sep 2016 08:51:41 +0200 (CEST) Message-ID: <383545525.3921.1473403901626@ox.pcextreme.nl> References: <1843378866.3701.1473189527273@ox.pcextreme.nl> <37bd7608-3352-5057-d238-e04517640f5b@redhat.com> <77759347.3838.1473317968120@ox.pcextreme.nl> <1735254177.3909.1473341381129@ox.pcextreme.nl> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: Received: from smtp01.mail.pcextreme.nl ([109.72.87.137]:58091 "EHLO smtp01.mail.pcextreme.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751234AbcIIGvr (ORCPT ); Fri, 9 Sep 2016 02:51:47 -0400 In-Reply-To: Sender: ceph-devel-owner@vger.kernel.org List-ID: To: Sage Weil Cc: Gregory Farnum , Josh Durgin , ceph-devel > Op 8 september 2016 om 15:38 schreef Sage Weil : > > > On Thu, 8 Sep 2016, Wido den Hollander wrote: > > > Op 8 september 2016 om 15:24 schreef Sage Weil : > > > On Thu, 8 Sep 2016, Wido den Hollander wrote: > > > > > Op 8 september 2016 om 3:08 schreef Gregory Farnum : > > > > > > > > > > > > > > > On Tue, Sep 6, 2016 at 4:16 PM, Josh Durgin wrote: > > > > > > On 09/06/2016 12:18 PM, Wido den Hollander wrote: > > > > > >> > > > > > >> Hi, > > > > > >> > > > > > >> wido@wido-laptop:~$ python -c "import rados; r = rados.Rados(); > > > > > >> print(r.version())" > > > > > >> 0.69.1 > > > > > >> wido@wido-laptop:~$ dpkg -l|grep rados|awk '{print $2" "$3}' > > > > > >> librados-dev 10.2.2-1trusty > > > > > >> librados2 10.2.2-1trusty > > > > > >> libradosstriper1 10.2.2-1trusty > > > > > >> python-rados 10.2.2-1trusty > > > > > >> wido@wido-laptop:~$ > > > > > >> > > > > > >> Looking at librados.h in master I see: > > > > > >> > > > > > >> #define LIBRADOS_VER_MAJOR 0 > > > > > >> #define LIBRADOS_VER_MINOR 69 > > > > > >> #define LIBRADOS_VER_EXTRA 1 > > > > > >> > > > > > >> Is this something which has just been forgotten to update? > > > > > > > > > > > > > > > > > > Pretty much. Not much has relied on the librados/librbd version numbers > > > > > > of this style. Adding tests for particular functions can be more > > > > > > reliable than checking version numbers, since sometimes functions are > > > > > > backported. > > > > > > > > > > > >> Looking at the 'ceph' tool I see: > > > > > >> > > > > > >> CEPH_GIT_NICE_VER="10.2.2" > > > > > >> > > > > > >> This is updated during packaging/build it seems. > > > > > >> > > > > > >> Should we maybe do that for librados.h as well? > > > > > > > > > > > > > > > > > > I see no reason not to. > > > > > > > > > > We at one point were trying to only increment the librados library > > > > > version when stuff actually changed. Shockingly, that manual > > > > > maintenance mostly resulted in it not getting updated. ;) > > > > > But is it really that helpful to just provide another way of exposing > > > > > the package version, instead of doing something that actually > > > > > illustrates what functions are around? :/ > > > > > > > > I was using some Python code to gather Ceph information ( > > > > https://github.com/42on/ceph-collect ). One of the things I wanted to > > > > know is the installed Ceph version and I used the version() method. > > > > > > > > It kept giving back 0.69.1 so I started looking into that. > > > > > > > > I just try to avoid calling subprocesses when this isn't required. > > > > > > > > Since the version() method is unreliable in giving back the version, > > > > what to do with it? > > > > > > Let's remove those #defines from the header, and then do something like > > > include the auto-generated ceph_ver.h in the package, and update the > > > version functions to #include that and return an accurate string... > > > > > > > yeah, one thing though. > > > > #define CEPH_GIT_VER @CEPH_GIT_VER@ > > #define CEPH_GIT_NICE_VER "@CEPH_GIT_NICE_VER@" > > > > librados.h: > > > > CEPH_RADOS_API void rados_version(int *major, int *minor, int *extra); > > > > We don't have major, minor and extra stored separately in ceph_ver.h at the moment. > > Let's just replace it with > > CEPH_RADOS_API const char *rados_version_string(); > CEPH_RADOS_API const char *rados_git_version_string(); > > We could parse the components out of the string, but I'm not sure it's > worth the effort? > No, I don't think so. Seems good to me to add these functions and remove the old version() method. Wido > sage