Nicolò Balzarotti <anothersms@gmail.com> writes:

> seems to be ok (applied fine).  A few notes about the patch
>
> [...]
>>          `(list ;; Taken from NixOS. Only way I could get libLLVM-6.0.so
>> -           "-DCMAKE_BUILD_TYPE=Release"
>> +          "-DCMAKE_BUILD_TYPE=Release"
>> +
>> +          ;; Build a native compiler and the NVPTX backend (NVIDIA) since
>> +          ;; Julia insists on it, nothing more.  This reduces build times and
>> +          ;; disk usage.
>> +          ,(string-append "-DLLVM_TARGETS_TO_BUILD=" (system->llvm-target))
>> +          "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=NVPTX"
>>  
>> -           ;; Build a native compiler and the NVPTX backend (NVIDIA) since
>> -           ;; Julia insists on it, nothing more.  This reduces build times and
>> -           ;; disk usage.
>> -           ,(string-append "-DLLVM_TARGETS_TO_BUILD=" (system->llvm-target))
>> -           "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=NVPTX"
> Here it's difficult to see if there are changes, you should undo the
> indent edit.  If indentation is wrong, it should be fixed in a separate commit.
>
>> +          ;; "-DLLVM_HOST_TRIPLE=${stdenv.hostPlatform.config}"
>> +          ;; "-DLLVM_DEFAULT_TARGET_TRIPLE=${stdenv.hostPlatform.config}"
>> +          ;; "-DLLVM_EXPERIMENTAL_TARGETS_TO_BUILD=WebAssembly"
> Those seems to be taken from nixos, but is there a reason why are those
> commented?
>
>> +                        (invoke "patch" flag "-i" (string-append
>> "julia-src/deps/patches/" patchname ".patch")))))
> Line too long
>
>> +                 (mkdir-p "julia-src")
>> +                 (invoke "tar" "xf" (assoc-ref inputs "julia-source")
>> "-C" "julia-src" "--strip-components=1")
> DITTO
>
>> +                 (map (lambda (patchname)
>> +                        (patch patchname "-p1")) (list
>> "llvm-D27629-AArch64-large_model_6.0.1"
> Here we can slpit the line before (list
>
>>  
>> -           "-DLLVM_LINK_LLVM_DYLIB=ON"))))))
>> +                 #t)))))))
> Returning #t should not be needed anymore
>
>> +    (inputs
>> +     `(("julia-source" ,(package-source julia))
>> +       ,@(package-inputs llvm-11)))))
>>  
>> -                                         "suitesparse"))
>> +                                         "suitesparse" "libfortran"))
>>                                    ":"))
>>               #t))
> We can remove this #t now
>
>> +                      (string-append "stdlib/" pkgname "_jll/src/"
>> pkgname "_jll.jl")))
> Long line
>> +           (lambda* (#:key inputs #:allow-other-keys)
>> +             ;; some tests execute julia in an environment that needs
>> to propagate GUIX_LOCPATH
> Good catch, I was wondering which ENV variable was needed
>
>> +             (substitute* "test/cmdlineargs.jl"
>> +               (("\"HOME\"\\s=>\\shomedir\\(\\)") "\"HOME\" =>
>> homedir(), \"GUIX_LOCPATH\" => ENV[\"GUIX_LOCPATH\"]"))
>>               #t))
> again, long line and #t
>> +         (add-before 'install 'symlink-libraries ;; FIXME change
>> build to install
> What does this comment refer to?
>>          (string-append "prefix=" (assoc-ref %outputs "out"))
>>  
>> -         ;; Passing the MARCH flag is necessary to build binary substitutes for
>> -         ;; the supported architectures.
>> -         ,(match (or (%current-target-system)
>> -                     (%current-system))
>> -                 ("x86_64-linux" "MARCH=x86-64")
>> -                 ("i686-linux" "MARCH=pentium4")
>> -                 ("aarch64-linux" "MARCH=armv8-a")
>> -                 ;; Prevent errors when querying this package on unsupported
>> -                 ;; platforms, e.g. when running "guix package --search="
>> -                 (_ "MARCH=UNSUPPORTED"))
>> +        ;; Passing the MARCH flag is necessary to build binary substitutes for
>> +        ;; the supported architectures.
>> +        ,(match (or (%current-target-system)
>> +                    (%current-system))
>> +           ("x86_64-linux" "MARCH=x86-64")
>> +           ("i686-linux" "MARCH=pentium4")
>> +           ("aarch64-linux" "MARCH=armv8-a")
>> +           ;; Prevent errors when querying this package on unsupported
>> +           ;; platforms, e.g. when running "guix package --search="
>> +           (_ "MARCH=UNSUPPORTED"))
>>
> Again the indentation stuff
>
>