| Message ID | 20210326155659.26806-1-ruppe.charlie@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [bug#47413] Added DeaDBeeF package to music.scm | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/submitting builds | success | |
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | success | View Laminar job |
| cbaines/issue | success | View issue |
On Fri, 2021-03-26 at 11:56 -0400, Charlie Ruppe wrote: > --- > 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’. > 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))) I looked at <https://github.com/DeaDBeeF-Player/deadbeef/blob/master/.gitmodules>, and it seems this repository does not pin the commits used, so this will break when the submodule repositories are updated. > + (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. > + (inputs `(("gettext" ,gettext-minimal) These packages ... > + ("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. > + ("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). > + (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.) > + (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. > + (home-page "https://deadbeef.sourceforge.io/") > + (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
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")