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

Add support for loading libraries from toplevel #2952

Merged
4 commits merged into from
Mar 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ Unreleased
- `dune upgrade` will now try to upgrade projects using versions <2.0 to version
2.0 of the dune language. (#3174, @voodoos)

- Add a `top` command to integrate dune with any toplevel, not just
utop. It is meant to be used with the new `#use_output` directive of
OCaml 4.11 (#2952, @mbernat, @diml)

2.4.0 (06/03/2020)
------------------

Expand Down
1 change: 1 addition & 0 deletions bin/main.ml
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ let all =
; Upgrade.command
; Caching.command
; Describe.command
; Top.command
]

let common_commands_synopsis =
Expand Down
58 changes: 58 additions & 0 deletions bin/top.ml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
open Stdune
open Import

let doc =
"Print a list of toplevel directives for including directories and loading \
cma files."

let man =
[ `S "DESCRIPTION"
; `P
{|Print a list of toplevel directives for including directories and loading cma files.|}
; `P
{|The output of $(b,dune toplevel-init-file) should be evaluated in a toplevel
to make a library available there.|}
; `Blocks Common.help_secs
]

let info = Term.info "top" ~doc ~man

let link_deps link ~lib_config =
List.concat_map link ~f:(fun t ->
Dune.Lib.link_deps t Dune.Link_mode.Byte lib_config)

let term =
let+ common = Common.term
and+ dir = Arg.(value & pos 0 string "" & Arg.info [] ~docv:"DIR")
and+ ctx_name =
Common.context_arg ~doc:{|Select context where to build/run utop.|}
in
Common.set_common common ~targets:[];
Scheduler.go ~common (fun () ->
let open Fiber.O in
let* setup = Import.Main.setup common in
let sctx =
Dune.Context_name.Map.find setup.scontexts ctx_name |> Option.value_exn
in
let dir =
Path.Build.relative
(Super_context.build_dir sctx)
(Common.prefix_target common dir)
in
let scope = Super_context.find_scope_by_dir sctx dir in
let db = Dune.Scope.libs scope in
let libs = Dune.Utop.libs_under_dir sctx ~db ~dir:(Path.build dir) in
let requires = Dune.Lib.closure ~linking:true libs |> Result.ok_exn in
let include_paths = Dune.Lib.L.include_paths requires in
let lib_config = sctx |> Super_context.context |> Context.lib_config in
let files = link_deps requires ~lib_config in
let* () = do_build (List.map files ~f:(fun f -> Target.File f)) in
let files_to_load =
List.filter files ~f:(fun p ->
let ext = Path.extension p in
ext = Dune.Mode.compiled_lib_ext Byte || ext = Dune.Cm_kind.ext Cmo)
in
Dune.Toplevel.print_toplevel_init_file ~include_paths ~files_to_load;
Fiber.return ())

let command = (term, info)
9 changes: 9 additions & 0 deletions doc/dune.inc
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,15 @@
(package dune)
(files dune-subst.1))

(rule
(with-stdout-to dune-top.1
(run dune top --help=groff)))

(install
(section man)
(package dune)
(files dune-top.1))

(rule
(with-stdout-to dune-uninstall.1
(run dune uninstall --help=groff)))
Expand Down
1 change: 1 addition & 0 deletions doc/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ Welcome to dune's documentation!
known-issues
migration
caching
toplevel-integration
14 changes: 14 additions & 0 deletions doc/toplevel-integration.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
********************
Toplevel integration
********************

It's possible to load dune projects in any toplevel. This is achieved in two stages.

First, `dune toplevel-init-file` builds the project and produces a list of toplevel pragmas
(#directory and #load). Copying the output of this command to a toplevel lets you
interact with the project's modules.

Second, to enhance usability, dune also provides a toplevel script, which does the above
manual work for you. To use it, make sure to have `topfind` available in your toplevel by
invoking `#use "topfind";;`. Afterwards you can run `#use "dune";;` and your
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need topfind for this? At the moment we only generate #directory and load directives and these are available by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, I thought the Toplevel module comes from topfind, but apparently it's available by default. I'll update the docs.

modules should be available.
2 changes: 2 additions & 0 deletions src/dune/context.ml
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,8 @@ let create ~(kind : Kind.t) ~path ~env ~env_nodes ~name ~merlin ~targets
(Config.local_install_dir ~context:name)
"lib/stublibs"))
; extend_var "OCAMLPATH" ~path_sep:ocamlpath_sep local_lib_path
; extend_var "OCAMLTOP_INCLUDE_PATH"
(Path.relative local_lib_path "toplevel")
; extend_var "OCAMLFIND_IGNORE_DUPS_IN" ~path_sep:ocamlpath_sep
local_lib_path
; extend_var "MANPATH"
Expand Down
7 changes: 7 additions & 0 deletions src/dune/toplevel.ml
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,13 @@ let setup_rules t =
(Build.symlink ~src:(Path.build src) ~dst);
setup_module_rules t

let print_toplevel_init_file ~include_paths ~files_to_load =
let includes = Path.Set.to_list include_paths in
List.iter includes ~f:(fun p ->
print_endline ("#directory \"" ^ Path.to_absolute_filename p ^ "\";;"));
List.iter files_to_load ~f:(fun p ->
print_endline ("#load \"" ^ Path.to_absolute_filename p ^ "\";;"))

module Stanza = struct
let setup ~sctx ~dir ~(toplevel : Dune_file.Toplevel.t) =
let source = Source.of_stanza ~dir ~toplevel in
Expand Down
3 changes: 3 additions & 0 deletions src/dune/toplevel.mli
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ val setup_rules : t -> unit

val make : cctx:Compilation_context.t -> source:Source.t -> t

val print_toplevel_init_file :
include_paths:Path.Set.t -> files_to_load:Path.t list -> unit

module Stanza : sig
val setup :
sctx:Super_context.t
Expand Down
2 changes: 2 additions & 0 deletions src/dune/utop.mli
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,6 @@ val utop_exe : string

val is_utop_dir : Path.Build.t -> bool

val libs_under_dir : Super_context.t -> db:Lib.DB.t -> dir:Path.t -> Lib.L.t

val setup : Super_context.t -> dir:Path.Build.t -> unit
10 changes: 10 additions & 0 deletions test/blackbox-tests/dune.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1910,6 +1910,14 @@
test-cases/tests-stanza-action-syntax-version
(progn (run dune-cram run run.t) (diff? run.t run.t.corrected)))))

(rule
(alias toplevel-integration)
(deps (package dune) (source_tree test-cases/toplevel-integration))
(action
(chdir
test-cases/toplevel-integration
(progn (run dune-cram run run.t) (diff? run.t run.t.corrected)))))

(rule
(alias toplevel-stanza)
(deps (package dune) (source_tree test-cases/toplevel-stanza))
Expand Down Expand Up @@ -2632,6 +2640,7 @@
(alias tests-stanza)
(alias tests-stanza-action)
(alias tests-stanza-action-syntax-version)
(alias toplevel-integration)
(alias toplevel-stanza)
(alias trace-file)
(alias transitive-deps-mode)
Expand Down Expand Up @@ -2891,6 +2900,7 @@
(alias tests-stanza)
(alias tests-stanza-action)
(alias tests-stanza-action-syntax-version)
(alias toplevel-integration)
(alias toplevel-stanza)
(alias trace-file)
(alias transitive-deps-mode)
Expand Down
50 changes: 50 additions & 0 deletions test/blackbox-tests/test-cases/toplevel-integration/run.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
Test toplevel-init-file on a tiny project
----------------------------------------------------
$ cat >dune-project <<EOF
> (lang dune 2.1)
> (name test)
> EOF
$ cat >dune <<EOF
> (library
> (name test)
> (public_name test))
> EOF
$ touch test.opam
$ cat >main.ml <<EOF
> let hello () = print_endline "hello"
> EOF

$ dune top
#directory "$TESTCASE_ROOT/_build/default/.test.objs/byte";;
#directory "$TESTCASE_ROOT/_build/default/.test.objs/native";;
#load "$TESTCASE_ROOT/_build/default/test.cma";;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test testing the whole story? i.e. doing #use "..." from a toplevel? And also one testing an error case, just to see what the error looks like. You can do the following:

  $ ocaml -stdin <<EOF
  > #use "..."
  > ...use code that was just loaded...
  > EOF

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I have several questions here:

  1. Even outside of tests, doing just #use "dune.top" doesn't work in ocaml(it can't find the file), but it does in utop. Is ocaml using OCAML_TOPLEVEL_PATH or does it resolve paths in some other fashion? I wasn't able to figure this out by googling and I only use plain ocaml rarely. But of course, we want to support it, that's the whole point of this PR! :)
  2. When running the tests, OCAML_TOPLEVEL_PATH still points to my user-local directory. Am I supposed to run export OCAML_TOPLEVEL_PATH="_build/install/default/lib/toplevel/dune.top" or is there a nicer way to refer to the local installation we're testing?
  3. What error case do you have in mind?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a bit of research and right now, packages that install toplevel scripts such as ocamlfind or down install them in both <libdir>/toplevel and <libdir>/ocaml. The latter is immediately available in the toplevel.

OCAML_TOPLEVEL_PATH is only used by utop and not by the vanilla toplevel. However, since 4.08.0, there is a new variable OCAMLTOP_INCLUDE_PATH that is supported natively. I opened a ticket to discuss setting this variable in opam: ocaml/opam-repository#15547. Let's see what happens there. At worse we could install dune.top in <libdir>/ocaml as well.

On the dune side, dune already extends a few environment variable so that when running dune or other tools inside dune, various tools will look for things in _build/install in addition to things installed globally. We should do this for OCAMLTOP_INCLUDE_PATH as well. More precisely, you can look for extend_var "OCAMLPATH" in src/dune/context.ml and add the following line: extend_var "OCAMLTOP_INCLUDE_PATH" (Path.relative local_lib_path "toplevel").

Then, in the test #use "dune.top" will work out of the box with OCaml >= 4.08. We run the testsuite with 4.09 in the CI so having the test only work with OCaml >= 4.08 is fine.

Regarding the error case, I was thinking of a case where compilation fails. For instance, what does the user see if one of the ml file that dune tries to compile doesn't compile? Or if a dune file is invalid.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all this great info and sorry about my (usual) late response!

Here's what I've done for now. Since there are still several unknowns, I've also had to make several questionable choices.

  1. I've added extend_var as you suggested.
  2. I've also installed dune.top into <libdir>/ocaml, since we can't rely on the opam PR being finished anytime soon (right?).
  3. I've put back the documentation bit that people need to do #use "topfind" first. Indeed, it turns out that without that I don't have Toplevel module available in dune.top (I missed this because I had #use "topfind" in my .ocamlinit). Do you think it's possible to remove the dependency on #use "topfind" or, even better, on the Toplevel module?
  4. I've added a few tests, including failures. However, the failures expose compilation internals and I'm not sure whether the tests won't break randomly: https://github.com/mbernat/dune/blob/toplevel/test/blackbox-tests/test-cases/toplevel-integration/run.t#L35

Please let me know what you think about this.

In case I don't get around to this again for a week, I wish you a very Merry Christmas and thanks again for all your help! 🎄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at those errors more closely, they actually reference my local paths, so they're definitely wrong.

How do you handle test errors containing local paths? And more importantly, what kind of error behavior should we expect from running dune toplevel-init-file and dune.top?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just back from holidays. Thanks, hope you had a nice Christmas too!

For 2, that seems reasonable.

For 3, we can do the same as the topfind script, i.e. temporary add the +compiler-libs directory:

#directory "+compiler-libs";;
...
#remove_directory "+compiler-libs";;

For 4, I was surprised by this output as normally Dune doesn't print the full compilation command line if it detects that the error messages already points to a file, i.e. is of the form File "xxx", .... After a bit of digging, I found that we always print the full compilation command when dune is run inside dune because of this in src/dune/config.ml:

let show_full_command_on_error () =
  inside_dune || inside_ci || !Clflags.always_show_command_line

@rgrinberg, I don't remember the details. Was the inside_dune || part intentional?


$ ocaml -stdin <<EOF
> #use "topfind";;
> #use "use_output_compat";;
> #use_output "dune top";;
> Test.Main.hello ();;
> EOF
hello

$ cat >error.ml <<EOF
> let oops () = undefined_function ()
> EOF

$ dune top
File "error.ml", line 1, characters 14-32:
1 | let oops () = undefined_function ()
^^^^^^^^^^^^^^^^^^
Error: Unbound value undefined_function
[1]

$ ocaml -stdin <<EOF
> #use "topfind";;
> #use "use_output_compat";;
> #use_output "dune top";;
> EOF
File "error.ml", line 1, characters 14-32:
1 | let oops () = undefined_function ()
^^^^^^^^^^^^^^^^^^
Error: Unbound value undefined_function
Command exited with code 1.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
(* -*- tuareg -*- *)

let try_finally ~always f =
match f () with
| x ->
always ();
x
| exception e ->
always ();
raise e

let use_output command =
let fn = Filename.temp_file "ocaml" "_toploop.ml" in
try_finally
~always:(fun () -> try Sys.remove fn with Sys_error _ -> ())
(fun () ->
match
Printf.ksprintf Sys.command "%s > %s" command (Filename.quote fn)
with
| 0 -> ignore (Toploop.use_file Format.std_formatter fn : bool)
| n -> Format.printf "Command exited with code %d.@." n)

let () =
let name = "use_output" in
if not (Hashtbl.mem Toploop.directive_table name) then
Hashtbl.add Toploop.directive_table name
(Toploop.Directive_string use_output)