Add support for dune rtop alongside dune utop

[cyrus-]

We are using dune for a Reason project, and dune utop is really useful. It would be great to support rtop in the same way. I think the API for rtop is identical to utop, so it should just be a matter of changing the executable name?

[ghost]

Rather than keep on adding stuff to Dune, we'd like to go in a slightly different direction. More precisely, allow to integrate with Dune from within the toplevel. For instance, you'd start any toplevel without wrapping with dune, do #load "dune" inside it and the magic would happen at this point.

[Khady]

Note that in the meantime it is possible to "load" rtop from inside utop IIRC. But unfortunately I can't find again the page where I read how to do it.

[mbernat]

Rather than keep on adding stuff to Dune, we'd like to go in a slightly different direction. More precisely, allow to integrate with Dune from within the toplevel. For instance, you'd start any toplevel without wrapping with dune, do #load "dune" inside it and the magic would happen at this point.

This sounds wonderful!
How much work will be required and how soon could we expect this feature?
And how to best approach reason+dune+utop in the meantime?

[ghost]

It's not much really but we have other priorities at the moment. If you are happy to spend a bit of time on this, I can give you some pointers.

[mbernat]

Sure, please send the pointers, I'll see if I can help. Thanks.

[ghost]

Great :)

So at a high level, this will work as follow: Dune will install a file <libdir>/toplevel/dune. This file will invoke dune in the current directory in order to:

• build all the relevant libraries
• get a list of include directories and cma files to load

We can make dune print a file that the toplevel will evaluate. For instance, we could run dune toplevel-init-file, capture its output and use Toploop.use_file to evaluate it.

To add a toplevel-init-file command, you can add a file bin/toplevel_init_file.ml with the following contents:

open Stdune
open Dune
open Import
open Fiber.O

let doc = "..."

let man =
  [ `S "DESCRIPTION"
  ; `P "..."
  ; `Blocks Common.help_secs
  ]

let info = Term.info "toplevel-init-file" ~doc ~man

let term =
  let+ common = Common.term  in
  Common.set_common common ~targets:[];
  Scheduler.go ~common (fun () ->
    let* setup = Import.Main.setup common in
    let sctx =
      Context_name.Map.find setup.super_contexts "default"
      |> Option.value_exn
    in
    let dir =
      Path.Build.relative
        (Super_context.build_dir sctx)
        (Common.prefix_target common dir)
    in
    ... (* insert code similar to the one in the [setup] function in
           [src/dune/utop] to compute [requires] *)
    let include_paths = Lib.L.include_paths requires in
    let files = Lib.L.link_deps requires ~mode:Link_mode.Byte in
    let* () = do_build (Build.paths files) in
    let files_to_load =
      List.filter files ~f:(fun p ->
        match Path.Build.extension p with
          | ".cma" | ".cmo" -> true
          | _ -> false)
    in
    Toplevel.print_toplevel_init_file ~include_paths ~files_to_load;
    Fiber.return ())

let command = (term, info)

Where:

• Lib.L.link_deps is a new function to write that returns the list of files needed when linking. You can take inspiration from Lib.L.compile_and_link_flags for this, it's pretty much the same except that we want to return just files rather than command line arguments
• Toplevel.print_toplevel_init_file is a new function to write that print the file to evaluate in the toplevel, composed of #directory and #load directives

Then you just have to add Toplevel_init.file.command to the all list in bin/main.ml.

[mbernat]

Thanks for this nice overview!
I've implemented most of it here: mbernat@6469d3c

But since I am very unfamiliar with Dune, I eventually got stuck when dealing with paths:

File "bin/toplevel_init_file.ml", line 42, characters 18-23:
Error: This expression has type Dune.Import.Path.t list
       but an expression was expected of type Stdune.Path.Build.t list
       Type Dune.Import.Path.t = Stdune.Path.t is not compatible with type
         Stdune.Path.Build.t = Stdune.Path.Build.w Stdune.Path.Local_gen.t 

Could you please give me some hints as to how various kinds of paths work? And also whether I'm going roughly in the right direction overall?

One other thing I noticed is that originally this functionality also worked as dune utop LIB and I think so far you haven't mentioned how passing that LIB argument should work from toplevel.

[ghost]

Sure, so we have:

• Path.Source.t, for paths that point to the source directory. All of these are relative to the root of the workspace
• Path.Build.t, for paths that point inside _build. All of these are relative to the _build directory
• Path.t, for all kinds of paths, i.e. source ones, build ones and external ones

Here, some of the files to load will be built by dune and so will be in the _build directory while others will be from installed libraries, so Path.t is indeed the right type to use. To fix the above error, you should use Path.extension rather than Path.Build.extension.

And also whether I'm going roughly in the right direction overall?

It looks to me that you are going in the right direction!

One other thing I noticed is that originally this functionality also worked as dune utop LIB and I think so far you haven't mentioned how passing that LIB argument should work from toplevel.

Here the idea is that you'd load all the libraries in the tree rooted at the current working directory. That said, we could add a new directive, or even overload the #require directive to build the libraries available in the project/workspace on the fly. Thought, we should live that for a separate PR :)

[mbernat]

Thanks! I'll try to sort it out. Dune is a bit overwhelming and it takes me a lot of time to figure out what pieces there are and how they fit together but, with your guidance, it seems doable :)

we should live that for a separate PR :)

Yeah, I'll happily leave that for another PR, I was just wondering :)

[mbernat]

I have some progress. I finished my hacky version of toplevel-init-file (mbernat@f2d1c8a)
and created a simple toplevel script (mbernat@95039ff).

I rely on the Unix module to capture subprocess output, which I guess is bad because dune is supposed to work on Windows too? Which also leads to me the question: can't we simply expose the required functionality as a library and call it directly, instead of using a binary and going through IO? It seems topfind is using this library approach. What do you think?

Anyway, at this stage, I can manually copy my new dune.exe and the toplevel script into my projects and it seems to do the right thing by loading the deps and my libs.

So what remains is improving my code (since it's ugly and I'm sure it will break in all kinds of settings), and adding the toplevel script to the dune installer. How do I do these things? Many thanks :)

[ghost]

For Unix, it should work on Windows. However, loading libraries with stubs in the toplevel doesn't work well on all platforms, so it is indeed better to not rely on Unix. Using a temporary file is the most portable way.

Regarding loading a library, there are a few concerns:

• that would mean running dune in bytecode, which is slower
• we want the dune package to be completely independent of any particular OCaml version, so it cannot install a library. As a result, if we followed the library approach users would have to install one additional package to get the toplevel integration

Overall, I feel like the system will be more flexible if we invoke dune as a plain tool.

So what remains is improving my code (since it's ugly and I'm sure it will break in all kinds of settings), and adding the toplevel script to the dune installer. How do I do these things? Many thanks :)

To install the script, you can use the following stanza:

(install
 (section lib_root)
 (files (dune.mlt as toplevel/dune)))

(the .mlt extension is for toplevel files).

Then the next step is to submit a PR so that we can review the code :) Oh, and if you could add a test and documentation that would be great!

For the test, you can add a directory in test/blackbox-tests/test-cases/ with a run.t file. Such files contains shell commands to run interleaved with their expected output. You can look at misc/run.t for instance. Once you have added a directory, you can run dune runtest --auto-promote once for the test to be added to the list of tests, and then dune runtest to run the new test. Inside run.t, you have access to dune and all the other files it installs as if it had been installed on the system.

For the documentation, I suppose you can add a chapter "Toplevel integration" to the manual. You can do that by creating a new .rst file in doc/ and adding it to the list in doc/index.rst.

[mbernat]

Hello @diml, once again thanks for all the great info and sorry about taking so long to get back to you.

However, loading libraries with stubs in the toplevel doesn't work well on all platforms, so it is indeed better to not rely on Unix

Oh, I see. So I suppose, I'll just use Sys.command then. Is that portable? I could try setting up a windows env, but I don't have access to one right now, so can't test this directly.

I'll also add the install stanza, a test, and docs, and post a PR ASAP.

[ghost]

No problem. Sys.command is indeed portable, you just need to make sure to quote the filenames with Filename.quote. We already use it in the bootstrap.

Looking forward to the PR!

[mbernat]

@diml I'm basically ready with my initial version: https://github.com/mbernat/dune/tree/toplevel

But I still need to rebase that on top of the current master to be able to create a PR and am a bit stuck because of a conflict.

It's about the link_flags function in src/dune/lib.ml. I wasn't really sure what's going on there, but I was able to add the essentially identical link_deps previously. But on current master the function changed considerably and has now both hidden and exit deps and I'm really clueless as to what this means and which bits I should use in toplevel-init-file, if any.

Please help :)

[ghost]

Indeed, this function is getting more complicated. I'm going to prepare a PR that refactors it a bit and adds the link_deps function.

[ghost]

Here it is: #2937

[mbernat]

Awesome!

[ghost]

And it's now merged!

[mbernat]

Thanks! I'll post the PR tomorrow.

…

On Tue, Dec 3, 2019 at 9:25 AM Jérémie Dimino ***@***.***> wrote: And it's now merged! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#2694?email_source=notifications&email_token=AAE5DYBO33JXKKZMGEY7E53QWYJWXA5CNFSM4I5FYPEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFYQE7I#issuecomment-561054333>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE5DYAONISJOFKJXXNHPK3QWYJWXANCNFSM4I5FYPEA> .

[mbernat]

Welp, it's finally here. Happy to address any issues (I'm sure there'll be plenty): #2952

[rgrinberg]

Long since fixed.