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.
>