-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
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
ocamlPackages.memprof-limits: init at 0.2.1 #318800
Conversation
Signed-off-by: Ali Caglayan <[email protected]>
Co-authored-by: Vincent Laporte <[email protected]>
This does not build with OCaml 5.2. It might be nice to throw an evaluation-time error. |
Yes, it seems we need to restrict < 5.0. |
@vbgl I've added a check. Do you think it would be better if we introduced maximalOCamlVersion in buildDunePackage? |
It sounds like a good idea. In particular, this would help avoiding silly mistakes such as swapping the arguments to |
pkgs/build-support/ocaml/dune.nix
Outdated
@@ -8,7 +8,8 @@ let Dune = | |||
; in | |||
|
|||
if (args ? minimumOCamlVersion && lib.versionOlder ocaml.version args.minimumOCamlVersion) || | |||
(args ? minimalOCamlVersion && lib.versionOlder ocaml.version args.minimalOCamlVersion) | |||
(args ? minimalOCamlVersion && lib.versionOlder ocaml.version args.minimalOCamlVersion) || |
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.
Is it correct that our minimal ocaml version bound is strict?
Please leave the refactoring of |
@vbgl OK, I'll put that in a separate PR then. |
While you’re at it, please double-check what happens to |
I've dropped the commit touching buildDunePackage (I'll handle that in another PR) and updated the version check. I am using versionOlder and the arguments should be in the correct order this time. |
Signed-off-by: Ali Caglayan <[email protected]>
Co-authored-by: Vincent Laporte <[email protected]>
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.