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: Upgrade ocamlformat and ppxlib #109288
Conversation
@@ -1,21 +1,30 @@ | |||
{ lib, fetchFromGitHub, buildDunePackage, ocaml | |||
, version ? if lib.versionAtLeast ocaml.version "4.07" then "0.15.0" else "0.13.0" | |||
, version ? if lib.versionAtLeast ocaml.version "4.07" then "0.18.0" else "0.13.0" |
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.
leave the default version at 0.15.0
, >= 0.16.0
breaks a lot of packages currently.
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.
To be precise this currently breaks:
dune-release jackline libbap patdiff python37Packages.bap python38Packages.bap python39Packages.bap satysfi
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.
Fixed, thanks
@@ -39,7 +41,23 @@ buildDunePackage rec { | |||
useDune2 = true; | |||
|
|||
buildInputs = | |||
if lib.versionAtLeast version "0.14" | |||
if lib.versionAtLeast version "0.15.1" |
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.
Any special reason for having 0.15.0 and 0.15.1?
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.
The only difference between 0.15.0 and 0.15.1 is the change in dependency (ocaml-migrate-parsetree vs ppxlib) but the exact version should be specified and checked into projects that use this tool. This means that every versions of OCamlformat are required.
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.
Alright fair, bit annoying that this means we specify a few not strictly required buildInputs for 0.15.1, but doesn't matter too much I think.
@GrahamcOfBorg build jackline dune-release libbap satysfi patdiff |
Result of 17 packages built:
|
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.
I think this is good, you might want to squash e2ccb362a04762c20dd1cfa181ba3441cf1e6767 and 70558af into one commit.
The default version is still 0.15.0 to avoid breaking other packages.
e2ccb36
to
b3da48e
Compare
Just squashed. |
Merged into master as bf010da. I’ve made two changes to the first commit. I’ve first increased the |
Thanks ! |
Motivation for this change
Add the newer versions of OCamlformat. Ppxlib is a dependency.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)