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

Require sexplib < v0.9.0 #19

Merged
merged 1 commit into from
Jun 8, 2017
Merged

Require sexplib < v0.9.0 #19

merged 1 commit into from
Jun 8, 2017

Conversation

talex5
Copy link
Collaborator

@talex5 talex5 commented May 17, 2017

Otherwise, building fails on OCaml 4.03 and 4.04 with:

# File "./make_includes.ml", line 1:
# Error: Reference to undefined global `Ephemeron'
# *** omake: targets were not rebuilt because of errors:
#    src/compiler/includes.ml
#       depends on: src/compiler/make_includes.ml
#       depends on: src/runtime/common-inc.ml
#       depends on: src/runtime/reader-inc.ml
#       depends on: src/runtime/builder-inc.ml

See also: #16

@avsm
Copy link
Collaborator

avsm commented Jun 6, 2017

This is actually due to a bug in OCaml 4.04.0 (that was missing Ephemereon from the standard include list). My understanding is that 4.04.1 should fix this, so perhaps we should just make this library require the latest OCaml -- that fits in with its use of flambda anyway, that requires a fairly modern compiler.

@talex5
Copy link
Collaborator Author

talex5 commented Jun 6, 2017

flambda works with 4.02. I don't know if anyone is using this library with 4.02 or 4.03. It seems a shame to drop them just to support a newer sexplib (a library which we don't appear to be using anyway).

@avsm
Copy link
Collaborator

avsm commented Jun 6, 2017

There have been significant improvements/changes in flambda between 4.03 and 4.04. You may be able to get away with conflicting with just 4.04.0 and allowing 4.03.0 to install.

@talex5
Copy link
Collaborator Author

talex5 commented Jun 6, 2017

Dropping < 4.04.1 affects everyone using the library though, not just people who care about the flambda optimisations. Wouldn't it make more sense for the broken versions of sexplib to have the constraint?

@avsm
Copy link
Collaborator

avsm commented Jun 6, 2017

That's true -- the challenge is just figuring out where to put the constraint.

Although, it occurs to me that this is a non-issue if #24 is merged, since jbuilder seems to just work correctly with ppx_driver in all the versions of OCaml. Have you tried relaxing the constraint on sexplib with #24?

@talex5
Copy link
Collaborator Author

talex5 commented Jun 6, 2017

#24 breaks the optmisations on all versions of OCaml, due to https://caml.inria.fr/mantis/view.php?id=7538

@talex5
Copy link
Collaborator Author

talex5 commented Jun 7, 2017

It seems that only the top-level is broken. With the removal of camlp4, we don't need to run the make-includes.ml script anyway.

Otherwise, building fails on OCaml 4.03 and 4.04 with:

    # File "./make_includes.ml", line 1:
    # Error: Reference to undefined global `Ephemeron'
    # *** omake: targets were not rebuilt because of errors:
    #    src/compiler/includes.ml
    #       depends on: src/compiler/make_includes.ml
    #       depends on: src/runtime/common-inc.ml
    #       depends on: src/runtime/reader-inc.ml
    #       depends on: src/runtime/builder-inc.ml
@talex5
Copy link
Collaborator Author

talex5 commented Jun 8, 2017

I tried making a version of the no-camlp4 branch without this patch, but it then breaks for other reasons when used with modern versions of core_kernel. So I'm going to merge this now, then merge the camlp4 work, then upgrade to a more recent core/base to get rid of it again.

@talex5 talex5 merged commit 7dbc238 into capnproto:master Jun 8, 2017
@talex5 talex5 deleted the fix-travis branch June 8, 2017 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants