Skip to content

ocamlPackages: fix eval and let ci check that it still evals#406555

Merged
wolfgangwalther merged 4 commits intoNixOS:stagingfrom
jopejoe1:fix-ocamlPackages
Oct 18, 2025
Merged

ocamlPackages: fix eval and let ci check that it still evals#406555
wolfgangwalther merged 4 commits intoNixOS:stagingfrom
jopejoe1:fix-ocamlPackages

Conversation

@jopejoe1
Copy link
Member

@jopejoe1 jopejoe1 commented May 12, 2025

The OCaml package set currently does not get tested by CI or build by hydra, let's change that so that no new errors are silently sneaking them self in.
Closes #277698
Closes #126934

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. label May 12, 2025
@nix-owners nix-owners bot requested a review from ulrikstrid May 12, 2025 19:55
@github-actions github-actions bot added 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. labels May 12, 2025
@nix-owners nix-owners bot requested a review from atagen May 12, 2025 20:00
@jopejoe1 jopejoe1 force-pushed the fix-ocamlPackages branch from 7a2d709 to 2a9d909 Compare May 13, 2025 19:10
@jopejoe1 jopejoe1 changed the base branch from master to staging May 13, 2025 19:10
@jopejoe1 jopejoe1 requested a review from vbgl May 13, 2025 19:26
@vbgl
Copy link
Contributor

vbgl commented May 13, 2025

A few packages do not build. It could be better to disable them. For instance ocamlPackages.janeStreet_0_15.typerep.

@jopejoe1
Copy link
Member Author

Marked the ones failing to build with nixpkgs-review as broken

@jopejoe1 jopejoe1 force-pushed the fix-ocamlPackages branch from f714544 to 3e3089f Compare June 20, 2025 22:10
@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Jul 25, 2025
@jopejoe1 jopejoe1 added 6.topic: cross-compilation Building packages on a different platform than they will be used on 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions and removed 6.topic: cross-compilation Building packages on a different platform than they will be used on labels Jul 25, 2025
@jopejoe1
Copy link
Member Author

jopejoe1 commented Jul 25, 2025

@wolfgangwalther now with #426629 this would fail CI, as OCaml packages use a throw to chech which version they are compatible with Any idea how this could be otherwise handled in ocamlPackages so that it conforms to the better ci?

@jopejoe1 jopejoe1 force-pushed the fix-ocamlPackages branch from 3e3089f to 3ad3598 Compare July 25, 2025 21:01
@wolfgangwalther
Copy link
Contributor

now with #426629 this would fail CI, as OCaml packages use a throw to chech which version they are compatible with Any idea how this could be otherwise handled in ocamlPackages so that it conforms to the better ci?

I dealt with a similar cases in #426640 for luaPackages. I also have a local branch looking into more of these kinds of errors for python3Packages, which makes heavy use of disabled as well. I was experimenting with marking disabled = true packages as meta.broken at the same time. Depending on where the assert is placed, this could already be enough.

I'm still investigating these things myself, so there's not a 100%-ish "do it like this" answer, yet. Open to suggestions, too.

@nixpkgs-ci nixpkgs-ci bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Aug 10, 2025
@nixpkgs-ci nixpkgs-ci bot removed 2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions labels Aug 19, 2025
@vbgl
Copy link
Contributor

vbgl commented Aug 19, 2025

To the best of my knowledge, packages marked with meta.broken are meant to be fixed, isn’t it?

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Oct 12, 2025
@jopejoe1
Copy link
Member Author

Would like to get this merged soonish if no-one else wants to take a stab at implementing this in a better way, following this comment from @wolfgangwalther.

Copy link
Contributor

@wolfgangwalther wolfgangwalther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see anyone coming up with a more principled approach, so let's do it this way.

Went through the changes, got some feedback.

@nixpkgs-ci nixpkgs-ci bot added 12.approvals: 3+ This PR was reviewed and approved by three or more persons. and removed 12.approvals: 2 This PR was reviewed and approved by two persons. labels Oct 13, 2025
@jopejoe1 jopejoe1 force-pushed the fix-ocamlPackages branch 2 times, most recently from 6078605 to 2f3051a Compare October 18, 2025 19:12
@nix-owners nix-owners bot requested a review from akshatagarwl October 18, 2025 19:17
@jopejoe1
Copy link
Member Author

This gets way too many merge conflicts. I would really like to get this merged soon.

@wolfgangwalther
Copy link
Contributor

I'm happy with commits 3 & 4. I think we are very close, the first two commits should be fixable. I'm happy to merge soon once they are.

@wolfgangwalther
Copy link
Contributor

Thank you for this PR and all the other small fixes to release stuff / recursion etc. - very much appreciated.

@wolfgangwalther wolfgangwalther added this pull request to the merge queue Oct 18, 2025
Merged via the queue into NixOS:staging with commit 683d4c3 Oct 18, 2025
26 of 30 checks passed
@jopejoe1 jopejoe1 deleted the fix-ocamlPackages branch October 18, 2025 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 1001-2500 This PR causes many rebuilds on Darwin and should most likely target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 1001-2500 This PR causes many rebuilds on Linux and should target the staging branches. 12.approvals: 3+ This PR was reviewed and approved by three or more persons.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants