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

[Mark H Weaver <mhw@netris.org>]

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