[PATCH] gnu: Add wmbattery

[PATCH] gnu: Add wmbattery

[Kei Yamashita]

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

The source tarball comes from Debian. Hopefully this patch is formatted
correctly.

[-- Attachment #2: 0001-gnu-Add-wmbattery.patch --]
[-- Type: application/octet-stream, Size: 2467 bytes --]

From ed508727588e234a89e1a34920d29d118c848509 Mon Sep 17 00:00:00 2001
From: Kei Yamashita <kei@openmailbox.org>
Date: Mon, 4 Apr 2016 16:59:02 -0400
Subject: [PATCH] gnu: Add wmbattery

* gnu/packages/gnustep.scm (wmbattery): New variable.
---
 gnu/packages/gnustep.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/gnu/packages/gnustep.scm b/gnu/packages/gnustep.scm
index edd159d..a5722a2 100644
--- a/gnu/packages/gnustep.scm
+++ b/gnu/packages/gnustep.scm
@@ -22,6 +22,10 @@
   #:use-module (guix build-system gnu)
   #:use-module (guix licenses)
   #:use-module (gnu packages xorg)
+  #:use-module (gnu packages gnome)
+  #:use-module (gnu packages texinfo)
+  #:use-module (gnu packages autotools)
+  #:use-module (gnu packages glib)
   #:use-module (gnu packages fontutils)
   #:use-module (gnu packages image)
   #:use-module (gnu packages pkg-config))
@@ -83,3 +87,42 @@ interface.  It is fast, feature rich, easy to configure, and easy to use.")
 
     ;; Artwork is distributed under the WTFPL.
     (license gpl2+)))
+
+(define-public wmbattery
+  (package
+    (name "wmbattery")
+    (version "2.50")
+    (source (origin
+	      (method url-fetch)
+	      (uri (string-append
+		    "http://http.debian.net/debian/pool/main/w/wmbattery/\
+wmbattery_2.50.orig.tar.gz"))
+	      (sha256
+	       (base32
+		"0hi6bivv3xd2k68w08krndfl68wdx7nmc2wjzsmcd4q3qgwgyk44"))))
+    (build-system gnu-build-system)
+    (arguments
+     `(#:phases (modify-phases %standard-phases
+                  (add-before
+                      'configure 'autoconf
+                    (lambda _ (zero? (system* "autoreconf" "-vfi"))))
+                  (delete 'check))))
+    (inputs
+     `(("glib" ,glib)
+       ("libx11" ,libx11)
+       ("libxext" ,libxext)
+       ("libxpm" ,libxpm)
+       ("upower" ,upower)))
+    (native-inputs
+     `(("autoconf" ,autoconf)
+       ("automake" ,automake)
+       ("pkg-config" ,pkg-config)))
+    (home-page "http://windowmaker.org/dockapps/?name=wmbattery")
+    (synopsis "Display laptop battery info")
+    (description
+     "Wmbattery displays the status of your laptop's battery in a small icon.
+This includes if it is plugged in, if the battery is charging, how many minutes
+of battery life remain, battery life remaining (with both a percentage and a
+graph), and battery status (high - green, low - yellow, or critical - red).")
+    (license gpl2)))
+
-- 
2.7.3

Re: [PATCH] gnu: Add wmbattery

[Danny Milosavljevic]

On Mon, 4 Apr 2016 17:04:51 -0400
Kei Yamashita <kei@openmailbox.org> wrote:

> The source tarball comes from Debian. Hopefully this patch is formatted
> correctly.

Spurios tab characters here:

	      (method url-fetch)
	      (uri (string-append
		    "http://http.debian.net/debian/pool/main/w/wmbattery/wmbattery_2.50.orig.tar.gz"))
	      (sha256
	       (base32
		"0hi6bivv3xd2k68w08krndfl68wdx7nmc2wjzsmcd4q3qgwgyk44"))))
^^^^^

Please run "guix lint wmbattery".

Otherwise I've tested it and it does install and run fine. 

Re: [PATCH] gnu: Add wmbattery

[Leo Famulari]

On Mon, Apr 04, 2016 at 05:04:51PM -0400, Kei Yamashita wrote:

Thanks for the patch!

As suggested by Danny, please run `guix lint` on it.

+    (version "2.50")
+    (source (origin
+	      (method url-fetch)
+	      (uri (string-append
+		    "http://http.debian.net/debian/pool/main/w/wmbattery/\
+wmbattery_2.50.orig.tar.gz"))

Can you re-use the version variable in the uri? There are many examples
of how to do this in the 'gnu/packages' directory of the Guix source
code.

Also, can you add a comment explaining why we fetch the source from
Debian?

+                  (delete 'check))))

Can you instead use the form '#:tests? #f', and add a comment explaining
why the tests are disabled? There are also examples of this in
'gnu/packages'.

Can you send an updated patch?

Re: [PATCH] gnu: Add wmbattery

[Alex Kost]

Kei Yamashita (2016-04-05 00:04 +0300) wrote:

> The source tarball comes from Debian. Hopefully this patch is formatted
> correctly.
>
> From ed508727588e234a89e1a34920d29d118c848509 Mon Sep 17 00:00:00 2001
> From: Kei Yamashita <kei@openmailbox.org>
> Date: Mon, 4 Apr 2016 16:59:02 -0400
> Subject: [PATCH] gnu: Add wmbattery
                                     ^
Conventions everywhere: a period in the end of the commit message :-)

> * gnu/packages/gnustep.scm (wmbattery): New variable.
> ---
>  gnu/packages/gnustep.scm | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
>
> diff --git a/gnu/packages/gnustep.scm b/gnu/packages/gnustep.scm
> index edd159d..a5722a2 100644
> --- a/gnu/packages/gnustep.scm
> +++ b/gnu/packages/gnustep.scm
> @@ -22,6 +22,10 @@
>    #:use-module (guix build-system gnu)
>    #:use-module (guix licenses)
>    #:use-module (gnu packages xorg)
> +  #:use-module (gnu packages gnome)
> +  #:use-module (gnu packages texinfo)
> +  #:use-module (gnu packages autotools)
> +  #:use-module (gnu packages glib)
>    #:use-module (gnu packages fontutils)
>    #:use-module (gnu packages image)
>    #:use-module (gnu packages pkg-config))
> @@ -83,3 +87,42 @@ interface.  It is fast, feature rich, easy to configure, and easy to use.")
>  
>      ;; Artwork is distributed under the WTFPL.
>      (license gpl2+)))
> +
> +(define-public wmbattery
> +  (package
> +    (name "wmbattery")
> +    (version "2.50")
> +    (source (origin
> +	      (method url-fetch)
   ^^^^
As Danny pointed we don't use tabulations.

> +	      (uri (string-append
> +		    "http://http.debian.net/debian/pool/main/w/wmbattery/\
> +wmbattery_2.50.orig.tar.gz"))

We don't split strings like this; with 'string-append' you can make as
many substrings as needed.  Also I changed it to use "mirror://debian/…".

> +	      (sha256
> +	       (base32
> +		"0hi6bivv3xd2k68w08krndfl68wdx7nmc2wjzsmcd4q3qgwgyk44"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     `(#:phases (modify-phases %standard-phases
> +                  (add-before
> +                      'configure 'autoconf

It looks better when the phases names are moved to the previous line.

Also this autoreconf phase should be added after 'unpack', not before
'configure', because between 'unpack' and 'configure' there are other
phases that fix potential problem on other systems (mips and arm).

> +                    (lambda _ (zero? (system* "autoreconf" "-vfi"))))
> +                  (delete 'check))))

Instead of deleting "check" phase, we disable tests explaining the reason.

I made the mentioned changes and applied your patch as f9ab1da¹.  Thanks!

¹ http://git.savannah.gnu.org/cgit/guix.git/commit/?id=f9ab1da0fa1a4cdb100209c2607c7afbbacfb7c6

-- 
Alex

Re: [PATCH] gnu: Add wmbattery

[Danny Milosavljevic]

On Mon, 4 Apr 2016 17:04:51 -0400
Kei Yamashita <kei@openmailbox.org> wrote:

> The source tarball comes from Debian. 

The wmbattery version <http://http.debian.net/debian/pool/main/w/wmbattery/wmbattery_2.50.orig.tar.gz> that we use in Guix has a memory leak. It's been eating up all memory of my laptop for some time now, so I tracked it down.

The fix would be:

diff -ur orig/dockapps-30b9edb/upower.c dockapps-30b9edb/upower.c
--- orig/dockapps-30b9edb/upower.c	2015-08-31 01:58:13.000000000 +0200
+++ dockapps-30b9edb/upower.c	2016-05-12 07:21:27.550374114 +0200
@@ -56,7 +56,8 @@
 
 int upower_supported(void)
 {
-	up = up_client_new();
+	if (!up)
+		up = up_client_new();
 
 	if (!up) {
 		return 0;
@@ -78,7 +79,8 @@
 	GPtrArray *devices = NULL;
 	static int retries = 0;
 
-	up = up_client_new();
+	if (!up)
+		up = up_client_new();
 
 	if (!up)
 		return -1;

I added Joey Hess, the author of the program, to Cc.

@Joey Hess: what do you think?

Re: [PATCH] gnu: Add wmbattery

[Joey Hess]

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

I'm not maintaining wmbattery anymore. I've cced Doug, its new
maintainer.

Danny Milosavljevic wrote:
> On Mon, 4 Apr 2016 17:04:51 -0400
> Kei Yamashita <kei@openmailbox.org> wrote:
> 
> > The source tarball comes from Debian. 
> 
> The wmbattery version <http://http.debian.net/debian/pool/main/w/wmbattery/wmbattery_2.50.orig.tar.gz> that we use in Guix has a memory leak. It's been eating up all memory of my laptop for some time now, so I tracked it down.
> 
> The fix would be:
> 
> diff -ur orig/dockapps-30b9edb/upower.c dockapps-30b9edb/upower.c
> --- orig/dockapps-30b9edb/upower.c	2015-08-31 01:58:13.000000000 +0200
> +++ dockapps-30b9edb/upower.c	2016-05-12 07:21:27.550374114 +0200
> @@ -56,7 +56,8 @@
>  
>  int upower_supported(void)
>  {
> -	up = up_client_new();
> +	if (!up)
> +		up = up_client_new();
>  
>  	if (!up) {
>  		return 0;
> @@ -78,7 +79,8 @@
>  	GPtrArray *devices = NULL;
>  	static int retries = 0;
>  
> -	up = up_client_new();
> +	if (!up)
> +		up = up_client_new();
>  
>  	if (!up)
>  		return -1;
> 
> I added Joey Hess, the author of the program, to Cc.
> 
> @Joey Hess: what do you think?

-- 
see shy jo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 811 bytes --]

Re: [PATCH] gnu: Add wmbattery

[Kei Yamashita]

[-- Attachment #1.1: Type: text/plain, Size: 1260 bytes --]

On Fri, 13 May 2016 12:25:07 +0200
Danny Milosavljevic <dannym@scratchpost.org> wrote:

> On Mon, 4 Apr 2016 17:04:51 -0400
> Kei Yamashita <kei@openmailbox.org> wrote:
> 
> > The source tarball comes from Debian.   
> 
> The wmbattery version
> <http://http.debian.net/debian/pool/main/w/wmbattery/wmbattery_2.50.orig.tar.gz>
> that we use in Guix has a memory leak. It's been eating up all memory
> of my laptop for some time now, so I tracked it down.
> 
> The fix would be:
> 
> diff -ur orig/dockapps-30b9edb/upower.c dockapps-30b9edb/upower.c
> --- orig/dockapps-30b9edb/upower.c	2015-08-31
> 01:58:13.000000000 +0200 +++ dockapps-30b9edb/upower.c
> 2016-05-12 07:21:27.550374114 +0200 @@ -56,7 +56,8 @@
>  
>  int upower_supported(void)
>  {
> -	up = up_client_new();
> +	if (!up)
> +		up = up_client_new();
>  
>  	if (!up) {
>  		return 0;
> @@ -78,7 +79,8 @@
>  	GPtrArray *devices = NULL;
>  	static int retries = 0;
>  
> -	up = up_client_new();
> +	if (!up)
> +		up = up_client_new();
>  
>  	if (!up)
>  		return -1;
> 
> I added Joey Hess, the author of the program, to Cc.
> 
> @Joey Hess: what do you think?

Until the patch is applied upstream, this patch should keep things in
order within Guix.

[-- Attachment #1.2: 0001-gnu-wmbattery-Add-patch-upower-phase.patch --]
[-- Type: text/plain, Size: 1231 bytes --]

From e07912c6b5ea2ff45d5b923dccad26fef3ca932d Mon Sep 17 00:00:00 2001
From: Kei Yamashita <kei@openmailbox.org>
Date: Sat, 14 May 2016 22:27:15 -0400
Subject: [PATCH] gnu: wmbattery: Add `patch-upower' phase.

* gnu/packages/gnustep.scm (wmbattery): Add `patch-upower' phase.
---
 gnu/packages/gnustep.scm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/gnustep.scm b/gnu/packages/gnustep.scm
index 31fca5b..961df0e 100644
--- a/gnu/packages/gnustep.scm
+++ b/gnu/packages/gnustep.scm
@@ -108,7 +108,13 @@ interface.  It is fast, feature rich, easy to configure, and easy to use.")
        #:phases
        (modify-phases %standard-phases
          (add-after 'unpack 'autoconf
-           (lambda _ (zero? (system* "autoreconf" "-vfi")))))))
+           (lambda _ (zero? (system* "autoreconf" "-vfi"))))
+         (add-before 'configure 'patch-upower
+           (lambda _
+             (substitute* "upower.c"
+               (("up = up_client_new\\(\\);")
+                (string-append "if (!up)\n"
+                               "                up = up_client_new();"))))))))
     (inputs
      `(("glib" ,glib)
        ("libx11" ,libx11)
-- 
2.7.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH] gnu: Add wmbattery

[Ludovic Courtès]

Kei Yamashita <kei@openmailbox.org> skribis:

> Until the patch is applied upstream, this patch should keep things in
> order within Guix.

Good idea.

> From e07912c6b5ea2ff45d5b923dccad26fef3ca932d Mon Sep 17 00:00:00 2001
> From: Kei Yamashita <kei@openmailbox.org>
> Date: Sat, 14 May 2016 22:27:15 -0400
> Subject: [PATCH] gnu: wmbattery: Add `patch-upower' phase.
>
> * gnu/packages/gnustep.scm (wmbattery): Add `patch-upower' phase.

Could you make this a ‘snippet’ in the origin?  I think that such source
modifications should be either plain patches, or snippets.

Thanks!

Ludo’.

Re: [PATCH] gnu: Add wmbattery

[Kei Kebreau]

[-- Attachment #1.1: Type: text/plain, Size: 802 bytes --]

On Tue, 17 May 2016 23:05:17 +0200
ludo@gnu.org (Ludovic Courtès) wrote:

> Kei Yamashita <kei@openmailbox.org> skribis:
> 
> > Until the patch is applied upstream, this patch should keep things
> > in order within Guix.  
> 
> Good idea.
> 
> > From e07912c6b5ea2ff45d5b923dccad26fef3ca932d Mon Sep 17 00:00:00
> > 2001 From: Kei Yamashita <kei@openmailbox.org>
> > Date: Sat, 14 May 2016 22:27:15 -0400
> > Subject: [PATCH] gnu: wmbattery: Add `patch-upower' phase.
> >
> > * gnu/packages/gnustep.scm (wmbattery): Add `patch-upower' phase.  
> 
> Could you make this a ‘snippet’ in the origin?  I think that such
> source modifications should be either plain patches, or snippets.
> 
> Thanks!
> 
> Ludo’.

Like attached?

-- 
Kei (GPG Key: 4096R/E6A5EE3C19467A0D)

[-- Attachment #1.2: 0001-gnu-wmbattery-Add-patch-upower-phase.patch --]
[-- Type: text/plain, Size: 1607 bytes --]

From b0729d68e78906c0b750935c17a6d98bfac981cf Mon Sep 17 00:00:00 2001
From: Kei Yamashita <kei@openmailbox.org>
Date: Tue, 17 May 2016 18:52:16 -0400
Subject: [PATCH] gnu: wmbattery: Add `patch-upower' phase.

* gnu/packages/gnustep.scm (wmbattery): Add `patch-upower' phase.
---
 gnu/packages/gnustep.scm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/gnustep.scm b/gnu/packages/gnustep.scm
index 31fca5b..1faf117 100644
--- a/gnu/packages/gnustep.scm
+++ b/gnu/packages/gnustep.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014 Ludovic Courtès <ludo@gnu.org>
-;;; Copyright © 2016 Kei Yamashita <kei@openmailbox.org>
+;;; Copyright © 2016 Kei Kebreau <kei@openmailbox.org>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -101,7 +101,13 @@ interface.  It is fast, feature rich, easy to configure, and easy to use.")
                     version ".orig.tar.gz"))
               (sha256
                (base32
-                "0hi6bivv3xd2k68w08krndfl68wdx7nmc2wjzsmcd4q3qgwgyk44"))))
+                "0hi6bivv3xd2k68w08krndfl68wdx7nmc2wjzsmcd4q3qgwgyk44"))
+              (modules '((guix build utils)))
+              (snippet
+               '(substitute* "upower.c"
+                  (("up = up_client_new\\(\\);")
+                   (string-append "if (!up)\n"
+                                  "                up = up_client_new();"))))))
     (build-system gnu-build-system)
     (arguments
      `(#:tests? #f              ; no "check" target
-- 
2.7.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] gnu: Add wmbattery

[Ludovic Courtès]

Kei Kebreau <kei@openmailbox.org> skribis:

> On Tue, 17 May 2016 23:05:17 +0200
> ludo@gnu.org (Ludovic Courtès) wrote:

[...]

>> > From e07912c6b5ea2ff45d5b923dccad26fef3ca932d Mon Sep 17 00:00:00
>> > 2001 From: Kei Yamashita <kei@openmailbox.org>
>> > Date: Sat, 14 May 2016 22:27:15 -0400
>> > Subject: [PATCH] gnu: wmbattery: Add `patch-upower' phase.
>> >
>> > * gnu/packages/gnustep.scm (wmbattery): Add `patch-upower' phase.  
>> 
>> Could you make this a ‘snippet’ in the origin?  I think that such
>> source modifications should be either plain patches, or snippets.

[...]

> From b0729d68e78906c0b750935c17a6d98bfac981cf Mon Sep 17 00:00:00 2001
> From: Kei Yamashita <kei@openmailbox.org>
> Date: Tue, 17 May 2016 18:52:16 -0400
> Subject: [PATCH] gnu: wmbattery: Add `patch-upower' phase.
>
> * gnu/packages/gnustep.scm (wmbattery): Add `patch-upower' phase.

I adjusted the commit log and committed, thanks!

Ludo’.