-
Notifications
You must be signed in to change notification settings - Fork 329
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
Fixup constraints #2760
base: main
Are you sure you want to change the base?
Fixup constraints #2760
Conversation
Older versions like 0.1.1 exit fail with this error: ``` File "tool/ood-gen/lib/success_story.ml", line 13, characters 12-79: 13 | [@@deriving of_yaml, stable_record ~version:t ~add:[ slug; body_md; body_html ]] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Error: Ppxlib.Deriving: 'of_yaml' is not a supported type deriving generator ```
Previous versions fail with ``` File "tool/ood-gen/lib/academic_institution.ml", line 41, characters 53-79: 41 | str ^ "T00:00:00+00:00" |> Ptime.of_rfc3339 |> Ptime.rfc3339_string_error ^^^^^^^^^^^^^^^^^^^^^^^^^^ Error: Unbound value Ptime.rfc3339_string_error ```
The old version 0.10.6 fails with ``` File "src/ocamlorg_web/lib/ocamlorg_web.ml", line 12, characters 35-69: 12 | Mirage_crypto_rng_lwt.initialize (module Mirage_crypto_rng.Fortuna); ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Error: This expression is packed module, but the expected type is unit ``` So this adds the dependency to the build files as well as to the OPAM files.
8d7d045
to
cb3fba3
Compare
Since we're managing dependency versions via a pin on opam-repository, can we close this now? @cuihtlauac |
I'm tempted to merge this one. I find it nice to have a .opam file that allows building without a switch. |
this is not enabled by this patch, since it's all just lower bounds and the upper bound is provided by the opam-repository pin |
@Leonidas-from-XIV told me he successfully built without creating a local switch and without a repo pin, only adding this PR |
I think this is trying to solve a problem that shouldn't exist in the first place, since everyone should be developing against the set of dependencies we're deploying with - in order to avoid wasting time debugging inconsistencies or bugs with dependencies in different people's switches |
I still think that the dependencies are wrongly described in that case. If the versions are fixed then the dependencies should either be removed from the Futhermore, pinning the opam-repository only fixes the upper bound but this PR fixes the lower bound, so the problem I ran into is still possible even when using a pin. |
I’ve been discussing this matter with @mtelvers and @shonfeder. Thanks to them, I’m now convinced we should move away from the repo pin and pin the docker image instead. That will enable caching opam switch building stage, which should speed up the ocaml-ci builds. When building in Docker, either with pinned repo or pinned image, we don’t need any versionning information, what’s under the pin's cap does the job. What’s needed is a way to have the same switch as in Docker when building outside Docker, for day-to-day development. Maybe, a GitHub action could derive an Opam switch dump when there's a pin change and create a PR out of that. Something automatic would be handy. Maintaining consistency on this is a weak spot in our set up. |
While updating an older OPAM switch I realized that the versions that are installed are too old to successfully compile, so I had to add a few constraints on versions and add dependencies that were not declared before.