-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Release ppxlib and ppxlib-tools 0.36.2 #28610
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
Conversation
|
There's this error in I'm not quite sure where that comes from, it seems dune's expanding ppxlib's version to the empty string somehow. |
|
I added upper bounds on the |
d405516 to
98ee139
Compare
|
Looking at the CI it seems the test were broken on ppx_factory and ppx_deriving_jsonschema before this release! |
98ee139 to
307bd6a
Compare
|
The failure on ppx_show is unrelated to this release! |
|
ping @ocaml/opam-repository! I'd like to get this in before a final release of the 5.4 compatible 0.37.0 release! I don't think any of the remaining errors are tied to these changes but I might have missed something! |
|
are there any reasons why this is still a draft PR? |
Because I forgot to undraft it... My bad |
Can you check that the generated META file that is installed mentions the version? |
307bd6a to
a4bb466
Compare
|
Indeed, when I run Should we explicitly set the version in the dune-project when releasing? That's probably something we should fix in dune-release somehow as manually setting it isn't ideal. |
|
Indeed that seems to fix the META files! It does result in adding the version field to the opam files as well which I thought was discouraged now, am I correct? |
CHANGES: - Make Ast_builder's default `value_binding` constructor generate the proper `pvb_constraint` from the pattern and expression arguments. (ocaml-ppx/ppxlib#589, @NathanReb) - Fix pprintast to output correct syntax from `Ppat_constraint (pat, Ptyp_poly ...)` nodes until they are completely dropped. (ocaml-ppx/ppxlib#588, @NathanReb)
a4bb466 to
b75aaf2
Compare
|
dune-release does strip the version field from the opam files for the release so that's all good. |
|
|
Nice to see the version issue is fixed! |
|
Hi there, after this upgrade building package fstar on OCaml 4.14.2 fails with: See here. It seems like 4.14.2 does not have this syntax for the mod operator. Should the OCaml version constraint here be tighter? (I'm not familiar with this syntax, not sure when it was introduced.) |
I'll take a look and see if I can reproduce this and track down the cause but from the look of it, I'm not sure that strictly ppxlib's doing. I need to know a bit more about fstar first! |
|
@smorimoto to circle back on those errors, The other two packages though seem to have had those test failures before ppxlib.0.36.2, I assume their test suite have been unmaintained for a little while! I can take a look and send PRs upstream if that helps with opam's revdeps CI in the future. |
@NathanReb fstar.2025.08.07 and fstar.2025.09.04 fails on ocaml.4.14.2, ocaml.5.0.0, ocaml.5.1.1 after the new release of ppxlib |
F*'s build is a bit complicated, but I think what's happening here is that we used to print |
|
Can you open an issue on ppxlib please? That'll give us a more suited discussion channel for this! |
|
Should we add a conflict at least to fstar here on the repo in the meantime? |
|
@mtzguido wrote:
This |
When was this change introduced in ppxlib? |
|
We didn't properly update our copy of Note that this should not impact regular ppx users as we don't use the driver's source output but instead marshall out the AST. From a quick look it seems fstar uses ppxlib's library for some Ocaml code generation which is leading to this. One possible fix for this in fstar would be to pretty print using the compiler's pprintast, i.e. migrate the AST to the installed compiler version and use its Pprintast to make sure the syntax is compatible. Note that this bit you with the identifier compat feature but it could also for other syntax changes. |
|
This is the raw literal notation added in 5.2 to make it possible to use an old library using a new keyword (aka |
|
In the meantime, in ppxlib, we could prevent this syntax from being used pre 5.2 which would fix the issue for fstar. |
|
I've made a patch here: NathanReb/ppxlib@230ffe1 just in case but I'm not quite sold on what's the best approach from ppxlib's perspective here. |
|
See also ocaml/ocaml#14279 for the pretty-printer patch that makes sure to print |
There is currently a problem with OCaml's pprintast/ppxlib in that it generates "raw identifiers" like `\#module` which are not valid in OCaml below 5.2.0 See ocaml/opam-repository#28610 (comment) for some discussion. I think this will be fixed upstream, but bumping to OCaml 5.3.0 seems useful anyway. (We can also work around it by using `" module"` instead of `"module"` in lib/GenCtypes.ml, but that's nasty.)
|
Thanks! I'll incorporate that in ppxlib's version! |
|
@lukstafi also noticed some new errors with this new release: https://opam.ci.ocaml.org/github/ocaml/opam-repository/commit/31317f993252923ac49cad459b61d7caaf5f9db2/variant/compilers,5.3,ppx_minidebug.2.4.0,tests |
|
@kit-ty-kate this is a result of the inital bug fix justifying this release! This is a follow up to #28160 which contains the changes leading to the diff you linked. We did not properly update our internal I think what happened here is that I'm sorry for the mess this caused. I think 0.36.0 being an AST bump release and therefore breaking so much ppx-es, it made it easy to let some of those errors slip through as we basically had no revdeps build at the time. |
|
should 0.36.0 be made unavailable (or at the very least |
|
I guess we could do that yes! |
See ocaml#28610 Signed-off-by: Marcello Seri <[email protected]>
|
Done in #28663, thanks! I am adding avoid version so far, to avoid disrupting too much for the people that don't see these issues |
As per usual, starting with a draft to see how revdeps look like.
This should be an improvement over the failed attempt at releasing 0.36.1 as users are less susceptible of producing nodes that will be incorrectly printed.
It is still possible to manually construct them but
Ast_builderusers should not have that problem anymore.Changes
value_bindingconstructor generate the properpvb_constraintfrom the pattern and expression arguments.(Fix Ast_builder's
value_bindingto generate the correctpvb_constraintocaml-ppx/ppxlib#589, @NathanReb)Ppat_constraint (pat, Ptyp_poly ...)nodes until they are completely dropped. (Fix pprintast with ppat_constraint in let bindings ocaml-ppx/ppxlib#588, @NathanReb)