[PATCH] Update ncmpcpp to v0.6.2

[PATCH] Update ncmpcpp to v0.6.2

[Paul van der Walt]

[-- Attachment #1: Type: text/plain, Size: 211 bytes --]

Hello again,

A patch to update ncmpcpp, the mpd frontend. I hope my patch makes more
sense now -- the build system is a little finicky, so perhaps there's a
neater way to accomplish what i'm doing.

Thanks,
p.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-ncmpcpp-Update-to-0.6.2.patch --]
[-- Type: text/x-diff, Size: 2709 bytes --]

From e1382a9df3e7df94e794df8aa247920b7e28c0c6 Mon Sep 17 00:00:00 2001
From: Paul van der Walt <paul@denknerd.org>
Date: Fri, 27 Feb 2015 10:54:07 +0100
Subject: [PATCH] gnu: ncmpcpp: Update to 0.6.2.

* gnu/packages/mpd.scm (ncmpcpp): Update to 0.6.2. Add readline, boost,
  autotools. Patch build system to find boost correctly (without -mt suffix).
---
 gnu/packages/mpd.scm | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/mpd.scm b/gnu/packages/mpd.scm
index a1b4272..1daf261 100644
--- a/gnu/packages/mpd.scm
+++ b/gnu/packages/mpd.scm
@@ -27,6 +27,8 @@
   #:use-module (guix utils)
   #:use-module (guix build-system gnu)
   #:use-module (gnu packages avahi)
+  #:use-module (gnu packages boost)
+  #:use-module (gnu packages readline)
   #:use-module (gnu packages compression)
   #:use-module (gnu packages curl)
   #:use-module (gnu packages doxygen)
@@ -35,6 +37,7 @@
   #:use-module (gnu packages mp3)
   #:use-module (gnu packages ncurses)
   #:use-module (gnu packages pkg-config)
+  #:use-module (gnu packages autotools)
   #:use-module (gnu packages pulseaudio)
   #:use-module (gnu packages databases)
   #:use-module (gnu packages video)
@@ -153,7 +156,7 @@ terminal using ncurses.")
 (define ncmpcpp
   (package
     (name "ncmpcpp")
-    (version "0.5.10")
+    (version "0.6.2")
     (source (origin
               (method url-fetch)
               (uri
@@ -161,11 +164,29 @@ terminal using ncurses.")
                               version ".tar.bz2"))
               (sha256
                (base32
-                "1a54g6dary1rirrny9fd0hpxpyyffypni3mpbdpvmjnrl9v56vgz"))))
+                "1mrd6m6ph0fscxp9x96ipxh6ai7w0n1miapcfqrqfy058qx5zbck"))))
     (build-system gnu-build-system)
     (inputs `(("libmpdclient" ,libmpdclient)
               ("ncurses" ,ncurses)))
-    (native-inputs `(("pkg-config" ,pkg-config)))
+    (native-inputs
+     `(("pkg-config" ,pkg-config)
+       ("readline" ,readline)
+       ("automake" ,automake)
+       ("autoconf" ,autoconf)
+       ("libtool" ,libtool)
+       ("boost" ,boost)))
+    (arguments
+     '(#:configure-flags
+       '("BOOST_LIB_SUFFIX=")
+       #:phases
+       (alist-cons-after
+        'unpack 'autogen
+        (lambda _
+          (substitute* "autogen.sh"
+            (("/bin/sh") (which "bash")))
+          (setenv "NOCONFIGURE" "true")
+          (zero? (system* "bash" "autogen.sh")))
+        %standard-phases)))
     (synopsis "Featureful ncurses based MPD client inspired by ncmpc")
     (description "Ncmpcpp is an mpd client with a UI very similar to ncmpc,
 but it provides new useful features such as support for regular expressions
-- 
2.3.1

Re: [PATCH] Update ncmpcpp to v0.6.2

[David Thompson]

Paul van der Walt <paul@denknerd.org> writes:

> A patch to update ncmpcpp, the mpd frontend. I hope my patch makes more
> sense now -- the build system is a little finicky, so perhaps there's a
> neater way to accomplish what i'm doing.

Thanks for updating this!

> From e1382a9df3e7df94e794df8aa247920b7e28c0c6 Mon Sep 17 00:00:00 2001
> From: Paul van der Walt <paul@denknerd.org>
> Date: Fri, 27 Feb 2015 10:54:07 +0100
> Subject: [PATCH] gnu: ncmpcpp: Update to 0.6.2.
>
> * gnu/packages/mpd.scm (ncmpcpp): Update to 0.6.2. Add readline, boost,
>   autotools. Patch build system to find boost correctly (without -mt suffix).
> ---
>  gnu/packages/mpd.scm | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)

Could you please add a copyright line for yourself at the top of this
file?

>
> diff --git a/gnu/packages/mpd.scm b/gnu/packages/mpd.scm
> index a1b4272..1daf261 100644
> --- a/gnu/packages/mpd.scm
> +++ b/gnu/packages/mpd.scm
> @@ -27,6 +27,8 @@
>    #:use-module (guix utils)
>    #:use-module (guix build-system gnu)
>    #:use-module (gnu packages avahi)
> +  #:use-module (gnu packages boost)
> +  #:use-module (gnu packages readline)
>    #:use-module (gnu packages compression)
>    #:use-module (gnu packages curl)
>    #:use-module (gnu packages doxygen)
> @@ -35,6 +37,7 @@
>    #:use-module (gnu packages mp3)
>    #:use-module (gnu packages ncurses)
>    #:use-module (gnu packages pkg-config)
> +  #:use-module (gnu packages autotools)
>    #:use-module (gnu packages pulseaudio)
>    #:use-module (gnu packages databases)
>    #:use-module (gnu packages video)
> @@ -153,7 +156,7 @@ terminal using ncurses.")
>  (define ncmpcpp
>    (package
>      (name "ncmpcpp")
> -    (version "0.5.10")
> +    (version "0.6.2")
>      (source (origin
>                (method url-fetch)
>                (uri
> @@ -161,11 +164,29 @@ terminal using ncurses.")
>                                version ".tar.bz2"))
>                (sha256
>                 (base32
> -                "1a54g6dary1rirrny9fd0hpxpyyffypni3mpbdpvmjnrl9v56vgz"))))
> +                "1mrd6m6ph0fscxp9x96ipxh6ai7w0n1miapcfqrqfy058qx5zbck"))))
>      (build-system gnu-build-system)
>      (inputs `(("libmpdclient" ,libmpdclient)
>                ("ncurses" ,ncurses)))
> -    (native-inputs `(("pkg-config" ,pkg-config)))
> +    (native-inputs
> +     `(("pkg-config" ,pkg-config)
> +       ("readline" ,readline)
> +       ("automake" ,automake)
> +       ("autoconf" ,autoconf)
> +       ("libtool" ,libtool)
> +       ("boost" ,boost)))

I doubt that readline and boost should be native inputs.  Please add
them as regular inputs.

> +    (arguments
> +     '(#:configure-flags
> +       '("BOOST_LIB_SUFFIX=")
> +       #:phases
> +       (alist-cons-after
> +        'unpack 'autogen
> +        (lambda _
> +          (substitute* "autogen.sh"
> +            (("/bin/sh") (which "bash")))

Is "autogen.sh" not executable?  If it starts with a shebang, Guix
should patch the file appropriately.  Does the build fail with this
removed?

> +          (setenv "NOCONFIGURE" "true")
> +          (zero? (system* "bash" "autogen.sh")))

If its executable, you can say:

    (zero? (system* "./autogen.sh"))

It's strange that the project used to ship a pre-built Makefile but now
requires users to do the autotools bootstrapping.  Oh well.

> +        %standard-phases)))
>      (synopsis "Featureful ncurses based MPD client inspired by ncmpc")
>      (description "Ncmpcpp is an mpd client with a UI very similar to ncmpc,
>  but it provides new useful features such as support for regular expressions
> -- 
> 2.3.1
>

Could you send an updated patch?  Thanks!

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] Update ncmpcpp to v0.6.2

[Mark H Weaver]

David Thompson <dthompson2@worcester.edu> writes:

> Paul van der Walt <paul@denknerd.org> writes:
[...]
>> +    (arguments
>> +     '(#:configure-flags
>> +       '("BOOST_LIB_SUFFIX=")
>> +       #:phases
>> +       (alist-cons-after
>> +        'unpack 'autogen
>> +        (lambda _
>> +          (substitute* "autogen.sh"
>> +            (("/bin/sh") (which "bash")))
>
> Is "autogen.sh" not executable?  If it starts with a shebang, Guix
> should patch the file appropriately.

The problem here is that this 'autogen' phase is run before the
'patch-source-shebangs' phase, at my suggestion.  It has to be that way,
because 'patch-source-shebangs' (and 'patch-usr-bin-file') will need to
be run on the files created by 'autogen.sh'.

>> +          (setenv "NOCONFIGURE" "true")
>> +          (zero? (system* "bash" "autogen.sh")))
>
> If its executable, you can say:
>
>     (zero? (system* "./autogen.sh"))

Yes, after the 'substitute*' above, this should presumably work.  My
guess is that Paul added the "bash" here before he realized that he
also needed the 'substitute*'.

      Mark

Re: [PATCH] Update ncmpcpp to v0.6.2

[David Thompson]

Mark H Weaver <mhw@netris.org> writes:

> David Thompson <dthompson2@worcester.edu> writes:
>
>> Paul van der Walt <paul@denknerd.org> writes:
> [...]
>>> +    (arguments
>>> +     '(#:configure-flags
>>> +       '("BOOST_LIB_SUFFIX=")
>>> +       #:phases
>>> +       (alist-cons-after
>>> +        'unpack 'autogen
>>> +        (lambda _
>>> +          (substitute* "autogen.sh"
>>> +            (("/bin/sh") (which "bash")))
>>
>> Is "autogen.sh" not executable?  If it starts with a shebang, Guix
>> should patch the file appropriately.
>
> The problem here is that this 'autogen' phase is run before the
> 'patch-source-shebangs' phase, at my suggestion.  It has to be that way,
> because 'patch-source-shebangs' (and 'patch-usr-bin-file') will need to
> be run on the files created by 'autogen.sh'.

Oh, okay.  Right.  Thanks for explaining.

>
>>> +          (setenv "NOCONFIGURE" "true")
>>> +          (zero? (system* "bash" "autogen.sh")))
>>
>> If its executable, you can say:
>>
>>     (zero? (system* "./autogen.sh"))
>
> Yes, after the 'substitute*' above, this should presumably work.  My
> guess is that Paul added the "bash" here before he realized that he
> also needed the 'substitute*'.

Makes sense.

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] Update ncmpcpp to v0.6.2

[Paul van der Walt]

Hi,

I'm a little busy for now, but i'll get to this soon. Thanks for your
comments!

On 2015-02-27 at 17:13, quoth Mark H Weaver:
>> Paul van der Walt <paul@denknerd.org> writes:
> [...]
>>> +    (arguments
>>> +     '(#:configure-flags
>>> +       '("BOOST_LIB_SUFFIX=")
>>> +       #:phases
>>> +       (alist-cons-after
>>> +        'unpack 'autogen
>>> +        (lambda _
>>> +          (substitute* "autogen.sh"
>>> +            (("/bin/sh") (which "bash")))
>>
>> Is "autogen.sh" not executable?  If it starts with a shebang, Guix
>> should patch the file appropriately.
>
> The problem here is that this 'autogen' phase is run before the
> 'patch-source-shebangs' phase, at my suggestion.  It has to be that way,
> because 'patch-source-shebangs' (and 'patch-usr-bin-file') will need to
> be run on the files created by 'autogen.sh'.

This is the case. Is this therefore the cleanest way to do it?

>>> +          (setenv "NOCONFIGURE" "true")
>>> +          (zero? (system* "bash" "autogen.sh")))
>>
>> If its executable, you can say:
>>
>>     (zero? (system* "./autogen.sh"))
>
> Yes, after the 'substitute*' above, this should presumably work.  My
> guess is that Paul added the "bash" here before he realized that he
> also needed the 'substitute*'.

This is what happened. I'll modify that. One last question:

>> From e1382a9df3e7df94e794df8aa247920b7e28c0c6 Mon Sep 17 00:00:00 2001
>> From: Paul van der Walt <paul@denknerd.org>
>> ...
>
> Could you please add a copyright line for yourself at the top of this
> file?

Are these from lines not enough? My previous patch which i generated the
same way was accepted.

Bye! Have good weekends!
p.

Re: [PATCH] Update ncmpcpp to v0.6.2

[David Thompson]

Paul van der Walt <paul@denknerd.org> writes:

> On 2015-02-27 at 17:13, quoth Mark H Weaver:
>>> Paul van der Walt <paul@denknerd.org> writes:
>> [...]
>>>> +    (arguments
>>>> +     '(#:configure-flags
>>>> +       '("BOOST_LIB_SUFFIX=")
>>>> +       #:phases
>>>> +       (alist-cons-after
>>>> +        'unpack 'autogen
>>>> +        (lambda _
>>>> +          (substitute* "autogen.sh"
>>>> +            (("/bin/sh") (which "bash")))
>>>
>>> Is "autogen.sh" not executable?  If it starts with a shebang, Guix
>>> should patch the file appropriately.
>>
>> The problem here is that this 'autogen' phase is run before the
>> 'patch-source-shebangs' phase, at my suggestion.  It has to be that way,
>> because 'patch-source-shebangs' (and 'patch-usr-bin-file') will need to
>> be run on the files created by 'autogen.sh'.
>
> This is the case. Is this therefore the cleanest way to do it?

Yes.

>> Could you please add a copyright line for yourself at the top of this
>> file?
>
> Are these from lines not enough? My previous patch which i generated the
> same way was accepted.

Each source code file has copyright information at the very top for each
person that has contributed code to that file.  Follow the pattern of
the other contributor(s) and add your information.  Adding copyright
information is not necessary for trivial changes, but you've definitely
made non-trivial changes here.

Thanks!

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] Update ncmpcpp to v0.6.2

[Mark H Weaver]

Paul van der Walt <paul@denknerd.org> writes:

> On 2015-02-27 at 17:13, quoth Mark H Weaver:
>>> Paul van der Walt <paul@denknerd.org> writes:
>> [...]
>>>> +    (arguments
>>>> +     '(#:configure-flags
>>>> +       '("BOOST_LIB_SUFFIX=")
>>>> +       #:phases
>>>> +       (alist-cons-after
>>>> +        'unpack 'autogen
>>>> +        (lambda _
>>>> +          (substitute* "autogen.sh"
>>>> +            (("/bin/sh") (which "bash")))
>>>
>>> Is "autogen.sh" not executable?  If it starts with a shebang, Guix
>>> should patch the file appropriately.
>>
>> The problem here is that this 'autogen' phase is run before the
>> 'patch-source-shebangs' phase, at my suggestion.  It has to be that way,
>> because 'patch-source-shebangs' (and 'patch-usr-bin-file') will need to
>> be run on the files created by 'autogen.sh'.
>
> This is the case. Is this therefore the cleanest way to do it?

Actually, having looked more carefully, I see that the only occurrence
of /bin/sh in autogen.sh is the shebang at the top.  Therefore, you
shouldn't need the 'substitute*' call at all, because of the way you are
running it below:

>>>> +          (setenv "NOCONFIGURE" "true")
>>>> +          (zero? (system* "bash" "autogen.sh")))

Also, since the shebang asked for /bin/sh, it's probably marginally more
correct to use "sh" instead of "bash" here.  Bash modifies its behavior
somewhat when run as "sh".

On the whole, I think the preferable approach (and the one I've used in
a large commit that cleans up all of these autogen-style phases) is
this:

--8<---------------cut here---------------start------------->8---
       (alist-cons-after
        'unpack 'autogen
        (lambda _
          (setenv "NOCONFIGURE" "true")
          (zero? (system* "sh" "autogen.sh")))
--8<---------------cut here---------------end--------------->8---

> One last question:
>
>>> From e1382a9df3e7df94e794df8aa247920b7e28c0c6 Mon Sep 17 00:00:00 2001
>>> From: Paul van der Walt <paul@denknerd.org>
>>> ...
>>
>> Could you please add a copyright line for yourself at the top of this
>> file?
>
> Are these from lines not enough? My previous patch which i generated the
> same way was accepted.

That was my mistake.  I asked you to add a copyright line, but then
forgot about it when I looked at the updated patch.  Can you submit a
patch that adds your copyright line for your previous package update?

Or is there a reason you'd like to avoid adding your copyright?

     Mark

Re: [PATCH] Update ncmpcpp to v0.6.2

[Paul van der Walt]

On 2015-02-27 at 19:21, quoth Mark H Weaver:
> On the whole, I think the preferable approach (and the one I've used in
> a large commit that cleans up all of these autogen-style phases) is
> this:
>
> --8<---------------cut here---------------start------------->8---
>        (alist-cons-after
>         'unpack 'autogen
>         (lambda _
>           (setenv "NOCONFIGURE" "true")
>           (zero? (system* "sh" "autogen.sh")))
> --8<---------------cut here---------------end--------------->8---

OK, i'll apply those changes and send in a cleaner patch. But not right
now, now it's best i go enjoy my weekend :)

> Or is there a reason you'd like to avoid adding your copyright?

No that's fine, i just hadn't understood what you were referring to the
first time.

Cheers!
p.