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

From: Tomasz Jeneralczyk <[email-masked]>
Date: Thu, 28 Jul 2022 14:58:19 +0000

On 2022-07-27 22:05, Maxime Devos wrote:
> On 27-07-2022 21:00, Tomasz Jeneralczyk wrote:
> 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.
You're right. I dropped it with no negative consequences.

> This sounds like something upstream should be informed about, otherwise 
> they wouldn't know there is something to fix.
I had a friend on arch linux run those tests and everything worked just 
fine, so it might have something to do with guix
itself or an incomplete package definition. In any case, I'll make sure 
to write a proper bug report in the upstream repo later.

> 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).
Thanks, this macro is not documented in manual, but it looks much nicer 
now.
The reason I made a new package is simply because someone on irc 
recommended me to do as such. Though your question made me realize
that just adding one flag worked because all the necessary packages to 
build python buildings were in opencv package already...
maybe originally it was intended for the python bindings to be included 
in opencv? And so I added the flag to opencv and removed
my original package - it works all the same.

> 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 though I missed something so I ran lint again, but it only said 
there's a new version of hydrus available.
Nevertheless I changed the invocation to what you recommended. I also 
updated hydrus to version 493.

> (I just scrolled quickly through the patches, a more full review will
> have to come later.)
Thank you for all your comments so far.
I made changes but I wont send them as patches until I fix all the 
problems or get a confirmation that there's nothing more to do.

However I'm attaching a "preview" diff of the changes I'll want to 
iclude in v2 of my patches.
I suppose I should only send v2 of the ones I changed, right?.
diff --git a/gnu/packages/image-processing.scm b/gnu/packages/image-processing.scm
index 9496155ec7..6417233245 100644
--- a/gnu/packages/image-processing.scm
+++ b/gnu/packages/image-processing.scm
@@ -511,6 +511,8 @@ (define-public opencv
              ;; DISPATCH is the list of optional dispatches.
              "-DCPU_BASELINE=SSE2"
 
+             "-DBUILD_opencv_python3=ON"
+
              ,@(match (%current-system)
                  ("x86_64-linux"
                   '("-DCPU_DISPATCH=NEON;VFPV3;FP16;SSE;SSE2;SSE3;SSSE3;SSE4_1;SSE4_2;POPCNT;AVX;FP16;AVX2;FMA3;AVX_512F;AVX512_SKX"
@@ -658,17 +660,6 @@ (define-public opencv
     (home-page "https://opencv.org/")
     (license license:bsd-3)))
 
-(define-public opencv-with-python
-  (package
-    (inherit opencv)
-    (name "opencv-with-python")
-    (arguments
-     (let* ((args (package-arguments opencv)))
-       (assoc-set! args #:configure-flags
-                   (append (list "-DBUILD_opencv_python3=ON")
-                           (assoc-ref args #:configure-flags)))
-       args))))
-
 (define-public vips
   (package
     (name "vips")
diff --git a/gnu/packages/image-viewers.scm b/gnu/packages/image-viewers.scm
index 9574384761..4ebc891427 100644
--- a/gnu/packages/image-viewers.scm
+++ b/gnu/packages/image-viewers.scm
@@ -985,7 +985,7 @@ (define-public xzgv
 (define-public hydrus-network
   (package
     (name "hydrus-network")
-    (version "492")
+    (version "493")
     (source
      (origin
        (method git-fetch)
@@ -995,7 +995,7 @@ (define-public hydrus-network
        (file-name (git-file-name name version))
        (sha256
         (base32
-         "0cyc499is97r8wri0y86yw6kpfcvc0a1yslr8g8sk4vhlly8gnra"))))
+         "1rr2mx3cxjmkbgqdp7827yl3smpgrjs58ljmhx1k1c7pa5cac4xi"))))
     (build-system python-build-system)
     (arguments
      (list
@@ -1012,8 +1012,7 @@ (define-public hydrus-network
                   (setenv "DISPLAY" ":0")
                   (setenv "XDG_CACHE_HOME" (getcwd))
                   (setenv "HOME" (getcwd))
-                  (system "Xvfb &")
-                  (invoke "python" "test.py")))
+                  (invoke "xvfb-run" "python" "test.py")))
               (delete 'build)
               (add-before 'install 'patch-variables
                 (lambda* (#:key outputs #:allow-other-keys)
@@ -1060,10 +1059,10 @@ (define-public hydrus-network
     ;; All native-inputs are only needed for the the check phase
     (native-inputs
      (list
+      xvfb-run
       python-nose
       python-mock
-      python-httmock
-      xorg-server-for-tests))
+      python-httmock))
     ;; All python packages were taken from static/build_files/linux/requirements.txt
     (propagated-inputs
      (list
@@ -1075,7 +1074,7 @@ (define-public hydrus-network
       python-lxml
       python-lz4
       python-numpy
-      opencv-with-python ; drop-in replacement for opencv-python-headless
+      opencv                     ; drop-in replacement for opencv-python-headless
       python-pillow
       python-psutil
       python-pylzma
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 5bc40ecaea..ddfb2b12a4 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -30222,22 +30222,21 @@ (define-public python-mpv
          "10w6j3n62ap45sf6q487kz8z6g58sha37i14fa2hhng794z7a8jh"))
        (modules '((guix build utils)))
        (snippet
-        (with-extensions (list mpv)
-          #~(begin
-              ;; Without an absolute path it is not able find and
-              ;; load the libmpv library.
-              (substitute* "mpv.py"
-                (("(sofile = )(.*)" _ pre post)
-                 (string-append pre "\"" #$mpv "/lib/\" + " post)))
-              ;; One of the tests never completes, so neutering it using
-              ;; early return allows other test to run without issue.
-              (substitute* "tests/test_mpv.py"
-                ;; Note the typo in "prooperty" - this was fixed later in
-                ;; upstream but has no effect on whether the tests hangs or not.
-                (("test_wait_for_prooperty_event_overflow.*" line)
-                 ;; The long whitespace between \n and return is to match the
-                 ;; identation level, which is significant in python.
-                 (string-append line "\n        return\n"))))))))
+        #~(begin
+            ;; Without an absolute path it is not able find and
+            ;; load the libmpv library.
+            (substitute* "mpv.py"
+              (("(sofile = )(.*)" _ pre post)
+               (string-append pre "\"" #$mpv "/lib/\" + " post)))
+            ;; One of the tests never completes, so neutering it using
+            ;; early return allows other test to run without issue.
+            (substitute* "tests/test_mpv.py"
+              ;; Note the typo in "prooperty" - this was fixed later in
+              ;; upstream but has no effect on whether the tests hangs or not.
+              (("test_wait_for_prooperty_event_overflow.*" line)
+               ;; The long whitespace between \n and return is to match the
+               ;; identation level, which is significant in python.
+               (string-append line "\n        return\n")))))))
     (build-system python-build-system)
     (arguments
      (list #:phases