Skip to content
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

[new-release] ppx_deriving_yojson 3.8.0 #25720

Merged
merged 4 commits into from
May 24, 2024

Conversation

NathanReb
Copy link
Contributor

@NathanReb NathanReb commented Apr 24, 2024

Changes

@NathanReb
Copy link
Contributor Author

I'm creating this as a draft first to get the result of the revdep builds.

@NathanReb NathanReb force-pushed the release-ppx_deriving_yojson.3.8.0 branch 2 times, most recently from d34f67d to 1466509 Compare April 24, 2024 09:22
@NathanReb
Copy link
Contributor Author

NathanReb commented Apr 24, 2024

I'm going through the errors here:

  • goblint & co: This is a legit error here, similar to what happened with ppx_deriving, we broke the Ppx_deriving_yojson_runtime API when dropping the result compat package dependency. I submitted a fix in Restore Ppx_deriving_yojson_runtime.Result for compatibility ocaml-ppx/ppx_deriving_yojson#157
  • morbig & co: Same as above, same fix.
  • satyrographos: Same
  • safemoney: they were trying to use ppx_deriving_yojson on exceptions which was silently failing so far. The port to ppxlib turned that silent error into an actual one which is causing the build failure. I'll update the upper bound in the relevant opam files and already reported this issue upstream: Misuse of [@@deriving yojson] on exceptions gborough/safemoney#5
  • pa_ppx: The test suite fails, I haven't quite put my finger on the exact reason yet but it does seem to be a legitimate error so I'll look into it.

Other failures (framac-lang, links-mysql, ocluster and sihl) seem unrelated.

@NathanReb
Copy link
Contributor Author

I've looked into pa_ppx's test failures both here and with ppx_deriving. It seems the issue comes from how the rewriters are applied in the test suite. I didn't quite get how they were running ppx-es there but it seems the input name is not properly set for ppxlib.

I'll simply add an upper bound here as if a fix is needed, it's either on pa_ppx or ppxlib's side.

Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
This will prevent build breakage following a fix in ppx_deriving_yojson
that now makes it properly report errors when `[@@deriving yojson]` is
used on nodes it does not support, such as exception declaration.

Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
@NathanReb
Copy link
Contributor Author

morbig still does not build but this time it seems to be unrelated to this release! I'll undraft this!

Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
@NathanReb NathanReb marked this pull request as ready for review May 23, 2024 09:03
@mseri
Copy link
Member

mseri commented May 24, 2024

pa_ppx_q_ast requires a lower bound on pa_ppx, from which it takes pcre2 transitively. I will send a fix separately. Thanks

@mseri mseri merged commit 7f4efa6 into ocaml:master May 24, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants