diff mbox series

[bug#69677,gnome-team,1/1] gnu: opam: Fix build.

Message ID 3a4e1befc3e3da925fccec1d88f53fff1ffff83b.1709986125.git.vivien@planete-kraus.eu
State New
Headers show
Series Fix opam on gnome-team | expand

Commit Message

Vivien Kraus March 9, 2024, 9:46 a.m. UTC
* gnu/packages/ocaml.scm (ocaml-opam-core) [#:phases]: Add 'always-use-base64-compat-5'.

Change-Id: I8d9aec14ca2af92b9a4ac346ac57c22e6b76c716
---
Sorry, I messed up my cover letter, so it was not delivered to you.

Here it is:

Dear Guix,

For some unknown reason, opam on master decides that the ocaml base64 library
is not installed well enough to use it, but on gnome-team, it seems that it
does.

Opam can adapt to both cases, by using a different compatibility layer in
each.  On master, it uses the compatibility layer version 5, and on
gnome-team, the compatibility layer version 6.

However, the compatibility layer version 6 does not work.  An easy fix is to
force opam to use compatibility layer version 5 every time.

I am not proud of my regular expressions, but I couldn’t find a way to
factorize the optional `i` so that it matches either .ml or .mli.

Best regards,

Vivien

 gnu/packages/ocaml.scm | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Comments

Maxim Cournoyer March 9, 2024, 6:18 p.m. UTC | #1
Hi Vivien,

Vivien Kraus <vivien@planete-kraus.eu> writes:

> * gnu/packages/ocaml.scm (ocaml-opam-core) [#:phases]: Add 'always-use-base64-compat-5'.

[...]

>  gnu/packages/ocaml.scm | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
> index 0f4c351141..5051524a53 100644
> --- a/gnu/packages/ocaml.scm
> +++ b/gnu/packages/ocaml.scm
> @@ -854,6 +854,26 @@ (define ocaml-opam-core
>                   #:tests? #f
>                   #:phases
>                   (modify-phases %standard-phases
> +                   (add-after 'unpack 'always-use-base64-compat-5
> +                     (lambda _
> +                       (call-with-input-file "src/client/dune"
> +                         (lambda (port)
> +                           (display "ok")
> +                           (newline)))

Shouldn't this be 'call-with-output-file' ?  It seems you want to
*write* a file containing "ok\n".

> +                       ;; By default, the opamBase64Compat module will be
> +                       ;; compatibility version 6, which is just an empty
> +                       ;; file.  Opam-client cannot build with an
> empty file.

What does upstream say about this?  Is this a bug?  Or some understood
limitation they don't foresee fixing?

> +                       (substitute* "src/client/dune"
> +                         (("opamBase64Compat\\.ml\\.6")
> +                          "opamBase64Compat.ml.5")
> +                         (("opamBase64Compat\\.mli\\.6")
> +                          "opamBase64Compat.mli.5")
> +                         ;; Undo the change in the rule to generate the .6
> +                         ;; files
> +                         (("with-stdout-to opamBase64Compat\\.ml\\.5")
> +                          "with-stdout-to opamBase64Compat.ml.6")
> +                         (("with-stdout-to opamBase64Compat\\.mli\\.5")
> +                          "with-stdout-to opamBase64Compat.mli.6"))))

I'd like to hear about upstream.  Perhaps our package is simply buggy?
It'd be nice to have an upstream issue to link to.
Vivien Kraus March 9, 2024, 6:21 p.m. UTC | #2
Le samedi 09 mars 2024 à 13:18 -0500, Maxim Cournoyer a écrit :
> Shouldn't this be 'call-with-output-file' ?  It seems you want to
> *write* a file containing "ok\n".
Oops, I forgot to remove that, how embarrassing. Sorry.
Vivien Kraus March 9, 2024, 7:39 p.m. UTC | #3
Le samedi 09 mars 2024 à 13:18 -0500, Maxim Cournoyer a écrit :
> 
> I'd like to hear about upstream.  Perhaps our package is simply
> buggy?
> It'd be nice to have an upstream issue to link to.
With a closer investigation, it looks lik we are doing things wrong
indeed.

From within the failed build directory, if I set up the environment
variable, run ocaml -I <the bunch of include directories>, and in the
prompt, run:

#load "base64.cma";;
Base64.decode_exn;;

I get an error message:

Error: The files /gnu/store/wh2ipi0bjywcf5pyis6z1s52adf5cqyd-ocaml-
base64-3.5.1/lib/ocaml/site-lib/base64/base64.cma
       and /gnu/store/i01v0c5yzhw62qmvrlv3y436igwhcafh-ocaml-extlib-
1.7.9/lib/ocaml/site-lib/extlib/base64.cmi
       make inconsistent assumptions over interface Base64

My guess is that we have 2 ocaml libraries implementing Base64 (ocaml-
base64 and ocaml-extlib, propagated from ocaml-dose3). Dune sees that
we have a powerful ocaml-base64 installed, so it lays out the
compatibility version 6 for us, but when it comes to actually building
the project, ocamlfind prioritizes the ocaml-extlib path by adding it
first, and it shadows our powerful ocaml-base64 implementation.

With this new extlib actor in the plot, I could search better.  It
seems like Opam is aware of the issue:

https://github.com/ocaml/opam/issues/5694

The solution seems to be for extlib to ship a minimal version. But
maybe this should primarily concern the ocaml team, not gnome-team, as
it looks to me like a recipe for breaking at least half of the ocaml
ecosystem.

Best regards,

Vivien
Liliana Marie Prikler March 9, 2024, 8:35 p.m. UTC | #4
Hi Vivien,

Am Samstag, dem 09.03.2024 um 20:39 +0100 schrieb Vivien Kraus:
> My guess is that we have 2 ocaml libraries implementing Base64
> (ocaml-base64 and ocaml-extlib, propagated from ocaml-dose3). Dune
> sees that we have a powerful ocaml-base64 installed, so it lays out
> the compatibility version 6 for us, but when it comes to actually
> building the project, ocamlfind prioritizes the ocaml-extlib path by
> adding it first, and it shadows our powerful ocaml-base64
> implementation.
> 
> With this new extlib actor in the plot, I could search better.  It
> seems like Opam is aware of the issue:
> 
> https://github.com/ocaml/opam/issues/5694
> 
> The solution seems to be for extlib to ship a minimal version. But
> maybe this should primarily concern the ocaml team, not gnome-team,
> as it looks to me like a recipe for breaking at least half of the
> ocaml ecosystem.
How much of it is broken with gnome-team already (vs. broken on other
branches)?  It wouldn't be nice of us to say "here, we broke it, you
fix it please".  It also appears as though opam only has two dependants
– are we not tracking this correctly or is a minimal version in use
anyway?

Cheers
Vivien Kraus March 9, 2024, 9:29 p.m. UTC | #5
Le samedi 09 mars 2024 à 21:35 +0100, Liliana Marie Prikler a écrit :
> > The solution seems to be for extlib to ship a minimal version. But
> > maybe this should primarily concern the ocaml team, not gnome-team,
> > as it looks to me like a recipe for breaking at least half of the
> > ocaml ecosystem.
> How much of it is broken with gnome-team already (vs. broken on other
> branches)?  It wouldn't be nice of us to say "here, we broke it, you
> fix it please".  It also appears as though opam only has two
> dependants
> – are we not tracking this correctly or is a minimal version in use
> anyway?

I meant that the fix should maybe be proposed to the OCaml team, not
the GNOME team.  I also over-estimated the number of packages that
depend on ocaml-extlib, so it is probably safe.

I opened #69682 for this.

Best regards,

Vivien
Julien Lepiller March 9, 2024, 9:35 p.m. UTC | #6
Le 9 mars 2024 21:35:43 GMT+01:00, Liliana Marie Prikler <liliana.prikler@gmail.com> a écrit :
>Hi Vivien,
>
>Am Samstag, dem 09.03.2024 um 20:39 +0100 schrieb Vivien Kraus:
>> My guess is that we have 2 ocaml libraries implementing Base64
>> (ocaml-base64 and ocaml-extlib, propagated from ocaml-dose3). Dune
>> sees that we have a powerful ocaml-base64 installed, so it lays out
>> the compatibility version 6 for us, but when it comes to actually
>> building the project, ocamlfind prioritizes the ocaml-extlib path by
>> adding it first, and it shadows our powerful ocaml-base64
>> implementation.
>> 
>> With this new extlib actor in the plot, I could search better.  It
>> seems like Opam is aware of the issue:
>> 
>> https://github.com/ocaml/opam/issues/5694
>> 
>> The solution seems to be for extlib to ship a minimal version. But
>> maybe this should primarily concern the ocaml team, not gnome-team,
>> as it looks to me like a recipe for breaking at least half of the
>> ocaml ecosystem.

Sounds like we could switch extlib to the dune build-system, possibly adding a #:package argument to make sure it doesn't build the compat version.

>How much of it is broken with gnome-team already (vs. broken on other
>branches)?  It wouldn't be nice of us to say "here, we broke it, you
>fix it please".  It also appears as though opam only has two dependants
>– are we not tracking this correctly or is a minimal version in use
>anyway?

opam is not used to build ocaml packages, so it's not a surprise to me it has so few dependents. Would be nice not to break it though, it's still a useful program by itself :)

>
>Cheers
>
>
>
diff mbox series

Patch

diff --git a/gnu/packages/ocaml.scm b/gnu/packages/ocaml.scm
index 0f4c351141..5051524a53 100644
--- a/gnu/packages/ocaml.scm
+++ b/gnu/packages/ocaml.scm
@@ -854,6 +854,26 @@  (define ocaml-opam-core
                  #:tests? #f
                  #:phases
                  (modify-phases %standard-phases
+                   (add-after 'unpack 'always-use-base64-compat-5
+                     (lambda _
+                       (call-with-input-file "src/client/dune"
+                         (lambda (port)
+                           (display "ok")
+                           (newline)))
+                       ;; By default, the opamBase64Compat module will be
+                       ;; compatibility version 6, which is just an empty
+                       ;; file.  Opam-client cannot build with an empty file.
+                       (substitute* "src/client/dune"
+                         (("opamBase64Compat\\.ml\\.6")
+                          "opamBase64Compat.ml.5")
+                         (("opamBase64Compat\\.mli\\.6")
+                          "opamBase64Compat.mli.5")
+                         ;; Undo the change in the rule to generate the .6
+                         ;; files
+                         (("with-stdout-to opamBase64Compat\\.ml\\.5")
+                          "with-stdout-to opamBase64Compat.ml.6")
+                         (("with-stdout-to opamBase64Compat\\.mli\\.5")
+                          "with-stdout-to opamBase64Compat.mli.6"))))
                    (add-before 'build 'pre-build
                      (lambda* (#:key inputs make-flags #:allow-other-keys)
                        (let ((bash (assoc-ref inputs "bash"))