[bug#54021] [PATCH] Add rhino javascript package

[Frank Pursel]

Thanks for the careful review.  The $@ was intended to allow the introduction of additional command line arguments but since it starts a repl and might be a source of nefariousness I've just removed it.  I did not understand the bash-minimal comment.  Are you saying that (search-input-file inputs "/bin/bash") will not work in the way I intended?  I've changed all the inputs to native-inputs now. 

I'll get back to you after I do some more testing.

Thank you for your helpful/thoughtful comments.  

Regards,

Frank

On Wed, Feb 16, 2022 at 5:21 PM Maxime Devos <maximedevos@telenet.be> wrote:

Frank Pursel schreef op di 15-02-2022 om 17:58 [-0800]:
> +(define-public rhino
> +  (let* ((rel-ver "1.7.7.2") (rel-git-tag "Rhino1_7_7_2_Release")
> +         (git-commitv "935942527ff434b205e797df4185518e5369466e")
> +         (git-short-commit (substring git-commitv 0 6))
> +         (hash "09i4yr98hs6855fs7fhgmrpiwpr90lhxdv2bvfj97nn4rv1d7wl8"))

This is quite a bit more complicated than necessary ...

 * 'rel-git-tag' is unused.
 * Conventionally, a let-bound commit string has as name 'commit', not
   'git-commitv'.  (guix upstream) expects 'commit', not 'git-commitv',
   and will fail at auto-updating if a different name is used.
 * (see later)

> +    (package
> +      (name "rhino")
> +      (version git-short-commit)

'git-short-commit' is a (shortened) commit string, not a version
number.  This needs to be (version "1.7.7.2") instead.

> +      (source (origin
> +                (method git-fetch)
> +                (uri (git-reference
> +                      (url "https://github.com/mozilla/rhino.git")
> +                      (commit git-short-commit)))

There is no need to shorten the commit string, you can use the full
"git-commitv" here.

> +                (file-name (git-file-name name git-short-commit))

We have a version number, so you can do (git-file-name name version)
here.

> +                (sha256
> +                 (base32
> +                  hash))))

The hash is only used in one place, so you can write it here directly.
This has as benefit that (guix packages) can do some checks on the hash
at compile time.

[...]

> +      (arguments
> +       `(#:phases (modify-phases %standard-phases
> +                    (add-after 'unpack 'clean-jars
> +                      (lambda _
> +                        (for-each (lambda (jarf)
> +                                    (delete-file jarf)
> +                                    (format #t "Deleted: ~s
> +" jarf))
> +                                  (find-files "." ".*\\.jar$")) #t))

Cleaning the source code of binaries, making sure that the source code
actually consists of source code, seems more something for origin
snippets.  It's not really explicitly mentioned anywhere I think,
but the following from ‘(guix)Snipepts versus Phases’ seems close:

The boundary between using an origin snippet versus a build phase to
modify the sources of a package can be elusive.  Origin snippets are
typically used to remove unwanted files such as bundled libraries,
nonfree sources, or to apply simple substitutions.  [...]

> +                    (replace 'install
> +                      (lambda* (#:key inputs outputs #:allow-other-keys)
> +                        (let* ((out (assoc-ref outputs "out"))
> +                               (pkg+ver (string-append ,name ,rel-ver))

You can refer to the 'version' field of the package here:

  (pkg+ver (string-append ,name ,version))

> +                               (bin (string-append out "/bin"))
> +                               (rhino (string-append bin "/rhino")))
> +                          (mkdir-p bin)
> +                          (install-file (string-append "build/" pkg+ver
> +                                                       "/js.jar")
> +                                        (string-append out "/share/java"))
> +                          (with-output-to-file rhino
> +                            (lambda _
> +                              (format #t "#!~a
> +~a -jar ~a $@
> +"
> +                                      (search-input-file inputs "/bin/bash")

'bash-minimal' is missing from 'inputs'.  Not including it only works
when compiling natively.

> +                                      (which "java")
> +                                      (string-append out "/share/java/js.jar"))))


'format' has a port argument, so you could do

(call-with-output-file rhino
  (lambda (port)
    (format port "#!~a ~a -jar -a $@"
            (search-input-file [...]) [...])))

Also, what's this "$@"?

Greetings,
Maxime.