-
Notifications
You must be signed in to change notification settings - Fork 414
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
fix: package management working with ocaml.5.3.0 #11310
Conversation
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.
Looks good, it's nice that we can solve it by implementing an existing TODO
comment, so it shows we're on the right path.
Some thinks might need to be made a little bit nicer though.
There's no need to log changes to components that are advertised as experimental |
Unrelated changes should be moved to separate PR's |
4676a05
to
7633f6b
Compare
Signed-off-by: Etienne Marais <[email protected]>
Signed-off-by: Etienne Marais <[email protected]>
7633f6b
to
c0f9083
Compare
* fix: allow ocaml.5.3.0 to be built with package management Signed-off-by: Etienne Marais <[email protected]> * fix: apply reviews Signed-off-by: Etienne Marais <[email protected]> --------- Signed-off-by: Etienne Marais <[email protected]>
…1365) Signed-off-by: Etienne Marais <[email protected]>
* fix: allow ocaml.5.3.0 to be built with package management Signed-off-by: Etienne Marais <[email protected]> * fix: apply reviews Signed-off-by: Etienne Marais <[email protected]> --------- Signed-off-by: Etienne Marais <[email protected]>
The release of
ocaml.5.3.0
introduces a new transitive dependency for the compiler:ocaml
->ocaml-base-compiler
->ocaml-compiler
. Consequently the solution is to markocaml-compiler
as a toolchain element and compute the transitive dependencies to be able to fetch the binaries before installing. It also creates a newconfig.status
file to make the target understand the compiler is already installed (file required by5.3.0
).Fixes #11309 (with the deep help of @Leonidas-from-XIV and @art-w)
(Take the opportunity to remove the unused
ocaml
function at the same time).This PR will require to release a new version of Dune to make package management work with it.
@rgrinberg should we start to log this kind of changes in a lock file, it would be important for this release. WDTY?