Subject: Re: [bug#29359] [PATCH 07/31] gnu: Add java-jansi-native.

From: Ricardo Wurmus <[email-masked]>
Date: Sun, 17 Dec 2017 23:57:02 +0100

Julien Lepiller <[email-masked]> writes:

> * gnu/packages/java.scm (java-jansi-native): New variable.

[…]

> +(define-public java-jansi-native
> +  (package
> +    (name "java-jansi-native")
> +    (version "1.7")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/fusesource/jansi-native/"
> +                                  "archive/jansi-native-" version ".tar.gz"))
> +              (sha256
> +               (base32
> +                "0j2ydlgxbzbgshqkwghbxxxnbnx1mmjgd6k5fw6xfvxw1z956yqf"))))
> +    (build-system ant-build-system)
> +    (arguments
> +     `(#:jar-name "jansi-native.jar"
> +       #:source-dir "src/main/java"
> +       #:tests? #f; no tests
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-before 'build 'build-native
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             (substitute* "src/main/native-package/src/jansi_ttyname.c"
> +               (("#include \"jansi_.*") ""))

Move this under the “with-directory-excursion” to shorten the file name.

> +             ;; there are more required files for windows in windows/
> +             (with-directory-excursion "src/main/native-package/src"
> +               (system* "gcc" "-c" "jansi_ttyname.c" "-o" "jansi_ttyname.o"
> +                        (string-append "-I" (assoc-ref inputs "java-hawtjni")
> +                                       "/include")
> +                        (string-append "-I" (assoc-ref inputs "jdk")
> +                                       "/include/linux")
> +                        "-fPIC" "-O2")
> +               (system* "gcc" "-o" "libjansi.so" "-shared" "jansi_ttyname.o"))

We shouldn’t throw away the return value.  Maybe use (and (zero? …)  …)
here?

> +             ;; FIXME: detect one of linux{32,64}, freebsd{32,64}, osx, windows{32,64}
> +             ;; This package will only work on x86_64
> +             (mkdir-p "build/classes/META-INF/native/linux64")
> +             (copy-file "src/main/native-package/src/libjansi.so"
> +                        "build/classes/META-INF/native/linux64/libjansi.so")

Is this really necessary or can we change the code to look for
libjansi.so elsewhere?  That might be easier.  Otherwise we should just
check the value of “system” and distinguish between at least i686 and
x86_64.

> +             #t))
> +         (add-after 'install 'install-native
> +           (lambda* (#:key outputs #:allow-other-keys)
> +             (mkdir-p (string-append (assoc-ref outputs "out") "/include"))
> +             (copy-file "src/main/native-package/src/jansi.h"
> +                        (string-append (assoc-ref outputs "out")
> +                                       "/include/jansi.h"))

Better use “install-file” here (without the target file name).

> +    (description "Java-jansi-native contains the native library for
> jansi.")

Please write “…for the jansi frobnication library/framework.”

--
Ricardo

GPG: BCA6 89B6 3655 3801 C3C6  2150 197A 5888 235F ACAC
https://elephly.net