[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Fixed gen_win.py to display Serf library version#

From: Lieven Govaerts <svnlgo_at_mobsol.be>
Date: Sat, 12 Sep 2009 13:00:17 +0200

Edmund,

I'm not able to test this patch myself, but in general it looks good
to me. Some small remarks inline.

On Fri, Sep 11, 2009 at 11:06 AM, Edmund Wong<ed_at_kdtc.net> wrote:
>> Hi,
>>
>> Given my previous message regarding gen-make.py not outputting
>> the Serf library even when it finds it, I have created a patch
>> that displays the library version if it finds it.  In actual
>> fact, this patch modifies gen_win.py and not gen-make.py.
>>
>> Unfortunately, there is a caveat for this patch.  It depends
>> entirely on the fact that the Serf library's version # will always
>> be in serf.h AND that the format is the same; that is,
>> the major, minor and patch #s are under individual defined
>> constants:
>>
>> #define SERF_MAJOR_VERSION 0
>> #define SERF_MINOR_VERSION 3
>> #define SERF_PATCH_VERSION 0
>>
>> If the library changes the format of getting the version
>> #, this patch is broken.
>>
> Log:
>
> [[[
>
> Fixed gen_win.py such that it checks for the presence of the
> Serf library (which it does prior to this patch) and also
> outputs the library version (which it doesn't do) with the caveat
> that the way of retrieving the Serf.h library version numbers
> remain as-is.
>
> * build/generator/gen_win.py
>      (_get_serf_version): New function to return the serf library
>        version in a 3-tuple format.
>      (_find_serf): Added a version check.  If the 3-tuple version
>        structure has -1 in the first index, the serf library is
>        not built; otherwise, the library is set to be built.  Then
>        a message is displayed informing the user of the results.
>
> ]]]
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2393570
> Index: build/generator/gen_win.py
> ===================================================================
> --- build/generator/gen_win.py  (revision 39236)
> +++ build/generator/gen_win.py  (working copy)
> @@ -1318,6 +1318,38 @@
>
>     sys.stderr.write(msg)
>
> +  def _get_serf_version(self):
> +    "Retrieves the serf version from serf.h"
> +
> +    ver_maj = -1
> +    ver_min = -1
> +    ver_patch = -1
> +
> +    if self.serf_path and os.path.exists(self.serf_path):
> +       fp = open(os.path.join(self.serf_path, 'serf.h'))
> +       txt = fp.read()
> +       maj_t = re.compile(r'SERF_MAJOR_VERSION (\d+)')
> +       min_t = re.compile(r'SERF_MINOR_VERSION (\d+)')
> +       patch_t = re.compile(r'SERF_PATCH_VERSION (\d+)')
> +       ver_maj_re = maj_t.search(txt)
> +       ver_min_re = min_t.search(txt)
> +       ver_patch_re = patch_t.search(txt)
> +
> +       if ver_maj_re:
> +          ver_maj_ray = map(int, ver_maj_re.groups())
> +          ver_maj = ver_maj_ray[0]

Given you use only the first match, why do you convert all of them to
integers first?

> +
> +       if ver_min_re:
> +          ver_min_ray = map(int, ver_min_re.groups())
> +          ver_min = ver_min_ray[0]
> +
> +       if ver_patch_re:
> +          ver_patch_ray = map(int, ver_patch_re.groups())
> +          ver_patch = ver_patch_ray[0]
> +
> +    return ([ver_maj, ver_min, ver_patch])
> +
> +
>   def _find_serf(self):
>     "Check if serf and its dependencies are available"
>
> @@ -1325,6 +1357,14 @@
>     if self.serf_path and os.path.exists(self.serf_path):
>       if self.openssl_path and os.path.exists(self.openssl_path):
>         self.serf_lib = 'serf'
> +        version = self._get_serf_version()
> +        if version[0] != -1:
> +          self.serf_ver = `version[0]` +"."+`version[1]` + "."+`version[2]`

To improve readability, I suggest storing the return value of
_get_serf_version() in a tuple (ver_maj, ver_min, ver_patch).

Lieven

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2393832
Received on 2009-09-12 13:00:59 CEST

This is an archived mail posted to the Subversion Dev mailing list.