diff mbox series

[bug#54852] gnu: Add openjdk18.

Message ID 20220411125335.4047-1-Rostislav.Svoboda@gmail.com
State Accepted
Headers show
Series [bug#54852] gnu: Add openjdk18. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Rostislav Svoboda April 11, 2022, 12:53 p.m. UTC 
---
 gnu/packages/java.scm | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

Comments

M April 11, 2022, 1:43 p.m. UTC | #1 
Rostislav Svoboda schreef op ma 11-04-2022 om 14:53 [+0200]:
> +    (native-inputs
> +     `(("autoconf" ,autoconf)
> +       ("openjdk17:jdk" ,openjdk17 "jdk")

Does it need openjdk17 to build, or would openjdk16 suffice?
That would avoid increasing the bootstrap chain length.

Greetings,
MAxime.
M April 11, 2022, 1:44 p.m. UTC | #2 
Rostislav Svoboda schreef op ma 11-04-2022 om 14:53 [+0200]:
> +    (arguments
> +     (substitute-keyword-arguments (package-arguments openjdk16)
> +       ((#:phases phases)
> +        `(modify-phases ,phases
> +           (replace 'fix-java-shebangs
> +             (lambda _
> +               ;; This file was "fixed" by patch-source-shebangs, but it requires
> +               ;; this exact first line.
> +               (substitute* "make/data/blockedcertsconverter/blocked.certs.pem"
> +                 (("^#!.*") "#! java BlockedCertsConverter SHA-256\n"))))))))

Why is this duplicated?  It is exactly the same phase as for openjdk17,
so inherit should take care of it.

Greetings,
Maxime.
Julien Lepiller April 11, 2022, 2:15 p.m. UTC | #3 
Hi!

The patch looks great, but I wonder why you need to change che phases to tomething that looks like openjdk 17's phases? Am I missing something? Wouldn't it work without any change to the arguments?

I can't build and test because of my limited bandwidth. Hopefully another maintainer can rur the tests and maybe push if my issues aren't solved soon…

On April 11, 2022 2:53:35 PM GMT+02:00, Rostislav Svoboda <rostislav.svoboda@gmail.com> wrote:
>---
> gnu/packages/java.scm | 33 +++++++++++++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
>
>diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm
>index 6738b5dc35..10c84d2973 100644
>--- a/gnu/packages/java.scm
>+++ b/gnu/packages/java.scm
>@@ -2263,6 +2263,39 @@ (define-public openjdk17
>                  (("^#!.*") "#! java BlockedCertsConverter SHA-256\n"))))))))
>     (home-page "https://openjdk.java.net/projects/jdk/17")))
> 
>+(define-public openjdk18
>+  (package
>+    (inherit openjdk17)
>+    (name "openjdk")
>+    (version "18")
>+    (source (origin
>+              (method git-fetch)
>+              (uri (git-reference
>+                    (url "https://github.com/openjdk/jdk18u")
>+                    (commit (string-append "jdk-" version "-ga"))))
>+              (file-name (git-file-name name version))
>+              (sha256
>+               (base32
>+                "1bv6bdhkmwvn10l0xy8yi9xibds640hs5zsvx0jp7wrxa3qw4qy8"))))
>+    (native-inputs
>+     `(("autoconf" ,autoconf)
>+       ("openjdk17:jdk" ,openjdk17 "jdk")
>+       ("pkg-config" ,pkg-config)
>+       ("unzip" ,unzip)
>+       ("which" ,which)
>+       ("zip" ,zip)))
>+    (arguments
>+     (substitute-keyword-arguments (package-arguments openjdk16)
>+       ((#:phases phases)
>+        `(modify-phases ,phases
>+           (replace 'fix-java-shebangs
>+             (lambda _
>+               ;; This file was "fixed" by patch-source-shebangs, but it requires
>+               ;; this exact first line.
>+               (substitute* "make/data/blockedcertsconverter/blocked.certs.pem"
>+                 (("^#!.*") "#! java BlockedCertsConverter SHA-256\n"))))))))
>+    (home-page "https://openjdk.java.net/projects/jdk/18")))
>+
> (define-public icedtea icedtea-8)
> 
> >
>-- 
>2.35.1
>
>
>
>
Rostislav Svoboda April 12, 2022, 1:03 p.m. UTC | #4 
> Rostislav Svoboda schreef op ma 11-04-2022 om 14:53 [+0200]:
> > +    (native-inputs
> > +     `(("autoconf" ,autoconf)
> > +       ("openjdk17:jdk" ,openjdk17 "jdk")
>
> Does it need openjdk17 to build, or would openjdk16 suffice?
> That would avoid increasing the bootstrap chain length.

Having
       ("openjdk16:jdk" ,openjdk16 "jdk")

leads to:

configure: Found potential Boot JDK using java(c) in PATH
configure: Potential Boot JDK found at
/gnu/store/j9lyv24nkwgznddiqfv29j2czfn15a9b-openjdk-16.0.1-jdk is
incorrect JDK version (openjdk version "16.0.1" 2021-04-20 OpenJDK
Runtime Environment (build 16.0.1+0-adhoc..source) OpenJDK 64-Bit
Server VM (build 16.0.1+0-adhoc..source, mixed mode, sharing));
ignoring
configure: (Your Boot JDK version must be one of: 17 18)
configure: Could not find a valid Boot JDK. OpenJDK distributions are
available at http://jdk.java.net/.
configure: This might be fixed by explicitly setting --with-boot-jdk
configure: error: Cannot continue

Cheers
Bost
Björn Höfling April 13, 2022, 7:51 a.m. UTC | #5 
Hi,

On Mon, 11 Apr 2022 16:15:46 +0200
Julien Lepiller <julien@lepiller.eu> wrote:

 
> I can't build and test because of my limited bandwidth. Hopefully
> another maintainer can rur the tests and maybe push if my issues
> aren't solved soon…

I tried but stumbled upon some limits:

/gnu/store/4y5m9lb8k3qkb1y9m02sw9w9a6hacd16-bash-minimal-5.1.8/bin/bash: fork: retry: Resource temporarily unavailable
g++: fatal error: cannot execute '/gnu/store/069aq2v993kpc41yabp5b6vm4wb9jkhg-gcc-10.3.0/libexec/gcc/x86_64-unknown-linux-gnu/10.3.0/cc1plus': vfork: Resource temporarily unavailable
compilation terminated.

Is this in my shell or on the daemon? How can I increase it?

Not sure if I'm faster than your download, but let's see ...

Björn
M May 8, 2022, 4:41 p.m. UTC | #6 
How long did it take to compile openjdk@18 on your computer?  It's
taking many hours on my computer ...
M May 9, 2022, 7:11 p.m. UTC | #7 
The following simplified package definition ‘worked’ for me:

(define-public openjdk18
  (package
    (inherit openjdk17)
    (name "openjdk")
    (version "18")
    (source (origin
              (method git-fetch)
              (uri (git-reference
                    (url "https://github.com/openjdk/jdk18u")
                    (commit (string-append "jdk-" version "-ga"))))
              (file-name (git-file-name name version))
              (sha256
               (base32
                "1bv6bdhkmwvn10l0xy8yi9xibds640hs5zsvx0jp7wrxa3qw4qy8"))))
    (native-inputs
      (modify-inputs (package-native-inputs openjdk17)
        (replace "openjdk16:jdk" openjdk17)))
    [home-page ...]))

‘worked’, because it took to long for me so I interrupted the build in
the build phase.

About the labels issue: maybe "icedtea-8" could be changed to "icedtea",
"openjdkN" to "openjdk" and "openjdkN:jdk" to "openjdk:jdk" at some point
in the future, such that the package name matches the input label and
the input alist can be changed to a package list.

Greetings,
Maxime.
Rostislav Svoboda May 9, 2022, 10:20 p.m. UTC | #8 
Hi Maxime,

> The following simplified package definition ‘worked’ for me:
>
> (define-public openjdk18
>   (package
>     (inherit openjdk17)
>     (name "openjdk")
>     (version "18")
>     (source (origin
>               (method git-fetch)
>               (uri (git-reference
>                     (url "https://github.com/openjdk/jdk18u")
>                     (commit (string-append "jdk-" version "-ga"))))
>               (file-name (git-file-name name version))
>               (sha256
>                (base32
>                 "1bv6bdhkmwvn10l0xy8yi9xibds640hs5zsvx0jp7wrxa3qw4qy8"))))
>     (native-inputs
>       (modify-inputs (package-native-inputs openjdk17)
>         (replace "openjdk16:jdk" openjdk17)))
>     [home-page ...]))

I'm a bit puzzled. Here is how I run the build:
The `/home/bost/dev/guix` contains a clone of
https://git.savannah.gnu.org/git/guix.git

```shell
$ pwd
/home/bost/dev/guix/gnu/packages
$ time guix build --check --load-path=. openjdk
...
Executed in   73.08 secs    fish           external
usr time  151.31 secs    0.00 micros  151.31 secs
sys time    0.48 secs  759.00 micros    0.48 secs
```

So my build time is about 2,5 minutes. During the build I get plenty
of warnings like:
    guix build: warning: failed to load '(SOME-THING)':
    no code for module (SOME-THING)
    ./SOME-THING.scm:20:0: warning: module name (gnu packages
SOME-THING) does not match file name 'SOME-THING.scm'
    hint: File `./SOME-THING.scm' should probably start with:

    (define-module (SOME-THING))

and it looks like the warnings can be ignored.

Regarding the compilation itself, first of all I had to exclude the
271b2e43bef96f17f3f1e1085394b4bb144c5768 due to the
https://issues.guix.gnu.org/55255 from my repo.
Then I had to fix the `[home-page ...]` and put there `(home-page
"https://openjdk.java.net/projects/jdk/18")` and even after that it
doesn't compile. I get:
    The following graft will be made:
       /gnu/store/42kdy7fs7pykx79m69v2gpxw7gmph745-openjdk-18.drv
    guix build: error: some outputs of
`/gnu/store/42kdy7fs7pykx79m69v2gpxw7gmph745-openjdk-18.drv' are not
valid, so checking is not possible

And here are the definitions which work for me:
;; I'd prefer:
(define-public openjdk18
  (package
    (inherit openjdk17)
    (name "openjdk")
    (version "18")
    (source (origin
              (method git-fetch)
              (uri (git-reference
                    (url "https://github.com/openjdk/jdk18u")
                    (commit (string-append "jdk-" version "-ga"))))
              (file-name (git-file-name name version))
              (sha256
               (base32
                "1bv6bdhkmwvn10l0xy8yi9xibds640hs5zsvx0jp7wrxa3qw4qy8"))))
    (native-inputs
     (map (lambda (input)
            (match (car input)
              ("openjdk16:jdk" `("openjdk17:jdk" ,openjdk17 "jdk"))
              (_ input)))
          (package-native-inputs openjdk17)))
    (home-page "https://openjdk.java.net/projects/jdk/18")))

;; This works too:
(define-public openjdk18
  (package
    (inherit openjdk17)
    (name "openjdk")
    (version "18")
    (source (origin
              (method git-fetch)
              (uri (git-reference
                    (url "https://github.com/openjdk/jdk18u")
                    (commit (string-append "jdk-" version "-ga"))))
              (file-name (git-file-name name version))
              (sha256
               (base32
                "1bv6bdhkmwvn10l0xy8yi9xibds640hs5zsvx0jp7wrxa3qw4qy8"))))
    (modify-inputs (package-native-inputs openjdk17)
      (delete "openjdk16:jdk")
      (append `(("openjdk17:jdk" ,openjdk17 "jdk"))))
    (home-page "https://openjdk.java.net/projects/jdk/18")))

> About the labels issue: maybe "icedtea-8" could be changed to "icedtea",
> "openjdkN" to "openjdk" and "openjdkN:jdk" to "openjdk:jdk" at some point
> in the future, such that the package name matches the input label and
> the input alist can be changed to a package list.

Sure. Good idea.

Greetings,
Bost
M May 10, 2022, 6:59 a.m. UTC | #9 
Rostislav Svoboda schreef op di 10-05-2022 om 00:20 [+0200]:
> /home/bost/dev/guix/gnu/packages
> $ time guix build --check --load-path=. openjdk
> [...]
> and it looks like the warnings can be ignored

packages contains files like "base.scm" corresponding to the module
(gnu packages base).  However, now there are two (gnu packages base) --
the base from the guix you run, and the base from ./.  Additionally,
"base.scm" has a module (gnu packages base) so Guile expects it to be
located in ./gnu/package/base.scm instead.

Instead, try:

/home/bost/dev/guix
# do this inside a "guix shell -D guix" or whatever you use
# to set up a Guix development environment  
$ make && time ./pre-inst-env guix build openjdk

(also, don't do --check -- check is for verifying that the build was
reproducible, but it's a new package definition, so Guix doesn't have
anything to compare it against).

> The following graft will be made:
> [...]

Looks like Guix didn't detect the new package definition, otherwise
you'd see ‘The following packages will be build’ or the like.

> ;; I'd prefer: [...
> (delete "openjdk16:jdk")
> (append `(("openjdk17:jdk" ,openjdk17 "jdk"))))

If/when 

> About the labels issue: maybe "icedtea-8" could be changed to
> "icedtea", "openjdkN" to "openjdk" and "openjdkN:jdk" to
> "openjdk:jdk" at some point in the future, such that the package name
> matches the input label and the input alist can be changed to a
> package list.

is addressed, WDYT of (replace "openjdk" openjdk)?

Greetings,
Maxime.
Rostislav Svoboda May 11, 2022, 3:37 p.m. UTC | #10 
Hi Maxime,

> Instead, try:
>
> /home/bost/dev/guix
> # do this inside a "guix shell -D guix" or whatever you use
> # to set up a Guix development environment
> $ make && time ./pre-inst-env guix build openjdk

Ok thanks. (As you see, I need to learn a lot.)
Regarding the build time (just for the record), I run

$ cd /home/bost/dev/guix
$ git checkout dev
$ git --force -dx       # make sure everything is pristine clean and clear
$ guix shell --development guix
# and then:
[env]$ ./bootstrap && ./configure --localstatedir=/var && make -j24
[env]$ time ./pre-inst-env guix build openjdk
...
real 4m32.993s
user 0m26.175s
sys 0m1.357s

And my `dev` branch contains this openjdk18 definition:

(define-public openjdk18
  (package
    (inherit openjdk17)
    (name "openjdk")
    (version "18")
    (source (origin
              (method git-fetch)
              (uri (git-reference
                    (url "https://github.com/openjdk/jdk18u")
                    (commit (string-append "jdk-" version "-ga"))))
              (file-name (git-file-name name version))
              (sha256
               (base32
                "1bv6bdhkmwvn10l0xy8yi9xibds640hs5zsvx0jp7wrxa3qw4qy8"))))
    (native-inputs
     (map (lambda (input)
            (match (car input)
              ("openjdk16:jdk" `("openjdk17:jdk" ,openjdk17 "jdk"))
              (_ input)))
          (package-native-inputs openjdk17)))
    (home-page "https://openjdk.java.net/projects/jdk/18")))

> > ;; I'd prefer: [...
> > (delete "openjdk16:jdk")
> > (append `(("openjdk17:jdk" ,openjdk17 "jdk"))))
>
> If/when

What do you mean by that???

> > About the labels issue: maybe "icedtea-8" could be changed to
> > "icedtea", "openjdkN" to "openjdk" and "openjdkN:jdk" to
> > "openjdk:jdk" at some point in the future, such that the package name
> > matches the input label and the input alist can be changed to a
> > package list.
>
> is addressed, WDYT of (replace "openjdk" openjdk)?

???
Could you explain that please?
Because to me, out of any context, (e.g. when grep-ing over the source
code,) that looks like a "replace a-thing with a-thing". I.e.
effectively, a do-nothing command.
Thank you.

Greetings,
Bost
M May 11, 2022, 6:13 p.m. UTC | #11 
Rostislav Svoboda schreef op wo 11-05-2022 om 17:37 [+0200]:
> > > About the labels issue: maybe "icedtea-8" could be changed to
> > > "icedtea", "openjdkN" to "openjdk" and "openjdkN:jdk" to
> > > "openjdk:jdk" at some point in the future, such that the package
> > > name
> > > matches the input label and the input alist can be changed to a
> > > package list.
> > 
> > is addressed, WDYT of (replace "openjdk" openjdk)?
> 
> ???
> Could you explain that please?
> Because to me, out of any context, (e.g. when grep-ing over the
> source
> code,) that looks like a "replace a-thing with a-thing". I.e.
> effectively, a do-nothing command.

Oops, I meant (replace "openjdk" openjdkN). Basically, whenever we now
have (native-inputs `(("openjdkN" ,openjdkN) ("openjdkN" ,openjdkN
"jdk") [... other inputs ...])), let's replace it by

  ;; In the openjdkN+1 package
  (native-inputs
    (list `(("openjdk" ,openjdkN)
            ("openjdk:jdk" ,openjdkN "jdk")
            [...])))

such that we always use the same input labels for the openjdk packages
(here, N is 9, 10, 11, 12, 13, 14, 15 or 16).

Then the the native-inputs can be simplified to (using "guix style"):

  (native-inputs
    (list openjdkN `(,openjdkN "jdk") [...]))

and further to

  (native-inputs
    (modify-inputs (package-native-inputs openjdkN)
      (replace "openjdk" openjdkN)))

which means: take the 'native-inputs' of the previous version of
'openjdk', but replace the openjdk it uses (openjdkN-1) by itself
(openjdkN).
M May 11, 2022, 6:22 p.m. UTC | #12 
Rostislav Svoboda schreef op wo 11-05-2022 om 17:37 [+0200]:
> [env]$ time ./pre-inst-env guix build openjdk

Did you run this after it was already built, or before?  If the former,
it probably just counts time grafting (4min seems about accurate for
openjdk in my experience) due to some new grafts since last time.

For timing build times, I recommend:

# --no-grafts: don't do grafts to avoid counting time grafting
# --check --rounds=1: build it again
$ time ./pre-inst-env guix build openjdk --no-grafts --check --rounds=1

though that only does somethin usefu

> ...
> real 4m32.993s
> user 0m26.175s
> sys 0m1.357s

I compiled it with cores=1 or cores=4 or cores=8 (not sure which).
Since apparently you have a 24 core computer, multiplying by 24 would
give an estimate of 108m ≅ 2 hours for cores=1.  Though it ran much
longer than that on my computer (at least 12h), so I don't understand,
unless there was grafting involved.

I don't think it really matters though, so maybe we can drop this?

Greetings,
Maxime.
M May 11, 2022, 6:30 p.m. UTC | #13 
Rostislav Svoboda schreef op wo 11-05-2022 om 17:37 [+0200]:
> > > ;; I'd prefer: [...
> > > (delete "openjdk16:jdk")
> > > (append `(("openjdk17:jdk" ,openjdk17 "jdk"))))
> > 
> > If/when
> 
> What do you mean by that???

It's a sentence construction that I occasionally find useful but I
imagine it can be confusing to someone not used to it.  (Rest of
explanation continued later).

> 
> > > About the labels issue: maybe "icedtea-8" could be changed to
> > > "icedtea", "openjdkN" to "openjdk" and "openjdkN:jdk" to
> > > "openjdk:jdk" at some point in the future, such that the package
> > > name
> > > matches the input label and the input alist can be changed to a
> > > package list.
> > 
> > is addressed, WDYT of (replace "openjdk" openjdk)?

Basically, the full sentence is ‘If/when SUBJECT is addressed, WDYT of
(replace "openjdk" openjdk)?’.  Here, SUBJECT is the complete next
quote:

> About the labels issue: maybe "icedtea-8" could be changed to
> "icedtea", "openjdkN" to "openjdk" and "openjdkN:jdk" to
> "openjdk:jdk" at some point in the future, such that the package name
> matches the input label and the input alist can be changed to a
> package list.

In linguistic terms, the subject of the sentence is the above
paragraph.  Using a full paragraph as subject isn't really feasible in
verbal speech, but textual communication is more flexible in this
aspect, albeit with the usual ‘only if both communication partners
understand each other’ caveat.

Greetings,
Maxime.
Maxim Cournoyer Sept. 27, 2022, 2:17 p.m. UTC | #14 
Hello!

Thanks for the heads up regarding openjdk18.  I've pushed a series of
change that removed the duplication in our different openjdk packages,
and slimmed its weigh from 345 to 116 MiB.  I also added openjdk18 on
top and made it the default version in Guix.

See commits from e33ab2dd9e to fb6173b7c0.

Thank you!

Closing.

Maxim
diff mbox series

Patch

diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm
index 6738b5dc35..10c84d2973 100644
--- a/gnu/packages/java.scm
+++ b/gnu/packages/java.scm
@@ -2263,6 +2263,39 @@  (define-public openjdk17
                  (("^#!.*") "#! java BlockedCertsConverter SHA-256\n"))))))))
     (home-page "https://openjdk.java.net/projects/jdk/17")))
 
+(define-public openjdk18
+  (package
+    (inherit openjdk17)
+    (name "openjdk")
+    (version "18")
+    (source (origin
+              (method git-fetch)
+              (uri (git-reference
+                    (url "https://github.com/openjdk/jdk18u")
+                    (commit (string-append "jdk-" version "-ga"))))
+              (file-name (git-file-name name version))
+              (sha256
+               (base32
+                "1bv6bdhkmwvn10l0xy8yi9xibds640hs5zsvx0jp7wrxa3qw4qy8"))))
+    (native-inputs
+     `(("autoconf" ,autoconf)
+       ("openjdk17:jdk" ,openjdk17 "jdk")
+       ("pkg-config" ,pkg-config)
+       ("unzip" ,unzip)
+       ("which" ,which)
+       ("zip" ,zip)))
+    (arguments
+     (substitute-keyword-arguments (package-arguments openjdk16)
+       ((#:phases phases)
+        `(modify-phases ,phases
+           (replace 'fix-java-shebangs
+             (lambda _
+               ;; This file was "fixed" by patch-source-shebangs, but it requires
+               ;; this exact first line.
+               (substitute* "make/data/blockedcertsconverter/blocked.certs.pem"
+                 (("^#!.*") "#! java BlockedCertsConverter SHA-256\n"))))))))
+    (home-page "https://openjdk.java.net/projects/jdk/18")))
+
 (define-public icedtea icedtea-8)