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

ocaml 4.06.0 #20239

Closed
wants to merge 6 commits into from
Closed

ocaml 4.06.0 #20239

wants to merge 6 commits into from

Conversation

JCount
Copy link
Contributor

@JCount JCount commented Nov 3, 2017

Created with brew bump-formula-pr.

@JCount
Copy link
Contributor Author

JCount commented Nov 3, 2017

cc @ilovezfs

@JCount JCount mentioned this pull request Nov 3, 2017
@JCount
Copy link
Contributor Author

JCount commented Nov 4, 2017

coccinelle and lablgtk are blocked on updating camlp4.

@JCount JCount force-pushed the ocaml-4.06.0 branch 2 times, most recently from 1703239 to 09e9e8d Compare November 4, 2017 03:05
@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

This will fix OPAM: #20251

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

This fixes the coccinelle test: 5063c2c

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

This will fix lablgtk: #20252

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

This will fix ocamlsdl: #20253

@ilovezfs ilovezfs mentioned this pull request Nov 4, 2017
@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

🍏 but it seems to break a lot of the things with a build-time-only dependency so we may need to do some more surgery before merging.

@ilovezfs ilovezfs mentioned this pull request Nov 4, 2017
4 tasks
@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

This gets coq building again: #20255

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

ENV["OCAMLPARAM"] = "safe-string=0,_" seems to make most problems go away. So I think we can ship this.

@ilovezfs ilovezfs closed this in 30c3ccd Nov 4, 2017
@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

🍪

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 4, 2017

Your request can't be satisfied:
  - No package matches qcow-format.

needs OCaml 4.05.0 from OPAM #20294

File "src/typing/flow_js.ml", line 10205, characters 2-334:
Error: This kind of expression is not allowed as right-hand side of `let rec'
Command exited with code 2.
make[1]: *** [build-flow] Error 10
make: *** [all-homebrew] Error 2
==> opam config exec opam install batteries=2.7.0 zarith=1.5 yojson=1.4.0 pprint=20140424 stdint=0.4.2 menhir=20170712
[ERROR] pprint.20140424 is not available because your system doesn't comply
        with ocaml-version < "4.06".
[ERROR] batteries.2.7.0 is not available because your system doesn't comply
        with ocaml-version >= "3.12.1" & ocaml-version < "4.06.0".

needs OCaml 4.05.0 from OPAM #20293

@ilovezfs ilovezfs mentioned this pull request Nov 4, 2017
4 tasks
@JCount JCount deleted the ocaml-4.06.0 branch November 4, 2017 18:02
@ilovezfs ilovezfs mentioned this pull request Nov 4, 2017
4 tasks
@shawwn
Copy link

shawwn commented Nov 5, 2017

Eh...

@ilovezfs FWIW, ocaml 4.06.0 wreaks havoc when attempting to build haxe from source.

To build haxe: make opam_install && make && make install

This works fine on older ocaml versions, but 4.06.0 fails to install various libraries during the make opam_install step.

This fixes most of the problems:

OCAMLPARAM="safe-string=0,_" make opam_install

However, opam install ptmap still fails. I had to run opam pin add ptmap path/to/my/monkeypatched/ptmap and fix the build issue in my local ptmap dir.

At that point haxe compiles successfully.

You can reproduce the issue:

brew install opam
brew unlink ocaml
opam switch 4.06.0
git clone https://github.com/HaxeFoundation/haxe
cd haxe
make opam_install # kaboom!
make && make install

Hope this helps!

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 5, 2017

@shawwn yeah unsafe-string was enough for the current release version (see 7f64c31) but I haven't tried HEAD for haxe. Did you submit your monkey-patch for ptmap upstream?

@shawwn
Copy link

shawwn commented Nov 5, 2017

Nah, it's horrible. I don't know ocaml at all, so I did the bare minimum hack to get it to compile:

let rec update x f = function
    Empty ->
      begin match f None with
      | None -> Empty
      | Some data -> add x data empty
      end
    | Leaf (k,j) as t -> t
    | Branch (p,m,t0,t1) as t -> t

If you put that into ptmap.ml, it compiles and all the tests pass. But that update function is obviously bogus. It doesn't actually update anything.

The compile error was due to the fact that ptmap.ml has no update function, so the above snippet is just a way to sidestep the problem.

That only works because apparently nothing calls update. So even though the functionality is bogus, haxe still built successfully.

Good enough for me :P I'm out of time, so someone else will have to carry this from here.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 5, 2017

CC @andyli have you hit the above issue yet?

@shawwn
Copy link

shawwn commented Nov 5, 2017

ah, I see.. 7f64c31 you changed haxe's brew formula to set OCAMLPARAM="safe-string=0,_".

Makes sense. It might be good to add that OCAMLPARAM="safe-string=0,_" trick to haxe's makefile instead.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 5, 2017

@shawwn well really it needs to be fixed so that it works with safe-string on :)

@shawwn
Copy link

shawwn commented Nov 5, 2017

good luck! :) You'll have to submit patches to 3 or 4 opam repos. e.g. rope fails to install without that safe-string workaround.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 5, 2017

@shawwn yeah the whole ecosystem is going to take time to catch up.

@avsm
Copy link
Contributor

avsm commented Nov 9, 2017

It's unfortunate that this got merged so quickly. We're working hard on getting opam-repository to be compliant with safe-string, but it does take a few weeks to work through all the libraries. In the meanwhile, upgrading to 4.06.0 does break users as a result.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 9, 2017

@avsm we merge stable versions of each package as soon as they're green for our CI.

@ilovezfs
Copy link
Contributor

ilovezfs commented Nov 9, 2017

@avsm my suggestion would be that you ship an opam version 1.2.3 that sets OCAMLPARAM="safe-string=0,_" in the environment until such time as the opam repository is actually caught up.

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants