-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Use -p
instead of --release
in Dune builds
#27607
Conversation
b250433
to
a6262c8
Compare
looks like llvm packages break if you build them with -p |
5a50ee0
to
675b498
Compare
@mseri Hmm, looks like the CI logs are gone, I'll rebase (and remove the opsian fixes) and check the results. |
@mseri The package does something strange, it does a |
That seems to have helped. There's some lower bounds failures and FreeBSD failures but it seems fairly unrelated and probably a separate issue. |
I think it is all right with most of the packages to merge this change but I am worried about llvm. It is a complex package with complex build process that grew up workign around issues. I'd rather not touch llvm and instead have an upper bound on dune for those packages. The maintainers can fix this and remove the upper bounds in the future once they make a new release |
Ok, fair enough, I've removed the changes from llvm. The new versions of the package don't use dune, so I doubt that this will be a problem in the future. (Wouldn't add a constraint on <3.18 just yet, we haven't merged the feature, this PR is mostly to prepare things) |
Thanks a lot! If you let me know once this change is merged I will add the upper bound |
Thanks |
We plan to slightly change the semantics of
--release
in Dune going forward (ocaml/dune#11378) thus we took a look whether and how it is used in OPAM repository.My conclusion is that most packages can be just moved to
-p
(which implies--release
) and it continues working fine. Thus I'm opening this PR to see if we can migrate the packages that use it off from--release
and towards-p
.Some of the projects already use
-p
in newer versions (likeocaml-lsp-server
), for the rest I can make PRs ustream if this works fine.