Edmund Wong wrote on Mon, 14 Sep 2009 at 01:36 +0800:
> [[[
>
> 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 is what this patch does).
>
> * build/generator/gen_win.py
> (_get_serf_version): New function to return the serf library
> version in a 3-tuple format.
The '(' should be indented two spaces.
> (_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.
>
IMO, the part about -1 and 3-tuple is a low-enough implementation detail
to not belong in the log message. Just say what the added code thus
(extract the version, message if successful, skip building(?) otherwise).
> Index: build/generator/gen_win.py
> ===================================================================
> --- build/generator/gen_win.py (revision 39272)
> +++ build/generator/gen_win.py (working copy)
> @@ -1318,6 +1318,32 @@
>
> 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):
> + if (os.path.exists(os.path.join(self.serf_path, 'serf.h'))):
> + 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)
This reads the entire file to the variable 'txt'. It would be better to
read it incrementally (e.g., one line at a time).
(And perhaps relax the regexes by allowing any number of whitespace
there? This is almost a matter of personal taste, though.)
I can't test the patch because right now I'm not on windows.
> + if ver_maj_re:
> + ver_maj = int(ver_maj_re.groups()[0])
> + if ver_min_re:
> + ver_min = int(ver_min_re.groups()[0])
> + if ver_patch_re:
> + ver_patch = int(ver_patch_re.groups()[0])
> +
> + return (ver_maj, ver_min, ver_patch)
> +
> def _find_serf(self):
> "Check if serf and its dependencies are available"
>
> @@ -1325,6 +1351,15 @@
> 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 = str(version[0]) +"."+str(version[1]) + "." + \
> + str(version[2])
> + msg = 'Found serf version %s\n' % self.serf_ver
> + else:
> + msg = 'Could not determine version, ra_serf will not be built\n'
> + self.serf_lib = None
> + sys.stderr.write(msg)
> else:
> sys.stderr.write('openssl not found, ra_serf will not be built\n')
> else:
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2394370
Received on 2009-09-14 00:14:09 CEST