Skip to content

Preview release of ppxlib.0.37.0 with 5.4 compat #27867

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 27, 2025

Conversation

NathanReb
Copy link
Contributor

We're releasing from a 0.37 branch this time as main already includes the 5.3 AST bump which would make it harder to test OCaml 5.4 as it might break a few rev deps.

I'm doing this as a preview for now as we might want to incorporate a couple other features in the actual 0.37.0 release.

CC @patricoferris.

@mseri
Copy link
Member

mseri commented May 19, 2025

@mtelvers something seems of with the ci on the lint job, it was working a few days ago

@shonfeder
Copy link
Contributor

Hi @NathanReb, :) is this still pending possible additions?

@NathanReb
Copy link
Contributor Author

What do you mean @shonfeder ? This is a preview version to help testing OCaml 5.4. We'll delete this when the actual 0.37.0 release comes out.

@NathanReb
Copy link
Contributor Author

NathanReb commented May 20, 2025

If you're talking about this PR: no, I think it's pretty much ready to go.

If you're talking about ppxlib.0.37.0: yeah, we're hoping to include other features in there eventually!

@shonfeder
Copy link
Contributor

Sorry for being unclear and confused. I was referring to this PR.

Are the revdeps failures here expected?

@mseri
Copy link
Member

mseri commented May 26, 2025

There is only one failure that I am worried about:

#=== ERROR while compiling vlt.0.2.5 ==========================================#
# context              2.3.0 | linux/x86_64 | ocaml-base-compiler.4.14.2 | file:///home/opam/opam-repository
# path                 ~/.opam/4.14/.opam-switch/build/vlt.0.2.5
# command              ~/.opam/opam-init/hooks/sandbox.sh build dune build -p vlt -j 255 @install
# exit-code            1
# env-file             ~/.opam/log/vlt-7-050377.env
# output-file          ~/.opam/log/vlt-7-050377.out
### output ###
# File "ppx/dune", lines 2-11, characters 0-162:
#  2 | (rule
#  3 |  (deps (:< vlt_ppx.ml.cppo))
#  4 |  (targets vlt_ppx.ml)
#  5 |  (action
#  6 |   (with-stdout-to
#  7 |    %{targets}
#  8 |    (run %{bin:cppo} -V "PPXLIB:%{version:ppxlib}" %{<})
#  9 |   )
# 10 |  )
# 11 | )
# (cd _build/default/ppx && /home/opam/.opam/4.14/bin/cppo -V PPXLIB: vlt_ppx.ml.cppo) > _build/default/ppx/vlt_ppx.ml
# Error: Invalid version specification: "PPXLIB:"

Is this a problem in ppxlib, cppo or something else?

@NathanReb
Copy link
Contributor Author

I don't think this is coming from ppxlib!

@NathanReb
Copy link
Contributor Author

I tried reproducing this by building vlt locally but everything just builds fine, both on vlt#main and 0.2.5.

@NathanReb
Copy link
Contributor Author

Any way we can see the full logs? I'm curious to see what %{version:ppxlib} gets expanded to. I had suprising results running it locally, it does not seem to use the opam version.

@NathanReb
Copy link
Contributor Author

According to the dune documentation on the matter (https://dune.readthedocs.io/en/stable/advanced/package-version.html), I think it might not be very well suited to preview releases such as this one where we simply point to the github source archive for a regular commit.

I'm still surprised it would produce something that would get rejected by cppo though.

@mseri
Copy link
Member

mseri commented May 27, 2025

Thanks for looking into it. You'll get some very partial testing from the CI with this issue in place, but this is has the avoid-version flag so should not create disruption outside people working on bleeding edge 5.4 development (which are expecting troubles anyway 😅). Should we go on with merging or you want to see if there is a solution for this?

@NathanReb
Copy link
Contributor Author

Let's go ahead and merge, as you point out, this is mostly for 5.4 testing. If the issue arises again when I go through the actual 0.37.0 release, I'll investigate this more thoroughly but I suspect it won't!

@mseri mseri merged commit 92661d7 into ocaml:master May 27, 2025
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants