diff mbox

[bug#60840,0/3] gnu: volctl: Update to 0.9.3.

Message ID 878rhvfr0b.fsf@colimite.fr
State New
Headers show

Commit Message

Sergiu Ivanov Jan. 21, 2023, 10:30 p.m. UTC 
Hi,

Maxim Cournoyer <maxim.cournoyer@gmail.com> [2023-01-16T18:01:59+0100]:
> Sergiu Ivanov <sivanov@colimite.fr> writes:
>
>>>From b92cdb4ce99bc7ad45e0caba7f863db5931741db Mon Sep 17 00:00:00 2001
>>  
>> +(define-public python-pulsectl
>> +  (package
>> +    (name "python-pulsectl")
>> +    (version "22.3.2")
>> +    (source (origin
>> +              (method url-fetch)
>> +              (uri (pypi-uri "pulsectl" version))
>> +              (sha256
>> +               (base32
>> +                "115ha1cwpd2r84ssnxdbr59hgs0jbx0lz3xpqli64kmxxqf4w5yc"))))
>> +    (build-system python-build-system)
>> +    (inputs (list pulseaudio))
>> +    (arguments
>> +     `(#:tests? #f
>
> Tests are typically stripped from the pypi source archive (sdist).  If
> you look into the source repository, there are tests under
> pulsectl/tests, so it'd be better to fetch the source from git.

In fact, pulsectl's tests fail because they seem to want to start
a dummy PulseAudio instance, which I expect to fail because of the
restrictions of the build environment.  Here's my post on the mailing
list with some more details:

https://lists.gnu.org/archive/html/help-guix/2023-01/msg00038.html

I added a comment briefly explaining this, but maybe there is
a better way.

> Also note that for the cases where using #:tests? #f is actually needed
> (when there really are no test suite), a short explanatory comment is
> expected (;no test suite).
>
>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-after 'unpack 'patch-path
>> +           (lambda* (#:key inputs #:allow-other-keys)
>> +             (let ((pulse (assoc-ref inputs "pulseaudio")))
>> +               (substitute* "pulsectl/_pulsectl.py"
>> +                 (("libpulse.so.0")
>> +                  (string-append pulse "/lib/libpulse.so.0")))
>> +               #t))))))
>
> Please do not include trailing #t in phases or snippets anymore; they
> are not needed.

Fixed, thank you.

> Also prefer using a plain list for arguments and g-expressions
> (gexps).

I spent some time squinting at this remark and reading the manuals, but
I can't figure out what you mean.

Could you please give some more hints about the parts I should change
and how?

>> +    (home-page "https://github.com/mk-fg/python-pulse-control")
>> +    (synopsis
>> +     "Python bindings for mixer-like controls in PulseAudio")
>> +    (description
>> +     "Python high-level interface and ctypes-based bindings for
>> +PulseAudio (libpulse), to use in simple synchronous code.  This wrapper is
>> +mostly for mixer-like controls and introspection-related operations, as
>> +opposed to e.g. submitting sound samples to play and player-like
>> client.")
>
> I'd start the description with "This package provides a Python
> high-level interface [...]", to make it a complete sentence.
>
> I'd use plural for the last word (player-like clientS), as there could
> be more than one client available.

Done, thank you.

> Don't forget to CC my email when sending a revised v2 version with the
> above :-).

Done as well :D


Maxim Cournoyer <maxim.cournoyer@gmail.com> [2023-01-16T18:06:19+0100]:
> Hi again,
>
> Sergiu Ivanov <sivanov@colimite.fr> writes:
[...]
>> +    (arguments
>> +     `(#:tests? #f
>> +       #:phases
>> +       (modify-phases %standard-phases
>> +         (add-after 'unpack 'patch-path
>> +           (lambda* (#:key inputs #:allow-other-keys)
>> +             (let ((pulse (assoc-ref inputs "pulseaudio")))
>> +               (substitute* "pulsectl/_pulsectl.py"
>> +                 (("libpulse.so.0")
>> +                  (string-append pulse "/lib/libpulse.so.0")))
>
> Sorry, I forgot to mention in my previous reply: here, you could use
> (search-input-file inputs "lib/libpulse.so.0"), which has the added
> benefit of failing if the file cannot be found in the inputs arguments.

Oh, good to know, thank you for the suggestion!  search-input-file
actually simplified the code and allowed me to drop the let (which
I copied from the previous version of volctl in fact).

I updated patches 2 and 3 to use search-input-file and attach both to
these E-mails.

By the way, I'd be happy to know whether with debbugs it is better to
attach the updated patches to E-mails with comments, or rather sending
the patches as separate E-mails.

-
Sergiu
diff mbox

Patch

From f98307b017c0cfb1a4a46f9a882ec558cec294b1 Mon Sep 17 00:00:00 2001
From: Sergiu Ivanov <sivanov@colimite.fr>
Date: Sun, 15 Jan 2023 23:23:44 +0100
Subject: [PATCH 3/3] gnu: volctl: Update to 0.9.3.

* gnu/packages/gtk.scm (volctl): Update to 0.9.3.
---
 gnu/packages/gtk.scm | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/gnu/packages/gtk.scm b/gnu/packages/gtk.scm
index ea22d0e930..2e6a546308 100644
--- a/gnu/packages/gtk.scm
+++ b/gnu/packages/gtk.scm
@@ -33,6 +33,7 @@ 
 ;;; Copyright © 2022 Benjamin Slade <slade@lambda-y.net>
 ;;; Copyright © 2022 Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
 ;;; Copyright © 2022 Petr Hodina <phodina@protonmail.com>
+;;; Copyright © 2023 Sergiu Ivanov <sivanov@colimite.fr>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -67,6 +68,7 @@  (define-module (gnu packages gtk)
   #:use-module (guix build-system waf)
   #:use-module (gnu packages)
   #:use-module (gnu packages algebra)
+  #:use-module (gnu packages audio)
   #:use-module (gnu packages autotools)
   #:use-module (gnu packages base)
   #:use-module (gnu packages bash)
@@ -2949,7 +2951,7 @@  (define-public gtkdatabox
 (define-public volctl
   (package
     (name "volctl")
-    (version "0.8.2")
+    (version "0.9.3")
     (source (origin
               (method git-fetch)
               (uri (git-reference (url "https://github.com/buzz/volctl")
@@ -2957,26 +2959,21 @@  (define-public volctl
               (file-name (git-file-name name version))
               (sha256
                (base32
-                "1cx27j83pz2qffnzb85fbl1x6pp3irv1kbw7g1hri7kaw6ky4xiz"))))
+                "0fz80w3ywq54jn4v31frfdj01s5g9lz6v9cd7hpg3kirca0zisln"))))
     (build-system python-build-system)
     (arguments
      `(#:phases
        (modify-phases %standard-phases
          (add-after 'unpack 'patch-path
            (lambda* (#:key inputs #:allow-other-keys)
-             (let ((pulse (assoc-ref inputs "pulseaudio"))
-                   (xfixes (assoc-ref inputs "libxfixes")))
-               (substitute* "volctl/lib/xwrappers.py"
-                 (("libXfixes.so")
-                  (string-append xfixes "/lib/libXfixes.so")))
-               (substitute* "volctl/lib/pulseaudio.py"
-                 (("libpulse.so.0")
-                  (string-append pulse "/lib/libpulse.so.0")))
-               #t))))))
+             (substitute* "volctl/xwrappers.py"
+               (("libXfixes.so")
+                (string-append (search-input-file inputs "/lib/libXfixes.so")))))))))
     (inputs
-     (list gtk+ libxfixes pulseaudio))
+     (list libxfixes))
     (propagated-inputs
-     (list python-click python-pycairo python-pygobject python-pyyaml))
+     (list python-click python-pycairo python-pygobject python-pyyaml
+           python-pulsectl gtk+))
     (home-page "https://buzz.github.io/volctl/")
     (synopsis "Per-application volume control and on-screen display")
     (description "Volctl is a PulseAudio-enabled tray icon volume control and
-- 
2.38.1