[bug#48325] [PATCH v3b 4/5] julia: Update to 1.6.1: where apply upstream patches?

[zimoun]

Hi Jean-Baptiste,

Two comments here:

On Sun, 13 Jun 2021 at 20:54, Jean-Baptiste Volatier via Guix-patches via 
<guix-patches@gnu.org> wrote:

> Subject: [PATCH] DRAFT gnu: julia: Update to 1.6.1.

Usually, to ease the review, we increment the reroll count and send all
the patch set.  Otherwise, one would have to read all the thread to find
which patch is the last.

Could you send the v4 containing the 5 patches IIRC?

> * gnu/packages/julia.scm (julia): Update to 1.6.1.
> [arguments]: XXXX
> [inputs]: Add nghttp2:lib, Add gfortran:lib, Remove arpack-ng.
> [native-inputs]: Add nss-certs.
>
> Co-Authored-By: Nicolò Balzarotti <nicolo@nixo.xyz>.
> Co-Authored-By: zimoun <zimon.toutoune@gmail.com>.
> ---
>  gnu/packages/julia.scm | 353 ++++++++++++++++++++++++++++-------------
>  1 file changed, 239 insertions(+), 114 deletions(-)
>
> diff --git a/gnu/packages/julia.scm b/gnu/packages/julia.scm
> index 13c9f7baf1..9903bb009b 100644
> --- a/gnu/packages/julia.scm
> +++ b/gnu/packages/julia.scm

[...]

>  (define llvm-julia
>    (package
> -    (inherit llvm-9)
> +    (inherit llvm-11)
>      (name "llvm-julia")
> -    (source (origin
> -              (inherit (package-source llvm-9))
> -              ;; Those patches are inside the Julia source repo.
> -              ;; They are _not_ Julia specific 
> (https://github.com/julialang/julia#llvm)
> -              ;; but they are required to build Julia.
> -              ;; Discussion: 
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=919628
> -              (patches
> -               (map (match-lambda
> -                      ((name hash)
> -                       (julia-patch name hash)))
> -                    (list
> -                     '("llvm-D27629-AArch64-large_model_6.0.1"
> -                       
> "1qrshmlqvnasdyc158vfn3hnbigqph3lsq7acb9w8lwkpnnm2j4z")

[...]

> -                     '("llvm-9.0-D85499"
> -                       
> "0vxlr35srvbvihlgrxq15v6dylp90vgi0qahj22j01jgqmdasjkm"))))
> -              (patch-flags '("-p1"))))
>      (arguments
> -     (substitute-keyword-arguments (package-arguments llvm-9)
> +     (substitute-keyword-arguments (package-arguments llvm-11)
>         ((#:configure-flags flags)
>          `(list ;; Taken from NixOS. Only way I could get libLLVM-6.0.so
>             "-DCMAKE_BUILD_TYPE=Release"
> @@ -177,7 +141,61 @@
>             ;; "-DLLVM_DEFAULT_TARGET_TRIPLE=${stdenv.hostPlatform.config}"
>             ;; "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=WebAssembly"
>             "-DLLVM_ENABLE_DUMP=ON"
> -           "-DLLVM_LINK_LLVM_DYLIB=ON"))))))
> +           "-DLLVM_LINK_LLVM_DYLIB=ON"))
> +       ((#:phases phases)
> +        `(modify-phases ,phases
> +           ;; applying patches from julia
> +           ;; list of patches can be found in deps/llvm.mk in julia source
> +           (add-after 'unpack 'julia-patches
> +             (lambda* (#:key inputs outputs #:allow-other-keys)
> +               (let ((patch
> +                      (lambda (patchname flag)
> +                        (invoke "patch" flag "-i"
> +                                (string-append
> +                                 "julia-src/deps/patches/"
> +                                 patchname
> +                                 ".patch")))))
> +                 (mkdir-p "julia-src")
> +                 (invoke "tar" "xf"
> +                         (assoc-ref inputs "julia-source")
> +                         "-C" "julia-src" "--strip-components=1")
> +                 (map (lambda (patchname)
> +                        (patch patchname "-p1"))
> +                      (list "llvm-D27629-AArch64-large_model_6.0.1"

[...]

> +                            "llvm-11-D96283-dagcombine-half"))
> +                 (map (lambda (patchname)
> +                        (patch patchname "-p2"))
> +                      (list "llvm-11-AArch64-FastIsel-bug"
> +                            "llvm-11-D97435-AArch64-movaddrreg"
> +                            "llvm-11-D97571-AArch64-loh"
> +                            "llvm-11-aarch64-addrspace")))))))))
> +    (inputs
> +     `(("julia-source" ,(package-source julia))
> +       ,@(package-inputs llvm-11)))))

This modification of patch location should appear in the commit message.

As I mentioned earlier, I am not convinced it is the right place [1].  I get
your point [2] that some are applied with the option “-p1” and other
with “-p2”.  Ludo, any recommendation?

1: <http://issues.guix.gnu.org/issue/48325#3>
2: <http://issues.guix.gnu.org/issue/48325#8>


All the best,
simon

PS:
I will be offline until the end of next week (~June, 25th).