[lttng-dev] [PATCH lttng-ust] Fix: Corrected python version assumptions in lttng-gen-tp

[Mathieu Desnoyers]

----- Original Message -----
> From: "Jesper Derehag" <jderehag at hotmail.com>
> To: "Mathieu Desnoyers" <mathieu.desnoyers at efficios.com>
> Cc: lttng-dev at lists.lttng.org
> Sent: Tuesday, March 25, 2014 2:03:53 PM
> Subject: RE: [lttng-dev] [PATCH lttng-ust] Fix: Corrected python version assumptions in lttng-gen-tp
> 
> > Date: Tue, 25 Mar 2014 14:16:02 +0000
> > From: mathieu.desnoyers at efficios.com
> > To: jderehag at hotmail.com
> > CC: lttng-dev at lists.lttng.org
> > Subject: Re: [lttng-dev] [PATCH lttng-ust] Fix: Corrected python version
> > 	assumptions in lttng-gen-tp
> > 
> > ----- Original Message -----
> >> From: jderehag at hotmail.com
> >> To: lttng-dev at lists.lttng.org
> >> Cc: "Jesper Derehag" <jderehag at hotmail.com>
> >> Sent: Monday, March 24, 2014 10:29:44 AM
> >> Subject: [lttng-dev] [PATCH lttng-ust] Fix: Corrected python version
> >> 	assumptions in lttng-gen-tp
> >> 
> >> From: Jesper Derehag <jderehag at hotmail.com>
> >> 
> >> This modification calls the interpreter through env instead of
> >> directly, which should be more portable for those distros which do not
> >> install
> >> python into /usr/bin/.
> >> 
> >> Also, since lttng-gen-tp is not python2 compatible, I changed it to
> >> strictly
> >> call python3 which is the recommended way.
> >> See PEP 394.
> >> http://legacy.python.org/dev/peps/pep-0394/
> > 
> > Hi Jesper,
> > 
> > I somehow thought that lttng-gen-tp was meant to work with
> > Python 2.7 and Python 3. But maybe I'm missing something.
> > Where have you seen the dependency of this script on python
> > 3 in our docs ?
> > 
> > Thanks,
> > 
> > Mathieu
> 
> Hmm... The reason for why I changed into python3 was due to that I got syntax
> error when trying to run it:
> -----
> ./lttng-gen-tp
>   File "./lttng-gen-tp", line 100
>     except OSError as msg:
>                     ^
> SyntaxError: invalid syntax
> ------
> I *thought* I was running on python2.7 but as it turns out I was not.
> When resolving "python" I get python2.7 but when I check the explicit version
> laying at /usr/bin/python its actually a 2.4 version so no wonder it was
> failing.
> 
> Checking the PEP for when "except XX as yy" was introduced points to that it
> was introduced in python 2.6 so entirely my mistake.
> Reran it now with the a 2.7 version and it works as intended.
> 
> However, I still think you should run it through env as proposed in the patch
> and not use the explicit path to python.
> 
> If you agree, do you want me to send you another patch removing the "3"?

Yes, if you can do that it would be perfect!

> 
> Otherwise, I apologize for my blunder.

No worries! This is why we have code review :) Pinpointing
an issue and submitting a patch as well, even if imperfect,
does a lot to improve LTTng!

Thanks,

Mathieu

> 
> Regards,
> Jesper
> 
> > 
> > 
> >> ---
> >> tools/lttng-gen-tp | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/tools/lttng-gen-tp b/tools/lttng-gen-tp
> >> index c49e8a5..c7bf295 100755
> >> --- a/tools/lttng-gen-tp
> >> +++ b/tools/lttng-gen-tp
> >> @@ -1,4 +1,4 @@
> >> -#!/usr/bin/python
> >> +#!/usr/bin/env python3
> >> #
> >> # Copyright (c) 2012 Yannick Brosseau <yannick.brosseau at gmail.com>
> >> #
> >> --
> >> 1.7.9.5
> >> 
> >> 
> >> _______________________________________________
> >> lttng-dev mailing list
> >> lttng-dev at lists.lttng.org
> >> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
> >> 
> > 
> > --
> > Mathieu Desnoyers
> > EfficiOS Inc.
> > http://www.efficios.com

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com