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

[Gerlando Falauto]

On 05/14/2014 05:56 PM, Jérémie Galarneau wrote:
> On Wed, May 14, 2014 at 10:22 AM, Gerlando Falauto
> <gerlando.falauto at keymile.com> wrote:
>> Hi,
>>
>>
>> On 05/13/2014 08:55 PM, Jérémie Galarneau wrote:
>>>
>>> On Tue, May 13, 2014 at 4:38 AM, Gerlando Falauto
>>> <gerlando.falauto at keymile.com> wrote:
>>>>
>>>> Hi Jesper,
>>>>
>>>> [...]
>>>>
>>>>> 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.
>>>>
>>>>
>>>>
>>>> I still have some issues, even with this change.
>>>> As a matter of fact, my default python installation would be a python2.6:
>>>>
>>>> $ python
>>>>
>>>> python
>>>> Python 2.6.6 (r266:84292, Jan 22 2014, 05:06:49)
>>>>
>>>> I'm however cross-compiling, so I specify a sysroot and I explicitly set
>>>> the
>>>> python version in my configure script:
>>>>
>>>> $ . /opt/eldk/eldk_init ppc_6xx # set the env for cross-compiling
>>>>
>>>> $ python2.7
>>>> Python 2.7.3 (default, Aug  2 2013, 20:37:10)
>>>> [GCC 4.7.2] on linux2
>>>>
>>>> $ which python2.7
>>>> /opt/eldk/powerpc/sysroots/i686-eldk-linux/usr/bin/python2.7
>>>>
>>>> $ ac_cv_header_linux_perf_event_h=no PYTHON=python2.7 ./configure
>>>> --target=powerpc-linux --host=powerpc-linux --build=i686-linux
>>>> --with-libtool-sysroot=/opt/eldk/powerpc/sysroots/powerpc-linux
>>>> --prefix=/usr
>>>>
>>>> $ MAKEFLAGS= && DESTDIR=/opt/eldk/powerpc/sysroots/powerpc-linux make all
>>>> install install-strip pkgconfigdir=/tmp pkgconfig_DATA=
>>>>
>>>> This doesn't however help, as the script will call "python" from the
>>>> current
>>>> environment (thereby ignoring the PYTHON= value used by the configure
>>>> script
>>>> when checking for python version).
>>>>
>>>> So perhaps a solution would be to change the configure script (or
>>>> whatever
>>>> it comes from by means of autoconfig/automake) so to create a "python"
>>>> symlink in the working directory (to the version selected/detected) and
>>>> then
>>>> add it to the path (*).
>>>>
>>>> I guess a quick-and-dirty alternative could be to change the makefile so
>>>> to
>>>> use the "PYTHON" environment variable to explictly set what interpreter
>>>> to
>>>> use -- but that would defeat the whole purpose of the shebang so I
>>>> believe
>>>> it's a very bad idea:
>>>>
>>>> --- a/doc/examples/gen-tp/Makefile
>>>> +++ b/doc/examples/gen-tp/Makefile
>>>> @@ -46,7 +46,7 @@ sample.o: sample.c sample_tracepoint.h
>>>>           CFLAGS="$(CFLAGS) $(AM_CFLAGS)" \
>>>>           LDFLAGS="$(LDFLAGS) $(AM_LDFLAGS)" \
>>>>           CC="$(CC)" \
>>>> -       $(LTTNG_GEN_TP_PATH)lttng-gen-tp -o $@ $<
>>>> +       $(PYTHON) $(LTTNG_GEN_TP_PATH)lttng-gen-tp -o $@ $<
>>>>
>>>>    # The following rule can be used to generate all files instead of
>>>> having
>>>> one
>>>>    # for each file type. Note that the sample.o has a dependency on the
>>>> @@ -55,10 +55,10 @@ sample.o: sample.c sample_tracepoint.h
>>>>    #      lttng-gen-tp $<
>>>>
>>>>    %.h: %.tp
>>>> -       $(LTTNG_GEN_TP_PATH)lttng-gen-tp -o $@ $<
>>>> +       $(PYTHON) $(LTTNG_GEN_TP_PATH)lttng-gen-tp -o $@ $<
>>>>
>>>>    %.c: %.tp
>>>> -       $(LTTNG_GEN_TP_PATH)lttng-gen-tp -o $@ $<
>>>> +       $(PYTHON) $(LTTNG_GEN_TP_PATH)lttng-gen-tp -o $@ $<
>>>>
>>>>    .PHONY: clean
>>>>    clean:
>>>>
>>>>
>>>> What do you think?
>>>>
>>>
>>> I agree that this is not the proper fix...
>>>
>>> Note that this script is not required to build lttng-ust. It will not
>>> be used during the build process if your host doesn't have a suitable
>>> Python version installed;
>>
>>
>> Uhm, that's not entirely correct. Unfortunately, the configure script will
>> indeed try detect a suitable python version; however, that doesn't mean it
>> will use THAT version (in my case, the one reachable through "python2.7")
>> when needed (and that's the whole point of my email!).
>> I also tried enforcing detection of an older version (so to disable building
>> of the examples) by using PYTHON=python before ./configure but I get:
>>
>> checking whether python version is >= 2.7... no
>> configure: error: Python interpreter is too old
>>
>
> Which version of lttng-ust are you using? This has been fixed by
>
> commit dcb6ba251f8c612133e4a3e064690d915f54a146
> Author: Jérémie Galarneau <jeremie.galarneau at efficios.com>
> Date:   Tue Apr 1 11:49:58 2014 -0400
>
>      Fix: Override AM_PATH_PYTHON's default action-if-not-found
>
>      AM_PATH_PYTHON's default action aborts configure. This fix ensures that
>      configure follows through with a noop.
>
>      Signed-off-by: Jérémie Galarneau <jeremie.galarneau at efficios.com>
>      Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers at efficios.com>
>
>
> which is part of 2.4.1.

I'm using the master branch updated a few days ago, which (I checked) 
includes your fix. IIRC, this fix was your reaction to the email I'm 
replying to.

If I revert your patch, my workaround below would miserably fail:

$ am_cv_pathless_PYTHON=none ./configure
...
checking for a Python interpreter with version >= 2.7... (cached) none
configure: error: no suitable Python interpreter found

whereas your fix at least allows me to continue if I explicitly (and in 
a more-or-less hacky way) disable python altogether.

Thank you,
Gerlando