[PATCH] gnu: Add freeimage.

[PATCH] gnu: Add freeimage.

[David Thompson]

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

This patch adds the freeimage library.  It's a handy way to load images
of various formats via a single interface.

How does it look?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-freeimage.patch --]
[-- Type: text/x-diff, Size: 2660 bytes --]

From 908cad8230ecc89dbf36ab54c0687879e4cf9b22 Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Wed, 25 Jun 2014 19:50:30 -0400
Subject: [PATCH] gnu: Add freeimage.

* gnu/packages/image.scm (freeimage): New variable.
---
 gnu/packages/image.scm | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/gnu/packages/image.scm b/gnu/packages/image.scm
index 38e3ddd..b492399 100644
--- a/gnu/packages/image.scm
+++ b/gnu/packages/image.scm
@@ -27,6 +27,7 @@
   #:use-module (gnu packages ghostscript)         ;lcms
   #:use-module (gnu packages xorg)
   #:use-module (gnu packages perl)
+  #:use-module (gnu packages zip)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix packages)
   #:use-module (guix download)
@@ -341,3 +342,43 @@ supplies a generic doubly-linked list and some string functions.")
     ;; X11 license.
     (license (license:x11-style "file://COPYING"
                                 "See 'COPYING' in the distribution."))))
+
+(define-public freeimage
+  (package
+   (name "freeimage")
+   (version "3.16.0")
+   (source (origin
+            (method url-fetch)
+            (uri (string-append "mirror://sourceforge/freeimage/Source%20Distribution/"
+                                version "/FreeImage"
+                                (string-join (string-split version #\.) "")
+                                ".zip"))
+            (sha256
+             (base32
+              "0q1gnjnxgphsh4l8i9rfly4bi8xsczsb9ryzbm8hf38lc3fk5bq3"))))
+   (build-system gnu-build-system)
+   (arguments
+    '(#:phases (alist-replace
+                'unpack
+                (lambda* (#:key source #:allow-other-keys)
+                  (and (zero? (system* "unzip" source))
+                       (chdir "FreeImage")))
+                (alist-delete
+                 'configure
+                 (alist-cons-before
+                  'build 'patch-makefile
+                  (lambda* (#:key outputs #:allow-other-keys)
+                    (substitute* "Makefile.gnu"
+                      (("/usr") (assoc-ref outputs "out"))
+                      (("-o root -g root") "")))
+                  %standard-phases)))
+      #:make-flags '("CC=gcc")
+      #:tests? #f)) ; no check target
+   (native-inputs
+    `(("unzip" ,unzip)))
+   (synopsis "Library for handling popular graphics image formats")
+   (description
+    "FreeImage is a library for developers who would like to support popular
+graphics image formats like PNG, BMP, JPEG, TIFF and others.")
+   (license license:gpl2+)
+   (home-page "http://freeimage.sourceforge.net")))
-- 
2.1.1


[-- Attachment #3: Type: text/plain, Size: 136 bytes --]


-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] gnu: Add freeimage.

[Andreas Enge]

Hello!

On Sat, Nov 01, 2014 at 11:48:46AM -0400, David Thompson wrote:
> +            (uri (string-append "mirror://sourceforge/freeimage/Source%20Distribution/"
> +                                version "/FreeImage"
> +                                (string-join (string-split version #\.) "")
> +                                ".zip"))

This looks a bit longer than the 80 characters limit.

> +                'unpack
> +                (lambda* (#:key source #:allow-other-keys)
> +                  (and (zero? (system* "unzip" source))
> +                       (chdir "FreeImage")))

I wonder if this should not be moved to the standard unpack phase. Can we
determine the file format and use the appropriate tool, unzip or tar,
automatically? (Assuming that there are no .tar.zip out there, of course.)

Andreas

Re: [PATCH] gnu: Add freeimage.

[David Thompson]

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

Andreas Enge <andreas@enge.fr> writes:

> Hello!
>
> On Sat, Nov 01, 2014 at 11:48:46AM -0400, David Thompson wrote:
>> +            (uri (string-append "mirror://sourceforge/freeimage/Source%20Distribution/"
>> +                                version "/FreeImage"
>> +                                (string-join (string-split version #\.) "")
>> +                                ".zip"))
>
> This looks a bit longer than the 80 characters limit.

The first argument to string append goes over, but that's okay IMO
because it is a single atom.  In any case, I just added a newline after
string-append and the issue goes away.

>
>> +                'unpack
>> +                (lambda* (#:key source #:allow-other-keys)
>> +                  (and (zero? (system* "unzip" source))
>> +                       (chdir "FreeImage")))
>
> I wonder if this should not be moved to the standard unpack phase. Can we
> determine the file format and use the appropriate tool, unzip or tar,
> automatically? (Assuming that there are no .tar.zip out there, of
> course.)

I've been thinking about this, too.  I don't think I would like the GNU
build system to depend on unzip since very few packages require it and
it's not a format that GNU uses, but maybe a procedure that replaced the
standard unpack phase with one that uses unzip would be nice.  I think
that work could/should be saved for a future patch.

Updated patch attached.  Thanks for the review!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-Add-freeimage.patch --]
[-- Type: text/x-diff, Size: 2638 bytes --]

From 2972e6e41d7c13dd619f0dd7fbda7b2a4ec0c6cc Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Wed, 25 Jun 2014 19:50:30 -0400
Subject: [PATCH] gnu: Add freeimage.

* gnu/packages/image.scm (freeimage): New variable.
---
 gnu/packages/image.scm | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/gnu/packages/image.scm b/gnu/packages/image.scm
index 38e3ddd..f4f4c78 100644
--- a/gnu/packages/image.scm
+++ b/gnu/packages/image.scm
@@ -27,6 +27,7 @@
   #:use-module (gnu packages ghostscript)         ;lcms
   #:use-module (gnu packages xorg)
   #:use-module (gnu packages perl)
+  #:use-module (gnu packages zip)
   #:use-module ((guix licenses) #:prefix license:)
   #:use-module (guix packages)
   #:use-module (guix download)
@@ -341,3 +342,44 @@ supplies a generic doubly-linked list and some string functions.")
     ;; X11 license.
     (license (license:x11-style "file://COPYING"
                                 "See 'COPYING' in the distribution."))))
+
+(define-public freeimage
+  (package
+   (name "freeimage")
+   (version "3.16.0")
+   (source (origin
+            (method url-fetch)
+            (uri (string-append
+                  "mirror://sourceforge/freeimage/Source%20Distribution/"
+                  version "/FreeImage"
+                  (string-join (string-split version #\.) "")
+                  ".zip"))
+            (sha256
+             (base32
+              "0q1gnjnxgphsh4l8i9rfly4bi8xsczsb9ryzbm8hf38lc3fk5bq3"))))
+   (build-system gnu-build-system)
+   (arguments
+    '(#:phases (alist-replace
+                'unpack
+                (lambda* (#:key source #:allow-other-keys)
+                  (and (zero? (system* "unzip" source))
+                       (chdir "FreeImage")))
+                (alist-delete
+                 'configure
+                 (alist-cons-before
+                  'build 'patch-makefile
+                  (lambda* (#:key outputs #:allow-other-keys)
+                    (substitute* "Makefile.gnu"
+                      (("/usr") (assoc-ref outputs "out"))
+                      (("-o root -g root") "")))
+                  %standard-phases)))
+      #:make-flags '("CC=gcc")
+      #:tests? #f)) ; no check target
+   (native-inputs
+    `(("unzip" ,unzip)))
+   (synopsis "Library for handling popular graphics image formats")
+   (description
+    "FreeImage is a library for developers who would like to support popular
+graphics image formats like PNG, BMP, JPEG, TIFF and others.")
+   (license license:gpl2+)
+   (home-page "http://freeimage.sourceforge.net")))
-- 
2.1.1


[-- Attachment #3: Type: text/plain, Size: 136 bytes --]


-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] gnu: Add freeimage.

[Ludovic Courtès]

David Thompson <dthompson2@worcester.edu> skribis:

> Andreas Enge <andreas@enge.fr> writes:

[...]

>>> +                'unpack
>>> +                (lambda* (#:key source #:allow-other-keys)
>>> +                  (and (zero? (system* "unzip" source))
>>> +                       (chdir "FreeImage")))
>>
>> I wonder if this should not be moved to the standard unpack phase. Can we
>> determine the file format and use the appropriate tool, unzip or tar,
>> automatically? (Assuming that there are no .tar.zip out there, of
>> course.)
>
> I've been thinking about this, too.  I don't think I would like the GNU
> build system to depend on unzip since very few packages require it and
> it's not a format that GNU uses, but maybe a procedure that replaced the
> standard unpack phase with one that uses unzip would be nice.  I think
> that work could/should be saved for a future patch.

I think we could change ‘unpack’ in gnu-build-system.scm to invoke unzip
when the file name ends in .zip, while letting it the user’s
responsibility to add unzip as an input when needed (as is already the
case for lzip.)

But yes, this would be for core-updates.

> From 2972e6e41d7c13dd619f0dd7fbda7b2a4ec0c6cc Mon Sep 17 00:00:00 2001
> From: David Thompson <dthompson2@worcester.edu>
> Date: Wed, 25 Jun 2014 19:50:30 -0400
> Subject: [PATCH] gnu: Add freeimage.
>
> * gnu/packages/image.scm (freeimage): New variable.

LGTM, OK to push!

Ludo’.

Re: [PATCH] gnu: Add freeimage.

[David Thompson]

Ludovic Courtès <ludo@gnu.org> writes:

> David Thompson <dthompson2@worcester.edu> skribis:
>
>> Andreas Enge <andreas@enge.fr> writes:
>
> [...]
>
>>>> +                'unpack
>>>> +                (lambda* (#:key source #:allow-other-keys)
>>>> +                  (and (zero? (system* "unzip" source))
>>>> +                       (chdir "FreeImage")))
>>>
>>> I wonder if this should not be moved to the standard unpack phase. Can we
>>> determine the file format and use the appropriate tool, unzip or tar,
>>> automatically? (Assuming that there are no .tar.zip out there, of
>>> course.)
>>
>> I've been thinking about this, too.  I don't think I would like the GNU
>> build system to depend on unzip since very few packages require it and
>> it's not a format that GNU uses, but maybe a procedure that replaced the
>> standard unpack phase with one that uses unzip would be nice.  I think
>> that work could/should be saved for a future patch.
>
> I think we could change ‘unpack’ in gnu-build-system.scm to invoke unzip
> when the file name ends in .zip, while letting it the user’s
> responsibility to add unzip as an input when needed (as is already the
> case for lzip.)
>
> But yes, this would be for core-updates.

Okay, that does sound like a good idea.

>> From 2972e6e41d7c13dd619f0dd7fbda7b2a4ec0c6cc Mon Sep 17 00:00:00 2001
>> From: David Thompson <dthompson2@worcester.edu>
>> Date: Wed, 25 Jun 2014 19:50:30 -0400
>> Subject: [PATCH] gnu: Add freeimage.
>>
>> * gnu/packages/image.scm (freeimage): New variable.
>
> LGTM, OK to push!

Pushed.  Thanks!

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] gnu: Add freeimage.

[Andreas Enge]

On Sat, Nov 01, 2014 at 01:21:55PM -0400, David Thompson wrote:
> +    '(#:phases (alist-replace
> +                'unpack
> +                (lambda* (#:key source #:allow-other-keys)
> +                  (and (zero? (system* "unzip" source))
> +                       (chdir "FreeImage")))
> +                (alist-delete
> +                 'configure
> +                 (alist-cons-before
> +                  'build 'patch-makefile
> +                  (lambda* (#:key outputs #:allow-other-keys)
> +                    (substitute* "Makefile.gnu"
> +                      (("/usr") (assoc-ref outputs "out"))
> +                      (("-o root -g root") "")))
> +                  %standard-phases)))

I think that instead of adding patch-makefile before build, I would let it
replace configure. But this is just a matter of taste, so please do as you
please.

Andreas

Re: [PATCH] gnu: Add freeimage.

[David Thompson]

Andreas Enge <andreas@enge.fr> writes:

> On Sat, Nov 01, 2014 at 01:21:55PM -0400, David Thompson wrote:
>> +    '(#:phases (alist-replace
>> +                'unpack
>> +                (lambda* (#:key source #:allow-other-keys)
>> +                  (and (zero? (system* "unzip" source))
>> +                       (chdir "FreeImage")))
>> +                (alist-delete
>> +                 'configure
>> +                 (alist-cons-before
>> +                  'build 'patch-makefile
>> +                  (lambda* (#:key outputs #:allow-other-keys)
>> +                    (substitute* "Makefile.gnu"
>> +                      (("/usr") (assoc-ref outputs "out"))
>> +                      (("-o root -g root") "")))
>> +                  %standard-phases)))
>
> I think that instead of adding patch-makefile before build, I would let it
> replace configure. But this is just a matter of taste, so please do as you
> please.

That's a fair point, though I have already pushed.  Thanks, anyhow. :)

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] gnu: Add freeimage.

[Andreas Enge]

Unfortunately, it fails to build on mips:
   http://hydra.gnu.org/build/136438
Could you have a look? To me it looks as if there is bad assembly code
in the source:
/tmp/nix-build-freeimage-3.16.0.drv-0/cc1WaZEm.s: Assembler messages:
/tmp/nix-build-freeimage-3.16.0.drv-0/cc1WaZEm.s:360: Error: opcode not supported on this processor: mips3 (mips3) `madd $24,$10'
...

Andreas

Re: [PATCH] gnu: Add freeimage.

[David Thompson]

Andreas Enge <andreas@enge.fr> writes:

> Unfortunately, it fails to build on mips:
>    http://hydra.gnu.org/build/136438
> Could you have a look? To me it looks as if there is bad assembly code
> in the source:
> /tmp/nix-build-freeimage-3.16.0.drv-0/cc1WaZEm.s: Assembler messages:
> /tmp/nix-build-freeimage-3.16.0.drv-0/cc1WaZEm.s:360: Error: opcode not supported on this processor: mips3 (mips3) `madd $24,$10'
> ...

When I get the chance, I will just disable mips support for freeimage
and post a patch.

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] gnu: Add freeimage.

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> Could you have a look? To me it looks as if there is bad assembly code
> in the source:
> /tmp/nix-build-freeimage-3.16.0.drv-0/cc1WaZEm.s: Assembler messages:
> /tmp/nix-build-freeimage-3.16.0.drv-0/cc1WaZEm.s:360: Error: opcode not supported on this processor: mips3 (mips3) `madd $24,$10'
> ...

Same problem as with Valgrind, so I would suggest disabling builds on
MIPS, as done in commit 67a86d3.

Ludo’.

Re: [PATCH] gnu: Add freeimage.

[Andreas Enge]

On Mon, Nov 03, 2014 at 10:00:38AM +0100, Ludovic Courtès wrote:
> Same problem as with Valgrind, so I would suggest disabling builds on
> MIPS, as done in commit 67a86d3.

Speaking of valgrind, how is transitivity of disabled builds handled,
on hydra and by the users?

I noticed that some packages do not build on mipss due to a dependency
on valgrind, for instance petsc-openmpi and petsc-complex-openmpi:
   http://hydra.gnu.org/build/132921
   http://hydra.gnu.org/build/132781
It was a bit confusing to me at first, since valgrind was not in the
"still failing jobs" tab, until I realised it was also not in the "still
succeeding jobs" tab and in fact disabled.

The two packages in question depend indirectly on valgrind via openmpi.
Now openmpi seems to be disabled for mips on hydra also, although it is
not explicitly disabled in the package recipe. These two other packages
are not disabled, however. So does this mean that the dependency graph
for disabled inputs is followed only to depth 1?

Andreas

Re: [PATCH] gnu: Add freeimage.

[Ludovic Courtès]

Andreas Enge <andreas@enge.fr> skribis:

> On Mon, Nov 03, 2014 at 10:00:38AM +0100, Ludovic Courtès wrote:
>> Same problem as with Valgrind, so I would suggest disabling builds on
>> MIPS, as done in commit 67a86d3.
>
> Speaking of valgrind, how is transitivity of disabled builds handled,
> on hydra and by the users?

There’s nothing preventing users to build Valgrind on mips64el-linux,
currently.

On Hydra, build-aux/hydra/gnu-system.scm uses
‘package-transitive-supported-systems’, which returns the intersection
of a package’s ‘supported-systems’ and that of its inputs (with the
assumption that implicit inputs are all supported everywhere.)

> I noticed that some packages do not build on mipss due to a dependency
> on valgrind, for instance petsc-openmpi and petsc-complex-openmpi:
>    http://hydra.gnu.org/build/132921
>    http://hydra.gnu.org/build/132781
> It was a bit confusing to me at first, since valgrind was not in the
> "still failing jobs" tab, until I realised it was also not in the "still
> succeeding jobs" tab and in fact disabled.
>
> The two packages in question depend indirectly on valgrind via openmpi.
> Now openmpi seems to be disabled for mips on hydra also, although it is
> not explicitly disabled in the package recipe. These two other packages
> are not disabled, however. So does this mean that the dependency graph
> for disabled inputs is followed only to depth 1?

Oh right, that’s a bug.  Fixed in commit c37a74b.

Thanks for the report!

Ludo’.

Re: [PATCH] gnu: Add freeimage.

[David Thompson]

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

Andreas Enge <andreas@enge.fr> writes:

> Unfortunately, it fails to build on mips:
>    http://hydra.gnu.org/build/136438
> Could you have a look? To me it looks as if there is bad assembly code
> in the source:
> /tmp/nix-build-freeimage-3.16.0.drv-0/cc1WaZEm.s: Assembler messages:
> /tmp/nix-build-freeimage-3.16.0.drv-0/cc1WaZEm.s:360: Error: opcode not supported on this processor: mips3 (mips3) `madd $24,$10'
> ...

Here's a patch that drops MIPS support.  Okay to push?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-gnu-freeimage-Remove-MIPS-from-supported-systems.patch --]
[-- Type: text/x-diff, Size: 998 bytes --]

From 67515d4f69b7deda45be7fb6e7ebf260d5e0439b Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Mon, 3 Nov 2014 18:26:38 -0500
Subject: [PATCH] gnu: freeimage: Remove MIPS from supported-systems.

* gnu/packages/image.scm (freeimage): Drop support for "mips64el-linux".
---
 gnu/packages/image.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/image.scm b/gnu/packages/image.scm
index f4f4c78..7a22bf4 100644
--- a/gnu/packages/image.scm
+++ b/gnu/packages/image.scm
@@ -377,6 +377,8 @@ supplies a generic doubly-linked list and some string functions.")
       #:tests? #f)) ; no check target
    (native-inputs
     `(("unzip" ,unzip)))
+   ;; Fails to build on MIPS due to assembly code in the source.
+   (supported-systems (delete "mips64el-linux" %supported-systems))
    (synopsis "Library for handling popular graphics image formats")
    (description
     "FreeImage is a library for developers who would like to support popular
-- 
2.1.1


[-- Attachment #3: Type: text/plain, Size: 136 bytes --]


-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate

Re: [PATCH] gnu: Add freeimage.

[Ludovic Courtès]

David Thompson <dthompson2@worcester.edu> skribis:

> From 67515d4f69b7deda45be7fb6e7ebf260d5e0439b Mon Sep 17 00:00:00 2001
> From: David Thompson <dthompson2@worcester.edu>
> Date: Mon, 3 Nov 2014 18:26:38 -0500
> Subject: [PATCH] gnu: freeimage: Remove MIPS from supported-systems.
>
> * gnu/packages/image.scm (freeimage): Drop support for "mips64el-linux".

OK!

Thanks,
Ludo'.

Re: [PATCH] gnu: Add freeimage.

[David Thompson]

Ludovic Courtès <ludo@gnu.org> writes:

> David Thompson <dthompson2@worcester.edu> skribis:
>
>> From 67515d4f69b7deda45be7fb6e7ebf260d5e0439b Mon Sep 17 00:00:00 2001
>> From: David Thompson <dthompson2@worcester.edu>
>> Date: Mon, 3 Nov 2014 18:26:38 -0500
>> Subject: [PATCH] gnu: freeimage: Remove MIPS from supported-systems.
>>
>> * gnu/packages/image.scm (freeimage): Drop support for "mips64el-linux".
>
> OK!

Pushed!  Thanks for reviewing, Ludo and Andreas.

-- 
David Thompson
Web Developer - Free Software Foundation - http://fsf.org
GPG Key: 0FF1D807
Support the FSF: https://fsf.org/donate