@Zimmi48, thanks for your PR! By analyzing the history of the files in this pull request, we identified @aske, @vbgl and @sternenseemann to be potential reviewers.

Just added octavius (see 0a4da64).

Can you find a sane name for newBuildOcamlJane? In particular, “build” in nixpkgs usually refers to “actually building the package (e.g. compiling it)” (dixit the manual).

Thanks for octavius!

I've renamed newBuildOcamlJane into janePackage.

From my point of view, this PR is mostly ready. I can use the packages defined in it for my own purpose, which was to build coq-serapi.
I still have two items on my checklist before merging:

• The first one is to check the OCaml version for the janeStreet set of packages. I don't know what is the recommended way to do that.
• The second is that ideally, we would want all packages which are new in this PR to be automatically added as ocamlPackages.newPackage in addition to ocamlPackages.janeStreet.newPackage. I suppose there are tricks to do that but I'm not sure which exactly.

Does this otherwise look good to you @vbgl @maurer?

For me this PR is now ready to merge. The solutions to the two last items I was referring to in my previous comment can be found in commits 4f0d1ff and 2a0ca7a (squashed since then). Do they look like acceptable solutions?

Once this is merged we can also package https://github.com/janestreet/patdiff; I’m in no position to review this though. @sternenseemann?

@Profpatsch I've just added to this PR a few packages which patdiff depends on. Not core_extended though because it seems that there is some script patching to do.

@Zimmi48 is it normal that some derivations still seem to have the 113.33 part in their name?

EDIT: Nevermind, I'm not sure I did this correctly...

@Ptival This PR does not update existing derivations, but rather introduce new ones at version 0.9 under the prefix ocamlPackages.janeStreet. So if you start by with ocamlPackages_latest; with janeStreet; you should get version 0.9 in priority, but if you just start by with ocamlPackages_latest; you will get version 133.33 in priority.

@Profpatsch This now includes core_extended which means that you have almost all the dependencies of patdiff (to the exception of pcre which is not a Jane Street package). The problem with core_extended was that a shebang needed patching. I had no idea that even /usr/bin/env becomes unavailable during the build of a derivation.

@maurer It now seems that ocamlPackages.janeStreet is a super-set of the previously available Jane Street packages. So should we remove the previously available versions (for OCaml 4.03)?

It now seems that ocamlPackages.janeStreet is a super-set of the previously available Jane Street packages. So should we remove the previously available versions (for OCaml 4.03)?

The clean-up could actually go much further following what was proposed in #18819. Like making 4.04 the default and keeping just one older version and only the older packages which are needed as dependencies to some other applications. But this is not the place of this PR to do that. So IMO it is fully ready to merge as-is.

@maurer It now seems that ocamlPackages.janeStreet is a super-set of the previously available Jane Street packages. So should we remove the previously available versions (for OCaml 4.03)?

seems sensible, but this might break things outside of nixpkgs and requires fixing in nixpkgs (which should be manageable)

Your fix for darwin does not change anything.

The issue with re2 might be an upstream bug: they explicitly require g++ when they should use the environment variable CXX (https://github.com/janestreet/re2/blob/master/src/re2_c/jbuild#L25).

About your last remark about patdiff: does this package provides a library or only some binaries? If it’s not a library, it may not belong to ocamlPackages.

Indeed, I think patdiff is only an application, not a library. Does @Profpatsch confirm? I'll move it elsewhere. What location do you suggest? tools/misc?

@vbgl Excuse my lack of knowledge but can you explain the general issue with g++, nixpkgs and Darwin. I just had a look at the manual and it indicates that g++ is made available as part of stdenv and it makes no mention of Darwin. I'm guessing that the issue is that g++ is actually platform specific. Does nixpkgs provide clang instead on Darwin as part of stdenv? In that case, I might try to patch the software depending on the platform. Or you can do it @vbgl since you are better positioned to test the fix (write access to this branch is granted to maintainers of nixpkgs).

@vbgl That my fix for Darwin was not working was not a mystery because I had forgotten to actually include gcc in the buildInputs! Now testing this solution again. If it doesn't work I'll try instead to patch the file you mentioned to explicitly use stdenv.cc.
I'm still really surprised that https://nixos.org/nixpkgs/manual/#sec-tools-of-stdenv makes no mention that GNU gcc can alternatively be clang.

I have now moved patdiff in tools/misc. @Profpatsch let me know if you think the location is OK.

Travis' verdict: this still fails on Darwin. The error changed though. One can see something like this:

ARFLAGS=rsc

CXX=g++

CXXFLAGS="-Wall -O3 -g -pthread"

if ! true; then

  CXX="$CXX -m32"

fi

/nix/store/x3raqnfvxl19nam0gjr8qcw0f0f26l7x-gnumake-4.2.1/bin/make -s -C libre2 clean

/nix/store/x3raqnfvxl19nam0gjr8qcw0f0f26l7x-gnumake-4.2.1/bin/make -s -C libre2 ARFLAGS="$ARFLAGS" CXX="$CXX" CXXFLAGS="$CXXFLAGS" obj/libre2.a obj/so/libre2.so

cp libre2/obj/libre2.a libre2_c_stubs.a

cp libre2/obj/so/libre2.so dllre2_c_stubs.so

/nix/store/x3raqnfvxl19nam0gjr8qcw0f0f26l7x-gnumake-4.2.1/bin/make -s -C libre2 clean

')

clang-4.0: warning: argument unused during compilation: '-idirafter /nix/store/xxbrmw7y5xl4cigq9p22b5jnji32ivnl-Libsystem-osx-10.11.6/include' [-Wunused-command-line-argument]

clang-4.0: warning: argument unused during compilation: '-idirafter /nix/store/m9rsrcndr9dfhi3rxc95kamdjc90dkkh-gcc-5.4.0/lib/gcc/x86_64-apple-darwin16.3.0/5.4.0/include-fixed' [-Wunused-command-line-argument]

and then pages later:

g++: error: unrecognized command line option '-stdlib=libc++'

make[1]: *** [Makefile:181: obj/so/libre2.so] Error 1

make: *** [Makefile:5: default] Error 1

This seems to indicate that even with the gcc dependency, the backend is still clang somehow.
I'll try patching re2 instead but I wonder if the problem is not that they use a g++ specific option.

Last try (patching this file to have it use $CXX instead of gcc). If this still doesn't work, I'll just add platform information to make re2, core_extended and patdiff Linux-specific.

OK here's what I get (on Travis, on Darwin):

clang-4.0: warning: argument unused during compilation: '-fno-strict-overflow' [-Wunused-command-line-argument]
cc src/stubs.o (exit 1)

(cd _build/default/src && /nix/store/ryxnhgxyvz4j4z6vmdrv20kj0kyi9gkh-clang-wrapper-4.0.0/bin/cc -I /nix/store/2l4jlirard4ky8wydixlgfhp5hb8j4lx-ocaml-4.03.0/lib/ocaml -O2 -fno-strict-aliasing -fwrapv -Wall -D_FILE_OFFSET_BITS=64 -D_REENTRANT -I /nix/store/v40yq1wig900ppf9kx31b2z15s9bxigj-ocaml4.03.0-core_kernel-0.9.0/lib/ocaml/4.03.0/site-lib/core_kernel -I /nix/store/wh5kh4wjmd4a1drrkvhxlc6rlgfb5z20-ocaml4.03.0-base-0.9.1/lib/ocaml/4.03.0/site-lib/base -I /nix/store/wr6yk1hk02zg4hq9f6ndgdz740wwwjpn-ocaml4.03.0-jane-street-headers-0.9.0/lib/ocaml/4.03.0/site-lib/jane-street-headers -g -I re2_c/libre2 -o stubs.o -c stubs.cpp)

stubs.cpp:1:10: fatal error: 'vector' file not found

#include <vector>

         ^~~~~~~~

1 error generated.

builder for ‘/nix/store/jjrc1rkc9dm93jaw2cw5y745qk0x7y16-ocaml4.03.0-re2-0.9.0.drv’ failed with exit code 2

Any idea how to fix this? Otherwise I'll just change the platform like I said earlier.

Adding gcc to re2’s buildInputs make the build go through. But this looks like a bad idea.

Moreover, it results in crazy linking errors in patdiff build.

I think it’s worth telling upstream about this issue.

About patdiff again: does the build inputs need to be propagated?

Adding gcc to re2’s buildInputs make the build go through. But this looks like a bad idea.
Moreover, it results in crazy linking errors in patdiff build.

I am making re2 platform specific for now because I don't want to delay this PR forever but it would be interesting to find a better solution for this. In particular, given that nixpkgs already packages Google's re2 (the C library), could it be used instead of the C sources shipped in the OCaml package? (Although for now, it is a bit outdated compared to the version Jane Street's re2 is based upon.)

I think it’s worth telling upstream about this issue.

Yes, although I'd be interested to have you or another OSX person do it, since the only way I could test things was through Travis (not the most practical).

About patdiff again: does the build inputs need to be propagated?

Indeed, they don't, I was just following the general model without really thinking. I've updated this.

@vbgl @sternenseemann Can someone merge this? Travis passed. I don't think there is any reason to delay any longer.

Done.

I've been working to get a project that depends on core_kernel 0.10.0, and ended up bumping many packages up to 0.10.0. Considering the sheer number of packages that need bumping moving forward, is there any consensus on how to organize the janeStreet set? Is mechanically updating all packages up to 0.10.0 viable compatibility-wise? In that case, I've already done half the job, so I believe it'll be relatively easy.

Updating all this package set to 0.10.0 at once seems a good idea. Would you like to give it a try?

I'll take a shot at it 😄

I've managed to get ocaml-ng.ocamlPackages_4_06.janeStreet built (excluding a few exceptions), but the problem now is that ocamlPackages (OCaml 4.04) fails to build across the board. I believe bumping ocamlPackages to 4.06 is one solution, but entails much more work than anticipated and probably breaks a multitude of packages. How do you think we should proceed?
@Zimmi48 @vbgl

Here's where I am right now: #35439