graphql-cohttp: ocaml-crunch -> crunch

[anmonteiro]

Description of changes

Use crunch from the current scope of packages rather than a global one that might import another OCaml version.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 22.05 Release Notes (or backporting 21.11 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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[vbgl]

Suggested change

	nativeBuildInputs = [ crunch ];
	nativeBuildInputs = [ crunch.bin ];

Not tested.

[Et7f3]

I don't see where is the magic but it seems superfluous #213222 (comment)

[vbgl]

Also what is the interest of this change?

[anmonteiro]

In a project that depends on ocamlPackages_4_x.graphql-cohttp where x is different than ocamlPackages.ocaml.version I want to avoid bringing in 2 OCaml compilers.

[vbgl]

ocaml-crunch does not “bring in” any OCaml compiler. It is a single binary that only depends on glibc (and co.).

[anmonteiro]

Maybe I should have phrased it better. ocaml-crunch is compiled by an OCaml compiler which needs to be built / downloaded in addition to the compiler one might be using, if not the same as ocamlPackages.ocaml.version.

[anmonteiro]

The same happens with opaline, btw, which is kind of annoying.

[Et7f3]

In a project that depends on ocamlPackages_4_x.graphql-cohttp where x is different than ocamlPackages.ocaml.version I want to avoid bringing in 2 OCaml compilers.

The issue is also in the other sense: ocaml5.0.0-mirage-block-unix-2.14.2 depends on opaline which is build by default with ocaml4.14

Most package that use dune use the dune of ocamlPackages. So this change look good.

Dune is defined inside ocamlPackages, could we define opaline the same way so we can avoid double inclusion ? I understand that opaline is top-level package because the behavior doesn't depends on ocaml version but it does imply more package to be build which is not specially optimal.

[vbgl]

IMHO the current design is strictly better.

[anmonteiro]

IMHO the current design is strictly better.

Why?

[Et7f3]

IMHO the current design is strictly better.

See the globally pinned issue:
#204303 (comment)

And opaline take a package set

nixpkgs/pkgs/development/tools/ocaml/opaline/default.nix

Line 1 in c6fd903

{ lib, stdenv, fetchFromGitHub, ocamlPackages }:

Unless I am misunderstanding we will need to change the current design. It is either non explicit (so against the goal of nix ?), non-cross compilation friendly.

One advantage of the current design is that only one version is built by hydra, so one variant of ocaml get also cached by hydra. In the described scenario we will need to build another variant (ocamlPackages_4_x.ocaml). So we are storing two compiler to avoid rebuilding a 2Mo package that should be quick to compile (only fives .ml for ocaml-crunch and one for opaline). Re-building should be as fast as fetching ocaml-crunch/opaline and should be more space efficient. We can keep the top-level attribute (like dune) for binary cache entry so we get the best of both world.

[vbgl]

we don't have binary cache

Why not? The ocaml-crunch package is about 2Mio.

[Et7f3]

we don't have binary cache

Why not? The ocaml-crunch package is about 2Mio.

Ah my bad It is accessible from top level so it get a binary cache entry and packages in ocamlPackages sub level doesn't get cached. Ok updated my argument.

I still think this change is a good thing but I think the correct patch is to find why it keep reference on ocaml used to build it.