unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [Patch] gst-plugins-base
@ 2016-03-30 15:38 Efraim Flashner
  2016-03-30 21:53 ` Mark H Weaver
  0 siblings, 1 reply; 6+ messages in thread
From: Efraim Flashner @ 2016-03-30 15:38 UTC (permalink / raw)
  To: guix-devel


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

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.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-gnu-gst-plugins-base-Don-t-build-with-orc-on-arm-mip.patch --]
[-- Type: text/x-patch, Size: 1512 bytes --]

From c7a03250b130928a9d0c00e8861218abb08c22b5 Mon Sep 17 00:00:00 2001
From: Efraim Flashner <efraim@flashner.co.il>
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))
+           '())))
     (native-inputs
       `(("pkg-config" ,pkg-config)
         ("glib" ,glib "bin")
-- 
2.8.0.rc3


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch] gst-plugins-base
  2016-03-30 15:38 [Patch] gst-plugins-base Efraim Flashner
@ 2016-03-30 21:53 ` Mark H Weaver
  2016-03-30 21:57   ` Mark H Weaver
  0 siblings, 1 reply; 6+ messages in thread
From: Mark H Weaver @ 2016-03-30 21:53 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: guix-devel

Efraim Flashner <efraim@flashner.co.il> 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 <efraim@flashner.co.il>
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] gst-plugins-base
  2016-03-30 21:53 ` Mark H Weaver
@ 2016-03-30 21:57   ` Mark H Weaver
  2016-03-31  7:22     ` Efraim Flashner
  0 siblings, 1 reply; 6+ messages in thread
From: Mark H Weaver @ 2016-03-30 21:57 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: guix-devel

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

> Here's one way to do it:
>
>     (inputs
>      `(("cdparanoia" ,cdparanoia)
>        ,@(if (any (cute string-prefix? <> (or (%current-target-system)
>                                               (%current-system)))
>                   '("arm" "mips"))
>              '()
>              `(("orc" ,orc)))
>
> 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?

One more thing: please add a comment containing the string "FIXME",
explaining that we are omitting "orc" on arm and mips, and the reason
why.

     Thanks!
       Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] gst-plugins-base
  2016-03-30 21:57   ` Mark H Weaver
@ 2016-03-31  7:22     ` Efraim Flashner
  2016-03-31 17:17       ` Mark H Weaver
  0 siblings, 1 reply; 6+ messages in thread
From: Efraim Flashner @ 2016-03-31  7:22 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guix-devel


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

On Wed, Mar 30, 2016 at 05:57:24PM -0400, Mark H Weaver wrote:
> Mark H Weaver <mhw@netris.org> writes:
> 
> > Here's one way to do it:
> >
> >     (inputs
> >      `(("cdparanoia" ,cdparanoia)
> >        ,@(if (any (cute string-prefix? <> (or (%current-target-system)
> >                                               (%current-system)))
> >                   '("arm" "mips"))

64-bit Arm is Aarch64 and not arm64, so this probably won't capture
64-bit Arm if/when we start to support it.

> >              '()
> >              `(("orc" ,orc)))
> >
> > 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?
> 
> One more thing: please add a comment containing the string "FIXME",
> explaining that we are omitting "orc" on arm and mips, and the reason
> why.
> 
>      Thanks!
>        Mark

It turns out I didn't search gstreamer.scm well enough, so if this patch
looks good then I'll copy the syntax over to the other packages in
gstreamer and make patches for them too.

efraim@debian-netbook:~/workspace/guix$ grep \(\"orc\"\ \,orc\)
gnu/packages/*scm
gnu/packages/gstreamer.scm:           `(("orc" ,orc)))
gnu/packages/gstreamer.scm:       ("orc" ,orc)
gnu/packages/gstreamer.scm:       ("orc" ,orc)
gnu/packages/gstreamer.scm:       ("orc" ,orc)))
gnu/packages/gstreamer.scm:       ("orc" ,orc)

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #1.2: 0001-gnu-gst-plugins-base-Don-t-build-with-orc-on-arm-mip.patch --]
[-- Type: text/plain, Size: 1707 bytes --]

From b368c9c2c9948d32bb9167cd4e07aa4d49fa2d44 Mon Sep 17 00:00:00 2001
From: Efraim Flashner <efraim@flashner.co.il>
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 | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/gstreamer.scm b/gnu/packages/gstreamer.scm
index 5cf59cd..3694e6c 100644
--- a/gnu/packages/gstreamer.scm
+++ b/gnu/packages/gstreamer.scm
@@ -58,7 +58,9 @@
   #:use-module (gnu packages tls)
   #:use-module (gnu packages version-control)
   #:use-module (gnu packages yasm)
-  #:use-module (gnu packages xml))
+  #:use-module (gnu packages xml)
+  #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-26))
 
 (define-public orc
   (package
@@ -160,7 +162,14 @@ This package provides the core library and elements.")
      `(("gstreamer" ,gstreamer))) ; required by gstreamer-plugins-base-1.0.pc
     (inputs
      `(("cdparanoia" ,cdparanoia)
-       ("orc" ,orc)
+       ;; FIXME: On arm the orc related tests fail, and on mips "orc" fails to
+       ;; build.  For the time being we are disabling "orc" as an input on
+       ;; these architectures.
+       ,@(if (any (cute string-prefix? <> (or (%current-target-system)
+                                              (%current-system)))
+                  '("arm" "mips"))
+           '()
+           `(("orc" ,orc)))
        ("pango" ,pango)
        ("libogg" ,libogg)
        ("libtheora" ,libtheora)
-- 
2.8.0.rc3


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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch] gst-plugins-base
  2016-03-31  7:22     ` Efraim Flashner
@ 2016-03-31 17:17       ` Mark H Weaver
  2016-03-31 18:11         ` Efraim Flashner
  0 siblings, 1 reply; 6+ messages in thread
From: Mark H Weaver @ 2016-03-31 17:17 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: guix-devel

Efraim Flashner <efraim@flashner.co.il> writes:

> On Wed, Mar 30, 2016 at 05:57:24PM -0400, Mark H Weaver wrote:
>> Mark H Weaver <mhw@netris.org> writes:
>> 
>> > Here's one way to do it:
>> >
>> >     (inputs
>> >      `(("cdparanoia" ,cdparanoia)
>> >        ,@(if (any (cute string-prefix? <> (or (%current-target-system)
>> >                                               (%current-system)))
>> >                   '("arm" "mips"))
>
> 64-bit Arm is Aarch64 and not arm64, so this probably won't capture
> 64-bit Arm if/when we start to support it.

We shouldn't assume that the same problem will occur on Aarch64.
I'd prefer to wait and see.

> It turns out I didn't search gstreamer.scm well enough, so if this patch
> looks good then I'll copy the syntax over to the other packages in
> gstreamer and make patches for them too.
>
> efraim@debian-netbook:~/workspace/guix$ grep \(\"orc\"\ \,orc\)
> gnu/packages/*scm
> gnu/packages/gstreamer.scm:           `(("orc" ,orc)))
> gnu/packages/gstreamer.scm:       ("orc" ,orc)
> gnu/packages/gstreamer.scm:       ("orc" ,orc)
> gnu/packages/gstreamer.scm:       ("orc" ,orc)))
> gnu/packages/gstreamer.scm:       ("orc" ,orc)

To be honest, I'm having second thoughts about this entire approach.

My concern is that without orc, the codec implementations (especially
the video codecs) might be so slow as to be practically unusable.  I'm
reluctant to apply this workaround too broadly until we know whether it
results in anything good.  Really, we should be trying to investigate
and fix the underlying problems instead of sweeping them under the rug.

     Thoughts?
       Mark

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch] gst-plugins-base
  2016-03-31 17:17       ` Mark H Weaver
@ 2016-03-31 18:11         ` Efraim Flashner
  0 siblings, 0 replies; 6+ messages in thread
From: Efraim Flashner @ 2016-03-31 18:11 UTC (permalink / raw)
  To: Mark H Weaver; +Cc: guix-devel

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

On Thu, 31 Mar 2016 13:17:42 -0400
Mark H Weaver <mhw@netris.org> wrote:

> Efraim Flashner <efraim@flashner.co.il> writes:
> 
>  [...]  
>  [...]  
>  [...]  
> >
> > 64-bit Arm is Aarch64 and not arm64, so this probably won't capture
> > 64-bit Arm if/when we start to support it.  
> 
> We shouldn't assume that the same problem will occur on Aarch64.
> I'd prefer to wait and see.

Agreed. I normally try to look at supported-arch and the like while keeping
in mind that hopefully we'll have more architectures in the future.
 
> > It turns out I didn't search gstreamer.scm well enough, so if this patch
> > looks good then I'll copy the syntax over to the other packages in
> > gstreamer and make patches for them too.
> >
> > efraim@debian-netbook:~/workspace/guix$ grep \(\"orc\"\ \,orc\)
> > gnu/packages/*scm
> > gnu/packages/gstreamer.scm:           `(("orc" ,orc)))
> > gnu/packages/gstreamer.scm:       ("orc" ,orc)
> > gnu/packages/gstreamer.scm:       ("orc" ,orc)
> > gnu/packages/gstreamer.scm:       ("orc" ,orc)))
> > gnu/packages/gstreamer.scm:       ("orc" ,orc)  
> 
> To be honest, I'm having second thoughts about this entire approach.
> 
> My concern is that without orc, the codec implementations (especially
> the video codecs) might be so slow as to be practically unusable.  I'm
> reluctant to apply this workaround too broadly until we know whether it
> results in anything good.  Really, we should be trying to investigate
> and fix the underlying problems instead of sweeping them under the rug.
> 
>      Thoughts?
>        Mark

If its unusable without orc then definately we shouldn't substitute it out.
On the other hand, `guix refresh -l orc` shows the gnome metapackage,
enlightenment, libreoffice, iceweasel, etc. Other than rhythmbox and guitarix
nothing seems like it should rely too heavily on gstreamer or on its usage of
orc's "just-in-time tools for compiling and executing simple programs that
operate on arrays of data." Giving the other programs a chance to build on
arm and mips I'm sure would be nice for their users. The best would, of
course, be to test it, but otherwise it should be okay to go without it.

-- 
Efraim Flashner   <efraim@flashner.co.il>   אפרים פלשנר
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-03-31 18:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-30 15:38 [Patch] gst-plugins-base Efraim Flashner
2016-03-30 21:53 ` Mark H Weaver
2016-03-30 21:57   ` Mark H Weaver
2016-03-31  7:22     ` Efraim Flashner
2016-03-31 17:17       ` Mark H Weaver
2016-03-31 18:11         ` Efraim Flashner

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).