Subject: Re: [bug#56803] [PATCH 3/6] gnu: Add python-mpv.

From: Maxime Devos <[email-masked]>
Date: Thu, 28 Jul 2022 00:05:30 +0200

On 27-07-2022 21:00, Tomasz Jeneralczyk wrote:
> +        (with-extensions (list mpv)

IIUC, that's for using Guile modules defined in packages -- but mpv is 
not a Guile package, so I expect that the surrounding with-extensions 
can be dropped.

> +     (let* ((args (package-arguments opencv)))
> +       (assoc-set! args #:configure-flags
> +                   (append (list "-DBUILD_opencv_python3=ON")
> +                           (assoc-ref args #:configure-flags)))
Look for substitute-keyword-arguments, which isn't stateful and hence 
there is less risk of accidentally modifying the arguments of the parent 
package.  Also, any reason for not adding this to the original package? 
(Possibly there is one).

> +              (replace 'check
> +                (lambda _
> +                  (setenv "DISPLAY" ":0")
> +                  (setenv "XDG_CACHE_HOME" (getcwd))
> +                  (setenv "HOME" (getcwd))
> +                  (system "Xvfb &")
> +                  (invoke "python" "test.py")))
Run "./pre-inst-env guix lint hydrus-network", it will have a remark 
about this.  Also, technically this is racy -- it's possible for python 
to start before Xvfb is ready though so far this doesn't seem to have 
caused trouble for other packages yet AFAIK -- I recommend "xvfb-run" 
"--" "python" "test.py" instead.

(I just scrolled quickly through the patches, a more full review will 
have to come later.)

Greetings,
Maxime.