Skip to content
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

Adding some Jane Street packages at version 0.9 (in progress) #25534

Closed
wants to merge 16 commits into from

Conversation

Zimmi48
Copy link
Member

@Zimmi48 Zimmi48 commented May 5, 2017

This pull request is mostly to get comments from OCaml packages (especially Jane Street packages) maintainers.

It seems that Nixpkgs contains lots of Jane Street packages at version 113.33.03.
Recently they have been moving to 0.9 with (it seems) quite a few differences in the way those are compiled / installed.
I need a few of these packages at version 0.9+, among them sexplib.
I have created the builder newBuildOcamlJane to generically build any of those packages. I'd appreciate feedback on that.
I've also added a package for sexplib version 0.9.1: at first I just wanted to replace version 113.33.03 with version 0.9.1 but a lot of packages depend on it, so it implies updating many many packages. Also some of the new versions of these packages have new dependencies, so it requires adding new packages as well. I am ready to do it, but I'd like feedback before.

Do you mind the new builder and does it seem OK to you? I probably could have modified the current builder to check if the version is before or after v0.9 but that would have been more complex and would have created unnecessary dependencies.

Do you confirm that updating all those packages (at once) is the way to go?

cc @maurer @ericbmerritt @vbgl

@maurer
Copy link
Member

maurer commented May 5, 2017

The new builder seems fine, but rather than just upgrading everything either all at once, or piecemeal, what I'd suggest doing instead is temporarily making sexplib_0_9 packages and the like, leaving the default sexplib in place, that way we can process adding packages one at a time. Once we get everything relevant to _0_9, we can examine making that the default version bump to that all at once.

I don't think sexplib is one of the culprits, but istr there being some api breakage on the 113->0.9 transition.

tl;dr
I think we should add 0.9 versions of packages as we can, and when we have a complete working set, use nox-review and friends to see if it's safe to make it the new default, then drop 113.x

@maurer
Copy link
Member

maurer commented May 5, 2017

P.S., can someone who has that power retrigger the OSX build? It appears to have failed during the installing nix stage, which I'm 99% sure wasn't this change, but OCaml + OSX tends to have surprising failures so it'd be good to see whether it works.

@Zimmi48
Copy link
Member Author

Zimmi48 commented May 6, 2017

@maurer Thanks for your comment. After some thinking, I reached the same conclusion. Let me add a few other 0.9 packages to this PR so as to confirm that the new builder works fine for all of them.

@Zimmi48 Zimmi48 changed the title Sexplib 0.9 (Feedback wanted!) Adding some Jane Street packages at version 0.9 (in progress) May 6, 2017
@Zimmi48
Copy link
Member Author

Zimmi48 commented May 7, 2017

@vbgl Just found out while adding package core_kernel at version 0.9 that you had already added package base and a few others at version 0.9. Though, you are not testing for OCaml version >= 4.03.
About the file organization I wonder where this is going, since some Jane Street packages are in ocaml-modules/ and some others are in sub-directory janestreet/ (the most recent version being found indifferently in one or the other).

@Zimmi48 Zimmi48 force-pushed the sexplib-0.9 branch 2 times, most recently from 57b3b02 to 44c5c30 Compare May 7, 2017 10:59
@Zimmi48
Copy link
Member Author

Zimmi48 commented May 7, 2017

My newOcamlJaneBuilder was actually not working when building packages with dependencies. I've changed it to something reproducing what @vbgl did in his derivation of base.

@Zimmi48
Copy link
Member Author

Zimmi48 commented May 7, 2017

Actually, my new version does not work either (ppx_optcomp fails to build). I had incorrectly tested the build on my machine.

@Zimmi48
Copy link
Member Author

Zimmi48 commented May 9, 2017

OK fixed. This was because I was using buildInputs instead of propagatedBuildInputs for OCaml package dependencies.

@Zimmi48 Zimmi48 force-pushed the sexplib-0.9 branch 3 times, most recently from 970d12b to 0d49e0c Compare May 9, 2017 17:06
@Zimmi48 Zimmi48 mentioned this pull request May 9, 2017
7 tasks
@Zimmi48
Copy link
Member Author

Zimmi48 commented May 9, 2017

My approach here does not scale. Here is a better proposal: #25658

@Zimmi48 Zimmi48 closed this May 9, 2017
@Zimmi48 Zimmi48 deleted the sexplib-0.9 branch May 15, 2017 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants