[bug#43080] [PATCH] gnu: add j version 902.

[Ludovic Courtès]

Hi Joseph,

Did you have a chance to look into it?

Thanks,
Ludo’.

Ludovic Courtès <ludo@gnu.org> skribis:

> Hi Joseph,
>
> Joseph Novakovich <josephnovakovich@gmail.com> skribis:
>
>> From 798d0e3c2b635cfe3a936937fd3dd3ea1b8c5812 Mon Sep 17 00:00:00 2001
>> From: Joseph Novakovich <josephnovakovich@gmail.com>
>> Date: Thu, 27 Aug 2020 19:16:16 -0400
>> Subject: [PATCH] gnu: add j version 902.
>>
>> ---
>>  gnu/packages/j.scm | 180 +++++++++++++++++++++++++++++++++++++++++++++
>
> Sorry for the loooong delay!
>
> A few comments—if needed we can make those changes on your behalf, let
> us know.
>
> Could you add j.scm to ‘gnu/local.mk’?
>
>> +    (name "j")
>> +    (version "902")
>> +    (source
>> +     (origin
>> +       (method git-fetch)
>> +       (uri
>> +        (git-reference
>> +         (url "https://github.com/jsoftware/jsource";)
>> +         (commit "59324abbc6c9c3709d39096f5a41e0a4ef28e9f6")))
>
> Why this commit?  If there’s a tag, it would be clearer to refer to the
> tag (the ‘commit’ field can name a tag).
>
>> +    (outputs '("out"))
>
> Unnecessary, you can remove it.
>
>> +     `(#:modules
>> +       ((guix build gnu-build-system)
>> +        (guix build utils))
>
> Likewise.
>
>> +           (lambda* (#:key inputs outputs #:allow-other-keys)
>> +             (let ((jplatform "linux")
>> +                   (j64x "j64avx2"))
>> +               (chdir "make2")
>> +               (system
>> +                (format #f
>> +                        "jplatform=~a j64x=~a USE_SLEEF=1 
>> ./build_jconsole.sh"
>> +                        jplatform j64x))
>
> Note that we cannot assume AVX2 support (on x86_64, but especially on
> non-Intel platforms).  Can you check this doesn’t create binaries that
> require AVX2?
>
> Also, it would be more robust to replace ‘system’ by something like:
>
>   (Seton "jplatform" "linux")
>   (setenv "USE_SLEEF" "1")
>   …
>   (invoke "./build_jconsole.sh")
>
> since ‘invoke’ checks the exit code of the given command.
>
>> +           (lambda* (#:key inputs outputs #:allow-other-keys)
>> +             (let* ((bin-out (string-append (assoc-ref %outputs "out") 
>> "/bin"))
>> +                    (share-out (string-append (assoc-ref %outputs "out")
>> +                                              "/share/j"))
>> +                    (jconsole "jlibrary/bin/jconsole")
>> +                    (libj.so  "jlibrary/bin/libj.so"))
>
> I’d omit ‘-out’ from variable names.
>
>> +    (synopsis "APL Dialect")
>
> What about “Dialect of the APL programming language” for clarity?
>
>> +    (description "J is a programming language that works with arrays,
>> +verbs, adverbs, and conjunctions.  For example, +/x sums array x and
>> +/:~x sorts it.")
>
> Please use @code markup for code snippets.  Bonus points if you can
> expound a bit!
>
>   https://guix.gnu.org/manual/en/html_node/Synopses-and-Descriptions.html
>
>> +    (license gpl3)))
>
> I confirm that this is “version 3 only” per ‘license.txt’.
>
> Could you send an updated patch?
>
> Thanks,
> Ludo’.