Assuming testsuite pass, etc., this LGTM - up to you if you want the extra commit updating the otherlibs Makefile.

I wouldn't put this in 4.08, personally - I think it's fine to announce that the functions are effectively not present (but available with a workaround for the belligerent), and then definitely deleted in 4.09.

https://gist.github.com/4d8edbcf47138253509505166a57e68d

Here's the results of a quick map_file grep over opam source of the latest versions of packages using grep -r map_file *|grep Bigarray (so this doesnt cover cases where Bigarray has been opened previously)

Looks like a fair few packages still using the old form. The full extent will become more obvious once we get a working ocaml-migrate-parsetree with 4.08 and can unblock the bulk opam builds for 4.08+beta2

We can learn a valuable lesson from all this: when deprecating a feature, it is important to specify when the feature will effectively go away.

@avsm what do you conclude from this grepping?

In 4.08, the Bigarray.*.map_file functions are not accessible anyway. As @dra27 suggests, there is an ugly workaround to access them (that I don't recommend using), so in any cases released packages will need to be updated in order to be compatible with 4.08.

So personally I still don't see the harm of deleting this function in 4.08. At least it makes things clear.

We can learn a valuable lesson from all this: when deprecating a feature, it is important to specify when the feature will effectively go away.

Indeed, that would be extremely useful to specify in the original deprecation warning too.

what do you conclude from this grepping?

I think several of the packages that currently use it will need to support compiling with ocaml<4.06.0 (such as opam), frustrating though this is. Would it be possible to have a stdlib shim that will expose the right map_file function for the current compiler, and patch packages to use that instead?

To be clear, I think that this PR is the right thing to do in this case, but a compat shim for pre<4.06>=4.08 compilers would make life vastly easier for package maintainers.

I suppose. So the proposed plan is:

• merge this in trunk and 4.08
• provide a package Bigarray_with_map_file that has the same API as Bigarray in 4.07

This is similar to what @damiendoligez suggested, except that the compatibility code for the old map_file functions goes in the external package, leaving the compiler in a better state right now.

How about calling the package Unix_with_map_file instead, so that it forces people to move to the foo_of_genarray (Unix.map_file) mode now. Then when they want to drop <4.06 they can just replace Unix_with_map_file with Unix.map_file

@diml was suggesting a third-party package with the Bigarray interface, so naming it Unix sounds weird. Maybe the third-party package could expose only the map_file function? Then @avsm's naming scheme would work, but something simple like Mmap.genarray_of_file or Mmap.file could also make sense.

Yes just exposing only the Unix.map_file function is what I intended, so your names sound better.

Just adding an echo for the shim adding Unix.map_file, rather than the other way around - it seems a double-win to do it that way, as the shim never affects newer versions of OCaml and it's forcing adoption of the new API in old versions rather than providing a way of accessing the old API in new versions.

I wish we were more systematic in proposing compatibility shims right away, or interacting early with people who already do the work of building those shims (eg. stdcompat).

Ok, that makes sense. However, I suggest to call the module Mmap_shims. If we give it a nice short name, it won't feel like a transient package.

Do we actually need the package to be transient? It's fine if users depend on it indefinitely.

Ok, so there are no objection to merging this in 4.08 then. I moved the changelog entry to 4.08 as a result.

@gasche it's just that it doesn't bring anything compared to Unix.map_file.

There is one reason to call it just Mmap -- if it's just a single module with the map_file function, then it's easier for us to implement it in Mirage in the future without a full dependency on Unix.

An Mmap_xen module for instance could take advantage of virtual_modules in Dune and implement the map_file interface now that Bigarray is Unix-independent. This is why I'm in favour of breaking up the monolithic Unix module where practical, as it makes modular use of Unix facilities significantly easier.

(edit: I realise there is still a dependency on Unix.fd in the interface -- we work around that with a -dontlink unix hack right now which isn't ideal... but at least it doesn't pull in the full Unix.cmxa)

Ok, that makes sense.

I created the package: https://github.com/diml/mmap

@avsm if you want to integrate it in the mirage suite, I'm happy to transfer the project to mirage.

I can help do this for https://github.com/mirage/mirage-platform as I recently submitted a PR for 4.08 integration (mirage/mirage-platform#206).

Once the next beta is out I'll take care of it.

@dra27 could you approve this PR?

@diml thanks - if you add me as admin on there i'll do a repo transfer to mirage/

I've cherry-picked it to 4.08 as well

Thanks!

https://github.com/mirage/mmap now available. I'll release it to opam after setting up CI