janeStreet: 0.9.0 -> 0.10.0 [WIP] #35439

mt-caret · 2018-02-24T02:44:36Z

Motivation for this change

Attempt to update janeStreet packages.

Things done

Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- other Linux distributions
Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

vbgl · 2018-02-25T10:07:46Z

pkgs/development/ocaml-modules/janestreet/default.nix

@@ -34,8 +32,7 @@ rec {
    meta.description = "OCaml AST used by Jane Street ppx rewriters";
  } // (if lib.versionAtLeast ocaml.version "4.06"


This test should be dropped.

vbgl · 2018-02-25T10:10:01Z

pkgs/development/ocaml-modules/janestreet/default.nix

    propagatedBuildInputs = [ core_extended async ];
    meta = {
      description = "Shell helpers for Async";
    };
  };

+  # Broken due to ctypes build failure (upgrading ctypes may possibly work, but
+  # opens a world of pain)


I'll fix this when the PR (#35418) gets merged.

vbgl · 2018-02-25T10:14:13Z

pkgs/development/ocaml-modules/janestreet/default.nix

    propagatedBuildInputs = [ ppx_sexp_conv ];
    meta.description = "Binding to the posix *at functions";
-    meta.broken = lib.versionAtLeast ocaml.version "4.05";
+    meta.broken = !lib.versionAtLeast ocaml.version "4.05";


This line should be removed.

vbgl · 2018-02-25T10:14:34Z

pkgs/development/ocaml-modules/janestreet/default.nix

    propagatedBuildInputs = [ core_kernel ];
    meta.description = "Topological sort algorithm";
  };

  typerep_extended = janePackage {
    name = "typerep_extended";
+    version = "0.9.0";


Does it make sense to keep this package?

vbgl · 2018-03-01T08:13:58Z

ctypes has been updated. Can you please rebase?

mt-caret · 2018-03-02T02:39:46Z

Hmm, did I just mess up rebase? 😢

Ericson2314 · 2018-03-02T20:01:48Z

hehe yes. Use git rebase -i and try again.

mt-caret · 2018-03-03T02:58:48Z

I think I got it right this time.
@vbgl Sorry for the delay.

vbgl · 2018-03-03T09:56:18Z

pkgs/development/ocaml-modules/janestreet/janePackage.nix

@@ -1,6 +1,6 @@
 { stdenv, fetchFromGitHub, ocaml, jbuilder, findlib }:

-{ name, version ? "0.9.0", buildInputs ? [], hash, meta, ...}@args:
+{ name, version ? "0.10.0", buildInputs ? [], hash, meta, ...}@args:

 if !stdenv.lib.versionAtLeast ocaml.version "4.03"


Did you test with 4.03 ? I think 4.04 or above is required.

vbgl · 2018-03-03T15:11:13Z

@GrahamcOfBorg build ocamlPackages.bap

GrahamcOfBorg · 2018-03-03T15:14:55Z

Failure on x86_64-linux (full log)

Partial log (click to expand)

checking for references to /tmp/nix-build-ocaml4.04.2-core_kernel-0.10.0.drv-0 in /nix/store/v99rbljycd5b3p63kpp7qsqkbg2jzphg-ocaml4.04.2-core_kernel-0.10.0...
building path(s) ‘/nix/store/yqks4kxpgbmpnf9lq3sggad8nspldiih-ocaml-bap-1.4.0’
unpacking sources
unpacking source archive /nix/store/gch8cxi2xhh3dvjdd7nnza7708p9rd1q-source
source root is source
patching sources
configuring
configure flags: --prefix=/nix/store/yqks4kxpgbmpnf9lq3sggad8nspldiih-ocaml-bap-1.4.0 --enable-everything --disable-ida --disable-fsi-benchmark --with-llvm-config=/nix/store/9pylfc5v0a5mlg75i5wmd6sq2idfiya9-llvm-3.8.1/bin/llvm-config
builder for ‘/nix/store/ykjzvri5598i4kn1hx61rb833xyc082x-ocaml-bap-1.4.0.drv’ failed with exit code 1
error: build of ‘/nix/store/ykjzvri5598i4kn1hx61rb833xyc082x-ocaml-bap-1.4.0.drv’ failed

GrahamcOfBorg · 2018-03-03T15:17:05Z

Failure on aarch64-linux (full log)

Partial log (click to expand)

building '/nix/store/ns3bsw9nc0ni02k37li6d9wn3fpxqxwa-ocaml-bap-1.4.0.drv'...
unpacking sources
unpacking source archive /nix/store/gch8cxi2xhh3dvjdd7nnza7708p9rd1q-source
source root is source
patching sources
updateAutotoolsGnuConfigScriptsPhase
configuring
configure flags: --prefix=/nix/store/63zbi3rydzzfn1nasy72v99qc6pv42i3-ocaml-bap-1.4.0 --enable-everything --disable-ida --disable-fsi-benchmark --with-llvm-config=/nix/store/9f58zhm11iq8ja3y5d2lwhzkgcrfvm7z-llvm-3.8.1/bin/llvm-config
builder for '/nix/store/ns3bsw9nc0ni02k37li6d9wn3fpxqxwa-ocaml-bap-1.4.0.drv' failed with exit code 1
�[31;1merror:�[0m build of '/nix/store/ns3bsw9nc0ni02k37li6d9wn3fpxqxwa-ocaml-bap-1.4.0.drv' failed

vbgl · 2018-03-03T15:29:05Z

This PR breaks bad. That’s an annoying regression.

mt-caret · 2018-03-03T16:13:39Z

I guess the solution is to keep the 0.9.0 versions around (under a different name like janeStreet_0_9_0, perhaps) and use that version for all of bap's dependencies (and all of their dependencies, and so on). I have no idea how one will go about doing that or if that's even possible.

I've tried to inject the 0.9.0 versions of core_kernel and ppx_jane for bap, but of course that ends up with errors about other dependencies dependent on core_kernel/ppx_jane/etc. 0.10.0.

How should we proceed?

vbgl · 2018-03-03T17:20:18Z

Maybe ask upstream (bap) plans about compatibility with core_kernel-0.10.0?

vbgl · 2018-03-05T16:42:07Z

Possible solution: keep the old janestreet/default.nix file around (under a new name, fixing a few version strings that used to be implicit), import it (as before) under a specific attribute (say janeStreet_0_9_0) and use this attribute to feed the bap derivation (as in inherit (janeStreet_0_9_0) core_kernel ppx_jane parsexp).

This shouldn’t introduce regression and allow to update the whole bunch of janestreet libraries.

mt-caret · 2018-03-06T14:23:52Z

Thanks! I tried core_kernel and ppx_jane and it didn't work, but adding parsexp did the trick!

I think just keeping the old janestreet/default.nix works as a temporary workaround while we wait for bap upstream to upgrade to 0.10.0 (BinaryAnalysisPlatform/bap#786). As a bonus, if any other regressions pop up we can also fix them with janestreet/old.nix pretty easily.

vbgl · 2018-03-06T19:30:58Z

@GrahamcOfBorg build patdiff

GrahamcOfBorg · 2018-03-06T19:40:06Z

Failure on aarch64-linux (full log)

Partial log (click to expand)

shrinking /nix/store/ajlvy8hj05ph50a47zvysvc0v5zn50bg-ocaml4.04.2-re2-0.10.1/lib/ocaml/4.04.2/site-lib/stubslibs/dllre2_stubs.so
shrinking /nix/store/ajlvy8hj05ph50a47zvysvc0v5zn50bg-ocaml4.04.2-re2-0.10.1/lib/ocaml/4.04.2/site-lib/re2/c/re2_c.cmxs
shrinking /nix/store/ajlvy8hj05ph50a47zvysvc0v5zn50bg-ocaml4.04.2-re2-0.10.1/lib/ocaml/4.04.2/site-lib/re2/re2.cmxs
strip is /nix/store/lvx1acn1ig1j2km8jds5x3ggh3f2wa8v-binutils-2.28.1/bin/strip
stripping (with command strip and flags -S) in /nix/store/ajlvy8hj05ph50a47zvysvc0v5zn50bg-ocaml4.04.2-re2-0.10.1/lib
patching script interpreter paths in /nix/store/ajlvy8hj05ph50a47zvysvc0v5zn50bg-ocaml4.04.2-re2-0.10.1
checking for references to /build in /nix/store/ajlvy8hj05ph50a47zvysvc0v5zn50bg-ocaml4.04.2-re2-0.10.1...
cannot build derivation '/nix/store/nhh77gxxaajlq4lqc5k4xrpjc7ry6d39-ocaml4.04.2-core_extended-0.10.0.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/3h2568z057h71chsxk7g63k7kvczkrv6-ocaml4.04.2-patdiff-0.10.0.drv': 2 dependencies couldn't be built
�[31;1merror:�[0m build of '/nix/store/3h2568z057h71chsxk7g63k7kvczkrv6-ocaml4.04.2-patdiff-0.10.0.drv' failed

GrahamcOfBorg · 2018-03-06T19:40:09Z

Failure on x86_64-linux (full log)

Partial log (click to expand)

         | `Word of Core.String.t ] Patience_diff_lib__Patience_diff.Hunk.t
         list
       but an expression was expected of type
         'a Patdiff_lib.Import.Patience_diff.Hunks.t =
           'a Patience_diff_lib__Patience_diff.Hunk.t list

Waiting for 1 job to finish.
make: *** [Makefile:5: default] Error 1
builder for ‘/nix/store/df30fid1ygkpfd0cb3ly4fvqa39614mn-ocaml4.04.2-patdiff-0.10.0.drv’ failed with exit code 2
error: build of ‘/nix/store/df30fid1ygkpfd0cb3ly4fvqa39614mn-ocaml4.04.2-patdiff-0.10.0.drv’ failed

ivg · 2018-03-06T20:20:19Z

FYI, BAP has a half-a-year release cycle, so we will unlikely upgrade to the core.v0.10 earlier than that. Looping in @maurer.

vbgl · 2018-03-06T20:22:18Z

I guess patdiff needs to be updated too.

mt-caret · 2018-03-07T05:27:54Z

@vbgl Done.

mt-caret · 2018-03-07T08:10:59Z

🎉

mt-caret mentioned this pull request Feb 24, 2018

Janestreet 0.9 #25658

Merged

7 tasks

GrahamcOfBorg added 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Feb 24, 2018

vbgl requested changes Feb 25, 2018

View reviewed changes

mt-caret mentioned this pull request Feb 27, 2018

ocamlPackages.re2 build fails #35701

Closed

mt-caret requested review from edolstra, Ericson2314, FRidh, nbp, orivej, peti, rycee, ttuegel and zimbatm as code owners March 2, 2018 02:38

ttuegel removed their request for review March 2, 2018 14:54

janeStreet: 0.9.0 -> 0.10.0

84adbd0

vbgl reviewed Mar 3, 2018

View reviewed changes

bump janePackage's version lower bound

bef07b8

FRidh removed their request for review March 4, 2018 09:54

Ericson2314 removed their request for review March 5, 2018 16:14

fix bap regression

39cb0d8

fix patdiff hash

391b4d7

vbgl approved these changes Mar 7, 2018

View reviewed changes

vbgl merged commit 0e23b52 into NixOS:master Mar 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

janeStreet: 0.9.0 -> 0.10.0 [WIP] #35439

janeStreet: 0.9.0 -> 0.10.0 [WIP] #35439

mt-caret commented Feb 24, 2018 •

edited

vbgl Feb 25, 2018

vbgl Feb 25, 2018

mt-caret Feb 27, 2018 •

edited

vbgl Feb 25, 2018

vbgl Feb 25, 2018

vbgl commented Mar 1, 2018

mt-caret commented Mar 2, 2018 •

edited

Ericson2314 commented Mar 2, 2018

mt-caret commented Mar 3, 2018

vbgl Mar 3, 2018

vbgl commented Mar 3, 2018

GrahamcOfBorg commented Mar 3, 2018

GrahamcOfBorg commented Mar 3, 2018

vbgl commented Mar 3, 2018

mt-caret commented Mar 3, 2018 •

edited

vbgl commented Mar 3, 2018

vbgl commented Mar 5, 2018

mt-caret commented Mar 6, 2018 •

edited

vbgl commented Mar 6, 2018

GrahamcOfBorg commented Mar 6, 2018

GrahamcOfBorg commented Mar 6, 2018

ivg commented Mar 6, 2018

vbgl commented Mar 6, 2018

mt-caret commented Mar 7, 2018

mt-caret commented Mar 7, 2018

		@@ -34,8 +32,7 @@ rec {
		meta.description = "OCaml AST used by Jane Street ppx rewriters";
		} // (if lib.versionAtLeast ocaml.version "4.06"

janeStreet: 0.9.0 -> 0.10.0 [WIP] #35439

janeStreet: 0.9.0 -> 0.10.0 [WIP] #35439

Conversation

mt-caret commented Feb 24, 2018 • edited

Motivation for this change

Things done

vbgl Feb 25, 2018

Choose a reason for hiding this comment

vbgl Feb 25, 2018

Choose a reason for hiding this comment

mt-caret Feb 27, 2018 • edited

Choose a reason for hiding this comment

vbgl Feb 25, 2018

Choose a reason for hiding this comment

vbgl Feb 25, 2018

Choose a reason for hiding this comment

vbgl commented Mar 1, 2018

mt-caret commented Mar 2, 2018 • edited

Ericson2314 commented Mar 2, 2018

mt-caret commented Mar 3, 2018

vbgl Mar 3, 2018

Choose a reason for hiding this comment

vbgl commented Mar 3, 2018

GrahamcOfBorg commented Mar 3, 2018

GrahamcOfBorg commented Mar 3, 2018

vbgl commented Mar 3, 2018

mt-caret commented Mar 3, 2018 • edited

vbgl commented Mar 3, 2018

vbgl commented Mar 5, 2018

mt-caret commented Mar 6, 2018 • edited

vbgl commented Mar 6, 2018

GrahamcOfBorg commented Mar 6, 2018

GrahamcOfBorg commented Mar 6, 2018

ivg commented Mar 6, 2018

vbgl commented Mar 6, 2018

mt-caret commented Mar 7, 2018

mt-caret commented Mar 7, 2018

mt-caret commented Feb 24, 2018 •

edited

mt-caret Feb 27, 2018 •

edited

mt-caret commented Mar 2, 2018 •

edited

mt-caret commented Mar 3, 2018 •

edited

mt-caret commented Mar 6, 2018 •

edited