[bug#49783] [PATCH] Add vimpc mpd client package definition

[Maxime Devos]

Hi,

guil_fsf--- via Guix-patches via schreef op vr 30-07-2021 om 15:30 [+0200]:
> ---
> gnu/packages/mpd.scm | 50 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 50 insertions(+)

A commit message is missing.

> diff --git a/gnu/packages/mpd.scm b/gnu/packages/mpd.scm
> index b97abcafc3..2c1f189421 100644
> --- a/gnu/packages/mpd.scm
> +++ b/gnu/packages/mpd.scm
> @@ -513,3 +513,53 @@ for your favorite MPD client but an addition to get a 
> better
> album-experience.")
>      (home-page "https://gitlab.com/coderkun/mcg";)
>      (license license:gpl3+)))

> +
> +(define-module (vimpc)
> +  #:use-module (guix licenses)
> +  #:use-module (guix packages)
> +  #:use-module (guix build-system gnu)
> +  #:use-module (guix git-download)
> +  #:use-module (gnu packages mpd)
> +  #:use-module (gnu packages pcre)
> +  #:use-module (gnu packages mp3)
> +  #:use-module (gnu packages autotools)
> +  #:use-module (gnu packages curl)
> +  #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages ncurses))

This define-module block can be removed,
and the #:use-module can be moved to the top of the file.

> +(define-public vimpc
> +  (package
> +   (name "vimpc")
> +   (version "0.09.2")
> +
> +   (source (origin
> +            (method git-fetch)
> +            (uri (git-reference
> +                  (url "https://github.com/boysetsfrog/vimpc.git";)

I think there is a linter checking that ".git" is not included in GitHub
URLs.  Try "./pre-inst-env guix lint vimpc".

> +                  (commit (string-append "v" version))))
> +            (sha256 (base32 
> "0lswzkap2nm7v5h7ppb6a64cb35rajysd09nb204rxgrkij4m6nx"))
> +            (file-name (git-file-name name version))))
> +   (build-system gnu-build-system)
> +   (native-inputs `(("autoconf" ,autoconf)
> +     ("automake" ,automake)))
> +   (inputs `(("libmpdclient" ,libmpdclient)
> +      ("pcre" ,pcre)
> +      ("taglib" ,taglib)
> +      ("pkg-config" ,pkg-config)

"pkg-config" should most likely be in 'native-inputs', not 'inputs'.
The difference between 'inputs' and 'native-inputs' matters when
cross-compiling.

I'd suggest running
"./pre-inst-env guix build vimpc --target=x86_64-linux-gnu"
for testing, but meson-build-system (used by libmpdclient) doesn't
support cross-compilation yet (at least, on master it doesn't.  On the
core-updates branch, meson-build-system supports cross-compilation),
so that wouldn't work yet anyway.

> +      ("curl" ,curl)
> +      ("ncurses" ,ncurses)
> +      ))
> +   (arguments
> +       `(#:tests? #false

When disabling tests, always write a comment explaining why.
If the tests fail, that may indicate a real problem (that's
the purpose of tests).

> +
> +         #:phases
> +         (modify-phases %standard-phases
> +           (add-after 'unpack 'fix-hardcoded-paths
> +             (lambda _
> +                  (system* "./autogen")
> +        )))))

The formatting went wrong here (whitespace and lonely parentheses).
I'd recommend using "git format-patch HEAD^..HEAD" and sending the patch
as an attachment, or using "git send-email".

> +   (synopsis "Vi-like Mpd TUI client written in C++")
> +   (description "Vimpc provides an alternative to other mpd clients (such as 
> ncmpc and ncmpcpp) that tries to provide an interface similar to that of the 
> vim text editor.")

This description doesn't explain anything to people who don't know what
'Mpd', 'Vi', 'nmpc' and 'ncmpcpp' are.  I only know of the first two
(a music/video player and a customisable text editor), but only because
I've read something about it some time ago. 

Browsing through the source code, I notice files named "lyricsloader.cpp",
"lyricsfetcher.cpp", "song.cpp".  Maybe you could note that "vimpc"
is a music player?

Greetings,
Maxime.

signature.asc
Description: This is a digitally signed message part