[PATCH evemu 1/3] py: add a tool to check the libevemu python binding

Benjamin Tissoires benjamin.tissoires at gmail.com
Tue Jan 14 12:45:06 PST 2014


On Tue, Jan 14, 2014 at 12:15 AM, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> On Mon, Jan 13, 2014 at 06:30:30PM -0500, Benjamin Tissoires wrote:
>> The test check-API.py is launched at each make, and verifies if the
>> exported evemu API matches the internal libevemu python binding.
>> The check only matches function names, and not signature.
>>
>> Without the python/evemu/base.py modification, the make output is:
>>
>> $ make
>> /usr/bin/python check-API.py ../src/libevemu.ver
>> The following functions are missing in the python binding:
>>  * evemu_create_managed
>>  * evemu_get_devnode
>> make: *** [check_python_api] Error 1
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires at gmail.com>
>> ---
>>
>> Hi guys,
>>
>> this is a follow up of Daniel's series regarding python 3.0.
>>
>> I have a problem here (sorry I did not spend hours on this) regarding the
>> Makefile.am. I harcoded ../src/libevemu.ver, but $(srcdir) returned '.', so it
>> was of no use here.
>
> $(srcdir) always returns '.' unless you've got a build-dir specified (like
> make distcheck does, that's good way to test changes like this btw).
> automake should convert relative paths correctly, if in doubt use
> $(top_srcdir)/src/libevemu.ver

oh, right. Thanks!

>
>> Any comments welcome!
>>
>> Cheers,
>> Benjamin
>>
>> PS: sorry for having written the first stages of a compiler, but I did not found
>> any other elegant way to extract the info from libevemu.ver... :(
>
> generate the header, libevemu.ver and python bits from the same source
> instead of going the other way round. Have a look at
> https://github.com/whot/evemu/tree/wip/xml-header
> what do you think of that? nees a bit more polishing, but otherwise works.

Ok, here is my personal feelings about this:
- I like the fact to have only one entry point for the three redundant
information (headers, .ver and abi.py)
- I really dislike having to edit several xml file when I want to
change the ABI (it's a C project, so really, C should be the primary
way of writing stuff)
- How about handling several ABI versions? (OK, I know, add a xml node
for each function per version, but still...)
- I am pretty sure next time I will make a change to evemu.h, I will
edit the .h file instead of the xml, and the next make will destroy my
changes, and I will curse the python binding ;)

Maybe the solution could be to add some hints in the .h (like
EXPORT_SYMBOL_GPL() in the kernel, but regarding ABI version and
return code), and rely on this to generate the .ver and the python
ABI.

>
> errcheck needs to be added, and any special ABI generation would need to be
> handled too though that should be simple enough to add.

regarding errcheck, I made some tests with my version. ld is
particularly difficult regarding the .ver file. So when it is buggy,
it just failed, and so does my script. So if ld passes, then the
script should too :)

>
>>
>>  python/Makefile.am   |   7 ++-
>>  python/check-API.py  | 167 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  python/evemu/base.py |  12 ++++
>>  3 files changed, 184 insertions(+), 2 deletions(-)
>>  create mode 100644 python/check-API.py
>>
>> diff --git a/python/Makefile.am b/python/Makefile.am
>> index 3ce3946..ddaf44b 100644
>> --- a/python/Makefile.am
>> +++ b/python/Makefile.am
>> @@ -32,6 +32,9 @@ evemu-test-runner: evemu-test-runner.in Makefile
>>         $< >$@
>>       chmod +x $@
>>
>> -BUILT_SOURCES = evemu-test-runner
>> -EXTRA_DIST =  evemu-test-runner.in $(wildcard evemu/test*)
>> +check_python_api: check-API.py Makefile ../src/libevemu.ver evemu/base.py
>> +     $(PYTHON) check-API.py ../src/libevemu.ver
>> +
>> +BUILT_SOURCES = evemu-test-runner check_python_api
>> +EXTRA_DIST =  evemu-test-runner.in $(wildcard evemu/test*) check-API.py
>>  CLEANFILES = $(BUILT_SOURCES)
>> diff --git a/python/check-API.py b/python/check-API.py
>> new file mode 100644
>> index 0000000..e983542
>> --- /dev/null
>> +++ b/python/check-API.py
>> @@ -0,0 +1,167 @@
>> +#!/usr/bin/env python
>> +# -*- coding: utf-8 -*-
>> +
>> +# Copyright 2014 Red Hat, Inc.
>> +#
>> +#  This program is free software; you can redistribute it and/or modify
>> +#  it under the terms of the GNU General Public License as published by
>> +#  the Free Software Foundation; either version 2 of the License, or
>> +#  (at your option) any later version.
>> +#
>> +#  This program is distributed in the hope that it will be useful,
>> +#  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +#  GNU General Public License for more details.
>> +#
>> +#  You should have received a copy of the GNU General Public License
>> +#  along with this program; if not, write to the Free Software
>> +#  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
>> +#  MA 02110-1301, USA.
>> +#
>> +
>> +#
>> +# Check that src/libevemu.ver and the API defined in python/evemu/base.py match
>> +#
>> +
>> +from __future__ import print_function
>> +import evemu.base
>> +import re
>> +
>> +class LexicalError(Exception):
>> +     pass
>> +
>> +class SemanticError(Exception):
>> +     pass
>> +
>> +lex_regexps = (
>> +     ("state", re.compile(r'\s*(global|local)\s*:(.*)')),
>> +     ("token", re.compile(r'\s*([a-zA-Z_\*][a-zA-Z_0-9\.]*)(.*)')),
>> +     ("delim", re.compile(r'\s*([\{\}:;])(.*)')),
>> +)
>> +
>> +def next_lexem(input):
>> +     if input == "":
>> +             return None, None, None
>> +     for _type, regexp in lex_regexps:
>> +             m =  regexp.match(input)
>> +             if m:
>> +                     return m.group(1), _type, m.group(2)
>> +     raise LexicalError("unable to parse input string:'{0}'".format(input))
>> +
>> +def lexical_analysis(input):
>> +     """
>> +     Lexical analysis:
>> +     split the input file into tokens.
>> +     For example:
>> +     EVEMU_2.0 {
>> +             global:
>> +                     evemu_create;
>> +                     evemu_create_event;
>> +     };
>> +     is transformed into:
>> +     [('EVEMU_2.0', 'token'), ('{', 'delim'), ('global', 'state'), ('evemu_create', 'token'),
>> +      (';', 'delim'), ('evemu_create_event', 'token'), (';', 'delim'),
>> +      ('}', 'delim'), (';', 'delim')]
>> +     """
>> +     lex = []
>> +     lexem, _type, input = next_lexem(input)
>> +     while input != None:
>> +             lex.append((lexem, _type))
>> +             lexem, _type, input = next_lexem(input)
>
> did you test this with multi-ABI defines as well? do we need extensions to the parser then.

Yes, the check works pretty well with the libevemu.ver from v1.2.0,
which defines 2 ABIs.

>
>> +     return lex
>> +
>> +def semantic_analysis(lex):
>> +     """
>> +     Semantical analysis:
>> +     Consume the tokens in lex and build an abstract representation of the file.
>> +     The previous example gives:
>> +     {'EVEMU_2.0': {'global': ['evemu_create', 'evemu_create_event']}}
>> +     """
>> +     sem = {}
>> +     current_node = None
>> +     current_dict = None
>> +     current_state_list = None
>> +     for l, _type in lex:
>> +             if _type == "token":
>> +                     current_node = l
>> +             elif _type == "delim":
>> +                     if l == "{":
>> +                             current_dict = {}
>> +                             sem[current_node] = current_dict
>> +                             current_node = None
>> +                     elif l == "}":
>> +                             current_dict = None
>> +                             current_state_list = None
>> +                     elif l == ";":
>> +                             if current_state_list != None:
>> +                                     current_state_list.append(current_node)
>> +                                     current_node = None
>> +             elif _type == "state":
>> +                     current_state_list = []
>> +                     current_dict[l] = current_state_list
>> +
>> +     return sem
>> +
>> +def parse_lib_ver(path):
>> +     with open(path, "r") as lib_ver:
>> +             feed = "".join(lib_ver.readlines()).replace("\n", "")
>> +     lex = lexical_analysis(feed)
>> +     sem = semantic_analysis(lex)
>> +
>> +     # now that the semantical analysis is done, get only global scope functions
>> +     exported_functions = []
>> +     for lib in sem.values():
>> +             if "global" in lib:
>> +                     exported_functions.extend(lib['global'])
>> +     return exported_functions
>> +
>> +def compare_functions(exported, internal):
>> +     """
>> +     Compare two lists, returns:
>> +     error, missing_exported, missing_internal
>> +
>> +     with
>> +     - error: True if the two lists are not identical
>> +     - missing_exported: items in internal not in exported
>> +     - missing_internal: items in exported not in internal
>> +     """
>> +     missings_internal = []
>> +     missings_exported = []
>> +
>> +     for f in exported:
>> +             if f not in internal:
>> +                     missings_internal.append(f)
>> +     for f in internal:
>> +             if f not in exported:
>> +                     missings_exported.append(f)
>> +
>> +     error = len(missings_internal) > 0 or len(missings_exported) > 0
>> +
>> +     return error, missings_exported, missings_internal
>
>     return frozenset(exported).symmetric_difference(internal)
>
> gives you the elements ein either but not both. alternatively, you can do
>
>     missing_internal = frozenset(exported).difference(internal)
>     missing_exported = frozenset(internal).difference(exported)
>
> and just return the two lists and use len() as an error indicator, err is
> superfluous here (that's C code ;)

:-)
I was trying to "optimize" by pre-calculating if there is an error,
but yes, it is not very pretty.

>
>> +
>> +
>> +def main():
>> +     import sys
>> +     path = "src/libevemu.ver"
>> +     if len(sys.argv) > 1:
>> +             path = sys.argv[1]
>
> given that this will be invoked by make, I'd just make the arguments
> mandatory so we don't need hardcoded paths here.

true

>
>> +     exp_fun = parse_lib_ver(path)
>> +     int_fun = list(evemu.base.LibEvemu._api_prototypes.keys())
>> +     err, m_e, m_i = compare_functions(exp_fun, int_fun)
>> +
>> +     if not err:
>> +             "the two provided lists are identical, good job"
>
> no error message on success please.

I thought I removed it, but then, it stayed there...

>
>> +             sys.exit(0)
>> +
>> +     if len(m_e):
>> +             print("The following functions are not available anymore:")
>> +             for f in m_e:
>> +                     print(" *", f)
>> +     if len(m_i):
>> +             print("The following functions are missing in the python binding:")
>> +             for f in m_i:
>> +                     print(" *", f)
>> +     sys.exit(1)
>> +
>> +if __name__ == "__main__":
>> +     main()
>> diff --git a/python/evemu/base.py b/python/evemu/base.py
>> index b366e2f..1d903b2 100644
>> --- a/python/evemu/base.py
>> +++ b/python/evemu/base.py
>> @@ -186,6 +186,12 @@ class LibEvemu(LibraryWrapper):
>>              "argtypes": (c_void_p,),
>>              "restype": c_uint,
>>              },
>> +        #const char *evemu_get_devnode(struct evemu_device *dev);
>> +        "evemu_get_devnode": {
>> +            "argtypes": (c_void_p,),
>> +            "restype": c_char_p,
>> +            "errcheck": expect_not_none
>> +            },
>>          #const char *evemu_get_name(const struct evemu_device *dev);
>>          "evemu_get_name": {
>>              "argtypes": (c_void_p,),
>> @@ -394,6 +400,12 @@ class LibEvemu(LibraryWrapper):
>>              "restype": c_int,
>>              "errcheck": expect_eq_zero
>>              },
>> +        #int evemu_create_managed(struct evemu_device *dev);
>> +        "evemu_create_managed": {
>> +            "argtypes": (c_void_p,),
>> +            "restype": c_int,
>> +            "errcheck": expect_eq_zero
>> +            },
>>          #void evemu_destroy(struct evemu_device *dev);
>>          "evemu_destroy": {
>>              "argtypes": (c_void_p,),
>
> this should be a separate patch, Reviewed-by: Peter Hutterer <peter.hutterer at who-t.net>

OK, will split for v2.

Cheers,
Benjamin


More information about the Input-tools mailing list