Re: [Patch] gst-plugins-base

[Mark H Weaver]

Efraim Flashner <address@hidden> writes:

> Currently on both mips64 and on armhf gst-plugins-base doesn't build, and I'm
> not sure if it has any time recently. On arm, the orc related tests fail, and
> on mips orc fails to build entirely. Orc is a nice addition to
> gst-plugins-base, but it will build without it. This patch would have arm and
> mips go without it. Currently gst-plugins-base is the only program that
> directly depends on orc, so the other ~50 programs that depend on
> gst-plugins-base will have a chance of building now.

Sounds good.  See below for comments.

> From c7a03250b130928a9d0c00e8861218abb08c22b5 Mon Sep 17 00:00:00 2001
> From: Efraim Flashner <address@hidden>
> Date: Wed, 30 Mar 2016 18:03:53 +0300
> Subject: [PATCH] gnu: gst-plugins-base: Don't build with orc on arm/mips
>  architectures.
>
> * gnu/packages/gstreamer.scm (gst-plugins-base)[inputs]: Only use orc as
> an input on armhf or mips architectures.
> ---
>  gnu/packages/gstreamer.scm | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/gnu/packages/gstreamer.scm b/gnu/packages/gstreamer.scm
> index 5cf59cd..c44cbec 100644
> --- a/gnu/packages/gstreamer.scm
> +++ b/gnu/packages/gstreamer.scm
> @@ -160,7 +160,6 @@ This package provides the core library and elements.")
>       `(("gstreamer" ,gstreamer))) ; required by gstreamer-plugins-base-1.0.pc
>      (inputs
>       `(("cdparanoia" ,cdparanoia)
> -       ("orc" ,orc)
>         ("pango" ,pango)
>         ("libogg" ,libogg)
>         ("libtheora" ,libtheora)
> @@ -169,7 +168,12 @@ This package provides the core library and elements.")
>         ("zlib" ,zlib)
>         ("libXext" ,libxext)
>         ("libxv" ,libxv)
> -       ("alsa-lib" ,alsa-lib)))
> +       ("alsa-lib" ,alsa-lib)
> +       ,@(if (not (string-prefix? (or "armhf" "mips")
> +                                  (or (%current-target-system)
> +                                      (%current-system))))
> +           `(("orc" ,orc))
> +           '())))

Several issues here:

* (or "armhf" "mips") always evaluates to "armhf", so this doesn't do
  what you seem to be hoping for.  As you have it above, orc would still
  be included as an input on mips.

* When cross-compiling (%current-target-system) returns a GNU triplet
  string instead of a Nix system string, and those strings start with
  "arm" but not "armhf".

* If you put this ",@if" in the same place where ("orc" ,orc) is
  currently located, then we can avoid rebuilding a lot of stuff on
  intel architectures.

* Finally, by our indentation conventions, the consequent of the 'if'
  should be lined up under the condition.

Here's one way to do it:

--8<---------------cut here---------------start------------->8---
    (inputs
     `(("cdparanoia" ,cdparanoia)
       ,@(if (any (cute string-prefix? <> (or (%current-target-system)
                                              (%current-system)))
                  '("arm" "mips"))
             '()
             `(("orc" ,orc)))
--8<---------------cut here---------------end--------------->8---

You'll need to import (srfi srfi-1) for 'any' and (srfi srfi-26) for
'cute'.

What do you think?  Can you send an updated patch?

    Thank you!
       Mark