On (13/02/15 18:43), Jakub Hrozek wrote:
>On Fri, Feb 13, 2015 at 02:24:12PM +0100, Lukas Slebodnik wrote:
>> On (12/02/15 12:22), Petr Viktorin wrote:
>> >On 02/10/2015 11:36 PM, Lukas Slebodnik wrote:
>> >>On (02/02/15 13:32), Petr Viktorin wrote:
>> >>>On 02/02/2015 09:08 AM, Lukas Slebodnik wrote:
>> >>>[...]
>> >>>>>>Patch 1/5: Note that even under Python 2, `except
ValueError, KeyError:`
>> >>>>>>would not work (it catches ValueError, and stores the
exception instance
>> >>>>>>under the name KeyError); please test these changes
well.
>> >>>>>>
>> >>>>>It means that in python2 KeyError was not handled at all.
>> >>>>>
>> >>>>>python2 documentation says:
>> >>>>>"If an exception occurs which does not match the
exception named in the except
>> >>>>>clause, it is passed on to outer try statements; if no
handler is found, it is
>> >>>>>an unhandled exception and execution stops with a message as
shown above."
>> >>>>>
>> >>>>>In my patch, you can see that different exception was thrown
(ParsingError) in
>> >>>>>handler. I'm not sure hot to test it therefore you are
in CC ;-)
>> >>>>>Shall I remove KeyError?
>> >>>
>> >>>Yes, remove KeyError. Looking at the history, since 2009 the code
worked
>> >>>without catching KeyError (even if that wasn't the intention). I
see no point
>> >>>in adding it now.
>> >>>
>> >>Removed
>> >>
>> >>>Otherwise LGTM
>> >>>
>> >>>>>
>> >>>>>>Patch 4/5: you can use io.StringIO in both 2.6+ and
3.x+
>> >>>>>>
>> >>>>>Yes, but...
>> >>>>>Traceback (most recent call last):
>> >>>>> File "../src/sbus/sbus_codegen", line 931, in
<module>
>> >>>>> main()
>> >>>>> File "../src/sbus/sbus_codegen", line 919, in
main
>> >>>>> generate_source(parser.parsed_interfaces, filename,
options.include)
>> >>>>> File "../src/sbus/sbus_codegen", line 600, in
generate_source
>> >>>>> out("/* The following definitions are
auto-generated from %s */", basename)
>> >>>>> File "../src/sbus/sbus_codegen", line 217, in
out
>> >>>>> sys.stdout.write(str)
>> >>>>>TypeError: unicode argument expected, got 'str'
>> >>>>>
>> >>>>>So I'm find with such solution for StringIO. The
sbus_codegen is python script
>> >>>>>which generate C source file. I do not expect any unicode
characters.
>> >>>>>But I'm not opposed better version.
>> >>>
>> >>>I see.
>> >>>People could expect "io" to be the io module, so you could
write
>> >>>
>> >>> if sys.version_info[0] > 2:
>> >>> from io import StringIO
>> >>> else:
>> >>> from StringIO import StringIO
>> >>> ...
>> >>> sys.stdout = buf = StringIO()
>> >>>
>> >>Fixed
>> >>
>> >>>... but at this point that's just nitpicking.
>> >>>
>> >>>>>
>> >>>>>Thank you very much for review.
>> >>>>>I squashed some small patches.
>> >>>>>
>> >>>>>LS
>> >>>>
>> >>>>I realized there were hardcoded python2 on some places. The 3rd
patch should
>> >>>>fix it. I used "env pyhton" for internal python
scripts and for tests which are
>> >>>>not installed.
>> >>>>
>> >>>>I had a discussion with Pavel B. and we agreed
>> >>>>that patch "sbus_codegen: Port to python3" needn't
be push to upstream yet.
>> >>>>It would break his other patches. So current version would be
used mostly by
>> >>>>downstream.
>> >>>
>> >>Thank you very much for review.
>> >>
>> >>Updated version is attached.
>> >
>> >LGTM
>> >
>> Here is a link to ci result
>>
http://sssd-ci.duckdns.org/logs/job/7/32/summary.html.
>>
>> I think we should push these patches also to stable branch sssd-1.12.
>> So package maintainers can decide wheter to build with python2 or python3.
>> The other patches on the list which allow to build bindings for python2 and
>> python3 are quite invasive and changes some behavoiur therefore are not
>> intended to push to stable branch.
>>
>> These patches depends on Bohuslav's patch
>> "Python3 support in SSSD" 341a00311680a440d7f979f06c34c70d86c9367a
>> "BUILD: Include python-test.py in the tarball"
>> 51d65c4ad15c2cc23f38fa09dd6efeb15e4f3e86
>>
>> BTW We agreed with Pavel (pbrezina) to do not push patch sbus_codegen patch to
>> master because he did some changes in that file. So I will prepare next version
>> afther his patches will be pushed.
>>
>> LS
>
>Pushed all except codegen to master:
> * e8058322725ba050014777ee2484f7e833ab1e3a
> * a71004c112cd5d61d3a9e37a4cfc5760dc9a1cec
> * 1ac368d0962ef8cc83dcd642c7fec8b3cba5b6fe
Attached is a rebased sbus_codegen patch on top of current master.
There isn't significant change.
LS
Sorry, forgot to send push-mail:
* e239b5bedd877e6c5002b22ea11926a12b4c781c