ocamlPackages.topkg: refactoring #32680
Conversation
| @@ -13,14 +17,15 @@ stdenv.mkDerivation rec { | |||
| sha256 = "1slrzbmyp81xhgsfwwqs2d6gxzvqx0gcp34rq00h5iblhcq7myx6"; | |||
| }; | |||
|
|
|||
| nativeBuildInputs = [ opam ]; | |||
There was a problem hiding this comment.
Conceptually this line should be left as is because it changes the opam used for cross compilation.
There was a problem hiding this comment.
I’m not sure to understand your comment. I removed this line because no opam stuff from the environment ($PATH) is used during the build. Instead, the absolute path to opam-installer is resolved by nix when evaluating the installPhase attribute.
I’ve no clue how cross-compilation works with nixpkgs but I guess it’s not just a matter of having opam for the build architecture: how to get an OCaml cross-compiler? how to run it alongside ocaml for the build architecture (to run the build system, itself written in OCaml)?
There was a problem hiding this comment.
I know that cross compilation for OCaml is not implemented in Nixpkgs, so there is no practical reason to keep this line, but the way it works is that during native compilation callPackage topkg/default.nix will be called once, and the value of nativeBuildInputs will have no effect on the value of opam, but during cross compilation it will be called again, with the value of nativeBuildInputs from the first call affecting the value of opam in the second call.
There was a problem hiding this comment.
This should solved later, when the cross-compiling stuff has landed in nixpkgs.
| @@ -4,6 +4,10 @@ if !stdenv.lib.versionAtLeast ocaml.version "4.01" | |||
| then throw "topkg is not available for OCaml ${ocaml.version}" | |||
| else | |||
|
|
|||
| let | |||
| run = "ocaml -I ${findlib}/lib/ocaml/${ocaml.version}/site-lib/ pkg/pkg.ml"; | |||
There was a problem hiding this comment.
You may want to leave a comment that run and buildPhase are part of the interface for use by other Ocaml packages and should be changed with care.
|
Thanks for the reviews. |
Motivation for this change
Factorize some code.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)