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

Release 2.6.0 between Oct. 15 and Nov. 1 #275

Closed
aantron opened this issue Oct 2, 2016 · 7 comments
Closed

Release 2.6.0 between Oct. 15 and Nov. 1 #275

aantron opened this issue Oct 2, 2016 · 7 comments
Milestone

Comments

@aantron
Copy link
Collaborator

aantron commented Oct 2, 2016

This will be the last release to support 4.01. It will include the new Lwt_result, the many bugfixes from the last few months, and whatever additional minor issues can get fixed soon. I probably won't have time to get the MSVC build working in AppVeyor.

2.7.0 will probably be released soon after. With ocaml.deprecated, minor breaking changes (e.g. [1], [2]) should be "safer" to make in minor releases, i.e. 2.8.0 onward, with sufficient advance warning.

Big changes such as dropping Camlp4, dropping modules (Lwt_pqueue), or changing the packaging are left for a hypothetical 3.0.0. I have no plans to actually do them, but I would like to.

@aantron aantron modified the milestone: 2.6.0 Oct 2, 2016
@aantron
Copy link
Collaborator Author

aantron commented Nov 1, 2016

@aantron aantron closed this as completed Nov 1, 2016
@tte
Copy link

tte commented Nov 22, 2016

@aantron After upgrade from 2.5.2 to 2.6.0 we get an issue at compile step w/ our own Result module.
Can you please give any suggestions how to handle module collision?

For example:
Result.ml - our own Result module
A.mli have only one method which return result of type Result.t

val update_object_field: (string * t) -> t -> (t, string) Result.t

B.ml using Lwt and our A.ml

open Lwt.Infix
open A

So, if you try to compile, you will get an error:

File "source/std/a.mli", line 1:
Error: The files source/std/result.cmi and source/std/result.cmi
      make inconsistent assumptions over interface Result

PS: now we just pinned lwt to 2.5.2.

@aantron
Copy link
Collaborator Author

aantron commented Nov 22, 2016

Argh. Are you using OCaml 4.02, or >= 4.03? The Result module comes from here: https://github.com/janestreet/result. Does your Result have more than just the result type?

I guess there are several ways to solve this. Here are some suggestions:

  • Lwt could define result internally using conditional compilation as follows:

    • If on OCaml >= 4.03, alias Pervasives.result,
    • If on OCaml 4.02 and package result is available, alias Result.t,
    • If on OCaml 4.02 and package result is not available, define result directly.

    I'd then release Lwt 2.6.1 soon, with result as an optional dependency, and you would have to compile on a switch that doesn't have result installed. Not sure if that is feasible for you. There might be some reasonable way to get Lwt to ignore result even if it is installed. Probably, you would have to set an environment variable during build.

  • If your Result.t is compatible with the one in package result, you could factor out your Result and pin it as that package. This may be annoying, however.

There are many other approaches, such as simply renaming your Result, which I don't think is reasonable. I think the above two are the most promising.

@c-cube if you have other suggestions :)

@c-cube
Copy link
Collaborator

c-cube commented Nov 22, 2016

Well first, I always use the result pseudo package and the type Result.result; second, this kind of error is usually solved with make clean since it's probably a build file relying on a previous version of the compiler/lib… If it's a collision between the standard result and a local module, well, rename the local module, it's the only solution.

@aantron
Copy link
Collaborator Author

aantron commented Nov 28, 2016

@tte Let me know if you'd like this to be resolved on the Lwt side :)

@tte
Copy link

tte commented Nov 28, 2016

Thank you for your suggestions.

I don't think that a new flag is a right decision. I think I should handle this on my side by factor out or trivial renaming.

I'm using Ocaml 4.03 and my Result looks like original janestreet's module with some extra methods.

Result.mli

type ('a, 'b) t =
  |  Ok of 'a
  |  Error of 'b

val map: ('a -> 'b) -> ('a, 'c) t -> ('b, 'c) t

val bind: ('a, 'c) t -> ('a -> ('b, 'c) t) -> ('b, 'c) t

val (>>=): ('a, 'c) t -> ('a -> ('b, 'c) t) -> ('b, 'c) t

val return: 'a -> ('a, 'b) t

val force: ('a, 'b) t -> 'a

val combine: ('a, 'b) t list -> ('a list, 'b) t

val to_value: ('a, 'b) t -> ('a -> 'c) -> ('b -> 'c) -> 'c

val to_lwt: ('a, string) t -> 'a Lwt.t

I tried to suppress original module with OCAMLFIND_IGNORE_DUPS_IN

$ export OCAMLFIND_IGNORE_DUPS_IN=/Users/.../.opam/4.03.0/lib/result

But it only clears a warning messages.

Sorry, I'm pretty new to Ocaml and I don't understand why my own module with the same type haven't compiled yet. First thing - cause I have already compiled original module. But I'm using ocambuild with -use-ocamlfind option and I have already suppressed a native module byOCAMLFIND_IGNORE_DUPS_IN env variable. It's not clear for me.

I will check some tricks with options and then package my module or just refact.

@aantron
Copy link
Collaborator Author

aantron commented Nov 28, 2016

The easiest thing may be to rename your module to, say, Result_prime, begin it with type ('a, 'b) t = ('a, 'b) result, and refer to it as Result_prime in your own code. This should work for OCaml >= 4.03, and Result_prime.t will be the same as Pervasives.result, Result.t, and Lwt.result (when 'b = exn).

If you want to go the route of substituting your own package for result, you have to:

  • get it building, of course
  • write an opam file
  • pin that package under the name result, so opam pin add result .
  • make sure you also pin it wherever your code is deployed

This may be too annoying.

OCAMLFIND_IGNORE_DUPS_IN won't solve this issue, it just suppresses that warning. It's good to know about it, though – that warning can be annoying in other cases.

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

No branches or pull requests

3 participants