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

reversing the Unix and Bigarray dependency #6139

Closed
vicuna opened this Issue Aug 26, 2013 · 14 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

vicuna commented Aug 26, 2013

Original bug ID: 6139
Reporter: @avsm
Assigned to: @gasche
Status: resolved (set by @gasche on 2018-04-09T14:37:32Z)
Resolution: fixed
Priority: normal
Severity: feature
Target version: later
Fixed in version: 4.07.0+dev/beta2/rc1/rc2
Category: otherlibs
Related to: #6885
Monitored by: @hhugo @gasche @diml @glondu jmeber @hcarty @dbuenzli @Chris00

Bug description

With the splitting out otherlibs/ from the compiler distribution, I wanted to also see if we could remove the hard dependency on the Unix library from Bigarray. Currently, there is just one function (map_file) which causes Bigarray to depend on Unix.

Bigarray is increasingly used in the more exotic backends (js_of_ocaml, Mirage, ocamljava) as a convenient way to access externally allocated memory safely and fast. It would therefore make compilation much easier if the map_file were moved into the Unix module, thus introducing a Unix->Bigarray dependency rather than Bigarray->Unix. It's hard to override Bigarray via package management as it's installed in the standard library area and therefore picked up by default unless overridden for every case where it's used.

I'm happy to come up with a patch if there's general agreement that is a good idea. I can't see how not to break the existing interface, although telling people who to switch their existing use should be quite easy.

(two concrete reason I want this is to purge the last of POSIX from Mirage/Xen, which uses Bigarray, and also to more easily compile Core to Javascript for an interactive Real World OCaml toplevel).

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Aug 26, 2013

Comment author: @alainfrisch

I'd actually argue for making Bigarray part of the stdlib. AFAIK, it is supported on all platforms (at least for the core features, I'm unsure about map_file, precisely) and it has a special status w.r.t. the compiler (syntax and optimizations).

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Aug 26, 2013

Comment author: @avsm

Putting it in base, sans map_file, would be great. It's been very easy to port to all kinds of embedded platforms (as long as they have a malloc), and it has win32 support.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Aug 27, 2013

Comment author: @dbuenzli

FWIW I'd also like to have them in the stdlib. I did sometimes avoid them at legitimate places because of their special status (namely sources/destinations for codecs, though in the current state it would still be hard to support an efficient interface that allows to e.g. source from either an in_channel, a string or a bigarray).

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Aug 28, 2013

Comment author: @damiendoligez

If you want to maintain compatibility at all costs, you can rename your new version of the Bigarray module to something like Bigarray_core, then make a new Bigarray that just reexports all the functions plus map_file.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Nov 4, 2013

Comment author: @johnwhitington

Can I add my voice to the calls for reversing the dependency and putting Bigarray into the stdlib? It would be very useful.

Not really a strong argument, but recall that Bigarray has special syntax even without loading bigarray.cma:

$ ocaml
OCaml version 4.01.0

Bigarray.Array1.create;;

Error: Reference to undefined global `Bigarray'

let x = 0;;

val x : int = 0

x.{2};;

Error: This expression has type int but an expression was expected of type
('a, 'b, 'c) Bigarray.Array1.t

So in some sense, it's already in the stdlib.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Nov 6, 2013

Comment author: @alainfrisch

If you want to maintain compatibility at all costs, you can rename your new
version of the Bigarray module to something like Bigarray_core, then make a new
Bigarray that just reexports all the functions plus map_file.

What's the opinion of other proponents of the integration of Bigarray into stdlib? My preference would be to simply move the map_file functions into Unix (and not provide a compatibility module). It requires to fix client code, but the required change is trivial.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Nov 6, 2013

Comment author: @avsm

Yes, I'd prefer the simpler approach of moving Bigarray into the stdlib, and the map_file function into Unix. I'm happy to help fix up the resulting problems with an OPAM bulk build over the 4.2 release cycle.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Nov 6, 2013

Comment author: @dbuenzli

Also in favour of the last two comments. Let's put OCaml's type system and ocamlot to good use.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented May 8, 2014

Comment author: @dbuenzli

Has this a chance of occurring for 4.02 ?

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 18, 2015

Comment author: @johnwhitington

Was consensus reached on this? It would be a useful improvement.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Dec 24, 2015

Comment author: @alainfrisch

I don't think there has been any more discussion on this. Probably not for 4.03.

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Jan 5, 2017

Comment author: @dbuenzli

There is now a GPR #997

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Mar 3, 2017

Comment author: @dbuenzli

And another one here #1077

@vicuna

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Apr 9, 2018

Comment author: @gasche

Solved by merging #1685

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.