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
and all to sssd-1-12:
* 9687d7db79e15846de385537a99525d11cae6a15 (Slavek's patch)
* 42563a20baf2f334c01a8f821c5c2d98c208fc84 (tarball patch)
* d36ff71364db4abc08053d36d392aa602fc5860a
* af8e9134176b4517ecfdd2e10e6204fd3f0ad765
* d0a95d87f41721bb57149471897cc920a8730c20
* 46020d2dbb0f092fca7ec2a27e8c822543ab50fd (codegen)