Re: [PATCH] Attempt at packaging guile-emacs (needs help)

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

Christopher Allan Webber <cwebber@dustycloud.org> writes:

> I started working on an attempt to package guile-emacs, based on
>   http://www.emacswiki.org/emacs/GuileEmacs
>
> This includes both a package for guilemacs-guile and guilemacs-emacs,
> both BT Templeton's WIP branches.

Excellent! :)

> Unfortunately, guilemacs-guile does not complete building.  It compiles,
> but then fails "make check":
>   http://pamrel.lu/2aa8f/

These are known issues.  Perhaps we should just disable tests for now.

> From 4182f8c22f1ca4eea453e61a90c77b790f573d8a Mon Sep 17 00:00:00 2001
> From: Christopher Allan Webber <cwebber@dustycloud.org>
> Date: Sun, 10 May 2015 09:29:54 -0500
> Subject: [PATCH] guilemacs attempt, failing on "make check"
>
> ---
>  gnu/packages/guile.scm     |  3 +-
>  gnu/packages/guilemacs.scm | 83 ++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 85 insertions(+), 1 deletion(-)
>  create mode 100644 gnu/packages/guilemacs.scm
>
>
> diff --git a/gnu/packages/guile.scm b/gnu/packages/guile.scm
> index 751002f..33d5773 100644
> --- a/gnu/packages/guile.scm
> +++ b/gnu/packages/guile.scm
> @@ -41,7 +41,8 @@
>    #:use-module (guix build-system gnu)
>    #:use-module (guix build-system trivial)
>    #:use-module (guix utils)
> -  #:use-module (ice-9 match))
> +  #:use-module (ice-9 match)
> +  #:export (guile-2.0))

No need for this, because 'define-public' is used to define guile-2.0.

> diff --git a/gnu/packages/guilemacs.scm b/gnu/packages/guilemacs.scm
> new file mode 100644
> index 0000000..c370200
> --- /dev/null
> +++ b/gnu/packages/guilemacs.scm

I believe that bipt calls it "guile-emacs", so that's what we should
call it too.

This file needs to be added to GNU_SYSTEM_MODULES in gnu-system.am.

> +(define-public guilemacs-guile

I think the main package should just be called "guile-emacs", but I'm
not sure what to name this branch of guile.  "guile-for-guile-emacs"
comes to mind, but I admit that's not great.  Maybe it doesn't matter
since users are unlikely to install it directly.

> +  (package (inherit guile-2.0)
> +    (name "guilemacs-guile")
> +    (version "20150212.a930be6")
> +    (arguments
> +     '(#:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'autogen
> +                    (lambda _
> +                      (zero? (system* "sh" "autogen.sh"))))
> +         (add-before 'autogen 'patch-/bin/sh
> +           (lambda _
> +             (substitute* "build-aux/git-version-gen"
> +               (("#!/bin/sh") (string-append "#!" (which "sh")))))))))

Phases are supposed to return a boolean indicating whether the phase
succeeded, but the return value of 'substitute*' is not specified.  In
practice it tends to work, but I'd prefer to put an explicit #t in
there, like this:

--8<---------------cut here---------------start------------->8---
           (lambda _
             (substitute* "build-aux/git-version-gen"
               (("#!/bin/sh") (string-append "#!" (which "sh"))))
             #t)))))
--8<---------------cut here---------------end--------------->8---

> +    (native-inputs
> +     (append
> +      `(("autoconf" ,autoconf)
> +        ("automake" ,automake)
> +        ("libtool" ,libtool)
> +        ("flex" ,flex)
> +        ("texinfo" ,texinfo))
> +      (package-native-inputs guile-2.0)))
> +    (inputs
> +     (append
> +      `(("gettext" ,gnu-gettext))
> +      (package-inputs guile-2.0)))

This is okay, but it would be more consistent with our usual style to
let the quasiquote do the append, like this:

--8<---------------cut here---------------start------------->8---
    (native-inputs
     `(("autoconf" ,autoconf)
       ("automake" ,automake)
       ("libtool" ,libtool)
       ("flex" ,flex)
       ("texinfo" ,texinfo)
       ,@(package-native-inputs guile-2.0)))
    (inputs
     `(("gettext" ,gnu-gettext)
       ,@(package-inputs guile-2.0)))))
--8<---------------cut here---------------end--------------->8---

> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "git://git.hcoop.net/git/bpt/guile.git")
> +                    (commit "a930be6f0f0d1594873c7eb9d2316ecf926da67d")))
> +              (sha256
> +               (base32
> +                "1l520n482cq8h8qbkp11j5xcz5vf6fnbpc24i0xmd3ngj2ga3gh7"))))))

For consistency, we usually put the 'source' field just below the
'version' field, although it's not a big deal.

> +(define-public guilemacs-emacs
> +  (package (inherit emacs)
> +    (name "guilemacs-emacs")

Let's call it "guile-emacs" (both the variable and in the 'name' field).

> +    (version "20150212.a930be6")

This should be "20150212.8f2e203".

> +    (source (origin
> +              (method git-fetch)
> +              (uri (git-reference
> +                    (url "git://git.hcoop.net/git/bpt/emacs.git")
> +                    (commit "8f2e20304cbfdc046f4e3d3fca7b844f07e65076")))
> +              (sha256
> +               (base32
> +                "0f0i7ax0wi5q2w2kvr4bdzkcbzvcqvnbni0n8vdsrxc7ayl8zdi3"))))
> +    (inputs
> +     (cons
> +      `("guile" ,guilemacs-guile)
> +      (package-inputs emacs)))))

See my suggestion above about using ,@ within the quasiquote to do this.

Thanks for working on this!

      Mark