[PATCH] Added DeaDBeeF package to music.scm

  • Open
  • quality assurance status badge
Details
2 participants
  • Maxime Devos
  • Charlie Ruppe
Owner
unassigned
Submitted by
Charlie Ruppe
Severity
normal
C
C
Charlie Ruppe wrote on 26 Mar 2021 16:56
(address . guix-patches@gnu.org)(name . Charlie Ruppe)(address . ruppe.charlie@gmail.com)
20210326155659.26806-1-ruppe.charlie@gmail.com
---
gnu/packages/music.scm | 53 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)

Toggle diff (66 lines)
diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
index 98cd3583cc..eb3933a5d6 100644
--- a/gnu/packages/music.scm
+++ b/gnu/packages/music.scm
@@ -598,6 +598,59 @@ It is a fork of Clementine aimed at music collectors and audiophiles.")
many input formats and provides a customisable Vi-style user interface.")
(license license:gpl2+)))
+(define-public deadbeef
+ (package
+ (name "deadbeef")
+ (version "1.8.4")
+ (source (origin
+ (method git-fetch)
+ (uri (git-reference
+ (url "git://github.com/Alexey-Yakovenko/deadbeef.git")
+ (commit version)
+ (recursive? #t)))
+ (sha256
+ (base32
+ "1mwiblsfzlp2jhhj5mfcy84wxfna9nmx2ia0i5l570hnhj3gxpwn"))))
+ (build-system glib-or-gtk-build-system)
+
+ (arguments '(#:configure-flags '("--enable-silent-rules")
+ #:tests? #f))
+ (inputs `(("gettext" ,gettext-minimal)
+ ("libtool" ,libtool)
+ ("intltool" ,intltool)
+ ("autoconf" ,autoconf)
+ ("automake" ,automake)
+ ("pkg-config" ,pkg-config)
+ ("libsamplerate" ,libsamplerate)
+ ("gtk+" ,gtk+)
+ ("jansson" ,jansson)
+ ("alsa-lib" ,alsa-lib)
+ ("libvorbis" ,libvorbis)
+ ("libogg" ,libogg)
+ ("curl" ,curl)
+ ("imlib2" ,imlib2)
+ ("libjpeg-turbo" ,libjpeg-turbo)
+ ("libmad" ,libmad)
+ ("mpg123" ,mpg123)
+ ("flac" ,flac)
+ ("wavpack" ,wavpack)
+ ("libsndfile" ,libsndfile)
+ ("libcdio" ,libcdio)
+ ("ffmpeg" ,ffmpeg)
+ ("xlib" ,xorg-server)
+ ("dbus" ,dbus)
+ ("pulseaudio" ,pulseaudio)
+ ("faad2" ,faad2)
+ ("zlib" ,zlib)
+ ("libzip" ,libzip)
+ ))
+ (synopsis "Modular audio player for desktop operating systems")
+ (description "DeaDBeeF (as in 0xDEADBEEF) is a modular audio player for GNU/Linux, *BSD, OpenSolaris, macOS, and other UNIX-like systems.
+
+DeaDBeeF lets you play variety of audio formats, convert between them, customize the UI almost any way you want, and use many additional plugins which can extend it even more.")
+ (home-page "https://deadbeef.sourceforge.io/")
+ (license (list license:gpl2+ license:lgpl2.1 license:zlib))))
+
(define-public denemo
(package
(name "denemo")
--
2.31.0
M
M
Maxime Devos wrote on 26 Mar 2021 19:44
c1c7d01bedfb1d4d2d497b1c3eaa9997ccd26500.camel@telenet.be
On Fri, 2021-03-26 at 11:56 -0400, Charlie Ruppe wrote:
Toggle quote (4 lines)
> ---
> gnu/packages/music.scm | 53 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)

You need to write a commit message (see the git repository of guix
for plenty of examples). It's in the manual: ‘16.1 Submitting Patches’.

Toggle quote (19 lines)
> diff --git a/gnu/packages/music.scm b/gnu/packages/music.scm
> index 98cd3583cc..eb3933a5d6 100644
> --- a/gnu/packages/music.scm
> +++ b/gnu/packages/music.scm
> @@ -598,6 +598,59 @@ It is a fork of Clementine aimed at music collectors and audiophiles.")
> many input formats and provides a customisable Vi-style user interface.")
> (license license:gpl2+)))
>
> +(define-public deadbeef
> + (package
> + (name "deadbeef")
> + (version "1.8.4")
> + (source (origin
> + (method git-fetch)
> + (uri (git-reference
> + (url "git://github.com/Alexey-Yakovenko/deadbeef.git")
> + (commit version)
> + (recursive? #t)))

and it seems this repository does not pin the commits used, so this will break
when the submodule repositories are updated.

Toggle quote (8 lines)
> + (sha256
> + (base32
> + "1mwiblsfzlp2jhhj5mfcy84wxfna9nmx2ia0i5l570hnhj3gxpwn"))))
> + (build-system glib-or-gtk-build-system)
> +
> + (arguments '(#:configure-flags '("--enable-silent-rules")
> + #:tests? #f))

Why are tests disabled? When tests are disabled for a package,
a comment explaining why should be added.

Toggle quote (2 lines)
> + (inputs `(("gettext" ,gettext-minimal)

These packages ...
Toggle quote (5 lines)
> + ("libtool" ,libtool)
> + ("intltool" ,intltool)
> + ("autoconf" ,autoconf)
> + ("automake" ,automake)
> + ("pkg-config" ,pkg-config)
... should be in native-inputs (see 8.2.1 ‘‘package‘ Reference’
in the manual).

These following packages should be sorted by name.
Toggle quote (24 lines)
> + ("libsamplerate" ,libsamplerate)
> + ("gtk+" ,gtk+)
> + ("jansson" ,jansson)
> + ("alsa-lib" ,alsa-lib)
> + ("libvorbis" ,libvorbis)
> + ("libogg" ,libogg)
> + ("curl" ,curl)
> + ("imlib2" ,imlib2)
> + ("libjpeg-turbo" ,libjpeg-turbo)
> + ("libmad" ,libmad)
> + ("mpg123" ,mpg123)
> + ("flac" ,flac)
> + ("wavpack" ,wavpack)
> + ("libsndfile" ,libsndfile)
> + ("libcdio" ,libcdio)
> + ("ffmpeg" ,ffmpeg)
> + ("xlib" ,xorg-server)
> + ("dbus" ,dbus)
> + ("pulseaudio" ,pulseaudio)
> + ("faad2" ,faad2)
> + ("zlib" ,zlib)
> + ("libzip" ,libzip)
> + ))

Also, ("xlib" ,xorg-server) does not make much sense to me.
Did you mean "libx11" or "xorg-server" here?
Please move the trailing )) to after ,libzip).

Toggle quote (2 lines)
> + (synopsis "Modular audio player for desktop operating systems")

‘for desktop operating systems’ seems redundant. Since when can I
run Guix System on a so-called ‘smartphone’? (Why ‘quotes’? They are
just small computers that can access the wireless telephone network.)

Toggle quote (3 lines)
> + (description "DeaDBeeF (as in 0xDEADBEEF) is a modular audio player for GNU/Linux, *BSD, OpenSolaris, macOS, and other UNIX-like systems.
> +DeaDBeeF lets you play variety of audio formats, convert between them, customize the UI almost any way you want, and use many additional plugins which can extend it even more.")
>
This line is way to long. This would be pointed out to you if you ran "guix lint"
(see the manual, 16.1 ‘Submitting Patches’)

Avoid marketing wording (‘almost any way you want’, ‘even more’, ‘modular’).
Everthing new is (supposedly) modular, well-written, supports many file formats,
are customisable ... It's like it's the law or something. Just saying it is
extensible should be ok, though. (See the manual, ‘Synopses and Descriptions’

Also, I don't see much reason to mention the various operating systems
it supports. GNU Guix only supports GNU (/Linux & /Hurd).

It doesn't make much sense to mention DeaDBeeF has many plugins, if they
aren't packaged in Guix.

Toggle quote (4 lines)
> + (license (list license:gpl2+ license:lgpl2.1 license:zlib))))

*** Bundling problems

There is some bundling going on here! In tools/glade, there is an
outdated copy of glade from 2007 (https://glade.gnome.org/),there
are also a copy of gettext from 2010 (see intl/ChangeLog)

The submodule deadbeef-osx-deps purely consists of bundling.
There may be some other bundling issues as well (I think I saw
some VLC code somewhere but I don't recall where.

Some bundling of cURL: plugins/artwork/escape.c

*** (Security) bugs

Look e.g. at plugins/artwork/artwork.c. It assumes malloc always succeeds,
never returning NULL. The function local_image_file does simply call
strcpy and does some iteration with assignement without any comments on
why everything is in bounds. I probably could find similar issues elsewhere.

This project has some systematic bad security practices, so I believe a package
for this software should *not* be added to Guix proper (though feel free to
define it in your own channel, see the manual), particularily as there are plenty
of other sound / video players in Guix already.

That said, if "deadbeef" clears up it code base and the bundling issues are addressed
(as well as the much less serious issues of inputs vs. native-inputs and what exactly
goes into synopsis & description), I see no reason why the package couldn't go into
Guix.

I'm sorry if this is rather harsh, but attackers are a very real thing *and*
we have alternative software (e.g. vlc and gnome-music) that works just fine
and (I presume) are much more security-conscious and have more eyeballs looking
at the code ...

Greetings,
Maxime
-----BEGIN PGP SIGNATURE-----

iI0EABYIADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYF4rchccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7pN6AQDMAzIg6z/PGbutnO7PRPVryKSb
bDyp1DV06NRw1qnw3wD/avXdmWi1xwT/t8rIoGAEeg9z4TXf8oZ/pTlVKpqJrg8=
=8l6q
-----END PGP SIGNATURE-----


?
Your comment

Commenting via the web interface is currently disabled.

To comment on this conversation send an email to 47413@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 47413
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch