Subject: Re: [bug#50756] [PATCH] gnu: Add lttng-tools.

From: Xinglu Chen <[email-masked]>
Date: Fri, 24 Sep 2021 16:24:42 +0200

On Thu, Sep 23 2021, Olivier Dion via Guix-patches via wrote:

> * gnu/packages/linux.scm (lttng-tools): New variable.
>
> Note that tests are not yet working.

What is the reason for that?  There should be a comment explaining why.

> Signed-off-by: Olivier Dion <[email-masked]>

There is no need to “sign-off” your own commit; this is only done by
committers who push a patch on behalf of someone else.

> ---
>  gnu/packages/linux.scm | 54 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>
> diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
> index 9bb90e599e..a8cb4fd05a 100644
> --- a/gnu/packages/linux.scm
> +++ b/gnu/packages/linux.scm
> @@ -53,6 +53,7 @@
>  ;;; Copyright © 2020 pukkamustard <[email-masked]>
>  ;;; Copyright © 2021 B. Wilson <[email-masked]>
>  ;;; Copyright © 2021 Ivan Gankevich <[email-masked]>
> +;;; Copyright © 2021 Olivier Dion <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -8051,6 +8052,59 @@ enable and disable specific instrumentation points, and writes event records
>  to ring buffers shared with a consumer daemon.")
>      (license license:lgpl2.1+)))
>  
> +(define-public lttng-tools
> +  (package
> +    (name "lttng-tools")
> +    (version "2.12.5")

Version 2.13 is available; any reason for not using it?

> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://lttng.org/files/lttng-tools/"
> +                                  "lttng-tools-" version ".tar.bz2"))
> +              (sha256
> +               (base32 "0bgk35423v6z17j1w80m7dcza7gigs1pwyq24sdmgqwg6j2d1zmc"))))

Keep lines <=80 characters long.

> +
> +    (build-system gnu-build-system)

These newlines aren’t necessary.

> +    (arguments
> +     `(#:tests? #f
> +       #:parallel-tests? #f

There is no need to set #:parallel-tests? if #:tests? is set to #f.

> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'configure 'set-environment-variables
> +           (lambda _
> +             (setenv "HOME" "/tmp")
> +             (setenv "LTTNG_HOME" "/tmp"))))))
> +
> +    (inputs
> +     `(("liburcu" ,liburcu)
> +       ("popt" ,popt)
> +       ("numactl" ,numactl)))
> +
> +    (propagated-inputs
> +     `(("libkmod" ,kmod)
> +       ("modprobe" ,module-init-tools)))

Any reason for the labels not being the same as the package?

> +    (native-inputs
> +     `(("pkg-config" ,pkg-config)
> +       ("perl" ,perl)
> +       ("libpfm4" ,libpfm4)
> +       ("python" ,python-3)

While running the configure script, I get

  configure: You may configure with --enable-python-bindings if you want Python bindings.

So you would have to pass the ‘--enable-python-bindings’ flag, and
Python would be needed during runtime as well.

> +       ("procps" ,procps)
> +       ("which" ,which)
> +       ("flex" ,flex)
> +       ("bison" ,bison)
> +       ("asciidoc" ,asciidoc)
> +       ("libxml2" ,libxml2)
> +       ("lttng-ust" ,lttng-ust)))
>
> +    (home-page "https://lttng.org/")
> +    (synopsis "LTTng userspace tracer libraries")
> +    (description "The lttng-tools project provides a session
> +daemon (lttng-sessiond) that acts as a tracing registry, the \"lttng\" command

the @command{lttng} command

> +line for tracing control, a lttng-ctl library for tracing control and a
> +lttng-relayd for network streaming.")

I would use @code{lttng-ctl} and @code{lttng-relayd}.

> +    (license license:gpl2)))

The bottom of the homepage says

  …LTTng-tools is licensed under LGPLv2.1 and GPLv2…

so it would be (list license:gpl2 license:lgpl2.1)

Could you send an updated patch?

[1]: <https://lttng.org/docs/v2.13/#doc-building-from-source>