Skip to content

Commit

Permalink
dune utop: don't select implementations
Browse files Browse the repository at this point in the history
Signed-off-by: Lucas Pluvinage <lucas.pluvinage@gmail.com>
  • Loading branch information
TheLortex authored and rgrinberg committed Jun 28, 2019
1 parent 1956524 commit b91fdf9
Show file tree
Hide file tree
Showing 12 changed files with 39 additions and 2 deletions.
4 changes: 4 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
1.11.0 (unreleased)
-------------------

- Don't select all local implementations in `dune utop`. Instead, let the
default implementation selection do its job. (#2327, fixes #2323, @TheLortex,
review by @rgrinberg)

- Don't reserve the `Ppx` toplevel module name for ppx rewriters (#2242, @diml)

- Redesign of the library variant feature according to the #2134 proposal. The
Expand Down
9 changes: 7 additions & 2 deletions src/utop.ml
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,20 @@ let libs_under_dir sctx ~db ~dir =
begin match Lib.DB.find_even_when_hidden db
(Dune_file.Library.best_name l) with
| None -> acc (* library is defined but outside our scope *)
| Some lib ->
| Some lib when should_be_selected lib ->
(* still need to make sure that it's not coming from an external
source *)
let info = Lib.info lib in
let src_dir = Lib_info.src_dir info in
if Path.is_descendant ~of_:(Path.build dir) src_dir then
(* Only select libraries that are not implementations.
Implementations are selected using the default implementation
feature. *)
let not_impl = Option.is_none (Lib_info.implements info) in
if not_impl && Path.is_descendant ~of_:(Path.build dir) src_dir then
lib :: acc
else
acc (* external lib with a name matching our private name *)
| _ -> acc
end
| _ ->
acc)))
Expand Down
10 changes: 10 additions & 0 deletions test/blackbox-tests/dune.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1448,6 +1448,14 @@
(run %{exe:cram.exe} -skip-versions <4.05.0 -test run.t)
(diff? run.t run.t.corrected)))))

(alias
(name utop-default-implementation)
(deps (package dune) (source_tree test-cases/utop-default-implementation))
(action
(chdir
test-cases/utop-default-implementation
(progn (run %{exe:cram.exe} -test run.t) (diff? run.t run.t.corrected)))))

(alias
(name variants)
(deps (package dune) (source_tree test-cases/variants))
Expand Down Expand Up @@ -1716,6 +1724,7 @@
(alias use-meta)
(alias utop)
(alias utop-default)
(alias utop-default-implementation)
(alias variants)
(alias variants-external-declaration)
(alias variants-external-declaration-conflict)
Expand Down Expand Up @@ -1880,6 +1889,7 @@
(alias transitive-deps-mode)
(alias unreadable-src)
(alias upgrader)
(alias utop-default-implementation)
(alias variants)
(alias variants-external-declaration)
(alias variants-external-declaration-conflict)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
(lang dune 1.7)
(using library_variants 0.1)
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(library
(name forutop_impl_2)
(implements forutop))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let run () = print_endline "shouldn't be selected"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
(library
(name forutop_impl)
(implements forutop))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let run () = print_endline "selected by default impl"
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
(library
(name forutop)
(virtual_modules forutop)
(default_implementation forutop_impl))
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
val run : unit -> unit
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Forutop.run ();;
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
$ dune utop . init_forutop.ml
selected by default impl

0 comments on commit b91fdf9

Please sign in to comment.