Skip to content

Commit

Permalink
Fix separate installation of implementations
Browse files Browse the repository at this point in the history
When instantiating an implementation with a variant, make sure it matches vlib's
known implementations. It replaces the same-project check that was done when
instantiating an implementation.

Signed-off-by: Lucas Pluvinage <lucas.pluvinage@gmail.com>
  • Loading branch information
TheLortex committed Jul 5, 2019
1 parent 1956524 commit fdc6229
Show file tree
Hide file tree
Showing 15 changed files with 76 additions and 32 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)
-------------------

- When instantiating an implementation with a variant, make sure it matches
virtual library's list of known implementations. (..., fixes #2322, @TheLortex,
review by ...)

- 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
59 changes: 34 additions & 25 deletions src/lib.ml
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,28 @@ module Error = struct
(Lib_name.to_string not_vlib)
(Lib_name.to_string impl)
]

let vlib_known_implementation_mismatch ~loc ~name ~variant ~vlib_name =
make ~loc
[
Pp.textf "Virtual library %S doesn't know about implementation %S with \
variant %S. Instead of using (variant %s) here, you need to \
reference it in the virtual library project, using the \
external_variant stanza:\n\
(external_variant\n\
\ (virtual_library %s)\n\
\ (variant %s)\n\
\ (implementation %s))"
(Lib_name.to_string vlib_name)
(Lib_name.to_string name)
(Variant.to_string variant)
(Variant.to_string variant)
(Lib_name.to_string vlib_name)
(Variant.to_string variant)
(Lib_name.to_string name)
]


end

(* Types *)
Expand Down Expand Up @@ -864,32 +886,19 @@ let rec instantiate db name info ~stack ~hidden =
| None -> Ok vlib
| Some variant ->
(* If the library is an implementation tagged with a
variant, we must make sure that that it implements a
library that is part of the same project *)
let status_vlib = Lib_info.status vlib.info in
if Option.equal Dune_project.Name.equal
(Lib_info.Status.project_name status)
(Lib_info.Status.project_name status_vlib)
then
Ok vlib
else
variant, we must make sure that that it's correctly part of the
virtual library's known implementations. *)
let vlib_impls = Lib_info.known_implementations vlib.info in
Variant.Map.find vlib_impls variant
|> Option.bind
~f:(fun (_, impl_name) ->
let name = Lib_info.name info in
Option.some_if (Lib_name.equal impl_name name) (Ok vlib))
|> Option.value
~default:(
let vlib_name = Lib_info.name vlib.info in
let name = Lib_info.name info in
let name_vlib = Lib_info.name vlib.info in
Errors.fail loc
"Library implementation %a with variant %a implements@ a \
library outside the project.@ Instead of using \
(variant %a) here,@ you need to reference it in the \
virtual library project,@ using the external_variant stanza:@ \
(external_variant@\n\
\ (virtual_library %a)@\n\
\ (variant %a)@\n\
\ (implementation %a))"
Lib_name.pp name
Variant.pp variant
Variant.pp variant
Lib_name.pp name_vlib
Variant.pp variant
Lib_name.pp name)
Error.vlib_known_implementation_mismatch ~loc ~name ~variant ~vlib_name))
in
let default_implementation =
Lib_info.default_implementation info
Expand Down
10 changes: 5 additions & 5 deletions test/blackbox-tests/test-cases/variants-multi-project/run.t
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ variant.
File "prj2/dune", line 4, characters 13-20:
4 | (implements vlibfoo)
^^^^^^^
Error: Library implementation impl with variant "somevariant" implements
a library outside the project. Instead of using (variant "somevariant") here,
you need to reference it in the virtual library project,
using the external_variant stanza:
Error: Virtual library "vlibfoo" doesn't know about implementation "impl"
with variant "somevariant". Instead of using (variant somevariant) here, you
need to reference it in the virtual library project, using the
external_variant stanza:
(external_variant
(virtual_library vlibfoo)
(variant "somevariant")
(variant somevariant)
(implementation impl))
[1]
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
(library
(name impl2foo)
(public_name impl2foo)
(implements vlibfoo)
(variant somevariant2)
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
(lang dune 1.10)
(using library_variants 0.1)
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
let implme () = ()
Empty file.
26 changes: 24 additions & 2 deletions test/blackbox-tests/test-cases/variants-only-package/run.t
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
Check that local variant implementations are correctly exported in the list of
known_implementations implementations when using -p

$ dune build -p vlibfoo
$ cd project && dune build -p vlibfoo

$ cat _build/install/default/lib/vlibfoo/dune-package
$ cat project/_build/install/default/lib/vlibfoo/dune-package
(lang dune 1.11)
(name vlibfoo)
(library
Expand All @@ -23,3 +23,25 @@ known_implementations implementations when using -p
(kind virtual)
(intf)))
(wrapped true)))

Also check that the implementation correctly builds while using -p when part of the same project

$ cp -r project/_build/ opam

$ cd project && env OCAMLPATH=../opam/install/default/lib dune build -p implfoo

And fail if it's not part of the same project.

$ cd project-2 && env OCAMLPATH=../opam/install/default/lib dune build -p impl2foo
File "impl2foo/dune", line 4, characters 13-20:
4 | (implements vlibfoo)
^^^^^^^
Error: Virtual library "vlibfoo" doesn't know about implementation "impl2foo"
with variant "somevariant2". Instead of using (variant somevariant2) here,
you need to reference it in the virtual library project, using the
external_variant stanza:
(external_variant
(virtual_library vlibfoo)
(variant somevariant2)
(implementation impl2foo))
[1]

0 comments on commit fdc6229

Please sign in to comment.