[bug#31395] [PATCH 2/2] gnu: Add snap.

[Eric Bavier]

Hello Nicolas,

This package looks fun, thanks for working on it, I played with it for
a while in my browser :)

Just a few comments:

On Wed,  9 May 2018 23:46:22 +0200
Nicolas Goaziou <address@hidden> wrote:

> * gnu/packages/education.scm (snap): New variable.
> ---
>  gnu/packages/education.scm | 82 +++++++++++++++++++++++++++++++++++++-
>  1 file changed, 80 insertions(+), 2 deletions(-)
> 
[...]
> @@ -224,3 +230,75 @@ Useful support functions and an extensive progress 
> tracker, topical lessons
>  and the ability to create your own practice lessons make learning to type
>  easy.")
>      (license license:gpl2)))
> +
> +(define-public snap
> +  (package
> +    (name "snap")
> +    (version "4.1.2.7")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (string-append
> +             
> "https://github.com/jmoenig/Snap--Build-Your-Own-Blocks/archive/";
> +             version ".tar.gz"))

I think we're trying to stay away from Github's auto-generated tarballs
now, because they are not guaranteed to remain the same over time.  

Unfortunately this project doesn't seem to upload its own release
tarballs.  I would instead use a git checkout.

> +       (file-name (string-append name "-" version))
> +       (sha256
> +        (base32
> +         "0yf3x133n9ady4zr9z8cab98qfp9v69x9qjfl8ia05zxzy7d3y5z"))))
> +    (build-system trivial-build-system)
> +    (arguments
> +     `(#:modules ((guix build utils))
> +       #:builder
> +       (begin
> +         (use-modules (guix build utils))
> +         (let* ((out (assoc-ref %outputs "out"))
> +                (share (string-append out "/share/snap"))
> +                (root (string-append "Snap--Build-Your-Own-Blocks-"
> +                                     ,version)))
> +           (setenv "PATH" (string-append
> +                           (assoc-ref %build-inputs "gzip") "/bin:"
> +                           (assoc-ref %build-inputs "tar") "/bin"))
> +           (invoke "tar" "xvzf" (assoc-ref %build-inputs "source"))
> +           (mkdir-p share)
> +           (with-directory-excursion root
> +             (copy-recursively "." share))

This could be simplified to '(copy-recursively root share)', and the
mkdir-p can even be left out because copy-recursively will create it
for you.

> +           ;; Replace the sole minified file in the package.
> +           (with-directory-excursion share
> +             (delete-file "FileSaver.min.js")
> +             (symlink (string-append (assoc-ref %build-inputs "js-filesaver")
> +                                     "/share/javascript/FileSaver.min.js")
> +                      "FileSaver.min.js"))
> +           ;; Create a "snap" executable.
> +           (let* ((bin (string-append out "/bin"))
> +                  (script (string-append bin "/snap"))
> +                  (bash (string-append (assoc-ref %build-inputs "bash")
> +                                       "/bin/bash"))
> +                  (xdg-open (string-append (assoc-ref %build-inputs 
> "xdg-utils")
> +                                           "/bin/xdg-open"))
> +                  (snap (string-append share "/snap.html")))
> +             (mkdir-p bin)
> +             (call-with-output-file script
> +               (lambda (port)
> +                 (format port "#!~a\n~a '~a'" bash xdg-open snap)))

You could maybe use '(which "sh")' and '(which xdg-open)'.  A
matter of taste, I think.

> +             (chmod script #o555)))
> +         #t)))
> +    (native-inputs
> +     `(("gzip" ,gzip)
> +       ("tar" ,tar)
> +       ("js-filesaver" ,js-filesaver)))

js-filesave should go in "inputs", right?

> +    (inputs
> +     `(("bash" ,bash-minimal)
> +       ("xdg-utils" ,xdg-utils)))
> +    (home-page "https://snap.berkeley.edu";)
> +    (synopsis "Visual, blocks based programming language inspired by 
> Scratch")

We can leave out "inspired by Scratch" in the synopsis.

> +    (description "Snap! (formerly BYOB) is a visual, drag-and-drop
> +programming language.  It is an extended reimplementation of Scratch (a
> +project of the Lifelong Kindergarten Group at the MIT Media Lab) that
> +allows you to Build Your Own Blocks.  It also features first class
> +lists, first class procedures, and continuations.  These added
> +capabilities make it suitable for a serious introduction to computer
> +science for high school or college students.
> +
> +This package provides a @command{snap} executable calling @command{xdg-open}
> +to open a the application in a web browser.")
           ^
s/a the/the/

> +    (license license:agpl3+)))

Otherwise looks good and works for me.

Thanks,
`~Eric

pgpC8CHGHjpn0.pgp
Description: OpenPGP digital signature