[PATCH 1/5] doc: Make 'Lirc Service' a subsubheading of 'Various Services'.

[PATCH 1/5] doc: Make 'Lirc Service' a subsubheading of 'Various Services'.

[David Craven]

* doc/guix.texi (Various Services)[Lirc Service]: New node. New subsubheading.
  [lirc] New cindex.
---
 doc/guix.texi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 77f028e..1622d71 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -9901,6 +9901,10 @@ directories are created when the service is activated.
 @node Various Services
 @subsubsection Various Services
 
+@cindex lirc
+@node Lirc Service
+@subsubheading Lirc Service
+
 The @code{(gnu services lirc)} module provides the following service.
 
 @deffn {Scheme Procedure} lirc-service [#:lirc lirc] @
-- 
2.9.0

[PATCH 2/5] gnu: qemu: Reorder inputs alphabetically.

[David Craven]

* gnu/packages/qemu.scm (qemu): Reorder inputs alphabetically.
---
 gnu/packages/qemu.scm | 37 ++++++++++++++++++-------------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/gnu/packages/qemu.scm b/gnu/packages/qemu.scm
index 911ed4c..6b5a41b 100644
--- a/gnu/packages/qemu.scm
+++ b/gnu/packages/qemu.scm
@@ -118,29 +118,28 @@
                 (string-append "# " all)))
              #t)))))
     (inputs                                       ; TODO: Add optional inputs.
-     `(("sdl" ,sdl)
-       ("mesa" ,mesa)
-       ("libusb" ,libusb)                         ;USB pass-through support
-
-       ;; ("libaio" ,libaio)
+     `(("alsa-lib" ,alsa-lib)
+       ("attr" ,attr)
        ("glib" ,glib)
-       ("ncurses" ,ncurses)
-       ("libpng" ,libpng)
+       ;; ("libaio" ,libaio)
+       ("libattr" ,attr)
+       ("libcap" ,libcap)           ; virtfs support requires libcap & libattr
        ("libjpeg" ,libjpeg-8)
+       ("libpng" ,libpng)
+       ("libusb" ,libusb)                         ;USB pass-through support
+       ("mesa" ,mesa)
+       ("ncurses" ,ncurses)
+       ;; ("pciutils" ,pciutils)
        ("pixman" ,pixman)
-       ;; ("vde2" ,vde2)
+       ("sdl" ,sdl)
        ("util-linux" ,util-linux)
-       ("libcap" ,libcap)           ; virtfs support requires libcap & libattr
-       ("libattr" ,attr)
-       ;; ("pciutils" ,pciutils)
-       ("alsa-lib" ,alsa-lib)
-       ("zlib" ,zlib)
-       ("attr" ,attr)))
-    (native-inputs `(("pkg-config" ,pkg-config)
+       ;; ("vde2" ,vde2)
+       ("zlib" ,zlib)))
+    (native-inputs `(("glib:bin" ,glib "bin") ; gtester, etc.
+                     ("perl" ,perl)
+                     ("pkg-config" ,pkg-config)
                      ("python" ,python-2) ; incompatible with Python 3 according to error message
-                     ("glib" ,glib "bin") ; gtester, etc.
-                     ("texinfo" ,texinfo)
-                     ("perl" ,perl)))
+                     ("texinfo" ,texinfo)))
     (home-page "http://www.qemu-project.org")
     (synopsis "Machine emulator and virtualizer")
     (description
@@ -175,4 +174,4 @@ server and embedded PowerPC, and S390 guests.")
 
     ;; Remove dependencies on optional libraries, notably GUI libraries.
     (inputs (fold alist-delete (package-inputs qemu)
-                  '("sdl" "mesa" "libusb")))))
+                  '("libusb" "mesa" "sdl")))))
-- 
2.9.0

[PATCH 3/5] gnu: qemu: Enable spice support.

[David Craven]

* gnu/packages/qemu.scm (qemu)[inputs]: Add SPICE and VIRGLRENDERER.
  [arguments]: Pass --enable-spice.
---
 gnu/packages/qemu.scm | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/gnu/packages/qemu.scm b/gnu/packages/qemu.scm
index 6b5a41b..97642af 100644
--- a/gnu/packages/qemu.scm
+++ b/gnu/packages/qemu.scm
@@ -33,6 +33,7 @@
   #:use-module (gnu packages pkg-config)
   #:use-module (gnu packages python)
   #:use-module (gnu packages sdl)
+  #:use-module (gnu packages spice)
   #:use-module (gnu packages texinfo)
   #:use-module (gnu packages xdisorg)
   #:use-module (guix build-system gnu)
@@ -91,6 +92,7 @@
                          ,(string-append "--cc=" (which "gcc"))
                          "--disable-debug-info" ; save build space
                          "--enable-virtfs"      ; just to be sure
+                         "--enable-spice"
                          ,(string-append "--prefix=" out)
                          ,@configure-flags))))))
          (add-after 'install 'install-info
@@ -132,8 +134,10 @@
        ;; ("pciutils" ,pciutils)
        ("pixman" ,pixman)
        ("sdl" ,sdl)
+       ("spice" ,spice)
        ("util-linux" ,util-linux)
        ;; ("vde2" ,vde2)
+       ("virglrenderer" ,virglrenderer)
        ("zlib" ,zlib)))
     (native-inputs `(("glib:bin" ,glib "bin") ; gtester, etc.
                      ("perl" ,perl)
-- 
2.9.0

[PATCH 4/5] gnu: spice-vdagent: Set Exec path in spice-vdagent.desktop.

[David Craven]

* gnu/packages/spice.scm (spice-vdagent): Set Exec path in
spice-vdagent.desktop.
---
 gnu/packages/spice.scm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gnu/packages/spice.scm b/gnu/packages/spice.scm
index cfb6084..9e4a669 100644
--- a/gnu/packages/spice.scm
+++ b/gnu/packages/spice.scm
@@ -234,6 +234,13 @@ Internet and from a wide variety of machine architectures.")
                (((string-append "\\$\\(mkdir_p\\) \\$\\(DESTDIR\\)"
                                 "\\$\\(localstatedir\\)/run/spice-vdagentd"))
                  "-$(mkdir_p) $(DESTDIR)$(localstatedir)/run/spice-vdagentd"))
+             #t))
+         (add-after 'unpack 'patch-spice-vdagent.desktop
+           (lambda* (#:key outputs #:allow-other-keys)
+            (substitute* "data/spice-vdagent.desktop"
+              (("Exec=/usr/bin/spice-vdagent\n")
+               (string-append "Exec=" (assoc-ref outputs "out")
+                              "/bin/spice-vdagent")))
              #t)))))
     (inputs
       `(("alsa-lib" ,alsa-lib)
-- 
2.9.0

[PATCH 5/5] services: Add spice vdagent service.

[David Craven]

* gnu/services/spice.scm: New file.
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
* doc/guix.texi (Various Services): New node.
---
 doc/guix.texi          | 12 ++++++++
 gnu/local.mk           |  1 +
 gnu/services/spice.scm | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+)
 create mode 100644 gnu/services/spice.scm

diff --git a/doc/guix.texi b/doc/guix.texi
index 1622d71..eb87dde 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -9921,6 +9921,18 @@ Finally, @var{extra-options} is a list of additional command-line options
 passed to @command{lircd}.
 @end deffn
 
+@cindex spice
+@node Spice Service
+@subsubheading Spice Service
+
+The @code{(gnu services spice)} module provides the following service.
+
+@deffn {Scheme Procedure} spice-vdagent-service [#:spice-vdagent]
+Returns a service that runs @url{http://www.spice-space.org,VDAGENT}, a daemon
+that enables sharing the clipboard with a vm and setting the guest display
+resolution when the graphical console window resizes.
+@end deffn
+
 @subsubsection Dictionary Services
 The @code{(gnu services dict)} module provides the following service:
 
diff --git a/gnu/local.mk b/gnu/local.mk
index f94b123..306cf72 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -385,6 +385,7 @@ GNU_SYSTEM_MODULES =				\
   %D%/services/networking.scm			\
   %D%/services/shepherd.scm			\
   %D%/services/herd.scm				\
+  %D%/services/spice.scm				\
   %D%/services/ssh.scm				\
   %D%/services/web.scm				\
   %D%/services/xorg.scm				\
diff --git a/gnu/services/spice.scm b/gnu/services/spice.scm
new file mode 100644
index 0000000..26f072e
--- /dev/null
+++ b/gnu/services/spice.scm
@@ -0,0 +1,75 @@
+;;; GNU Guix --- Functional package management for GNU
+;;; Copyright © 2016 David Craven <david@craven.ch>
+;;;
+;;; This file is part of GNU Guix.
+;;;
+;;; GNU Guix is free software; you can redistribute it and/or modify it
+;;; under the terms of the GNU General Public License as published by
+;;; the Free Software Foundation; either version 3 of the License, or (at
+;;; your option) any later version.
+;;;
+;;; GNU Guix is distributed in the hope that it will be useful, but
+;;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;;; GNU General Public License for more details.
+;;;
+;;; You should have received a copy of the GNU General Public License
+;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+(define-module (gnu services spice)
+  #:use-module (gnu packages spice)
+  #:use-module (gnu services)
+  #:use-module (gnu services shepherd)
+  #:use-module (guix gexp)
+  #:use-module (guix records)
+  #:export (spice-vdagent-configuration
+            spice-vdagent-configuration?
+            spice-vdagent-service-type
+            spice-vdagent-service))
+
+(define-record-type* <spice-vdagent-configuration>
+  spice-vdagent-configuration make-spice-vdagent-configuration
+  spice-vdagent-configuration?
+  (spice-vdagent spice-vdagent-configuration-spice-vdagent
+                 (default spice-vdagent)))
+
+(define (spice-vdagent-activation config)
+  "Return the activation gexp for CONFIG."
+  #~(mkdir-p "/var/run/spice-vdagentd"))
+
+(define (spice-vdagent-shepherd-service config)
+  "Return a <shepherd-service> for spice-vdagentd with CONFIG."
+  (define spice-vdagent (spice-vdagent-configuration-spice-vdagent config))
+
+  (define spice-vdagentd-command
+    (list
+      #~(string-append #$spice-vdagent "/sbin/spice-vdagentd")
+      "-x"))
+
+  (list
+    (shepherd-service
+      (documentation "Spice vdagentd service")
+      (requirement '(udev))
+      (provision '(spice-vdagentd))
+      (start #~(make-forkexec-constructor #$@spice-vdagentd-command))
+      (stop #~(make-kill-destructor)))))
+
+(define spice-vdagent-profile
+  (compose list spice-vdagent-configuration-spice-vdagent))
+
+(define spice-vdagent-service-type
+  (service-type (name 'spice-vdagent)
+    (extensions
+      (list (service-extension shepherd-root-service-type
+                               spice-vdagent-shepherd-service)
+            (service-extension activation-service-type
+                               spice-vdagent-activation)
+            (service-extension profile-service-type
+                               spice-vdagent-profile)))))
+
+(define* (spice-vdagent-service
+          #:optional (config (spice-vdagent-configuration)))
+  "Start the @command{vdagentd} and @command{vdagent} deamons
+from @var{spice-vdagent} to enable guest window resizing and
+clipboard sharing."
+  (service spice-vdagent-service-type config))
-- 
2.9.0

Re: [PATCH 5/5] services: Add spice vdagent service.

[David Craven]

No comment means these patches are ok to merge?

Re: [PATCH 5/5] services: Add spice vdagent service.

[Andreas Enge]

On Fri, Aug 05, 2016 at 04:33:03PM +0200, David Craven wrote:
> No comment means these patches are ok to merge?

Or that nobody looked at them? :-)
Unfortunately I have no service expertise; hopefully someone else
can have a look at this.

Andreas

Re: [PATCH 5/5] services: Add spice vdagent service.

[Leo Famulari]

On Fri, Aug 05, 2016 at 08:12:29PM +0200, Andreas Enge wrote:
> On Fri, Aug 05, 2016 at 04:33:03PM +0200, David Craven wrote:
> > No comment means these patches are ok to merge?
> 
> Or that nobody looked at them? :-)
> Unfortunately I have no service expertise; hopefully someone else
> can have a look at this.

Likewise.

Re: [PATCH 5/5] services: Add spice vdagent service.

[David Craven]

Ludo looked at them and asked me to resubmit, it's mainly adding the
documentation and splitting a commit into two...

Re: [PATCH 3/5] gnu: qemu: Enable spice support.

[Leo Famulari]

On Tue, Aug 02, 2016 at 03:45:29PM +0200, David Craven wrote:
> * gnu/packages/qemu.scm (qemu)[inputs]: Add SPICE and VIRGLRENDERER.
>   [arguments]: Pass --enable-spice.

Do spice and virglrenderer belong in qemu-minimal, or should they be
removed from it?

> ---
>  gnu/packages/qemu.scm | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/gnu/packages/qemu.scm b/gnu/packages/qemu.scm
> index 6b5a41b..97642af 100644
> --- a/gnu/packages/qemu.scm
> +++ b/gnu/packages/qemu.scm
> @@ -33,6 +33,7 @@
>    #:use-module (gnu packages pkg-config)
>    #:use-module (gnu packages python)
>    #:use-module (gnu packages sdl)
> +  #:use-module (gnu packages spice)
>    #:use-module (gnu packages texinfo)
>    #:use-module (gnu packages xdisorg)
>    #:use-module (guix build-system gnu)
> @@ -91,6 +92,7 @@
>                           ,(string-append "--cc=" (which "gcc"))
>                           "--disable-debug-info" ; save build space
>                           "--enable-virtfs"      ; just to be sure
> +                         "--enable-spice"
>                           ,(string-append "--prefix=" out)
>                           ,@configure-flags))))))
>           (add-after 'install 'install-info
> @@ -132,8 +134,10 @@
>         ;; ("pciutils" ,pciutils)
>         ("pixman" ,pixman)
>         ("sdl" ,sdl)
> +       ("spice" ,spice)
>         ("util-linux" ,util-linux)
>         ;; ("vde2" ,vde2)
> +       ("virglrenderer" ,virglrenderer)
>         ("zlib" ,zlib)))
>      (native-inputs `(("glib:bin" ,glib "bin") ; gtester, etc.
>                       ("perl" ,perl)
> -- 
> 2.9.0
> 

Re: [PATCH 5/5] services: Add spice vdagent service.

[Leo Famulari]

On Fri, Aug 05, 2016 at 08:28:35PM +0200, David Craven wrote:
> Ludo looked at them and asked me to resubmit, it's mainly adding the
> documentation and splitting a commit into two...

Okay, then I think it's good.

To follow-up on my last comment, I believe that qemu-minimal is used to
build GuixSD VMs and disk images. So, that's the context in which to
decide whether qemu-minimal needs spice / virglrenderer support.

Re: [PATCH 5/5] services: Add spice vdagent service.

[David Craven]

> To follow-up on my last comment, I believe that qemu-minimal is used to
> build GuixSD VMs and disk images. So, that's the context in which to
> decide whether qemu-minimal needs spice / virglrenderer support.

I'll add spice and virglrenderer to the list of deleted packages for
qemu-minimal and rebuild qemu-minimal

Remove the @node from the documentation commits and amend the commit
messages (for some reason @node breaks make because of some warning,
but doesn't complaing when running make pdf, which I used to test)

And I'll cherry-pick the commit that adds downloads.kde.org to the
mirror list based on our discussion.

And if there aren't any further comments I'll push this tonight.

Re: [PATCH 3/5] gnu: qemu: Enable spice support.

[Mark H Weaver]

David Craven <david@craven.ch> writes:
> * gnu/packages/qemu.scm (qemu)[inputs]: Add SPICE and VIRGLRENDERER.
>   [arguments]: Pass --enable-spice.

Unfortunately, 'spice' only builds successfully on x86_64, so with this
patch, i686 and armhf lose 'qemu'.

One problem is that 'spice' depends on 'usbredir', and 'usbredir' only
builds successfully on x86_64.

     Mark

Re: [PATCH 5/5] services: Add spice vdagent service.

[Ricardo Wurmus]

David Craven <david@craven.ch> writes:

> No comment means these patches are ok to merge?

Usually we ask for about one week to pass before merging without
comment.  Otherwise there’s just not enough time for reviewers to look
over the patches.

~~ Ricardo

Re: [PATCH 3/5] gnu: qemu: Enable spice support.

[David Craven]

> Unfortunately, 'spice' only builds successfully on x86_64, so with this
> patch, i686 and armhf lose 'qemu'.

'usbredir' is needed by 'spice-gtk' which is only needed to run the tests
for spice. This can be fixed by removing the automated tests configure-flag
and removing spice-gtk from the dependencies on i686.

If supported-systems is set correctly, will guix try to build without
the required
package? This is useful in case the dependency is only optional.

Trying to build with guix build --system=armhf-linux tells me that I'm
x86-64-linux and the same for --system=mipsel-linux. It would be
nice if guix build would spin up a qemu instance in this case. It will
take me a while to test.

Re: [PATCH 5/5] services: Add spice vdagent service.

[David Craven]

> Usually we ask for about one week to pass before merging without
> comment.  Otherwise there’s just not enough time for reviewers to look
> over the patches.

Thank you for the information!

In case you are referring to the 'enable spice in qemu patch', it was first
submitted 2016/07/29. But I should have tested it on all platforms we
support.

Ludo already reviewed the spice-vdagent-service and I thought
that the changes he wanted me to make were minor corrections.

I guess I am trigger happy, if there is a trigger somewhere I'll pull
it =P

David

Re: [PATCH 3/5] gnu: qemu: Enable spice support.

[David Craven]

> Trying to build with guix build --system=armhf-linux tells me that I'm
> x86-64-linux and the same for --system=mipsel-linux. It would be
> nice if guix build would spin up a qemu instance in this case. It will
> take me a while to test.

Before reinventing the wheel or deploying my own hydra build farm
(or cuirass), is there a simple way to build a package for all supported
platforms?

> If supported-systems is set correctly, will guix try to build without
> the required package? This is useful in case the dependency is
> only optional.

It looks like guix ignores the supported-systems property and
doesn't make use of it anywhere. Say it doesn't prevent building
a package on i686-linux when only x86_64-linux is in the list.

Re: [PATCH 3/5] gnu: qemu: Enable spice support.

[Mark H Weaver]

Hi David,

David Craven <david@craven.ch> writes:

>> Unfortunately, 'spice' only builds successfully on x86_64, so with this
>> patch, i686 and armhf lose 'qemu'.
>
> 'usbredir' is needed by 'spice-gtk' which is only needed to run the tests
> for spice. This can be fixed by removing the automated tests configure-flag
> and removing spice-gtk from the dependencies on i686.

I would prefer to fix 'usbredir' on i686.  The same error occurs on all
non-x86_64 systems, so if you can fix it on i686, there's a good chance
it will work on mips64el and armhf as well.  Hydra will check that.

In any case, i686 is the most important for now.  You can see the failed
build log via this link:

  https://hydra.gnu.org/job/gnu/master/usbredir-0.7.1.i686-linux

Here's the relevant excerpt:

--8<---------------cut here---------------start------------->8---
  CC       libusbredirhost_la-usbredirhost.lo
usbredirhost.c: In function ‘usbredirhost_can_write_iso_package’:
usbredirhost.c:1023:13: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ [-Werror=format=]
             DEBUG("START dropping isoc packets %lu buffer > %lu hi threshold",
             ^
usbredirhost.c:1023:13: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘uint64_t’ [-Werror=format=]
usbredirhost.c:1028:13: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ [-Werror=format=]
             DEBUG("STOP dropping isoc packets %lu buffer < %lu low threshold",
             ^
usbredirhost.c:1028:13: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘uint64_t’ [-Werror=format=]
usbredirhost.c: In function ‘usbredirhost_set_iso_threshold’:
usbredirhost.c:1162:5: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 4 has type ‘uint64_t’ [-Werror=format=]
     DEBUG("higher threshold is %lu bytes | lower threshold is %lu bytes",
     ^
usbredirhost.c:1162:5: error: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘uint64_t’ [-Werror=format=]
cc1: all warnings being treated as errors
--8<---------------cut here---------------end--------------->8---

> If supported-systems is set correctly, will guix try to build without
> the required package?

No.  A package is considered "unsupported" on a given architecture if
any of its transitive inputs are unsupported there.  See
'package-transitive-supported-systems' and 'supported-package?'
in guix/packages.scm.

> Trying to build with guix build --system=armhf-linux tells me that I'm
> x86-64-linux and the same for --system=mipsel-linux. It would be
> nice if guix build would spin up a qemu instance in this case.

I agree that would be very nice, but no one has implemented it yet.

> Before reinventing the wheel or deploying my own hydra build farm
> (or cuirass), is there a simple way to build a package for all supported
> platforms?

No, it currently requires having access to a system of the desired
architecture running Guix.  If you have this, then you can arrange for
builds to be automatically "offloaded" to that machine as needed.  See
section 2.4.2 (Using the Offload Facility) in the Guix manual for
details.

Note that if GuixSD included support for mips64el or armhf running qemu,
then you could use "guix system vm" to create a virtual system of the
desired system, and then configure Guix to offload builds to it, and
this could be automated.  However, at present, you'd need to install a
different distro within qemu, install Guix within it, and then set up
offloading manually.

Anyway, it is always possible to build for i686 on an x86_64 system
without offloading, and fortunately that should be sufficient to make
progress on this issue.

> It looks like guix ignores the supported-systems property and
> doesn't make use of it anywhere.

It's used to determine which build jobs are attempted on Hydra, affects
the output of "guix package --list-available", and maybe a few other
things.

> Say it doesn't prevent building a package on i686-linux when only
> x86_64-linux is in the list.

Yes, Guix will attempt a build if you ask it to, even if the package is
listed as unsupported on your system.

Thanks for your efforts!

     Regards,
       Mark

[PATCH] gnu: spice: Fix usbredir for 32 bit platforms.

[David Craven]

* gnu/packages/spice.scm (usbredir)[origin]: Fetch source from git repo.
  [native-inputs]: Add AUTOCONF, AUTOMAKE and LIBTOOL.
  [arguments]: Add autogen phase.
---
 gnu/packages/spice.scm | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/gnu/packages/spice.scm b/gnu/packages/spice.scm
index 9e4a669..67e493a 100644
--- a/gnu/packages/spice.scm
+++ b/gnu/packages/spice.scm
@@ -18,6 +18,7 @@
 
 (define-module (gnu packages spice)
   #:use-module (gnu packages)
+  #:use-module (gnu packages autotools) ; remove after updating usbredir to 0.7.1+
   #:use-module (gnu packages compression)
   #:use-module (gnu packages gl)
   #:use-module (gnu packages glib)
@@ -37,6 +38,7 @@
   #:use-module (gnu packages xml)
   #:use-module (guix build-system gnu)
   #:use-module (guix download)
+  #:use-module (guix git-download) ; remove after updating usbredir to 0.7.1+
   #:use-module (guix packages)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix utils))
@@ -45,19 +47,39 @@
   (package
     (name "usbredir")
     (version "0.7.1")
+    ;(source (origin
+    ;          (method url-fetch)
+    ;          (uri (string-append
+    ;            "http://spice-space.org/download/usbredir/"
+    ;            "usbredir-" version ".tar.bz2"))
+    ;          (sha256
+    ;           (base32
+    ;            "1wsnmk4wjpdhbn1zaxg6bmyxspcki2zgy0am9lk037rnl4krwzj0"))))
+    ; FIXME: usbredir 0.7.1 release doesn't build on 32 bit systems.
+    ;        issue is fixed in HEAD
+    ;        remove 'autogen phase and autoconf, automake, libtool inputs
     (source (origin
-              (method url-fetch)
-              (uri (string-append
-                "http://spice-space.org/download/usbredir/"
-                "usbredir-" version ".tar.bz2"))
+              (method git-fetch)
+              (uri (git-reference
+                      (url "http://cgit.freedesktop.org/spice/usbredir")
+                      (commit "ac80a5971c6318d73d5fba4b5f13d3a9389558c9")))
               (sha256
                (base32
-                "1wsnmk4wjpdhbn1zaxg6bmyxspcki2zgy0am9lk037rnl4krwzj0"))))
+                "052fywgi72j68dr5ybldncg4vk8iqfrh58la7iazyxxpph9aag1g"))))
     (build-system gnu-build-system)
     (propagated-inputs
       `(("libusb" ,libusb)))
     (native-inputs
-      `(("pkg-config" ,pkg-config)))
+      `(("pkg-config" ,pkg-config)
+        ("autoconf" ,autoconf)
+        ("automake" ,automake)
+        ("libtool" ,libtool)))
+    (arguments
+     `(#:phases
+       (modify-phases %standard-phases
+         (add-after 'unpack 'autogen
+           (lambda* _
+             (system* "sh" "autogen.sh"))))))
     (synopsis "Tools for sending USB device traffic over a network")
     (description "Usbredir is a network protocol for sending USB device traffic
 over a network connection.  It can be used to redirect traffic from a USB device
-- 
2.9.0

Re: [PATCH] gnu: spice: Fix usbredir for 32 bit platforms.

[David Craven]

spice still fails to build on 32 bit platforms due to a test failure,
but this is a start.

Re: [PATCH] gnu: spice: Fix usbredir for 32 bit platforms.

[Mark H Weaver]

Hi David,

David Craven <david@craven.ch> writes:

> * gnu/packages/spice.scm (usbredir)[origin]: Fetch source from git repo.
>   [native-inputs]: Add AUTOCONF, AUTOMAKE and LIBTOOL.
>   [arguments]: Add autogen phase.

I see you already pushed this, but there are some problems.
See below.

> @@ -45,19 +47,39 @@
>    (package
>      (name "usbredir")
>      (version "0.7.1")
> +    ;(source (origin
> +    ;          (method url-fetch)
> +    ;          (uri (string-append
> +    ;            "http://spice-space.org/download/usbredir/"
> +    ;            "usbredir-" version ".tar.bz2"))
> +    ;          (sha256
> +    ;           (base32
> +    ;            "1wsnmk4wjpdhbn1zaxg6bmyxspcki2zgy0am9lk037rnl4krwzj0"))))
> +    ; FIXME: usbredir 0.7.1 release doesn't build on 32 bit systems.
> +    ;        issue is fixed in HEAD
> +    ;        remove 'autogen phase and autoconf, automake, libtool inputs
>      (source (origin
> -              (method url-fetch)
> -              (uri (string-append
> -                "http://spice-space.org/download/usbredir/"
> -                "usbredir-" version ".tar.bz2"))
> +              (method git-fetch)
> +              (uri (git-reference
> +                      (url "http://cgit.freedesktop.org/spice/usbredir")
> +                      (commit "ac80a5971c6318d73d5fba4b5f13d3a9389558c9")))
>                (sha256
>                 (base32
> -                "1wsnmk4wjpdhbn1zaxg6bmyxspcki2zgy0am9lk037rnl4krwzj0"))))
> +                "052fywgi72j68dr5ybldncg4vk8iqfrh58la7iazyxxpph9aag1g"))))

This is no longer version "0.7.1", so the version number needs to be
updated accordingly.  Please see section 7.6.3 (Version Numbers) in the
manual for our conventions for version numbers of VCS snapshots, and the
recommended code to generate those version numbers.

>      (build-system gnu-build-system)
>      (propagated-inputs
>        `(("libusb" ,libusb)))
>      (native-inputs
> -      `(("pkg-config" ,pkg-config)))
> +      `(("pkg-config" ,pkg-config)
> +        ("autoconf" ,autoconf)
> +        ("automake" ,automake)
> +        ("libtool" ,libtool)))
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'autogen
> +           (lambda* _
> +             (system* "sh" "autogen.sh"))))))

Phase procedures are supposed to return a boolean indicating whether
they succeeded, but 'system*' returns a number: a result code.  In
scheme, all numbers are considered true.  Also, you might as well use
'lambda' here instead of 'lambda*', so it should look like this:

           (lambda _
             (zero? (system* "sh" "autogen.sh")))

It might have been better to just use a simple patch to fix the format
strings than to use a VCS snapshot that might introduce more bugs, but
I guess we can see how it goes.

Anyway, can you push fixes for the other problems?

      Mark

Re: [PATCH] gnu: spice: Fix usbredir for 32 bit platforms.

[David Craven]

> Phase procedures are supposed to return a boolean indicating whether
> they succeeded, but 'system*' returns a number: a result code.  In
> scheme, all numbers are considered true.  Also, you might as well use
> 'lambda' here instead of 'lambda*', so it should look like this:

>           (lambda _
>             (zero? (system* "sh" "autogen.sh")))

The reason was that autogen.sh performs some check at the end that fails.
I'm running autoreconf directly now, so that the zero? doesn't cause the
phase to fail.

> This is no longer version "0.7.1", so the version number needs to be
> updated accordingly.  Please see section 7.6.3 (Version Numbers) in the
> manual for our conventions for version numbers of VCS snapshots, and the
> recommended code to generate those version numbers.

This could have also probably survived until the package gets updated, but
I fixed it.

> It might have been better to just use a simple patch to fix the format
> strings than to use a VCS snapshot that might introduce more bugs, but
> I guess we can see how it goes.

I considered this option also, but in at least one instance I was asked to
use substitute* instead of a patch, so I thought that patches are considered
a last resort. I also checked the history before selecting HEAD as the commit,
it looks like there where only a couple of bugfixes, but not much activity
otherwise. So hopefully this does not introduce any new bugs.

Thanks,
David

Re: [PATCH] gnu: spice: Fix usbredir for 32 bit platforms.

[Mark H Weaver]

David Craven <david@craven.ch> writes:

>> Phase procedures are supposed to return a boolean indicating whether
>> they succeeded, but 'system*' returns a number: a result code.  In
>> scheme, all numbers are considered true.  Also, you might as well use
>> 'lambda' here instead of 'lambda*', so it should look like this:
>
>>           (lambda _
>>             (zero? (system* "sh" "autogen.sh")))
>
> The reason was that autogen.sh performs some check at the end that fails.
> I'm running autoreconf directly now, so that the zero? doesn't cause the
> phase to fail.

Sounds good, thanks.

>> This is no longer version "0.7.1", so the version number needs to be
>> updated accordingly.  Please see section 7.6.3 (Version Numbers) in the
>> manual for our conventions for version numbers of VCS snapshots, and the
>> recommended code to generate those version numbers.
>
> This could have also probably survived until the package gets updated, but
> I fixed it.

Thank you.  I think it's important for the 'version' field to be
accurate.  If you could push this fix at your earliest convenience, I'd
be grateful.  (I don't see it in the git repo yet)

>> IT might have been better to just use a simple patch to fix the format
>> strings than to use a VCS snapshot that might introduce more bugs, but
>> I guess we can see how it goes.
>
> I considered this option also, but in at least one instance I was asked to
> use substitute* instead of a patch, so I thought that patches are considered
> a last resort.

They are certainly not a last resort in general.  The preferred method
of making changes prior to the build depends on the specific details.

However, one important consideration is that patches and snippets change
what is considered by Guix to be the "source" of the package as returned
by 'package-source' at the Scheme level and "guix build --source" as the
command line.

With this in mind, I'd say that for bug fixes, patches are generally the
preferred method.

> I also checked the history before selecting HEAD as the commit,
> it looks like there where only a couple of bugfixes, but not much activity
> otherwise. So hopefully this does not introduce any new bugs.

Oh, okay, that makes sense.  Thanks for working on it.

      Mark