diff mbox series

[bug#54852] gnu: Add openjdk18.

Message ID CAEtmmezbF7sKtaWTKjfO+7V2nE1yLkAdAV6UKomtce_69Q=RnQ@mail.gmail.com
State Accepted
Headers show
Series [bug#54852] gnu: Add openjdk18. | expand

Checks

Context Check Description
cbaines/applying patch fail View Laminar job
cbaines/issue success View issue

Commit Message

Rostislav Svoboda April 12, 2022, 1:24 p.m. UTC
Le lun. 11 avr. 2022 à 15:44, Maxime Devos <maximedevos@telenet.be> a écrit :
> > +    (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.

Le lun. 11 avr. 2022 à 16:16, Julien Lepiller <julien@lepiller.eu> a écrit :
> 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?

You're both right. We don't need that. (Sorry) I'm sending correction below.

Cheers
Bost


From 543aa7797308ea66d1d1140e72a48908a2e73419 Mon Sep 17 00:00:00 2001
From: Rostislav Svoboda <Rostislav.Svoboda@gmail.com>
Date: Mon, 11 Apr 2022 11:55:55 +0200
Subject: [PATCH] gnu: Add openjdk18.

---
 gnu/packages/java.scm | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Comments

Rostislav Svoboda April 21, 2022, 10:27 a.m. UTC | #1
Le mar. 12 avr. 2022 à 15:24, Rostislav Svoboda
<rostislav.svoboda@gmail.com> a écrit :
> +    (native-inputs
> +     `(("autoconf" ,autoconf)
> +       ("openjdk17:jdk" ,openjdk17 "jdk")
> +       ("pkg-config" ,pkg-config)
> +       ("unzip" ,unzip)
> +       ("which" ,which)
> +       ("zip" ,zip)))

And it looks like even this part can be deduplicated. Either by:

(native-inputs
     (map (lambda (input)
            (match (car input)
              ("openjdk16:jdk" `("openjdk17:jdk" ,openjdk17 "jdk"))
              (_ input)))
          (package-native-inputs openjdk17)))

or by:

(native-inputs
     (modify-inputs (append (package-native-inputs openjdk17)
                            `(("openjdk17:jdk" ,openjdk17 "jdk")))
       (delete "openjdk16:jdk")))

Which one do you like more?

Cheers
Bost
M April 21, 2022, 4:38 p.m. UTC | #2
Rostislav Svoboda schreef op do 21-04-2022 om 12:27 [+0200]:
> or by:
> 
> (native-inputs
>      (modify-inputs (append (package-native-inputs openjdk17)
>                             `(("openjdk17:jdk" ,openjdk17 "jdk")))
>        (delete "openjdk16:jdk")))
> 
> Which one do you like more?

Is something like

  (modify-inputs (package-native-inputs opendjk17)
    (replace "openjdk16:jdk" openjdk17))

possible?
Rostislav Svoboda April 22, 2022, 10:59 a.m. UTC | #3
> Is something like
>
>   (modify-inputs (package-native-inputs opendjk17)
>     (replace "openjdk16:jdk" openjdk17))
>
> possible?

I think more appropriate would be:

(modify-inputs (package-native-inputs openjdk17)
      (replace "openjdk16:jdk" `(,openjdk17 "jdk")))

however that would mean to assign a value Y to a variable named
"I-represent-a-value-of-X". So hmm, better not this way.

I personally prefer the:

(native-inputs
     (map (lambda (input)
            (match (car input)
              ("openjdk16:jdk" `("openjdk17:jdk" ,openjdk17 "jdk"))
              (_ input)))
          (package-native-inputs openjdk17)))

variant. IMO it better expresses the idea of substitution.

Here, (if such a pattern repeats) we could define a new syntax
`substitue` in the guix/packages.scm analogical to `delete`,
`prepend`, `append` and `replace`.

Cheers
Bost
M April 23, 2022, 2:38 p.m. UTC | #4
Rostislav Svoboda schreef op vr 22-04-2022 om 12:59 [+0200]:
> > Is something like
> > 
> >    (modify-inputs (package-native-inputs opendjk17)
> >      (replace "openjdk16:jdk" openjdk17))
> > 
> > possible?
> 
> I think more appropriate would be:
> 
> (modify-inputs (package-native-inputs openjdk17)
>       (replace "openjdk16:jdk" `(,openjdk17 "jdk")))
> 
> however that would mean to assign a value Y to a variable named
> "I-represent-a-value-of-X". So hmm, better not this way.
> 
> I personally prefer the:
> 
> (native-inputs
>      (map (lambda (input)
>             (match (car input)
>               ("openjdk16:jdk" `("openjdk17:jdk" ,openjdk17 "jdk"))
>               (_ input)))
>           (package-native-inputs openjdk17)))
> 
> variant. IMO it better expresses the idea of substitution.
> 
> Here, (if such a pattern repeats) we could define a new syntax
> `substitue` in the guix/packages.scm analogical to `delete`,
> `prepend`, `append` and `replace`.

Isn't this just (modify-inputs ... (replace "..." ...))?

What's the difference between 'replace' and 'substitute'?  They
conceptually seem to be about the same to me.  And in case of
substitute, I'm more thinking of 'substitute*' than package inputs.

Greetings,
Maxime.
Rostislav Svoboda April 23, 2022, 3:11 p.m. UTC | #5
> > Here, (if such a pattern repeats) we could define a new syntax
> > `substitue` in the guix/packages.scm analogical to `delete`,
> > `prepend`, `append` and `replace`.
>
> Isn't this just (modify-inputs ... (replace "..." ...))?
>
> What's the difference between 'replace' and 'substitute'?  They
> conceptually seem to be about the same to me.  And in case of
> substitute, I'm more thinking of 'substitute*' than package inputs.

The lists `inputs`, `native-inputs` and `propagated-inputs` are lists
of association lists that map input labels to file names. E.g.:

(native-inputs
     `(("autoconf" ,autoconf)
       ("openjdk16:jdk" ,openjdk16 "jdk")
       ("pkg-config" ,pkg-config)
       ("unzip" ,unzip)
       ("which" ,which)
       ("zip" ,zip)))

The `replace` just replaces the file name for a given input label (see
guix/packages.scm, line 1099):

(define (replace-input name replacement inputs)
  "Replace input NAME by REPLACEMENT within INPUTS."
  (map (lambda (input)
         (match input
           (((? string? label) _ . outputs)
            (if (string=? label name)
                (match replacement        ;does REPLACEMENT specify an output?
                  ((_ _) (cons label replacement))
                  (_     (cons* label replacement outputs)))
                input))))
       inputs))

(define-syntax replace
  (lambda (s)
    (syntax-violation 'replace
                      "'replace' may only be used within 'modify-inputs'"
                      s)))

See also https://guix.gnu.org/en/blog/2021/the-big-change/

> I'm more thinking of 'substitute*' than package inputs.

That thing which I named `substitute` doesn't exist. I was just
contemplating that we could write it, if we want to change the whole
association list, i.e. the pair input label & file name, not just the
file name. (And when I think about it again, such a name will lead to
confusion. So nah, forget about it.)

Cheers
Bost
M April 23, 2022, 4:10 p.m. UTC | #6
Rostislav Svoboda schreef op za 23-04-2022 om 17:11 [+0200]:
> The lists `inputs`, `native-inputs` and `propagated-inputs` are lists
> of association lists that map input labels to file names. E.g.:
>
> (native-inputs
>      `(("autoconf" ,autoconf)
>        ("openjdk16:jdk" ,openjdk16 "jdk")
>        ("pkg-config" ,pkg-config)
>        ("unzip" ,unzip)
>        ("which" ,which)
>        ("zip" ,zip)))

Nitpick: 'autoconf', 'openjdk16', ..., are package objects, not file
names.

> The `replace` just replaces the file name for a given input label (see
> guix/packages.scm, line 1099):
> 
> (define (replace-input name replacement inputs)
>   "Replace input NAME by REPLACEMENT within INPUTS."
>   (map (lambda (input)
>          (match input
>            (((? string? label) _ . outputs)
>             (if (string=? label name)
>                 (match replacement        ;does REPLACEMENT specify an output?
>                   ((_ _) (cons label replacement))
>                   (_     (cons* label replacement outputs)))
>                 input))))
>        inputs))

Replacing 'file name' by 'package object', isn't that what we need
here?  FWIW, 'replace' could be extended to also support modifying the
output, but that extra functionality doesn't appear to have any use
yet.

Greetings,
Maxime.
diff mbox series

Patch

diff --git a/gnu/packages/java.scm b/gnu/packages/java.scm
index 6738b5dc35..ec0b956388 100644
--- a/gnu/packages/java.scm
+++ b/gnu/packages/java.scm
@@ -2263,6 +2263,29 @@  (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)))
+    (home-page "https://openjdk.java.net/projects/jdk/18")))
+
 (define-public icedtea icedtea-8)