Re: [PATCH] gnu: Add exempi and eog.

[Mark H Weaver]

David Hashe <address@hidden> writes:

> The attached patches add Eye of GNOME and one of its dependencies,
> exempi. The Eye of GNOME (eog) patch depends on the exempi patch.

I agree with the comments by Ricardo and 宋文武, and have a few
additional comments:

> From cf0f69390f00852b9971e88e5e817ca540264d88 Mon Sep 17 00:00:00 2001
> From: David Hashe <address@hidden>
> Date: Wed, 15 Jul 2015 01:02:02 -0500
> Subject: [PATCH] gnu: Add exempi.
>
> * gnu/packages/freedesktop.scm (exempi): New variable.
> ---
>  gnu/packages/freedesktop.scm | 45 
> ++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/gnu/packages/freedesktop.scm b/gnu/packages/freedesktop.scm
> index eeb97cd..93a4470 100644
> --- a/gnu/packages/freedesktop.scm
> +++ b/gnu/packages/freedesktop.scm
> @@ -4,6 +4,7 @@
>  ;;; Copyright © 2015 Andy Wingo <address@hidden>
>  ;;; Copyright © 2015 Ludovic Courtès <address@hidden>
>  ;;; Copyright © 2015 Ricardo Wurmus <address@hidden>
> +;;; Copyright © 2015 David Hashe <address@hidden>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -40,6 +41,7 @@
>    #:use-module (gnu packages xdisorg)
>    #:use-module (gnu packages xorg)
>    #:use-module (gnu packages doxygen)
> +  #:use-module (gnu packages compression)
>    #:use-module (gnu packages libffi))
>  
>  (define-public xdg-utils
> @@ -232,3 +234,46 @@ display server running on Linux kernel modesetting and 
> evdev input devices, an X
>  application, or a wayland client itself.  The clients can be traditional
>  applications, X servers (rootless or fullscreen) or other display servers.")
>      (license license:x11)))
> +
> +(define-public exempi
> +    (package
> +      (name "exempi")
> +      (version (string-append "2.2.2"))
> +      (source (origin
> +               (method git-fetch)
> +               (uri (git-reference
> +                     (url "http://anongit.freedesktop.org/git/exempi.git";)
> +                     (commit version)))
> +               (sha256
> +                (base32
> +                 "1z25wij89fn86bm38d9ahhzfq8a2sgxaphdc4lrpyq87dgb766q9"))
> +               (file-name (string-append name "-" version))))

If we had used the git checkout, the file-name should end with
"-checkout", but we should use the release tarball instead.

> +      (build-system gnu-build-system)
> +      (arguments
> +        ;; FIXME: tests depend on boost, but unable to find headers when
> +        ;; used as an input
> +       `(#:configure-flags '("--disable-unittest")
> +         #:phases (alist-cons-after
> +                     'unpack 'fix-autogen
> +                     (lambda _
> +                     (substitute* "autogen.sh"
> +                      ;; autogen.sh tries to run configure before we
> +                      ;; are able to patch it
> +                      (("^.*topsrcdir/configure.*$") "")))
> +                   (alist-cons-before
> +                    'configure 'autogen
> +                    (lambda _
> +                     (zero? (system* "./autogen.sh")))
> +                     %standard-phases))))

Autogen/autoreconf/bootstrap phases should always go after 'unpack' (or
in this case, after 'fix-autogen'), *not* before 'configure'.  The
reason is that there are some other phases (patch-usr-bin-file and
patch-source-shebangs) which should have access to the generated files.
For example, letting 'patch-usr-bin-file' patch the generated
'configure' is important on some platforms.

However, this is moot because you should use the release tarball.

Can you send updated patches?

     Thanks,
       Mark