Skip to content
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

Merged
merged 6 commits into from
Mar 26, 2025

Conversation

Leonidas-from-XIV
Copy link
Contributor

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 (like ocaml-lsp-server), for the rest I can make PRs ustream if this works fine.

@mseri
Copy link
Member

mseri commented Mar 15, 2025

looks like llvm packages break if you build them with -p
Do you know why?

@Leonidas-from-XIV
Copy link
Contributor Author

@mseri Hmm, looks like the CI logs are gone, I'll rebase (and remove the opsian fixes) and check the results.

@Leonidas-from-XIV
Copy link
Contributor Author

Leonidas-from-XIV commented Mar 19, 2025

@mseri The package does something strange, it does a rm llvm.install and then calls install.sh which does dune install. I've simplified it to just let build create the llvm.install and will see what the CI thinks about this.

@Leonidas-from-XIV
Copy link
Contributor Author

That seems to have helped. There's some lower bounds failures and FreeBSD failures but it seems fairly unrelated and probably a separate issue.

@mseri
Copy link
Member

mseri commented Mar 20, 2025

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

@Leonidas-from-XIV
Copy link
Contributor Author

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)

@mseri
Copy link
Member

mseri commented Mar 20, 2025

Thanks a lot! If you let me know once this change is merged I will add the upper bound

@mseri mseri merged commit e77ebb8 into ocaml:master Mar 26, 2025
1 of 3 checks passed
@mseri
Copy link
Member

mseri commented Mar 26, 2025

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants