Re: perf install-python_ext installing .o files in python site-packages

[Josh Boyer]

On Fri, Feb 26, 2016 at 4:14 PM, Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> On Fri, Feb 26, 2016 at 03:38:09PM -0500, Josh Boyer wrote:
>> Hi Arnaldo,
>>
>> We've had a report [1] that the python-perf package in Fedora installs
>> a handful of .o files in /usr/lib64/python2.7/site-packages/.  This is
>> true on Linus' latest tree, but it goes back to arriving with the 4.2
>> kernel.
>>
>> Digging around a bit, it seems the files installed are those that are
>> found as relative paths in the utils/python-ext-sources directory:
>>
>> /usr/lib64/python2.7/site-packages/bitmap.o
>> /usr/lib64/python2.7/site-packages/find_bit.o
>> /usr/lib64/python2.7/site-packages/hweight.o
>> /usr/lib64/python2.7/site-packages/rbtree.o
>>
>> None of those files should really be installed at all, as they're
>> temporary object files used to build perf.so.
>>
>> As far as I can tell, this is a side effect of using the python
>> distutils module with it's setup.py script.  The sources are listed
>> there so they get included, but then the build_ext functionality
>> interprets it as e.g.:
>>
>> -c ../lib/hweight.c -o python_ext_build/tmp/../lib/hweight.o
>>
>> That leaves it in the lib/ directory of the build output alongside the
>> final perf.so instead of python_ext_build/tmp/util/ like the rest of
>> the object files.  It happens to be an unfortunate coincidence that
>> install-lib looks in said .../lib/ directory and we just happen to
>> conflict with that naming.  So when install-python_ext is called, it
>> simply copies everything in that directory, including the .o files.
>>
>> I'm not enough of a python wizard to figure out how to fix this
>> quickly.  We could remove the .o files in the .spec file, but that
>> seems hacky and this should probably be fixed in the perf build
>> system.
>>
>> Ideas?
>
> looks like the python extension build system mixes
> up the '../lib' source files with final lib, which
> by 'accident' is located in '../lib' as well ;-)

Right, that's what I was trying to say above.

> using full paths for sources sorted this for me (patch attached)

Ah, yeah I had the idea to use full paths, but couldn't quickly figure
out how to do that.

> but that will end up with following build tree:
>
> ./python_ext_build
> ./python_ext_build/lib
> ./python_ext_build/lib/perf.so
> ./python_ext_build/tmp
> ./python_ext_build/tmp/home
> ./python_ext_build/tmp/home/jolsa
> ./python_ext_build/tmp/home/jolsa/kernel
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/counts.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/util.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/thread_map.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/cpumap.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/strlist.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/evlist.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/evsel.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/trace-event.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/xyarray.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/python.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/ctype.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/cgroup.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/rblist.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/perf/util/string.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/lib
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/lib/hweight.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/lib/rbtree.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/lib/bitmap.o
> ./python_ext_build/tmp/home/jolsa/kernel/linux-perf/tools/lib/find_bit.o
>
> not sure we want to come up with some 'nicer' solution

I don't think anything under python_ext_build/tmp/ actually matters in
any significant way.  As long as this doesn't negatively impact
something via side-effect, it's probably good enough.

Will you write up a full patch?

josh

> ---
> diff --git a/tools/perf/util/setup.py b/tools/perf/util/setup.py
> index 1833103768cb..6c4bb2ea3594 100644
> --- a/tools/perf/util/setup.py
> +++ b/tools/perf/util/setup.py
> @@ -22,6 +22,7 @@ cflags = getenv('CFLAGS', '').split()
>  # switch off several checks (need to be at the end of cflags list)
>  cflags += ['-fno-strict-aliasing', '-Wno-write-strings', '-Wno-unused-parameter' ]
>
> +src_perf  = getenv('srctree') + '/tools/perf'
>  build_lib = getenv('PYTHON_EXTBUILD_LIB')
>  build_tmp = getenv('PYTHON_EXTBUILD_TMP')
>  libtraceevent = getenv('LIBTRACEEVENT')
> @@ -30,6 +31,8 @@ libapikfs = getenv('LIBAPI')
>  ext_sources = [f.strip() for f in file('util/python-ext-sources')
>                                 if len(f.strip()) > 0 and f[0] != '#']
>
> +ext_sources = map(lambda x: '%s/%s' % (src_perf, x) , ext_sources)
> +
>  perf = Extension('perf',
>                   sources = ext_sources,
>                   include_dirs = ['util/include'],