Re: [PATCH 0/5] Build trace-cruncher as Python pakage

["Yordan Karadzhov (VMware)" <y.karadz@gmail.com>]

Hi Douglas,

Happy new year!
Thanks a lot for trying trace-cruncher and providing us with useful 
feedback!

On 31.12.19 г. 20:37 ч., Douglas Raillard wrote:
> Hi Yordan,
> 
> Finally got around to trying trace-cruncher. I've rediscovered how much I hated C build
> systems issues in general, so here is the outcome of the first day of battle :-)
> 
> kernelshark bits:
> * kernelshark build system seems to ignore the standard CMAKE_INSTALL_PREFIX
>    but uses _INSTALL_PREFIX instead, might be worth aligning with the documented
>    standard stuff of CMake if that makes sense.
> * _INSTALL_PREFIX is used for almost everything, except a polkit file that is copied
>    to /usr/share/polkit-1 regardless of _INSTALL_PREFIX, which makes installing as
>    non root fail.
> 
> Now trace-cruncher-specific bits:
> * Building kernelshark gives shared objects with .so.X.Y.Z file names, but GCC
>    wants to link against .so
> * There are some hardcoded paths to /usr/local/ in setup.py. They don't seem
>    to break user-given include paths but it would probably be a good idea to
>    make that somewhat parametric.
> * libkshark-input.h does not seem to exist but is used by ksharkpy.c. I'm not sure
>    if I used the wrong version of something or if that's a genuine issue.
> 

I am going to send a second version of the patch-set that tries to 
address some of the problems you found. This second version of the 
patches goes a bit further and simplifies/automates the build process 
for the user. Now you only have to do:

 > make
 > sudo make install

and that's all. You also have "make clean" and "make uninstall". I hope 
this will fix the problems you reported above.

> Here is the command I used to try to build trace-cruncher in a venv with
> numpy and cython installed:
> $ pip install . --global-option=build_ext --global-option='-Itrace-cmd/install/include' --global-option='-Ltrace-cmd/install/lib/kernelshark:trace-cmd/install/lib/trace-cmd:trace-cmd/install/lib/traceevent'
> 
> I've pushed some WIP commits on top of your series on my fork if you
> want to get a look at them:
> 
> Branch: refactor
> remote: https://github.com/douglas-raillard-arm/trace-cruncher.git
> https://github.com/douglas-raillard-arm/trace-cruncher/commits/refactor
>

All new patches are available here:
https://github.com/vmware/trace-cruncher/tree/refactoring_WIP (branch 
"refactoring_WIP")

> For the sake of reproducibility until things become stable, it might be a good idea
> to maintain a WIP branch with submodules/subtrees for trace-cmd in the right version
> in trace-cruncher repo, and the series applied, so we don't have to jugle with the
> ML lore to get mbox plus other patches from the repo itself etc. Once we have that,
> we can easily include a script that calls all the build systems involved and install all the
> required bits in a venv, without polluting system-wide locations, and progressively
> turn that into a standalone setup.py (or the nearest approximation of that we
> manage to make).

In v2 I am trying to address this, partially by automating the patching 
and build of trace-cmd and partially by making all those patched 
libraries part of the package itself (exploiting some linker magic). 
However, your solution to use venv is probably by fare more common. It 
will be great if we can have some discussion here and decide which of 
the two solutions is the best.

Once again, thanks for helping us!
cheers,
Yordan

> 
> PS: apologies for the extra new lines and lack of reply markers,
> Office 365 webmail does not seem to play very nicely with plain text ...
> 
> Cheers,
> Douglas
> 
> From: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> 
> Sent: 12 December 2019 09:02
> 
> To: linux-trace-devel@vger.kernel.org <linux-trace-devel@vger.kernel.org>
> 
> Cc: rostedt@goodmis.org <rostedt@goodmis.org>; Valentin Schneider <Valentin.Schneider@arm.com>; Douglas Raillard <Douglas.Raillard@arm.com>; Yordan Karadzhov (VMware) <y.karadz@gmail.com>
> 
> Subject: [PATCH 0/5] Build trace-cruncher as Python pakage
> 
> 
> 
> 
> This patch-set is an attempt to restructure the project and to make it
> 
> build as a native Python package. Although it looks like a complete
> 
> rewrite, this is essentially just a switching from using Cython to using
> 
> directly the C API of Python. Cython is still being used but only for
> 
> the implementation of the NumPy data wrapper. The new package has its
> 
> own stand-alone build system (very primitive for the moment) that is
> 
> completely decoupled from the existing build system used by tracecruncher.
> 
> In order to build and install the new package do:
> 
> 
> 
> sudo python setup.py install --record files.txt
> 
> 
> 
> The patch-set does not  remove the old implementation yet. This will
> 
> happen in another successive patch-set.
> 
> 
> 
> Please review as careful as possible!
> 
> 
> 
> Yordan Karadzhov (VMware) (5):
> 
>    Refactor the part of the interface that relies on libkshark
> 
>    Refactor the part of the interface that relies on libtraceevent
> 
>    Refactor NumPy based data wrapper
> 
>    Add "utils"
> 
>    Adapt the sched_wakeup.py example script to use the new tracecruncher
> 
>      module
> 
> 
> 
>   examples/sched_wakeup.py  |  30 ++---
> 
>   setup.py                  |  61 +++++++++
> 
>   src/common.h              |  20 +++
> 
>   src/datawrapper.pyx       | 201 ++++++++++++++++++++++++++++
> 
>   src/ftracepy.c            | 234 +++++++++++++++++++++++++++++++++
> 
>   src/ksharkpy.c            | 268 ++++++++++++++++++++++++++++++++++++++
> 
>   src/trace2matrix.c        |  29 +++++
> 
>   tracecruncher/__init__.py |   0
> 
>   tracecruncher/utils.py    |  54 ++++++++
> 
>   9 files changed, 882 insertions(+), 15 deletions(-)
> 
>   create mode 100644 setup.py
> 
>   create mode 100644 src/common.h
> 
>   create mode 100644 src/datawrapper.pyx
> 
>   create mode 100644 src/ftracepy.c
> 
>   create mode 100644 src/ksharkpy.c
> 
>   create mode 100644 src/trace2matrix.c
> 
>   create mode 100644 tracecruncher/__init__.py
> 
>   create mode 100644 tracecruncher/utils.py
> 
> 
> 
> --
> 
> 2.20.1
> 
> 
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>