diff mbox series

[bug#53324] gnu: Add uftrace.

Message ID cf9e68fcec5d2a773245982e8ae25c58d64d5881.1642446918.git.olivier.dion@polymtl.ca
State Accepted
Headers show
Series [bug#53324] gnu: Add uftrace. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Olivier Dion Jan. 17, 2022, 7:20 p.m. UTC
* gnu/packages/instrumentation.scm: (uftrace): New variable.
---
 gnu/packages/instrumentation.scm | 68 +++++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

Comments

M Jan. 17, 2022, 7:34 p.m. UTC | #1
Olivier Dion via Guix-patches via schreef op ma 17-01-2022 om 14:20 [-
0500]:
> +    ;; NOTE!  User should add python-3 and luajit to their profile if they
> +    ;; want to do scripting.

This information seems more suite for ‘description’

> +    (propagated-inputs
> +     (list
> +      capstone
> +      elfutils
> +      libunwind
> +      ncurses))

Do these need to be propagated, or can the propagation be avoided with
sufficient application of 'substitute*'?  Propagation can cause profile
collisions which can be difficult to resolve.

> +      ncurses))

If 'ncurses' is used (and depropagated), you probably need to add the
search paths of ncurses (TERMINFO_DIRS) to make sure Guix knows to set
TERMINFO_DIRS.

Greetings,
Maxime.
M Jan. 17, 2022, 7:39 p.m. UTC | #2
Olivier Dion via Guix-patches via schreef op ma 17-01-2022 om 14:20 [-
0500]:
> +                       (invoke "./configure" [...]

Looking at the configure script, for cross-compilation, you need to set
--arch and --cross-compile.
                               
> +                       (let ([python (which "python3")])

Guix doesn't use [ square brackets ].

> +                         (for-each (lambda (path)
> +                                     (substitute* path
> +                                       (("/usr/bin/env python") python)))
> +                                   (cons "misc/gen-autoargs.py"

If you use 'python-wrapper' instead of 'python3', then the
'patch-shebangs' phase should take care of this automatically.
Olivier Dion Jan. 17, 2022, 9:02 p.m. UTC | #3
On Mon, 17 Jan 2022, Maxime Devos <maximedevos@telenet.be> wrote:
> Olivier Dion via Guix-patches via schreef op ma 17-01-2022 om 14:20 [-
> 0500]:
>> +                       (invoke "./configure" [...]
>
> Looking at the configure script, for cross-compilation, you need to set
> --arch and --cross-compile.
>                                
>> +                       (let ([python (which "python3")])

Thanks for the quick review!  I'm in the process of adding cross
compilation (currently building for aarch64).

In the mean time, would you have an idea on how to fix __FILE__ for the
build?  uftrace uses it for logging and it ends up expanding to
`/tmp/guix-build*/source/*`.  I tried to change the `srcdir` in the
various Makefile without much success.
M Jan. 17, 2022, 9:37 p.m. UTC | #4
Hi,

Olivier Dion schreef op ma 17-01-2022 om 16:02 [-0500]:
> On Mon, 17 Jan 2022, Maxime Devos <maximedevos@telenet.be> wrote:
> > Olivier Dion via Guix-patches via schreef op ma 17-01-2022 om 14:20 [-
> > 0500]:
> > > +                       (invoke "./configure" [...]
> > 
> > Looking at the configure script, for cross-compilation, you need to set
> > --arch and --cross-compile.
> >                                
> > 
> > > +                       (let ([python (which "python3")])
> 
> Thanks for the quick review!  I'm in the process of adding cross
> compilation (currently building for aarch64).
> 
> In the mean time, would you have an idea on how to fix __FILE__ for the
> build?  uftrace uses it for logging and it ends up expanding to
> `/tmp/guix-build*/source/*`.  I tried to change the `srcdir` in the
> various Makefile without much success.
> 

Maybe uftrace supports out-of-tree builds?  Also, does uftrace try to
read /tmp/guix-build*, or does only the file name appear in backtraces?
If the latter, it is presumably not much of a problem.  If it is,
maybe out-of-tree builds could help.  Automake supports out-of-tree
builds, but I don't know if uftrace's build system does.

The review was only a partial review, I didn't actually build uftrace.

Greetings,
Maxime.
Olivier Dion Jan. 17, 2022, 9:55 p.m. UTC | #5
On Mon, 17 Jan 2022, Maxime Devos <maximedevos@telenet.be> wrote:
> Olivier Dion schreef op ma 17-01-2022 om 16:02 [-0500]:
>> On Mon, 17 Jan 2022, Maxime Devos <maximedevos@telenet.be> wrote:
>>
>> In the mean time, would you have an idea on how to fix __FILE__ for
>> the build?  uftrace uses it for logging and it ends up expanding to
>> `/tmp/guix-build*/source/*`.  I tried to change the `srcdir` in the
>> various Makefile without much success.
>> 
>
> Maybe uftrace supports out-of-tree builds?  Also, does uftrace try to
> read /tmp/guix-build*, or does only the file name appear in
> backtraces?  If the latter, it is presumably not much of a problem.
> If it is, maybe out-of-tree builds could help.  Automake supports
> out-of-tree builds, but I don't know if uftrace's build system does.

It does support out-of-tree build juste like autotool.  The configure
script however does `readlink -f $(dirname $0)` for its `srcdir`.

For example, even if I build at `/tmp/uftrace` where the configure
script is at /home/old/documents/polymtl/bmi/uftrace, I will get
--------------------------------------------------------------------------------
`uftrace:/home/old/documents/polymtl/bmi/uftrace/cmds/record.c:1571:find_in_path`
--------------------------------------------------------------------------------
at runtime.
M Jan. 18, 2022, 7:58 a.m. UTC | #6
Hi,

Olivier Dion schreef op ma 17-01-2022 om 16:55 [-0500]:
> It does support out-of-tree build juste like autotool.  The configure
> script however does `readlink -f $(dirname $0)` for its `srcdir`.
> 
> For example, even if I build at `/tmp/uftrace` where the configure
> script is at /home/old/documents/polymtl/bmi/uftrace, I will get
> --------------------------------------------------------------------------------
> `uftrace:/home/old/documents/polymtl/bmi/uftrace/cmds/record.c:1571:find_in_path`
> --------------------------------------------------------------------------------
> at runtime.

That seems to be `srcdir` working as expected, because
/home/old/documents/polymtl/bmi/uftrace is the directory
with the source code, so I'm not sure what ‘however’ is doing in

‘The configure script however does `readlink -f $(dirname $0)` for its
`srcdir`.’.

Greetings,
Maxime.
Ludovic Courtès Jan. 20, 2022, 2:10 p.m. UTC | #7
Hi,

Maxime Devos <maximedevos@telenet.be> skribis:

> Olivier Dion via Guix-patches via schreef op ma 17-01-2022 om 14:20 [-

[...]

>> +      ncurses))
>
> If 'ncurses' is used (and depropagated), you probably need to add the
> search paths of ncurses (TERMINFO_DIRS) to make sure Guix knows to set
> TERMINFO_DIRS.

IMO we shouldn’t do that: ‘TERMINFO_DIRS’ “belongs” to ncurses, and we
do not add it to each and every program that depends on ncurses.

I understand that not adding ‘TERMINFO_DIRS’ in uftrace can be annoying
due to <https://issues.guix.gnu.org/20255>, but so far consensus has
been to keep search path specs where they belong.

Thoughts?

Ludo’.
M Jan. 20, 2022, 4:50 p.m. UTC | #8
Ludovic Courtès schreef op do 20-01-2022 om 15:10 [+0100]:
> > If 'ncurses' is used (and depropagated), you probably need to add
> > the
> > search paths of ncurses (TERMINFO_DIRS) to make sure Guix knows to
> > set
> > TERMINFO_DIRS.
> 
> IMO we shouldn’t do that: ‘TERMINFO_DIRS’ “belongs” to ncurses, and
> we
> do not add it to each and every program that depends on ncurses.
> 
> I understand that not adding ‘TERMINFO_DIRS’ in uftrace can be
> annoying
> due to <https://issues.guix.gnu.org/20255>, but so far consensus has
> been to keep search path specs where they belong.

Did you mean to refer to <https://issues.guix.gnu.org/22138> here
(‘Search paths of dependencies are not honored’)?

We do add SSL_CERT_DIR to guix even though guix it is only used through
guile+openssl (though we seem to forget to add it to many other
packages).  We sometimes add XDG_DATA_DIRS and XDG_CONFIG_DIRS to
‘leaf’ packages even though it is (usually) only used by some glib-
related things.

So aside from perhaps 22138, I don't see a problem here.  As long as
22138 isn't addressed, how else could Guix determine that some leaf
package needs TERMINFO_DIRS to be set?

Greetings,
Maxime.
diff mbox series

Patch

diff --git a/gnu/packages/instrumentation.scm b/gnu/packages/instrumentation.scm
index 86b80d65ec..9aafc28181 100644
--- a/gnu/packages/instrumentation.scm
+++ b/gnu/packages/instrumentation.scm
@@ -23,11 +23,16 @@  (define-module (gnu packages instrumentation)
   #:use-module (gnu packages datastructures)
   #:use-module (gnu packages documentation)
   #:use-module (gnu packages elf)
+  #:use-module (gnu packages engineering)
   #:use-module (gnu packages flex)
   #:use-module (gnu packages glib)
+  #:use-module (gnu packages haskell-xyz)
+  #:use-module (gnu packages libunwind)
   #:use-module (gnu packages linux)
   #:use-module (gnu packages llvm)
+  #:use-module (gnu packages lua)
   #:use-module (gnu packages man)
+  #:use-module (gnu packages ncurses)
   #:use-module (gnu packages perl)
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages popt)
@@ -43,7 +48,8 @@  (define-module (gnu packages instrumentation)
   #:use-module (guix gexp)
   #:use-module (guix git-download)
   #:use-module ((guix licenses) #:prefix license:)
-  #:use-module (guix packages))
+  #:use-module (guix packages)
+  #:use-module (guix utils))
 
 (define-public babeltrace
   (package
@@ -238,3 +244,63 @@  (define-public lttng-tools
 line for tracing control, a @code{lttng-ctl} library for tracing control and a
 @code{lttng-relayd} for network streaming.")
     (license (list  license:gpl2 license:lgpl2.1))))
+
+(define-public uftrace
+  (package
+    (name "uftrace")
+    (version "0.11")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/namhyung/uftrace")
+                    (commit (string-append "v" version))))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32 "0gk0hv3rnf5czvazz1prg21rf9qlniz42g5b389n8a29hqj4q6xr"))))
+    (build-system gnu-build-system)
+    (arguments `(#:make-flags
+                 (list
+                  (string-append "CC=" ,(cc-for-target)))
+                 ;; runtest hang at some point -- probably dues to
+                 ;; failed socket connection -- but we want to keep the
+                 ;; unit tests.  Change the target to "test" when fixed.
+                 #:test-target "unittest"
+                 #:phases
+                 (modify-phases %standard-phases
+                   (replace 'configure
+                     (lambda* (#:key inputs outputs #:allow-other-keys)
+                       (setenv "SHELL" (which "sh"))
+                       (invoke "./configure"
+                               (string-append "--prefix="
+                                              (assoc-ref outputs "out")))))
+
+                   (add-after 'unpack 'patch-python-shebangs
+                     (lambda _
+                       (let ([python (which "python3")])
+                         (for-each (lambda (path)
+                                     (substitute* path
+                                       (("/usr/bin/env python") python)))
+                                   (cons "misc/gen-autoargs.py"
+                                         (find-files "tests" "\\.py$")))))))))
+    ;; NOTE!  User should add python-3 and luajit to their profile if they
+    ;; want to do scripting.
+    (propagated-inputs
+     (list
+      capstone
+      elfutils
+      libunwind
+      ncurses))
+    (native-inputs
+     (list
+      luajit
+      pandoc
+      pkg-config
+      python-3))
+    (home-page "https://github.com/namhyung/uftrace")
+    (synopsis "Function graph tracer for C/C++/Rust")
+    (description "uftrace is a tool for tracing and analyzing the execution of
+programs written in C/C++.  It is heavily inspired by the ftrace framework of
+the Linux kernel, while supporting userspace programs.  It supports various
+kind of commands and filters to help analysis of the program execution and
+performance.  It provides the command @command{uftrace}.")
+    (license license:gpl2)))