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