GNU bug report logs - #62353
[PATCH] gnu: Add glava.

Previous Next

Package: guix-patches;

Reported by: Spencer Skylar Chan <schan12 <at> umd.edu>

Date: Tue, 21 Mar 2023 20:17:01 UTC

Severity: normal

Tags: patch

To reply to this bug, email your comments to 62353 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#62353; Package guix-patches. (Tue, 21 Mar 2023 20:17:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Spencer Skylar Chan <schan12 <at> umd.edu>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Tue, 21 Mar 2023 20:17:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Spencer Skylar Chan <schan12 <at> umd.edu>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: Add glava.
Date: Tue, 21 Mar 2023 13:58:46 -0400
From 17eb638d8ff5d5cb40c9bdf9f24544ef7f23088e Mon Sep 17 00:00:00 2001
From: Skylar Chan <schan12 <at> umd.edu>
Date: Mon, 30 Jan 2023 13:39:53 -0500
Subject: [PATCH] gnu: Add glava.

* gnu/packages/audio.scm (glava): New variable.
---
 gnu/packages/audio.scm | 75 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/gnu/packages/audio.scm b/gnu/packages/audio.scm
index 6f3fa2a580..a0e73ed55a 100644
--- a/gnu/packages/audio.scm
+++ b/gnu/packages/audio.scm
@@ -5109,6 +5109,81 @@ (define-public cava
 using ALSA, MPD, PulseAudio, or a FIFO buffer as its input.")
     (license license:expat)))

+(define-public glava
+  (package
+    (name "glava")
+    (version "1.6.3")
+    (source
+     (origin
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/jarcode-foss/glava")
+             (commit (string-append "v" version))))
+       (file-name (git-file-name name version))
+       (sha256
+        (base32
+         "0kqkjxmpqkmgby05lsf6c6iwm45n33jk5qy6gi3zvjx4q4yzal1i"))))
+    (build-system gnu-build-system)
+    (arguments
+     (list #:tests? #f ;no tests.
+           #:make-flags
+           #~(list
+              (string-append "DESTDIR=" #$output)
+              (string-append "PREFIX=" #$output)
+              (string-append "CC=" #$(cc-for-target)))
+           #:phases
+           #~(modify-phases %standard-phases
+               (add-after 'unpack 'fix-etc-references
+                 (lambda* (#:key outputs #:allow-other-keys)
+                   (substitute* (find-files ".*")
+                     (("/etc/xdg") (string-append #$output 
"/etc/xdg/glava")))))
+               (add-after 'unpack 'patch-makefile
+                 (lambda _
+                   (substitute* "Makefile"
+                     (("$(DESTDIR)$(SHADERDIR)")
+                      "$(SHADERDIR)"))))
+               (delete 'configure)
+               (add-after 'install 'make-wrapper
+                 ;; 
https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/misc/glava/default.nix
+                 (lambda* (#:key inputs outputs #:allow-other-keys)
+                   (let* ((wrapper (string-append #$output "/bin/glava")))
+                     (with-output-to-file wrapper
+                       (lambda _
+                         (display
+                          (string-append
+                           "#!/bin/sh\n"
+                           "case \"$1\" in\n"
+                           "  --copy-config|-C)\n"
+                           "# The binary would symlink it, which won't 
work in Guix because the\n"
+                           "# garbage collector will eventually remove 
the original files after\n"
+                           "# updates.\n"
+                           "  echo \"Guix wrapper: Copying glava config 
to ~/.config/glava\"\n"
+                           "  cp -r --no-preserve=all " #$output 
"/etc/xdg/glava ~/.config/glava\n"
+                           "  ;;\n"
+                           "*)\n"
+                           "  exec " #$output "/bin/.glava-real \"$@\"\n"
+                           "esac\n"))))
+                     (chmod wrapper #o555))))
+               (add-after 'install 'rename-binary
+                 (lambda* (#:key outputs #:allow-other-keys)
+                   (mkdir-p (string-append #$output "/bin"))
+                   (rename-file (string-append #$output "/usr/bin/glava")
+                                (string-append #$output 
"/bin/.glava-real"))
+                   (delete-file-recursively (string-append #$output 
"/usr")))))))
+    (inputs (list pulseaudio
+                  libx11
+                  libxext
+                  libxcomposite
+                  libxrender))
+    (native-inputs (list python))
+    (home-page "https://github.com/jarcode-foss/glava")
+    (synopsis "OpenGL audio spectrum visualizer")
+    (description "GLava is an OpenGL audio spectrum visualizer using 
PulseAudio or
+MPD's fifo output.  Its primary use case is for desktop windows or 
backgrounds.
+It is compatible with most EMWH compliant window managers.")
+    (license (list license:gpl3+
+                   license:expat)))) ;; khrplatform.h
+
 (define-public fluid-3
   (let ((commit "871c8ce2002e8b3c198f532fdb4fbcce7914f951"))
     (package

base-commit: 60a211ec705ac98483d76da7f2523f2b8966343a
-- 
2.39.2





Information forwarded to guix-patches <at> gnu.org:
bug#62353; Package guix-patches. (Wed, 29 Mar 2023 03:02:02 GMT) Full text and rfc822 format available.

Message #8 received at 62353 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Spencer Skylar Chan <schan12 <at> umd.edu>
Cc: 62353 <at> debbugs.gnu.org
Subject: Re: bug#62353: [PATCH] gnu: Add glava.
Date: Tue, 28 Mar 2023 23:01:34 -0400
Hi Spencer,

Spencer Skylar Chan <schan12 <at> umd.edu> writes:

>  From 17eb638d8ff5d5cb40c9bdf9f24544ef7f23088e Mon Sep 17 00:00:00 2001
> From: Skylar Chan <schan12 <at> umd.edu>
> Date: Mon, 30 Jan 2023 13:39:53 -0500
> Subject: [PATCH] gnu: Add glava.
>
> * gnu/packages/audio.scm (glava): New variable.

Thank you!

I'm having problem applying your patch; git says it's corrupted.  It
seems the longer lines were folded?  I'm using this opportunity to share
some review comments.

> ---
>   gnu/packages/audio.scm | 75 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 75 insertions(+)
>
> diff --git a/gnu/packages/audio.scm b/gnu/packages/audio.scm
> index 6f3fa2a580..a0e73ed55a 100644
> --- a/gnu/packages/audio.scm
> +++ b/gnu/packages/audio.scm
> @@ -5109,6 +5109,81 @@ (define-public cava
>   using ALSA, MPD, PulseAudio, or a FIFO buffer as its input.")
>       (license license:expat)))
>
> +(define-public glava
> +  (package
> +    (name "glava")
> +    (version "1.6.3")
> +    (source
> +     (origin
> +       (method git-fetch)
> +       (uri (git-reference
> +             (url "https://github.com/jarcode-foss/glava")
> +             (commit (string-append "v" version))))
> +       (file-name (git-file-name name version))
> +       (sha256
> +        (base32
> +         "0kqkjxmpqkmgby05lsf6c6iwm45n33jk5qy6gi3zvjx4q4yzal1i"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     (list #:tests? #f ;no tests.

nitpick: You can drop the trailing period since inline comments are not
complete sentence (contrary to standalone comments, which should be).

> +           #:make-flags
> +           #~(list
> +              (string-append "DESTDIR=" #$output)
> +              (string-append "PREFIX=" #$output)
> +              (string-append "CC=" #$(cc-for-target)))
> +           #:phases
> +           #~(modify-phases %standard-phases
> +               (add-after 'unpack 'fix-etc-references
> +                 (lambda* (#:key outputs #:allow-other-keys)
> +                   (substitute* (find-files ".*")

The first argument to find-files is a directory, not a regexp; so it
should probably be ".".  It's usually best to use a regexps as a 2nd
argument otherwise for large projects it wastes IO scanning tons of
files and sometimes fails on binary images for example.

> +                     (("/etc/xdg") (string-append #$output 
> "/etc/xdg/glava")))))
> 
> +               (add-after 'unpack 'patch-makefile
> +                 (lambda _
> +                   (substitute* "Makefile"
> +                     (("$(DESTDIR)$(SHADERDIR)")
> +                      "$(SHADERDIR)"))))

This substitution must do anything, as the pattern
"$(DESTDIR)$(SHADERDIR)" is a regexp, and $ matches the end of line
character.  You probably meant to escape regexp special characters like:

"\\$\\(DESTDIR\\)\\$\\(SHADERDIR\\)"

in the pattern.

> +               (delete 'configure)
> +               (add-after 'install 'make-wrapper
> +                 ;; 
> https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/misc/glava/default.nix

Please leave a verbose comment (complete sentence) explaining what is
the issue, and prefer linking to the 'raw' github file in a (see: $URL)
at the end of the explanation.

> +                 (lambda* (#:key inputs outputs #:allow-other-keys)

'inputs' appears unused.

> +                   (let* ((wrapper (string-append #$output "/bin/glava")))
> +                     (with-output-to-file wrapper
> +                       (lambda _
> +                         (display
> +                          (string-append
> +                           "#!/bin/sh\n"
> +                           "case \"$1\" in\n"
> +                           "  --copy-config|-C)\n"
> +                           "# The binary would symlink it, which won't 
> work in Guix because the\n"
> +                           "# garbage collector will eventually remove 
> the original files after\n"
> +                           "# updates.\n"
> +                           "  echo \"Guix wrapper: Copying glava config 
> to ~/.config/glava\"\n"
> +                           "  cp -r --no-preserve=all " #$output 
> "/etc/xdg/glava ~/.config/glava\n"
> +                           "  ;;\n"
> +                           "*)\n"
> +                           "  exec " #$output "/bin/.glava-real \"$@\"\n"
> +                           "esac\n"))))
> +                     (chmod wrapper #o555))))

Hm.  I'm not sure we want to preserve this behavior of messing with
the user's HOME files, but I haven't read the issue to get a grasp of
what is the exact problem it tries to solve.

> +               (add-after 'install 'rename-binary
> +                 (lambda* (#:key outputs #:allow-other-keys)
> +                   (mkdir-p (string-append #$output "/bin"))
> +                   (rename-file (string-append #$output "/usr/bin/glava")
> +                                (string-append #$output 
> "/bin/.glava-real"))
> +                   (delete-file-recursively (string-append #$output 
> "/usr")))))))
> +    (inputs (list pulseaudio
> +                  libx11
> +                  libxext
> +                  libxcomposite
> +                  libxrender))
> +    (native-inputs (list python))
> +    (home-page "https://github.com/jarcode-foss/glava")
> +    (synopsis "OpenGL audio spectrum visualizer")
> +    (description "GLava is an OpenGL audio spectrum visualizer using 
> PulseAudio or
> +MPD's fifo output.  Its primary use case is for desktop windows or 
> backgrounds.
> +It is compatible with most EMWH compliant window managers.")
> +    (license (list license:gpl3+
> +                   license:expat)))) ;; khrplatform.h

nitpick: Please use single ';' for inline comments :-)

Could you please send a revised version with the above taken care of?

-- 
Thanks,
Maxim




This bug report was last modified 2 years and 139 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.