Re: gnu: guile: Add guile-wisp

[Mark H Weaver]

Christopher Allan Webber <address@hidden> writes:

> From 8d29d47c0558c24562c2c0760e1f05a78b064838 Mon Sep 17 00:00:00 2001
> From: Christopher Allan Webber <address@hidden>
> Date: Fri, 18 Sep 2015 16:58:31 -0500
> Subject: [PATCH] gnu: guile: Add guile-wisp
>
> * gnu/packages/guile.scm (guile-wisp): New variable.
> ---
>  gnu/packages/guile.scm | 78 
> +++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 77 insertions(+), 1 deletion(-)
>
>
> diff --git a/gnu/packages/guile.scm b/gnu/packages/guile.scm
> index 60af92d..867f79a 100644
> --- a/gnu/packages/guile.scm
> +++ b/gnu/packages/guile.scm
> @@ -39,6 +39,7 @@
>    #:use-module (gnu packages texinfo)
>    #:use-module (gnu packages gettext)
>    #:use-module (gnu packages gdbm)
> +  #:use-module (gnu packages python)
>    #:use-module (guix packages)
>    #:use-module (guix download)
>    #:use-module (guix git-download)
> @@ -538,7 +539,7 @@ See http://minikanren.org/ for more on miniKanren 
> generally.")
>  
>             ;; compile to the destination
>             (compile-file gdbm.scm-dest
> -                         #:output-file gdbm.go-dest)))))
> +                           #:output-file gdbm.go-dest)))))

This hunk looks like a mistake, it should be omitted.

> +(define-public guile-wisp
> +  (package
> +    (name "guile-wisp")
> +    (version "0.8.6")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append 
> "https://bitbucket.org/ArneBab/wisp/downloads/wisp-";

Please keep lines to 80 columns or less.

> +                                  version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "021437nmnc5vqmbyyn2zgfl8fzvwv0phc5pph6hp2x98wf2lzvg8"))))
> +    (build-system gnu-build-system)
> +    (arguments
> +     '(#:phases
> +       (modify-phases %standard-phases
> +         (add-after
> +          'unpack 'bootstrap
> +          (lambda _
> +            (zero? (system* "bash" "bootstrap.sh"))))
> +         (add-before
> +          'configure 'substitute-before-config
> +          (lambda* (#:key inputs #:allow-other-keys)
> +            (let ((bash (assoc-ref inputs "bash")))
> +              ;; configure checks for guile-2.0, but ours is just named 
> "guile" :)
> +              (substitute* "configure"
> +                (("guile-2.0") "guile"))
> +              ;; Puts together some test files with /bin/bash hardcoded
> +              (substitute* "Makefile.in"
> +                (("/bin/bash")
> +                 (string-append bash "/bin/bash"))))))

I guess this 'bash' is used at build-time only, right?  If so, you can
just replace it with "bash" instead of the absolute path.  This also
allows you to get rid of the 'let' and change the 'lambda*' to just
(lambda _ ...).

Also, phase procedures are expected to return a boolean indicating
success (#t) or failure (#f), but the return value of 'substitute*' is
unspecified.  Please add a #t after the last 'substitute*' call.

> +         ;; auto complilation breaks, but if we set HOME to /tmp,

There's an extra 'l' in "complilation".

> +         ;; that works ok
> +         (add-before
> +          'check 'auto-compile-hacky-workaround
> +          (lambda _
> +            (setenv "HOME" "/tmp")))

As above, please add #t after the 'setenv' call.

> +         (replace
> +          'install
> +          (lambda* (#:key outputs inputs #:allow-other-keys)
> +            (use-modules (guix build utils)
> +                         (system base compile))
> +
> +            (let* ((out (assoc-ref outputs "out"))
> +                   (module-dir (string-append out "/share/guile/site/2.0"))
> +                   (language-dir
> +                    (string-append module-dir "/language/wisp"))
> +                   (guild (string-append (assoc-ref inputs "guile")
> +                                         "/bin/guild")))
> +              ;; Make installation directories.
> +              (for-each (lambda (x) (mkdir-p x))
> +                        (list module-dir language-dir))

(lambda (x) (mkdir-p x)) is equivalent to just 'mkdir-p', so you can
just write this as:

  (for-each mkdir-p (list module-dir language-dir))

However, in this case, I wonder if it's simpler and more readable to
just write:

  (mkdir-p module-dir)
  (mkdir-p language-dir)

What do you think?

> +
> +              ;; copy the source
> +              (copy-file "wisp-scheme.scm"
> +                         (string-append module-dir "/wisp-scheme.scm"))
> +              (copy-file "language/wisp/spec.scm"
> +                         (string-append language-dir "/spec.scm"))
> +
> +              ;; compile to the destination
> +              (compile-file "wisp-scheme.scm"
> +                            #:output-file (string-append
> +                                           module-dir "/wisp-scheme.go"))
> +              (compile-file "language/wisp/spec.scm"
> +                            #:output-file (string-append
> +                                           language-dir "/spec.go"))))))))

As above, please add #t after the last call to 'compile-file'.

> +    (home-page "http://draketo.de/english/wisp";)
> +    (inputs
> +     `(("guile" ,guile-2.0)
> +       ("python" ,python)))
> +    (synopsis "wisp is a whitespace to lisp syntax for Guile")
> +    (description "wisp is a syntax for Guile which provides a Python-like
> +whitespace-significant language.  It may be easier on the eyes for some
> +users and in some situations.")
> +    (license gpl3+)))

Otherwise it looks good to me.

    Thanks!
      Mark