-
Notifications
You must be signed in to change notification settings - Fork 585
refactor node config to remove use of optcomp #18204
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
base: compatible
Are you sure you want to change the base?
Conversation
ea52977 to
ca4844f
Compare
| | p -> | ||
| failwithf "Invalid dune profile %s" p () | ||
|
|
||
| include (val profile_to_use) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trick I learnt from src/lib/verifier/verifier.ml
| "ppx_bitstring.4.1.0" | ||
| "ppx_deriving.5.2.1" | ||
| "ppx_deriving_yojson.3.6.1" | ||
| "ppx_optcomp.v0.14.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only removed from roots instead of installed as well. It's used as a transitive deps for another lib for now. We'll get rid of it when bumping to corev0.16
No rebuild of toolchain needed here because technically we're still using the same set of packages.
| (pps | ||
| ppx_mina | ||
| ppx_version | ||
| ; ppx_jane except ppx_optcomp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this comment is not relevant so might as well delete it.
| (rule | ||
| (target comptime.ml) | ||
| (deps | ||
| (sandbox none) | ||
| (:< gen.sh) | ||
| (universe)) | ||
| (action | ||
| (run bash %{<} %{target}))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trick I learnt from src/lib/mina_version/normal/dune
ca4844f to
3590d4f
Compare
| dune_profile_val="None" | ||
| fi | ||
|
|
||
| printf 'let dune_profile : string option = %s\n' "$dune_profile_val" > "$1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the ugliest part of this solution. We could potentially use https://github.com/stedolan/ppx_stage. But that lib is too old and we need to have it update to date.
Right now I guess this hack is fine, as it's already used in mina_version.normal
26c5914 to
f1f74e6
Compare
971b975 to
8bdd1b6
Compare
|
|
||
| buildCommand = '' | ||
| mkdir -p $out/etc/coda/build_config | ||
| cp ${../src/config}/mainnet.mlh $out/etc/coda/build_config/BUILD.mlh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/etc/coda/build_config/BUILD.mlh will no longer be present, added a changelog.
8b29b67 to
869bfdd
Compare
869bfdd to
055e66a
Compare
|
!ci-build-me |
dkijania
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from infra/test related changes. Thanks fro removing mlh files from debians
As title.
This is actually an effort to bump to OCaml 5, which requires bumping core libs which in turn requires bumping ppx_optcomp.
ppx_optcomp is completely redesigned to another library and could not be used by us anymore, hence this refactor.
It turns out we can just go one step further and make the comp time dispatch to runtime and we can use same build for different profiles. This, with some CI effort, could divide the time it takes for building stuff by the number of profiles we have. I believe that number is 3/4 now.
Bonus: