[bug#52117] [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[bug#52117] [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[zimoun]

Hi,

With this series, all the Julia packages builds again.  The only one still
missing is 'julia-sundials-jll’.  It probably requires to introduce something
to julia-build-system but I have not investigated much.


Cheers,
simon


zimoun (6):
  gnu: julia: Test correctly '#:parallel-tests?'.
  build: julia-build-system: Correctly disable parallel tests.
  gnu: julia-aqua: Disable parallel tests.
  gnu: julia-interpolations: Disable parallel tests.
  gnu: julia-requires: Disable parallel tests.
  gnu: julia-unitful: Disable parallel tests.

 gnu/packages/julia-xyz.scm        | 8 ++++++++
 gnu/packages/julia.scm            | 9 ++++-----
 guix/build/julia-build-system.scm | 8 +++++---
 3 files changed, 17 insertions(+), 8 deletions(-)


base-commit: 138498feec335f68935f00f8a97924c90c7f59b0
-- 
2.32.0

[bug#52117] [PATCH 1/6] gnu: julia: Test correctly '#:parallel-tests?'.

[zimoun]

* gnu/packages/julia.scm (julia)[arguments]<#:phases>: Fix parallel-test.
---
 gnu/packages/julia.scm | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/gnu/packages/julia.scm b/gnu/packages/julia.scm
index ac5bf7db25..c123711da5 100644
--- a/gnu/packages/julia.scm
+++ b/gnu/packages/julia.scm
@@ -315,11 +315,10 @@ (define-public julia
                (substitute* (jlpath "Zlib")
                  (((from "libz")) (to "zlib" "libz"))))))
          (add-after 'unpack 'enable-parallel-tests
-           ;; FIXME: julia fails at networking in the container and falls back
-           ;; to a single worker, which causes the tests to not run in
-           ;; parallel (see: https://github.com/JuliaLang/julia/issues/43205).
-           (lambda* (#:key parallel-build? #:allow-other-keys)
-             (setenv "JULIA_CPU_THREADS" (if parallel-build?
+           ;; Parallel tests require 'julia-allow-parallel-build.patch'.
+           ;; https://github.com/JuliaLang/julia/issues/43205.
+           (lambda* (#:key parallel-tests? #:allow-other-keys)
+             (setenv "JULIA_CPU_THREADS" (if parallel-tests?
                                              (number->string (parallel-job-count))
                                              "1"))
              (format #t "JULIA_CPU_THREADS environment variable set to ~a~%"
-- 
2.32.0

[bug#52117] [PATCH 2/6] build: julia-build-system: Correctly disable parallel tests.

[zimoun]

Even providing '--procs=1' launches 2 workers which breaks some testsuite of
packages; therefore set '#:parallel-tests?' to '#false' was ineffective.

* guix/build/julia-build-system.scm (check): Fix unexpected behaviour from
'julia' command line option.
---
 guix/build/julia-build-system.scm | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/guix/build/julia-build-system.scm b/guix/build/julia-build-system.scm
index f0dc419c17..af478fd4a3 100644
--- a/guix/build/julia-build-system.scm
+++ b/guix/build/julia-build-system.scm
@@ -112,7 +112,10 @@ (define* (check #:key tests? source inputs outputs julia-package-name
            (builddir (string-append out "/share/julia/"))
            (jobs (if parallel-tests?
                      (number->string (parallel-job-count))
-                     "1")))
+                     "1"))
+           (nprocs (if parallel-tests?
+                       (string-append "--procs=" jobs)
+                       "")))
       ;; With a patch, SOURCE_DATE_EPOCH is honored
       (setenv "SOURCE_DATE_EPOCH" "1")
       (setenv "JULIA_DEPOT_PATH" builddir)
@@ -122,8 +125,7 @@ (define* (check #:key tests? source inputs outputs julia-package-name
                                  "")))
       (setenv "JULIA_CPU_THREADS" jobs)
       (setenv "HOME" "/tmp")
-      (invoke "julia" "--depwarn=yes"
-              (string-append "--procs=" jobs)
+      (invoke "julia" "--depwarn=yes" nprocs
               (string-append builddir "loadpath/"
                              package "/test/runtests.jl"))))
   #t)
-- 
2.32.0

[bug#52117] [PATCH 3/6] gnu: julia-aqua: Disable parallel tests.

[zimoun]

* gnu/packages/julia-xyz.scm (julia-aqua)[arguments]: Disable parallel tests.
---
 gnu/packages/julia-xyz.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/julia-xyz.scm b/gnu/packages/julia-xyz.scm
index 547d2bf81e..3ca32097e9 100644
--- a/gnu/packages/julia-xyz.scm
+++ b/gnu/packages/julia-xyz.scm
@@ -136,6 +136,8 @@ (define-public julia-aqua
         (sha256
          (base32 "1g0kyzcdykgs247j72jpc2qqall696jwgb3hnn4cxmbi8bkf7wpk"))))
     (build-system julia-build-system)
+    (arguments
+     `(#:parallel-tests? #f))
     (home-page "https://github.com/JuliaTesting/Aqua.jl")
     (synopsis "Automated quality assurance for Julia packages")
     (description "@acronym{Aqua.jl, Auto QUality Assurance for Julia packages},
-- 
2.32.0

[bug#52117] [PATCH 4/6] gnu: julia-interpolations: Disable parallel tests.

[zimoun]

* gnu/packages/julia-xyz.scm (julia-interpolations)[arguments]: Disable
parallel tests.
---
 gnu/packages/julia-xyz.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/julia-xyz.scm b/gnu/packages/julia-xyz.scm
index 3ca32097e9..f68cd1647b 100644
--- a/gnu/packages/julia-xyz.scm
+++ b/gnu/packages/julia-xyz.scm
@@ -2518,6 +2518,8 @@ (define-public julia-interpolations
         (sha256
          (base32 "1236c20k388qlh7k74mhf7hkbn0vf7ss8b1rgh1a6aj0234ayfnc"))))
     (build-system julia-build-system)
+    (arguments
+     `(#:parallel-tests? #f))
     (propagated-inputs
      `(("julia-axisalgorithms" ,julia-axisalgorithms)
        ("julia-offsetarrays" ,julia-offsetarrays)
-- 
2.32.0

[bug#52117] [PATCH 5/6] gnu: julia-requires: Disable parallel tests.

[zimoun]

* gnu/packages/julia-xyz.scm (julia-requires)[arguments]: Disable parallel
tests.
---
 gnu/packages/julia-xyz.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/julia-xyz.scm b/gnu/packages/julia-xyz.scm
index f68cd1647b..6ddf1429ca 100644
--- a/gnu/packages/julia-xyz.scm
+++ b/gnu/packages/julia-xyz.scm
@@ -3966,6 +3966,8 @@ (define-public julia-requires
        (sha256
         (base32 "03hyfy7c0ma45b0y756j76awi3az2ii4bz4s8cxm3xw9yy1z7b01"))))
     (build-system julia-build-system)
+    (arguments
+     `(#:parallel-tests? #f))
     (inputs                             ;required for test
      `(("julia-example" ,julia-example)))
     (propagated-inputs
-- 
2.32.0

[bug#52117] [PATCH 6/6] gnu: julia-unitful: Disable parallel tests.

[zimoun]

* gnu/packages/julia-xyz.scm (julia-unitful)[arguments]: Disable parallel
tests.
---
 gnu/packages/julia-xyz.scm | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/gnu/packages/julia-xyz.scm b/gnu/packages/julia-xyz.scm
index 6ddf1429ca..d2e57aeadc 100644
--- a/gnu/packages/julia-xyz.scm
+++ b/gnu/packages/julia-xyz.scm
@@ -4854,6 +4854,8 @@ (define-public julia-unitful
        (sha256
         (base32 "10qwscd15dnmvx116dwvg99m7kmwgmj5ahdkq7psiq48lcc554gq"))))
     (build-system julia-build-system)
+    (arguments
+     `(#:parallel-tests? #f))
     (propagated-inputs
      `(("julia-constructionbase" ,julia-constructionbase)))
     (home-page "https://painterqubits.github.io/Unitful.jl/stable/")
-- 
2.32.0

[bug#52117] [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[Maxim Cournoyer]

Hello Simon!

zimoun <zimon.toutoune@gmail.com> writes:

> * gnu/packages/julia.scm (julia)[arguments]<#:phases>: Fix parallel-test.
> ---
>  gnu/packages/julia.scm | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/gnu/packages/julia.scm b/gnu/packages/julia.scm
> index ac5bf7db25..c123711da5 100644
> --- a/gnu/packages/julia.scm
> +++ b/gnu/packages/julia.scm
> @@ -315,11 +315,10 @@ (define-public julia
>                 (substitute* (jlpath "Zlib")
>                   (((from "libz")) (to "zlib" "libz"))))))
>           (add-after 'unpack 'enable-parallel-tests
> -           ;; FIXME: julia fails at networking in the container and falls back
> -           ;; to a single worker, which causes the tests to not run in
> -           ;; parallel (see: https://github.com/JuliaLang/julia/issues/43205).
> -           (lambda* (#:key parallel-build? #:allow-other-keys)
> -             (setenv "JULIA_CPU_THREADS" (if parallel-build?
> +           ;; Parallel tests require 'julia-allow-parallel-build.patch'.
> +           ;; https://github.com/JuliaLang/julia/issues/43205.
> +           (lambda* (#:key parallel-tests? #:allow-other-keys)
> +             (setenv "JULIA_CPU_THREADS" (if parallel-tests?
>                                               (number->string (parallel-job-count))
>                                               "1"))
>               (format #t "JULIA_CPU_THREADS environment variable set to ~a~%"

I've moved the comment where I thought it made more sense, at the top of
the patch itself, like so:

--8<---------------cut here---------------start------------->8---
modified   gnu/packages/julia.scm
@@ -315,8 +315,6 @@ (define-public julia
                (substitute* (jlpath "Zlib")
                  (((from "libz")) (to "zlib" "libz"))))))
          (add-after 'unpack 'enable-parallel-tests
-           ;; Parallel tests require 'julia-allow-parallel-build.patch'.
-           ;; https://github.com/JuliaLang/julia/issues/43205.
            (lambda* (#:key parallel-tests? #:allow-other-keys)
              (setenv "JULIA_CPU_THREADS" (if parallel-tests?
                                              (number->string (parallel-job-count))
modified   gnu/packages/patches/julia-allow-parallel-build.patch
@@ -1,3 +1,8 @@
+Allow parallel tests with isolated environment.
+
+See https://github.com/JuliaLang/julia/issues/43205 and
+https://github.com/JuliaLang/julia/pull/43211.
+
 diff --git a/test/runtests.jl b/test/runtests.jl
 index 2f9cd058bb..150395e78c 100644
 --- a/test/runtests.jl
@@ -11,14 +16,11 @@ index 2f9cd058bb..150395e78c 100644
  using Base: Experimental
  
  include("choosetests.jl")
-@@ -83,11 +83,15 @@ prepend!(tests, linalg_tests)
+@@ -83,11 +83,12 @@ prepend!(tests, linalg_tests)
  import LinearAlgebra
  cd(@__DIR__) do
      n = 1
 -    if net_on
-+    # Allow parallel tests with isolated environment
-+    # https://github.com/JuliaLang/julia/issues/43205
-+    # https://github.com/JuliaLang/julia/pull/43211
 +    if net_on || haskey(ENV, "JULIA_CPU_THREADS")
          n = min(Sys.CPU_THREADS, length(tests))
          n > 1 && addprocs_with_testenv(n)
--8<---------------cut here---------------end--------------->8---

Maxim

[bug#52117] [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[Maxim Cournoyer]

Hello Simon!

zimoun <zimon.toutoune@gmail.com> writes:

> Even providing '--procs=1' launches 2 workers which breaks some testsuite of
                                                             ^ the 
> packages; therefore set '#:parallel-tests?' to '#false' was ineffective.
 ^ some               ^ setting

It's good to put the rationale here, as you did.

> * guix/build/julia-build-system.scm (check): Fix unexpected behaviour from
> 'julia' command line option.

But in the changelog message I'd expect to see foremost *what* it does
rather than a reformulation of *why* it does it :-).  E.g., something
like: do not pass the '--procs' argument when not running the tests in
parallel.

> ---
>  guix/build/julia-build-system.scm | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/guix/build/julia-build-system.scm b/guix/build/julia-build-system.scm
> index f0dc419c17..af478fd4a3 100644
> --- a/guix/build/julia-build-system.scm
> +++ b/guix/build/julia-build-system.scm
> @@ -112,7 +112,10 @@ (define* (check #:key tests? source inputs outputs julia-package-name
>             (builddir (string-append out "/share/julia/"))
>             (jobs (if parallel-tests?
>                       (number->string (parallel-job-count))
> -                     "1")))
> +                     "1"))
> +           (nprocs (if parallel-tests?
> +                       (string-append "--procs=" jobs)
> +                       "")))
>        ;; With a patch, SOURCE_DATE_EPOCH is honored
>        (setenv "SOURCE_DATE_EPOCH" "1")
>        (setenv "JULIA_DEPOT_PATH" builddir)
> @@ -122,8 +125,7 @@ (define* (check #:key tests? source inputs outputs julia-package-name
>                                   "")))
>        (setenv "JULIA_CPU_THREADS" jobs)
>        (setenv "HOME" "/tmp")
> -      (invoke "julia" "--depwarn=yes"
> -              (string-append "--procs=" jobs)
> +      (invoke "julia" "--depwarn=yes" nprocs

Here nprocs can be ""; is it really OK to pass an empty string argument
to julia?

>                (string-append builddir "loadpath/"
>                               package "/test/runtests.jl"))))
>    #t)

Trailing '#t' are no longer required.  Actually, looking at the output
of julia --help:

 -p, --procs {N|auto}      Integer value N launches N *additional* local worker processes

The key is 'additional' :-).  So to disable parallel processing it needs
to be 0.

I've modified it like so:

--8<---------------cut here---------------start------------->8---
(define* (check #:key tests? source inputs outputs julia-package-name
                parallel-tests? #:allow-other-keys)
  (when tests?
    (let* ((out (assoc-ref outputs "out"))
           (package (or julia-package-name (project.toml->name "Project.toml")))
           (builddir (string-append out "/share/julia/"))
           (job-count (if parallel-tests?
                          (parallel-job-count)
                          1))
           ;; The --proc argument of Julia *adds* extra processors rather than
           ;; specify the exact count to use, so zero must be specified to
           ;; disable parallel processing.
           (additional-procs (max 0 (1- job-count))))
      ;; With a patch, SOURCE_DATE_EPOCH is honored
      (setenv "SOURCE_DATE_EPOCH" "1")
      (setenv "JULIA_DEPOT_PATH" builddir)
      (setenv "JULIA_LOAD_PATH"
              (string-append builddir "loadpath/" ":"
                             (or (getenv "JULIA_LOAD_PATH")
                                 "")))
      (setenv "JULIA_CPU_THREADS" (number->string job-count))
      (setenv "HOME" "/tmp")
      (invoke "julia" "--depwarn=yes"
              "--procs" (number->string additional-procs)
              (string-append builddir "loadpath/"
                             package "/test/runtests.jl")))))
--8<---------------cut here---------------start------------->8---

And took the liberty to remove trailing #f in other phases.

Thank you!

Maxim

bug#52117: [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[Maxim Cournoyer]

Hello Simon,

zimoun <zimon.toutoune@gmail.com> writes:

> Hi,
>
> With this series, all the Julia packages builds again.  The only one still
> missing is 'julia-sundials-jll’.  It probably requires to introduce something
> to julia-build-system but I have not investigated much.
>
>
> Cheers,
> simon
>
>
> zimoun (6):
>   gnu: julia: Test correctly '#:parallel-tests?'.
>   build: julia-build-system: Correctly disable parallel tests.
>   gnu: julia-aqua: Disable parallel tests.
>   gnu: julia-interpolations: Disable parallel tests.
>   gnu: julia-requires: Disable parallel tests.
>   gnu: julia-unitful: Disable parallel tests.
>
>  gnu/packages/julia-xyz.scm        | 8 ++++++++
>  gnu/packages/julia.scm            | 9 ++++-----
>  guix/build/julia-build-system.scm | 8 +++++---
>  3 files changed, 17 insertions(+), 8 deletions(-)
>
>
> base-commit: 138498feec335f68935f00f8a97924c90c7f59b0

Now pushed to core-updates-frozen; I've made some changes to the
build-system one after finding out that --procs meant "adding" cores,
rather than specifying their exact count (and you cannot add 0!).

The parallel build of Julia packages uses a ton of RAM, but it's fast!
:-).

Thanks a lot,

Maxim

[bug#52117] [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[zimoun]

Hi Maxim,

Thanks for the review and the improved patch.

I am sorry if the commit message and/or changelog I provided was badly
worded, but somehow it was an attempt to explain the odd behaviour – at
least counter-intuitive since I initially felt into when sending the
very first patch allowing parallel tests and you felt too, I guess. :-)


On Fri, 26 Nov 2021 at 22:17, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

>> ---
>>  guix/build/julia-build-system.scm | 8 +++++---
>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/guix/build/julia-build-system.scm b/guix/build/julia-build-system.scm
>> index f0dc419c17..af478fd4a3 100644
>> --- a/guix/build/julia-build-system.scm
>> +++ b/guix/build/julia-build-system.scm
>> @@ -112,7 +112,10 @@ (define* (check #:key tests? source inputs outputs julia-package-name
>>             (builddir (string-append out "/share/julia/"))
>>             (jobs (if parallel-tests?
>>                       (number->string (parallel-job-count))
>> -                     "1")))
>> +                     "1"))
>> +           (nprocs (if parallel-tests?
>> +                       (string-append "--procs=" jobs)
>> +                       "")))
>>        ;; With a patch, SOURCE_DATE_EPOCH is honored
>>        (setenv "SOURCE_DATE_EPOCH" "1")
>>        (setenv "JULIA_DEPOT_PATH" builddir)
>> @@ -122,8 +125,7 @@ (define* (check #:key tests? source inputs outputs julia-package-name
>>                                   "")))
>>        (setenv "JULIA_CPU_THREADS" jobs)
>>        (setenv "HOME" "/tmp")
>> -      (invoke "julia" "--depwarn=yes"
>> -              (string-append "--procs=" jobs)
>> +      (invoke "julia" "--depwarn=yes" nprocs
>
> Here nprocs can be ""; is it really OK to pass an empty string argument
> to julia?

Yes it is OK.  When #:parallel-tests? sets to #f, my patch leads to the
call “julia --depwarn=yes” which is valid.  Your modified patch adds
another test but leads to the same call “julia --depwarn=yes”.

--8<---------------cut here---------------start------------->8---
+           (job-count (if parallel-tests?
+                          (parallel-job-count)
+                          1))
+           ;; The --proc argument of Julia *adds* extra processors rather than
+           ;; specify the exact count to use, so zero must be specified to
+           ;; disable parallel processing...

[..]

+      (apply invoke "julia"
+             `("--depwarn=yes"
+               ,@(if parallel-tests?
+                     ;; XXX: ... but '--procs' doesn't accept 0 as a valid
+                     ;; value, so just omit the argument entirely.
+                     (list (string-append  "--procs="
+                                           (number->string additional-procs)))
+                     '())
--8<---------------cut here---------------end--------------->8---

So because of 2 tests. I think your modified patch is more
“complicated”. :-)


About this,

--8<---------------cut here---------------start------------->8---
+           (additional-procs (max 0 (1- job-count))))
--8<---------------cut here---------------end--------------->8---

I considered that it was not a big deal; initially, I did something
similar in ’let’ but remove it because it changes nothing from my
experiments.  In fact, because ’--procs’ overrides JULIA_CPU_THREADS and
run Julia with n or n+1 is more or less the same for the Julia land,
IMHO.  Well, it is not clear what is the load for the main worker. And
JULIA_CPU_THREADS=1 is required for running using only one worker.
Anyway, this changes nothing, practically speaking. :-) Indeed, it is
better and more consistent.


Last, I am confused by Cuirass.  Because it says evaluation complete but
julia-* packages are scheduled.

    https://ci.guix.gnu.org/eval/48802?status=pending

And for instance,

    https://ci.guix.gnu.org/build/1853818/log/raw

BTW, Berlin has some issues I guess

#48720 - d508c5b was pushed CommitDate: Fri Nov 26 23:21:45 2021 +0100
       - 941f776 was pushed CommitDate: Sat Nov 27 01:22:32 2021 -0500
       - 9c4752b was pushed CommitDate: Sat Nov 27 10:24:12 2021 +0100 
#48802 - 1b8a18b was pushed CommitDate: Sat Nov 27 11:48:17 2021 +0100

I do not understand why 941f776 or 9c4752b had not been evaluated.

Could you give a look?  For example, by restarting the evaluation?


Cheers,
simon

[bug#52117] [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[zimoun]

Hi,

On Sat, 27 Nov 2021 at 13:38, zimoun <zimon.toutoune@gmail.com> wrote:

> Last, I am confused by Cuirass.  Because it says evaluation complete but
> julia-* packages are scheduled.
>
>     https://ci.guix.gnu.org/eval/48802?status=pending
>
> And for instance,
>
>     https://ci.guix.gnu.org/build/1853818/log/raw
>
> BTW, Berlin has some issues I guess
>
> #48720 - d508c5b was pushed CommitDate: Fri Nov 26 23:21:45 2021 +0100
>        - 941f776 was pushed CommitDate: Sat Nov 27 01:22:32 2021 -0500
>        - 9c4752b was pushed CommitDate: Sat Nov 27 10:24:12 2021 +0100 
> #48802 - 1b8a18b was pushed CommitDate: Sat Nov 27 11:48:17 2021 +0100
>
> I do not understand why 941f776 or 9c4752b had not been evaluated.
>
> Could you give a look?  For example, by restarting the evaluation?

Re-checking now, all is green. \o/
I am still confused by Cuirass but that’s another story, I guess.


Thanks for helping in this yak shaving. :-)

Cheers,
simon

[bug#52117] [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[Maxim Cournoyer]

Hello Simon,

zimoun <zimon.toutoune@gmail.com> writes:

> Hi Maxim,
>
> Thanks for the review and the improved patch.
>
> I am sorry if the commit message and/or changelog I provided was badly
> worded, but somehow it was an attempt to explain the odd behaviour – at
> least counter-intuitive since I initially felt into when sending the
> very first patch allowing parallel tests and you felt too, I guess. :-)

No worries.  Communicating changes (or anything) is always one of the
greatest challenges in programming and elsewhere, it seems :-).  The
nice thing about it is that it can be improved with perseverance and
some feedback.

>
> On Fri, 26 Nov 2021 at 22:17, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:
>
>>> ---
>>>  guix/build/julia-build-system.scm | 8 +++++---
>>>  1 file changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/guix/build/julia-build-system.scm b/guix/build/julia-build-system.scm
>>> index f0dc419c17..af478fd4a3 100644
>>> --- a/guix/build/julia-build-system.scm
>>> +++ b/guix/build/julia-build-system.scm
>>> @@ -112,7 +112,10 @@ (define* (check #:key tests? source inputs outputs julia-package-name
>>>             (builddir (string-append out "/share/julia/"))
>>>             (jobs (if parallel-tests?
>>>                       (number->string (parallel-job-count))
>>> -                     "1")))
>>> +                     "1"))
>>> +           (nprocs (if parallel-tests?
>>> +                       (string-append "--procs=" jobs)
>>> +                       "")))
>>>        ;; With a patch, SOURCE_DATE_EPOCH is honored
>>>        (setenv "SOURCE_DATE_EPOCH" "1")
>>>        (setenv "JULIA_DEPOT_PATH" builddir)
>>> @@ -122,8 +125,7 @@ (define* (check #:key tests? source inputs outputs julia-package-name
>>>                                   "")))
>>>        (setenv "JULIA_CPU_THREADS" jobs)
>>>        (setenv "HOME" "/tmp")
>>> -      (invoke "julia" "--depwarn=yes"
>>> -              (string-append "--procs=" jobs)
>>> +      (invoke "julia" "--depwarn=yes" nprocs
>>
>> Here nprocs can be ""; is it really OK to pass an empty string argument
>> to julia?
>
> Yes it is OK.  When #:parallel-tests? sets to #f, my patch leads to
> the call “julia --depwarn=yes” which is valid.  Your modified patch
> adds another test but leads to the same call “julia --depwarn=yes”.

No, it would invoke julia with the following argv list: "julia"
"-depwarn=yes" "" [...];

My point is that invoke is equivalent to doing an execlp system call;
and the arguments get passed as a list (including that empty string
argument when parallel-tests? is #f).  Whether this works or not is up
to the application, so I'd suggest not relying on it.  Consider for
example:

--8<---------------cut here---------------start------------->8---
(execlp "python" "python" "" "-c" "print('hello')")
/gnu/store/cwqv4z5bvb5x6i0zvqgc1j1dnr6w9vp8-profile/bin/python: can't
find '__main__' module in
'/home/maxim/src/guix-core-updates-next/gnu/packages/'
--8<---------------cut here---------------end--------------->8---

It fails because it interprets the empty string argument as the current
directory, apparently.  If that works with the above Julia invocation,
that's great, but it doesn't make it cleaner in my opinion :-).

> +           (job-count (if parallel-tests?
> +                          (parallel-job-count)
> +                          1))
> +           ;; The --proc argument of Julia *adds* extra processors rather than
> +           ;; specify the exact count to use, so zero must be specified to
> +           ;; disable parallel processing...
>
> [..]
>
> +      (apply invoke "julia"
> +             `("--depwarn=yes"
> +               ,@(if parallel-tests?
> +                     ;; XXX: ... but '--procs' doesn't accept 0 as a valid
> +                     ;; value, so just omit the argument entirely.
> +                     (list (string-append  "--procs="
> +                                           (number->string additional-procs)))
> +                     '())
>
>
> So because of 2 tests. I think your modified patch is more
> “complicated”. :-)

It is slightly more complex indeed; but I think it provides the reader
with useful knowledge of julia's quirks and is more correct.

> About this,
>
> +           (additional-procs (max 0 (1- job-count))))
>
> I considered that it was not a big deal; initially, I did something
> similar in ’let’ but remove it because it changes nothing from my
> experiments.  In fact, because ’--procs’ overrides JULIA_CPU_THREADS and
> run Julia with n or n+1 is more or less the same for the Julia land,
> IMHO.  Well, it is not clear what is the load for the main worker. And
> JULIA_CPU_THREADS=1 is required for running using only one worker.
> Anyway, this changes nothing, practically speaking. :-) Indeed, it is
> better and more consistent.

Yeah, I don't like that the behavior of --procs is to *add* workers,
rather than set its exact count; which make thing awkward.  Even
upstream get tricked by that by erroneously *adding* Sys.CPU_THREADS
workers because of this in test/runtest.jl [0]

[0]  https://github.com/JuliaLang/julia/pull/43217#pullrequestreview-817102530

Thanks,

Maxim

[bug#52117] [core-updates-frozen] [PATCH 0/6] Fix Julia packages.

[zimoun]

Hi Maxim,

On Sat, 27 Nov 2021 at 21:57, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> No, it would invoke julia with the following argv list: "julia"
> "-depwarn=yes" "" [...];
>
> My point is that invoke is equivalent to doing an execlp system call;
> and the arguments get passed as a list (including that empty string
> argument when parallel-tests? is #f).  Whether this works or not is up
> to the application, so I'd suggest not relying on it.  Consider for
> example:
>
> (execlp "python" "python" "" "-c" "print('hello')")
> /gnu/store/cwqv4z5bvb5x6i0zvqgc1j1dnr6w9vp8-profile/bin/python: can't
> find '__main__' module in
> '/home/maxim/src/guix-core-updates-next/gnu/packages/'

Thanks for the explanations.


> It fails because it interprets the empty string argument as the current
> directory, apparently.  If that works with the above Julia invocation,
> that's great, but it doesn't make it cleaner in my opinion :-).

Indeed, and it is expected to fail because:

--8<---------------cut here---------------start------------->8---
def _get_main_module_details(error=ImportError):
    # Helper that gives a nicer error message when attempting to
    # execute a zipfile or directory by invoking __main__.py
    main_name = "__main__"
    try:
        return _get_module_details(main_name)
    except ImportError as exc:
        if main_name in str(exc):
            raise error("can't find %r module in %r" %
                              (main_name, sys.path[0]))
        raise
--8<---------------cut here---------------end--------------->8---

It allows to do:

        $ mkdir /tmp/foo
        $ echo print(42) > /tmp/foo/__main__.py
        $ python /tmp/foo

Therefore, this

        $ python '' -c '0'

just fails.  Contrary to,

        $ cd /tmp/foo
        $ python '' -c '0'

which just passes.  To me, it is an oddity of the Python command-line
which silently accepts a path; it is not documented by “python -h”.

Anyway, I agree that the behaviour when passing "" is up to the
application, therefore it should be avoided.


Cheers,
simon