Skip to content

Bump to (OCaml4.14 + Core.v0.16) / (OCaml5.2 + Core.v0.17) - #912

Merged
SanabriaRusso merged 6 commits into
masterfrom
lyh/snarky-5.2.1
Aug 5, 2026
Merged

Bump to (OCaml4.14 + Core.v0.16) / (OCaml5.2 + Core.v0.17)#912
SanabriaRusso merged 6 commits into
masterfrom
lyh/snarky-5.2.1

Conversation

@glyh

@glyh glyh commented Nov 29, 2025

Copy link
Copy Markdown
Member

This is part of the bigger plan bumping Mina to OCaml 5.2.1 with Corev0.17.1

@glyh
glyh force-pushed the lyh/snarky-5.2.1 branch 2 times, most recently from 06367b7 to 99aff38 Compare November 29, 2025 14:18
@glyh
glyh marked this pull request as draft November 29, 2025 14:20
@glyh
glyh marked this pull request as ready for review November 30, 2025 00:44
@glyh
glyh marked this pull request as draft December 4, 2025 04:23
@glyh glyh changed the title Bump to OCaml 5.2.1 with Core.v0.17.1 series Bump to (OCaml4.14 + Core.v0.16) / (OCaml5.2 + Core.v0.17) Dec 6, 2025
@glyh
glyh force-pushed the lyh/snarky-5.2.1 branch from 850a6c3 to 3cac602 Compare January 22, 2026 11:48
Comment thread group_map/group_map.ml
end

module Make
(Constant : Field_intf.S) (F : sig

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are auto format

@glyh
glyh force-pushed the lyh/snarky-5.2.1 branch from 3cac602 to 384e29a Compare March 20, 2026 08:28
@glyh
glyh force-pushed the lyh/snarky-5.2.1 branch from 384e29a to 517a716 Compare March 20, 2026 08:40
@glyh
glyh marked this pull request as ready for review July 27, 2026 08:30
@dannywillems

Copy link
Copy Markdown
Contributor

FWIW: OCaml 5.2.x is already 2 years old, see https://ocaml.org/releases. OCaml 5.5.0 brings a set of interesting features, and there have been other improvements since 5.2.x.

@glyh

glyh commented Aug 2, 2026

Copy link
Copy Markdown
Member Author

We're bumping up step by step. But I'd love to bump to 5.5.0 if there's no obvious blockers as well -- it just need to be investigated.

Thanks for the recommendation @dannywillems :)

dkijania
dkijania previously approved these changes Aug 4, 2026
@dkijania

dkijania commented Aug 4, 2026

Copy link
Copy Markdown

@glyh question if existing tests are enough to cover this bump?

@glyh

glyh commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

@dkijania Good question — I went through the full diff to answer it. I believe the existing tests are sufficient for this bump, for two reasons:

1. The changes are mechanical, not semantic. The ~430 changed lines fall entirely into these buckets:

  • Core_kernelCore renames (Core v0.17 merged Core_kernel into Core) and Caml.*Stdlib.*;
  • Sys.getenv_opt → Core's Sys.getenv (same string option semantics) and Sys.getcwdSys_unix.getcwd;
  • ocamlformat 0.28 reformatting (functor argument indentation etc.);
  • dune/opam constraint bumps, CI and Dockerfile updates.

There are no algorithmic or data-structure changes, so what needs coverage is "does everything still compile and behave the same", not new logic.

2. What the suite exercises is the right regression surface for this kind of change. dune runtest runs: the src/tests Alcotest suite (Checked/Typ/As_prover machinery over the pure-OCaml test backend + the Fermat example), inline let%tests across snark0, utils, sponge, group_map and snarkette/fields, the fold_lib/interval_union suites, and the sponge Poseidon test vectors — the last one being a good end-to-end check that the Core bump didn't perturb hashing.

Two caveats, one of which is now addressed:

  • CI originally only built 5.2.1 after this PR, while the opam constraints still accept Core v0.16. I've added a symmetric CI matrix (c76de53) that pins the toolchain pairs explicitly — OCaml 5.2.1 + core.v0.17.2 and OCaml 4.14.2 + core.v0.16.2 — so both supported combinations are built and tested on every PR.
  • The ultimate acceptance test is building Mina against this branch, since snarky's own tests run on a dummy backend rather than the real Kimchi integration. That part is outside this repo's suite.

dkijania
dkijania previously approved these changes Aug 4, 2026
Pin core explicitly in each matrix leg — Core v0.17 also solves on
OCaml 4.14, so without the pin both legs would test the same series.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@glyh
glyh force-pushed the lyh/snarky-5.2.1 branch from 3ea6535 to 81e0652 Compare August 4, 2026 11:04
@SanabriaRusso
SanabriaRusso self-requested a review August 5, 2026 07:08
@SanabriaRusso
SanabriaRusso merged commit 3d1d68f into master Aug 5, 2026
5 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.

4 participants